Bug 693054

(In reply to comment #0)
> That file fails with -1 against pxlcolor (I think you may need to apply
> attachment 8598 [details] Bug 690585 to see it, or may be not - but that patch is
> "obviously" correct so should be applied anyway).
> 
> pcl6 -sDEVICE=pxlcolor -sOutputFile=a.p pcl5cfts/fts.0980
> Warning interpreter exited with error code -1
> Flushing to end of job

Just commenting that it appears that the attachment is needed to see the error code -1. (the -28 with pswrite is independent of this, as expected).

This is curious as the pcl6 main *should* be able to cope with switching from 'obviously wrong' no-op 'return 0' for pclxl_strip_copy_rop() to 'obviously correct but not optimized' gx_default_strip_copy_rop(). This suggests a bug in either pcl6 main or gx_default_strip_copy_rop().

pxlcolor and pswrite fails at the same place for both files.

(ESC * b) 11 W
   [Transfer Raster Row]
[i][file pos=28831]

for 0980.

(ESC * c) 5 G
   [Pattern ID]
ESC * c
(ESC * c) 3 P
   [Fill Rectangular Area]
[i][file pos=30939]

for 0982.

note that the failure with pxlcolor is directly related to switching from the wrong no-op to gx_default_strip_copy_rop(). This suggests the common problem is pcl6 main does not play nice with gx_default_strip_copy_rop() (which presumably pswrite also uses somehow, considered that it is one of the gx_default_* routines).

(In reply to comment #2)
> pxlcolor and pswrite fails at the same place for both files.
> 
> (ESC * b) 11 W
>    [Transfer Raster Row]
> [i][file pos=28831]
> 
> for 0980.
> 
> (ESC * c) 5 G
>    [Pattern ID]
> ESC * cht 
> (ESC * c) 3 P
>    [Fill Rectangular Area]
> [i][file pos=30939]
> 
> for 0982.
> 
> note that the failure with pxlcolor is directly related to switching from the
> wrong no-op to gx_default_strip_copy_rop(). This suggests the common problem is
> pcl6 main does not play nice with gx_default_strip_copy_rop() (which presumably
> pswrite also uses somehow, considered that it is one of the gx_default_*
> routines).

The issue is there is no way to read the destination (framebuffer) for the high level device to support a non trivial raster op or transparency, no "get_bits_rectangle".    We don't know of a good solution to this problem.  gx_default_strip_copy_rop() requires get_bits_rectangle(), see the comment above the procedure, so your patch is not appropriate.  If you change the lop parameter of gx_default_strip_copy_rop() to not use the destination (see gsropt.h for bit twiddling tricks for the parameter) then the procedure should work because get_bits_rectangle will not be called and we should get an approximate result, but I haven't experimented.  You can talk to Robin or I on IRC about this some more if you want.

(In reply to comment #3)
> (In reply to comment #2)
> > pxlcolor and pswrite fails at the same place for both files.
> > 
> > (ESC * b) 11 W
> >    [Transfer Raster Row]
> > [i][file pos=28831]
> > 
> > for 0980.
> > 
> > (ESC * c) 5 G
> >    [Pattern ID]
> > ESC * cht 
> > (ESC * c) 3 P
> >    [Fill Rectangular Area]
> > [i][file pos=30939]
> > 
> > for 0982.
> > 
> > note that the failure with pxlcolor is directly related to switching from the
> > wrong no-op to gx_default_strip_copy_rop(). This suggests the common problem is
> > pcl6 main does not play nice with gx_default_strip_copy_rop() (which presumably
> > pswrite also uses somehow, considered that it is one of the gx_default_*
> > routines).
> 
> The issue is there is no way to read the destination (framebuffer) for the high
> level device to support a non trivial raster op or transparency, no
> "get_bits_rectangle".    We don't know of a good solution to this problem. 
> gx_default_strip_copy_rop() requires get_bits_rectangle(), see the comment
> above the procedure, so your patch is not appropriate.  If you change the lop
> parameter of gx_default_strip_copy_rop() to not use the destination (see
> gsropt.h for bit twiddling tricks for the parameter) then the procedure should
> work because get_bits_rectangle will not be called and we should get an
> approximate result, but I haven't experimented.  You can talk to Robin or I on
> IRC about this some more if you want.

I would argue that's a problem with gx_default_strip_copy_rop() itself, rather than pxl's use of it - since pswrite also suffers from its limitation (and may be other drivers too). Besides, the patch does correct the rendering problems of some 20+ files. There is a way of optimizing *pxl* rop to by-pass gx_default for those 2 files, but that does not solve the general issue of gx_fault_* not working completely in all circunstances. It is supposed to be the fallback case if other alternatives are not available, and it isn't working completely that way.

(In reply to comment #3)
.. We don't know of a good solution to this problem. 
> gx_default_strip_copy_rop() requires get_bits_rectangle(), see the comment
> above the procedure, so your patch is not appropriate.

The patch is correct, it just exposes deficiency elsewhere. There is a quick compromise between improving 20+ files and not failing these two: run gx_default_strip_copy_rop() but ignore its return status and still returning zero.

The would get around the problem of get_bits_rectangle() not working, but still does not (1) solve rendering problem of these two, nor (2) address pswrite's and other use of  gx_default_strip_copy_rop() not working.

if you would prefer 'always finishing but not necessarily rendering correctly', ignoring and not passing the return status back up the language core is a possible answer.

(In reply to comment #5)
> (In reply to comment #3)
> >... We don't know of a good solution to this problem. 
> > gx_default_strip_copy_rop() requires get_bits_rectangle(), see the comment
> > above the procedure, so your patch is not appropriate.
> 
> The patch is correct, it just exposes deficiency elsewhere. There is a quick
> compromise between improving 20+ files and not failing these two: run
> gx_default_strip_copy_rop() but ignore its return status and still returning
> zero.

No, because the error code may be caused by other things, and simply ignoring it would be bad.

The best answer is probably to modify the rop value used.

In your patch, try adding:

  lop = rop3_know_D_0_(lop);

before the call.

That will modify the lop to one that no longer uses 'D' (and always assumes it's 0). The results will not be correct in all cases, but in the absence of someone fixing get_bits_rectangle it'll be as good as we can do.

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #3)
run
> > gx_default_strip_copy_rop() but ignore its return status and still returning
> > zero.
> 
> No, because the error code may be caused by other things, and simply ignoring
> it would be bad.

Ignoring the return status is no worse than the current situation, which is just making up a return value (zero).

> The best answer is probably to modify the rop value used.
> 
> In your patch, try adding:
> 
>   lop = rop3_know_D_0_(lop);
> 
> before the call.
> 
> That will modify the lop to one that no longer uses 'D' (and always assumes
> it's 0). The results will not be correct in all cases, but in the absence of
> someone fixing get_bits_rectangle it'll be as good as we can do.

The *best* answer is to do rop3 optimally & natively with pxl features, and  by-pass and "protect" gx_default_(). That still does not address its own limitation and other drivers (pswrite & possibly others)' use of gx_default().

The default routines do not have to work with all devices, your obvious patch was not implemented in the first place because it is wrong: the default routine is only possible for clients that support "get_bits_rectangle()".  pswrite's use of the "default" is wrong as well, it is not worth fooling with as the device will not be supported much longer.  Please use Robin and my suggestion to modify the lop or don't work on the problem and we will fix it.

(In reply to comment #8)
... Please use Robin and my suggestion
> to modify the lop or don't work on the problem and we will fix it.

Robin's suggestion is wrong. It is just switching from rendering wrongly a bunch of file one way, to rendering wrongly in a differently way. The correct way is to use pxl rop3 properly to by-pass gx_default. (i.e. fix bug 693056 to some extent).

Modified the patch to Bug 690585 to no longer cause interpreter failure. (at the cost of not fixing rendering problems of 2 of the 21 files).

This still needs to remain open, since gs_default_strip_copy_rop() doesn't work on other devices. (until all such usage are gone, but that's difficult to gaurantee...)

Marking as "LATER", not likely to do anything about this in the foreseeable future.