Summary: | The x11 device garbles output when -dMaxBitmap is used | ||
---|---|---|---|
Product: | Ghostscript | Reporter: | Ray Johnston <ray.johnston> |
Component: | X Display Driver | Assignee: | Ray Johnston <ray.johnston> |
Status: | RESOLVED FIXED | ||
Severity: | blocker | CC: | ralph.giles, till.kamppeter |
Priority: | P1 | ||
Version: | master | ||
Hardware: | All | ||
OS: | Linux | ||
Customer: | Word Size: | --- | |
Attachments: |
gdevbbox.c.patch
gdevbbox.c.patch |
Description
Ray Johnston
2009-01-29 11:02:31 UTC
I've traced this to part of Igor's changes when the 'fillpage' was added to the bbox device. He removed a check for 'RECT_IS_PAGE', but mistakenly left in the invocation of BBOX_INIT_BOX. The old code was: if (RECT_IS_PAGE(dev, x, y, w, h)) { if (!BBOX_INIT_BOX(bdev)) return code; } and he changed it to: if (!BBOX_INIT_BOX(bdev)) return code; in both places, so the bounding box information was getting reset to an empty box MUCH to often. This caused the x11 device to miss copying many areas from the image24 device to the screen. Since the bbox device is used by customers directly (not just in the x11 device) this regression is CRITICAL in my opinion and should be applied to 8.64 Created attachment 4773 [details]
gdevbbox.c.patch
This restores correct behavior to the bbox device and the x11 (and x11alpha)
devices now work at least as well as in 8.63.
Note that there is one formatting change in a comment that is not strictly
needed.
This patch fixes bug 690222. *** Bug 690222 has been marked as a duplicate of this bug. *** Before adding this to the release, please verify that the following bugs do not reappear when applying the patch: http://bugs.ghostscript.com/show_bug.cgi?id=689548 http://bugs.ghostscript.com/show_bug.cgi?id=689562 https://bugs.launchpad.net/ubuntu/+source/ghostscript/+bug/160203 I did a quick look and the files I tested give the wrong results. I have done some checks and I can at lrast reproduce that example3.ps of https://bugs.launchpad.net/ubuntu/+source/ghostscript/+bug/160203 gives an empty bounding box, but I am not sure whether this was already the case befgore applying the patch. example.ps and example2.ps of that bug report seem to be OK. I checked Bug 689548.ps and with the patch I get the correct results as described in that comment #11. http://bugs.ghostscript.com/show_bug.cgi?id=689548#c8 without the patch, it returns 0's W.R.T. bug 689562, I get the same results with or without the patch (the correct nnumbers, not 0's), so the patch has no effect. AFAICT, this file is the same as the 'inline' file in the 160203 Ubuntu bug reprot. With 'example2.ps' from 160203, I see correct numbers with the patch, and 0's without it. However, I still see 0's with example3.ps I still see 0's. I've determined that adding a line with %!PS-Addobe is enough to make it work. I will investigate this, but since this is a MUCH more rare case (a PS file without any sort of header), this may not be a 'blocker'. I determined that the one remaining problem with 'example3' returning 0's (related to ubuntu bug 160203) is caused by the poorly commented fix applied to ghostscript in rev 9314 (by till). The log message says: Fix segmentation fault on -sDEVICE=bbox, contributed to Debian by Yaroslav Halchenko. Since there is no bug reference or any other details, I cannot determine the conditions that caused the segfault, and whether or not this was really the correct fix. Please provide background information (links?) for this change so I can make an assessment on reverting this change. Here are the Debian bug reports about the bbox segfault: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=250290 http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=254877 The first of the two contains the patch. Feel free top revert it, there is nothing else known to me about it. I believe that this patch is archaic (since references to it in the other bugs mentioned date back to 2004) and I am fairly certain that there are other patches that make the check for bbox->is_open irrelevant, including: ============================================================================== r8161: Fix problem seen with stcolor device where the get_color_mapping_procs device proc was NULL, causing SEGV. Bug #689371. DETAILS: The fix was local to gdevstc.c. The other modules were changed to add protection in case other devices (such as all of the new devices in contrib) don't manage to change the get_color_mapping_procs from the usual default of NULL in the device structure. Rather than dprintf or error messages that may be supressed with #if DEBUG. I elected to use eprintf. Note that the get_color_mapping_procs is usually set up by gdevdflt gx_device_fill_in_procs, but there are several cases where we don't know what to do, so we set the gx_error_get_color_mapping_procs. ============================================================================== At this point I am leaning toward reverting the 9314 patch and proceding. Created attachment 4774 [details]
gdevbbox.c.patch
This patch includes the previous patch and the RECOMMENDED patch to remove
the check for bdev->is_open from the test for initialization of the 'black'
and 'white' colors. Having these colors both remain at the default of 0
causes painting black to be ignored, thus resulting in a bbox of 0's for
the 'example3.ps' case of the ubuntu bug repprt.
Ray: please commit the reversion of r9314 separately, Then commit your bbox fix for the breakage from r9288 on top of it. Both should go in for 8.64. Thanks for all your work tracking these down. In passing; this also fixes a problem I observed with pswrite where the BoundingBox and HiResBoundingBox comments were always zero. Patches committed separately as Ralph requested in revs 9430 and 9431. |