Indirectly noticed with automated testing that all these files have non trivial visual discrepancies with the pxlcolor device. pcl6 -r600 -sDEVICE=pxlcolor -dNOPAUSE -sOutputFile=foo.pxl ... tests_private/pcl/pcl5cfts/fts.0060 tests_private/pcl/pcl5cfts/fts.0090 tests_private/pcl/pcl5cfts/fts.0100 tests_private/pcl/pcl5cfts/fts.0110 tests_private/pcl/pcl5cfts/fts.0120 tests_private/pcl/pcl5cfts/fts.0180 tests_private/pcl/pcl5cfts/fts.0210 tests_private/pcl/pcl5cfts/fts.0220 tests_private/pcl/pcl5cfts/fts.0230 tests_private/pcl/pcl5cfts/fts.0670 tests_private/pcl/pcl5cfts/fts.0680 tests_private/pcl/pcl5cfts/fts.0730 tests_private/pcl/pcl5cfts/fts.0740 tests_private/pcl/pcl5cfts/fts.0750 tests_private/pcl/pcl5cfts/fts.0760 tests_private/pcl/pcl5cfts/fts.0774 tests_private/pcl/pcl5cfts/fts.0816 tests_private/pcl/pcl5cfts/fts.0825 tests_private/pcl/pcl5cfts/fts.0832 tests_private/pcl/pcl5cfts/fts.0840 tests_private/pcl/pcl5cfts/fts.0861 tests_private/pcl/pcl5cfts/fts.0861 tests_private/pcl/pcl5cfts/fts.0862 tests_private/pcl/pcl5cfts/fts.0865 tests_private/pcl/pcl5cfts/fts.0866 tests_private/pcl/pcl5cfts/fts.0880 tests_private/pcl/pcl5cfts/fts.0884 tests_private/pcl/pcl5cfts/fts.0891 tests_private/pcl/pcl5cfts/fts.0930 tests_private/pcl/pcl5cfts/fts.0954 tests_private/pcl/pcl5cfts/fts.0970 tests_private/pcl/pcl5cfts/fts.0976 tests_private/pcl/pcl5cfts/fts.0980 tests_private/pcl/pcl5cfts/fts.0982 tests_private/pcl/pcl5cfts/fts.0990 tests_private/pcl/pcl5cfts/fts.1020 tests_private/pcl/pcl5cfts/fts.1051 tests_private/pcl/pcl5cfts/fts.1055 tests_private/pcl/pcl5cfts/fts.1060 tests_private/pcl/pcl5cfts/fts.1100 tests_private/pcl/pcl5cfts/fts.1130 tests_private/pcl/pcl5cfts/fts.1290 tests_private/pcl/pcl5cfts/fts.1300 tests_private/pcl/pcl5cfts/fts.1310 tests_private/pcl/pcl5cfts/fts.1390 tests_private/pcl/pcl5cfts/fts.1400 tests_private/pcl/pcl5cfts/fts.1410 tests_private/pcl/pcl5cfts/fts.1420 tests_private/pcl/pcl5cfts/fts.1430 tests_private/pcl/pcl5cfts/fts.1440 tests_private/pcl/pcl5cfts/fts.1460 tests_private/pcl/pcl5cfts/fts.1552 tests_private/pcl/pcl5cfts/fts.1640 tests_private/pcl/pcl5cfts/fts.1740 tests_private/pcl/pcl5cfts/fts.1762 tests_private/pcl/pcl5cfts/fts.1800 tests_private/pcl/pcl5cfts/fts.1810 tests_private/pcl/pcl5cfts/fts.1816 tests_private/pcl/pcl5cfts/fts.1821 tests_private/pcl/pcl5cfts/fts.1832 tests_private/pcl/pcl5cfts/fts.1850 tests_private/pcl/pcl5cfts/fts.1870 tests_private/pcl/pcl5cfts/fts.1890 tests_private/pcl/pcl5cfts/fts.1920 tests_private/pcl/pcl5cfts/fts.1925 tests_private/pcl/pcl5cfts/fts.1930 tests_private/pcl/pcl5cfts/fts.1932 tests_private/pcl/pcl5cfts/fts.1935 tests_private/pcl/pcl5cfts/fts.1940 tests_private/pcl/pcl5cfts/fts.1944 tests_private/pcl/pcl5cfts/fts.1950 tests_private/pcl/pcl5cfts/fts.1960 tests_private/pcl/pcl5cfts/fts.1962 tests_private/pcl/pcl5cfts/fts.1965 tests_private/pcl/pcl5cfts/fts.2000 tests_private/pcl/pcl5cfts/fts.2024 tests_private/pcl/pcl5cfts/fts.2030 tests_private/pcl/pcl5cfts/fts.2092 tests_private/pcl/pcl5cfts/fts.2105 tests_private/pcl/pcl5cfts/fts.2110 tests_private/pcl/pcl5cfts/fts.2114 tests_private/pcl/pcl5cfts/fts.2180 tests_private/pcl/pcl5cfts/fts.2190 tests_private/pcl/pcl5cfts/fts.2200 tests_private/pcl/pcl5cfts/fts.2201 tests_private/pcl/pcl5cfts/fts.2210 tests_private/pcl/pcl5cfts/fts.2320 tests_private/pcl/pcl5cfts/fts.2330 tests_private/pcl/pcl5cfts/fts.2350 tests_private/pcl/pcl5cfts/fts.2354 tests_private/pcl/pcl5cfts/fts.5401
Assign to Henry (per his request) at least until Marcos returns from vacation.
If the visual discrepancies hasn't gone away with my recent changes in the pxlcolor device, this could be re-assigned to me. (although it looks like it is pcl6 driving pxlcolor rather than ghostscript).
I ran at -r72 quickly through the 184 files in tests_private/pcl/pcl5cfts/ - these 46 files shows significant differences (plus some showing small width/positioning differences). 43 of those were in Ray's list and the 3 additions are fts.0070 fts.0080 fts.0800. Out of the 46 files, about 14 of them are similiar to '690949 - colorred pattern fill appeared with portion painted with black' . Suggest revisit this after fixing 690949. ----------- fts.0060 fts.0070 fts.0080 fts.0110 fts.0210 fts.0220 fts.0230 fts.0740 fts.0750 fts.0760 fts.0800 fts.0825 fts.0832 fts.0861.new fts.0861 fts.0862 fts.0865 fts.0866 fts.0880 fts.0884 fts.0891 fts.0954 fts.0970 fts.0980 fts.0982 fts.0990 fts.1020 fts.1051 fts.1055 fts.1060 fts.1100 fts.1130 fts.1420 fts.1740 fts.1762 fts.1816 fts.1821 fts.1832 fts.1944 fts.2105 fts.2114 fts.2210a fts.2320 fts.2330 fts.2350 fts.2354
Fixed 690949; there are still 39 cases. (12 'compare: images too dissimilar' and 27 smaller visible differences. The 12 are: fts.0825 fts.0832 fts.0861 fts.0861.new fts.0866 fts.0980 fts.1420 fts.1740 fts.1762 fts.1821 fts.2210a fts.2350 The 27 are: fts.0060 fts.0070 fts.0080 fts.0110 fts.0210 fts.0220 fts.0230 fts.0740 fts.0750 fts.0760 fts.0800 fts.0862 fts.0865 fts.0880 fts.0884 fts.0891 fts.0970 fts.0982 fts.0990 fts.1051 fts.1055 fts.1130 fts.2105 fts.2114 fts.2320 fts.2330 fts.2354 fts.1821 fts.1832 fts.2210a shows IllegalAttributeDataType SetLineJoin error so persumably it will work when that's fixed. fts.0602 (outside of the 39) - Failed to interpret TT instructions for glyph index 53.
The three invalid line joins are all trying to set value to 4. (PXL line joins only goes up to 3). After reviewing what other driver do - most of them either set default and/or output a warning, this is what I'd commit the next time: diff --git a/base/gdevpx.c b/base/gdevpx.c index c6f48dc..b23b9ed 100644 --- a/base/gdevpx.c +++ b/base/gdevpx.c @@ -1053,6 +1053,11 @@ pclxl_setlinejoin(gx_device_vector * vdev, gs_line_join join) { stream *s = gdev_vector_stream(vdev); + /* better not to set an invalid value */ + if ((join < 0) || (join > 3)) { + eprintf1("Igoring invalid linejoin enumerator %d\n", join); + return 0; + } /* The PCL XL join styles just happen to be identical to PostScript. */ px_put_ub(s, (byte) join); px_put_ac(s, pxaLineJoinStyle, pxtSetLineJoin);
Hin-Tak were you going to look at this more? I've made it bountiable.
(In reply to comment #6) > Hin-Tak were you going to look at this more? I've made it bountiable. Not to any extent - probably will come back to it from time to time. The diff in comment 5 was committed as r10400 . comment 4 was probably tested after r10403 but not sure if r10404 was included in the testing; as I remember many of the remaining diff were user pattern-related so 10404 was probably relevant.
A few of the problems were fixed by trunk@10403 (and possibly also trunk@10404). commit 2c6b4186758461bfd17658f34abdedee4a111c0e Author: Hin-Tak Leung <hintak_leung@yahoo.co.uk> Date: Mon Nov 30 02:41:19 2009 +0000 set ROP to paint through character glyphs (pattern tiles). ... Fixes bug 690949, also some of the problems in bug 690585 (for ghostpdl). This would have cut the numbers down to a bit lower than 39 cases, except there seems to be some new color-related issue so it stays at 38.
Created attachment 8598 [details] pclxl_strip_copy_rop() should call gx_default_* pclxl_strip_copy_rop() should calling gx_default_strip_copy_rop() instead of returning 0. This fixes 19 of the problems. fts.0060 fts.0070 fts.0080 fts.0110 fts.0210 fts.0220 fts.0230 fts.0740 fts.0750 fts.0760 fts.0866 fts.0880 fts.0884 fts.0891 fts.1051 fts.1130 fts.2105 fts.2114 fts.2350
Filed 3 new bug reports for continuations. AFAIK there are only 38 differences, 19 is fixed by comment 9 (and continue for optimization in bug 693056), 2 shows interpreter failure, and 17 with slight gray shifts (bug 693055). bug 693054 interpreter failure with fts.0980 and fts.0982 bug 693055 gray shifts with color round-trip pxl->png vs png conversion bug 693056 optimized version of pclxl_strip_copy_rop() The patch in comment 9 should be applied. Pending outcome of 0980/0982 after somebody else fixes the interpreter failure, this bug can be closed.
(In reply to comment #9) > Created an attachment (id=8598) [details] > pclxl_strip_copy_rop() should call gx_default_* > > pclxl_strip_copy_rop() should > calling gx_default_strip_copy_rop() instead of returning 0. This patch also improves bug 692853. (without this patch, the raster pattern is skipped and output almost empty).
Created attachment 8668 [details] pclxl_strip_copy_rop() should call gx_default_* when D is not involved. Calling gx_default_* conditional on !rop3_uses_D(lop) . gx_default_strip_copy_rop() cannot cope with rop3 involving the destination. It turns out that out of the 19 files fixed by the previous patch, 17 uses only 0xfc(TSo) and 0xff(1) (and only fts.0884 fts.0891 uses 0xff(1)). The two files go back to being rendered wrongly are fts.2105 and fts.2114 . I.e. this patch fixes rendering problems with 17 of the files (as well as the two samples in bug 692853, which uses only TSo), without causing interpreter failure in bug 693054.
I suggest committing the patch in comment 12, close this bug report and following up with trying to make the remaining 4 files work in bug 693056.
A small refinement suggestion: a warning should be emitted when dropped through to "return 0" - e.g. "copy_rop not implemented; raster may not render correctly.". Since nobody had noticed the lack of such much, I am not strongly for it though.
(In reply to comment #13) > I suggest committing the patch in comment 12, close this bug report and > following up with trying to make the remaining 4 files work in bug 693056. Done you can also collect the bounty for this problem. Fixed in: commit 76aad1b9e4eb2ae30d42b995a3ba8998b8e0820f Author: Henry Stiles <henry.stiles@artifex.com> Date: Wed Jun 13 13:29:35 2012 -0600 Bug 690585 - Use the defalt strip copy rop routine when the destination is not included. Thanks to Hin-Tak Leung for this fix. CLUSTER UNTESTED diff --git a/gs/base/gdevpx.c b/gs/base/gdevpx.c index b8338a3..0d2bd33 100644 --- a/gs/base/gdevpx.c +++ b/gs/base/gdevpx.c @@ -1719,9 +1719,18 @@ pclxl_strip_copy_rop(gx_device * dev, const byte * sdata, int sourcex, const gx_color_index * tcolors, int x, int y, int width, int height, int phase_x, int phase_y, gs_logical_operation_t lop) -{ /* We can't do general RasterOps yet. */ -/****** WORK IN PROGRESS ******/ - return 0; +{ + /* Improvements possible here using PXL ROP3 + for some combinations of args; use gx_default for now */ + if (!rop3_uses_D(lop)) /* gx_default() cannot cope with D ops */ + return gx_default_strip_copy_rop(dev, sdata, sourcex, + sraster, id, + scolors, + textures, + tcolors, + x, y, width, height, + phase_x, phase_y, lop); + return 0; } /* ------ High-level images ------ */