Created attachment 10724 [details] Input PS file that contains a multi-color image We have just upgraded our use of GS as a display engine in our WYSIWYG Editor product from version 8.63 to version 9.10. We still have some Win2000 Server systems that are used to Remote Desktop into the office (for remote employees), and these systems are limited to 256-colors for Terminal Window sessions. Some of our customers may still be using similar setups. With the 256-color limited display, viewing the attached PS file [has_color_image.ps] (doing "./bin/gs -sDEVICE=x11 has_color_image.ps" using Exceed as the X/Server) no longer works - gs just exits with the following error: Artifex Ghostscript 9.10: Unrecoverable error, exit code 25 In GS 8.63 in gxcmap.c at the end of cmap_rgb_direct() there is the following code (that handles this scenario): /* encode as a color index */ color = dev_proc(dev, encode_color)(dev, cv); /* check if the encoding was successful; we presume failure is rare */ if (color != gx_no_color_index) color_set_pure(pdc, color); else cmap_rgb_halftoned(r, g, b, pdc, pis, dev, select); In GS 9.10 in base/gxicolor.c at line 906 in image_render_color_icc() there is the following code: /* Now we can do an encoding directly or we have to apply transfer and or halftoning */ if (must_halftone || has_transfer) { /* We need to do the tranfer function and/or the halftoning */ cmap_transfer_halftone(&(conc[0]), pdevc_next, pis, dev, has_transfer, must_halftone, gs_color_select_source); } else { /* encode as a color index. avoid all the cv to frac to cv conversions */ color = dev_proc(dev, encode_color)(dev, &(conc[0])); /* check if the encoding was successful; we presume failure is rare */ if (color != gx_no_color_index) color_set_pure(pdevc_next, color); } Notice that unlike the GS 8.63 code there is nothing to handle the "rare" case when color == gx_no_color_index - there is no 'else' to handle it. Following the "pattern" of what was happening in the old code and what's happening right around this area of the new code, I found that I could successfully display the attachment on a 256-color display by adding an 'else' so that the code looks like this: ... if (color != gx_no_color_index) color_set_pure(pdevc_next, color); else cmap_transfer_halftone(&(conc[0]), pdevc_next, pis, dev, false, true, gs_color_select_source); } where it had to be that both has_transfer and must_halftone were 'false', otherwise cmap_transfer_halftone() would already have been called, and I've duplicated the cmap_transfer_halftone() call that was just above and left the has_transfer parameter as 'false' but forced the must_halftone parameter to 'true'. This seems to fix the problem, but I am not confident that this is the "right" solution.
I'm afraid Michael is travelling until Friday, so I'll take a look at this, but it may well have to wait for his return.
On a more common 24-bit true-color X system, you can nonetheless do Xvfb :1 -ac -screen 0 800x600x8 & to set up an 8-bit virtual display, then DISPLAY=:1 gs -sDEVICE=x11 ... to "display" to it. You won't see any outcome though. Ken/Michael can use this trick without reconfiguring their linux system/vm . BTW, -sDEVICE=x11mono/x11cmyk/x11cmyk8 with gs 9.10 seems to work; though with current head, a few of these seg-faulted. I suspect this is another regression from Ray's recent clist improvement... YMMV.
Xnest and Xephyr supposedly allows you to run an embedded X server (of a different depth) as a client of the main one; these might be more suitable than Xvfb, though I cannot claim any first-hand usage experience with those two, unlike Xvfb.
Handing off to Ray to have a look at this.
I have got to the bottom of this issue. The suggested diff from the reporter is definitely wrong. The correct fix is this. Near line 150 of base/gxicolor.c, when it tries to set up "must_halftone" from gx_device_must_halftone(): penum->icc_setup.must_halftone = gx_device_must_halftone(penum->dev); gx_device_must_halftone() is a macro in base/gxdevice.h near line 500: #define gx_device_must_halftone(dev)\ ((gx_device_has_color(dev) ? (dev)->color_info.max_color :\ (dev)->color_info.max_gray) < MIN_CONTONE_LEVELS) The x11 device is very unusual and unique - when it is operating under a truecolor display (almost everything recent and native on linux/Mac OS X), max_color is 255 and dither_color is 256, and encode_color always works; but under an 8-bit color display, max_color is still 255, but dither_color is only 5, and encode_color fails very often; so whether half-toning is needed depends on what display monitor it is running against (or against an emulator like Xvfb or remote display on Exceed). Most devices have "dither_colors = max_color + 1" (comments in base/gxdevcli.h). The strange values of x11 under an 8-bit color display are I think correct though - it is geniunely capable of doing a lot of shades of any one color component, *as long as* you don't ask for all colors at the same time. In details, under an 8-bit display, it does 5x5x5 standard colors, plus another 60+ dynamically (and another 60+ reserved by the X server for house-keeping, etc to make 256). Because some of it is dynamically allocated, it can geniunely do 60+ shades of red, if you *only* want different shades of red. But if you want a even spread of rainbow colors, it cannot do much more than 6x6x6 (=216). So i think the correct fix is modifying the macro to read: #define gx_device_must_halftone(dev)\ ((gx_device_has_color(dev) ? (dev)->color_info.dither_colors - 1 :\ (dev)->color_info.max_gray) < MIN_CONTONE_LEVELS) or, if this is considered too important to change, modify line 150 of base/gxicolor.c to read: penum->icc_setup.must_halftone = gx_device_must_halftone(penum->dev) || (penum->dev.dither_colors < MIN_CONTONE_LEVELS + 1); I think the first way is more correct, but more invasive; the 2nd one is less so, but have the advantage of having very little effect on code elsewhere. Neither of these changes should have any effect on other devices, since most devices are supposed to have dither_color = max_color + 1. While looking on this, I also found a 3rd way - modifying gdev_x_map_rgb_color() in devices/gdevxcmp.c (used by encode_color()) to return gx_no_color_index less often; the submitted file then works, but the change produces very noticeable blocky colors against say, examples/colorcir.ps, so while this 3rd way is least invasive, it is quite undesirable; I think encode_color() is supposed to occasionally fail and return gx_no_color_index, as long as it does not do so too often. I could push either option 1 or option 2 to cluster test at some point. Anyway, it should be back to Michael to comment, and perhaps Ray likes to comment as well?
I suppose a slight variation of option 1 is to change the macro to: #define gx_device_must_halftone(dev)\ ((gx_device_has_color(dev) ? \ min((dev)->color_info.dither_colors - 1, (dev)->color_info.max_color):\ (dev)->color_info.max_gray) < MIN_CONTONE_LEVELS) This is somewhat better than option 2, which probably needs an gx_device_has_color(dev) conditional there also there to parallel this.
oh, the icc-related enhancements is new to 9.x . x11 under 8.x almostly certainly uses a different code path - I haven't checked, but I can accept that it went through cmap_rgb_direct() in 8.x .
All the ideas in comment 5 and 6 causes a fair number of segfaults. This suggests the "dither_colors = max_color + 1" comments in base/gxdevcli.h is wrong for those "common devices" used in cluster tests. OTOH, segfaults are probably easier to fix than rendering differences or postscript errors.
So it looks like there is another more general bug - the comment about "dither_colors = max_color + 1" is wrong and unqualified since it was written. I'll possibly pick one of the segfaults to look at soon.
The clipper device is used by pbmraw under some circumstances. It also seems to be unusual in having dither_colors = 4 but max_color = 255 . Not too sure how that comes about, since superficially the clipper device should get color_info from its target device. Anyway, there is one other way which does work - try detecting x11 and treat it special. But the x11 device uses this code in two different ways, (besides behaving differently based on *runtime* environment of whether it is against 8-bit display or true-color) - either directly, when -dMaxBitmap=0, or through the image8/image24 device, so this approach needs some horrid conditional like: !strncmp(dev->dname, "x11", 3) || (!strncmp(dev->dname, "image8", 6) && !strncmp(((gx_device_forward *)dev)->target->dname, "x11", 3)) anyway, if ray/michael/ken can chip in whether targetting pbmraw should ever have a clipper with max_color = 255 and dither_colors = 4?
something like this (plus another similiar change in the 8-bit gray section - search for ramp_size) does work, and have no effect on other devices or on x11 under 24-bit truecolor display: diff --git a/gs/devices/gdevxcmp.c b/gs/devices/gdevxcmp.c index d91e78d..1be5759 100644 --- a/gs/devices/gdevxcmp.c +++ b/gs/devices/gdevxcmp.c @@ -364,6 +364,8 @@ gdev_x_setup_colors(gx_device_X * xdev) while (!xdev->cman.dither_ramp && ramp_size >= 2) { xdev->color_info.dither_grays = xdev->color_info.dither_colors = ramp_size; + xdev->color_info.max_gray = + xdev->color_info.max_color = ramp_size - 1; if (!setup_cube(xdev, ramp_size, true)) { #ifdef DEBUG emprintf3(xdev->memory, Though I am not convinced max_color should be 4 (5-1) though...
Created attachment 11069 [details] limit max_color/max_gray to color cube size. This is a refinement of the one in comment 11. This change has the advantage of being entirely limited to the x11 device, and also limited to the rarely used code-path of going running against a limited-color display i.e. it has no effect on modern linux/mac os X usage against truecolor displays, and entirely safe to be applied. It does has the disadvantage of not fulling using the available colours on the lagacy display in the discussion - against a 8-bit display, the x11 device should be able to allocate 190+ colors, some 60+ beyond the 5x5x5 color cube. This may not be visually noticeable though, so possibly acceptable as a work-around for the time being.
I have isolated the problematic commits - unsurprisingly, it is trunk@11306 : commit 6a82ae29ea4826048fc923388f4f59823e3a55c6 Merge: 254698b 53e03bb Author: Michael Vrhel <michael.vrhel@artifex.com> Date: Mon May 24 16:31:58 2010 +0000 Merge of icc_work branch into trunk. The test worked before that, and broke after. Bisecting through the icc_work branch, the brokage happen in two stages - icc_work@10472 - from rendering wrongly to aborting with error -255/-100 : commit 3141735bd33655bed1b208fa8146597f034e7fad Author: Michael Vrhel <michael.vrhel@artifex.com> Date: Tue Dec 8 20:17:27 2009 +0000 Fix to set device color procs during image color render. ... And from ok to rending wrongly to a brown blob via 3 commits that segfaults: commit 57867ab64305ea425b8b15fde77226d60a680c55 icc_work@9994 brown blob commit 3c0d497fb536df41b429e59098f22f0c011bd552 icc_work@9986 segfaults commit d07f9a38a8c9e307c7bdd0ef849e74d06eda80cb icc_work@9984 segfaults commit 0b01c8607335f2a339cd8703d842aac77cd53b74 icc_work@9983 segfaults commit 1077ae2f0f1530756c868d5d1d809e0579b34f77 icc_work@9982 ok I am fairly sure 3141735bd33655bed1b208fa8146597f034e7fad / icc_work@10472 is a red-herring, though it would surely be more friendly to display something wrongly than an unhelpful error code. The change between ok and brown blob through the segfaults is however too large and complicated to my comprehension, and I am not having much luck with it. Anyway, this is firmly in the area of regression from icc change, and been broken for 4 year unnoticed. I am not going to look much further in the near future so somebody else could have a go.
Looked at this with the windows display device for various bit depths and could not find any issue. Passing to Henry to see if he can emulate the 256 color display
(In reply to Michael Vrhel from comment #14) > Looked at this with the windows display device for various bit depths and > could not find any issue. Passing to Henry to see if he can emulate the 256 > color display The reporter was using Exceedw as X/Server so this is really a X11 Display problem. (see below). I am working on a fix for this together with bug 691432 and 695081 (both to do with interacting with X server with limited color capabilities) as they are somewhat related. I am thinking the graphic core should go back to dithering when the X server's color allocation table is full (i.e. we have already asked for as many colors as the limited X server is happy to give) and scale back max_color/max_gray according to the full allocation table. On a limited color X display, max_color/max_gray is some where between dither_gray/dither_color-1 (about 5 or 6) and 255, and not necessarily as high as MIN_CONTONE_LEVELS (which is 31 or 32). (In reply to Jonathan Dagresta from comment #0) ... > With the 256-color limited display, viewing the attached PS file > [has_color_image.ps] (doing "./bin/gs -sDEVICE=x11 has_color_image.ps" using > Exceed as the X/Server) no longer works - gs just exits with the following > error: ...
You can see the failure on a typical linux system using this procedure: (In reply to Hin-Tak Leung from comment #2) > On a more common 24-bit true-color X system, you can nonetheless do > > Xvfb :1 -ac -screen 0 800x600x8 & > > to set up an 8-bit virtual display, then > > DISPLAY=:1 gs -sDEVICE=x11 ...
Actually the dynamic color table does not become full - the X server fails to allocate further after about 60-ish requests, after the 125 color cube and server over head. I am thinking of scaling back max_color/max_gray to the current allocation table's capacity when allocation fails.
Hin-Tak are you still working on this? It is bountiable and in progress I wanted to give you a chance to finish if you want. I can arrange for a partial bounty for work you've already done on this problem, if you'd like another staff member to look at it.
Hin-Tak can you update the status of this bug? Thanks.
(From Hin-Tak Leung from comment #5) > I have got to the bottom of this issue. The suggested diff from the reporter > is definitely wrong. > (From Hin-Tak Leung from comment #7) > oh, the icc-related enhancements is new to 9.x . x11 under 8.x almost > certainly uses a different code path - I haven't checked, but I can accept > that it went through cmap_rgb_direct() in 8.x . So, the ongoing work to find a solution to this problem has gone on a lot longer than I expected. I'm not sure why my original suggested diff was rejected offhand, since my proposed fix was actually taken from what was being done in the 8.x code. Although I understand that there were ICC-related enhancements in 9.x that may mean the "old" way of handling the 256-color limitations is not really "correct", I will say that since I initially proposed a fix (Feb 2014) we've incorporated those changes into the version of GS code (currently 9.14) that we're using in our product and the code changes have worked fine when using a limited X11 256-color display and we have not observed any negative effects when using a non-limited (e.g. true color) X11 display. Of course, our product only is used with an X11 display so we're not testing many other scenarios as far as the proposed fix. What is it about the proposed fix (to use the 8.x way of dealing with a color-limited display) that is not correct, or that has a negative impact on other things?
Fixed with http://git.ghostscript.com/?p=ghostpdl.git;a=commit;h=47d23e2dfbae8db3b142b70aaef086c1bd6e097d The device should no longer fail in the encoding process.