Bug 690260 - The x11 device garbles output when -dMaxBitmap is used
Summary: The x11 device garbles output when -dMaxBitmap is used
Status: RESOLVED FIXED
Alias: None
Product: Ghostscript
Classification: Unclassified
Component: X Display Driver (show other bugs)
Version: master
Hardware: All Linux
: P1 blocker
Assignee: Ray Johnston
URL:
Keywords:
: 690222 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-01-29 11:02 UTC by Ray Johnston
Modified: 2009-02-02 00:59 UTC (History)
2 users (show)

See Also:
Customer:
Word Size: ---


Attachments
gdevbbox.c.patch (1.03 KB, patch)
2009-01-31 19:57 UTC, Ray Johnston
Details | Diff
gdevbbox.c.patch (1.31 KB, patch)
2009-02-01 21:48 UTC, Ray Johnston
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ray Johnston 2009-01-29 11:02:31 UTC
The x11 output is badly garbled when -dMaxBitmap is large enough to cause the
image buffer to be used.

The command line that shows this is:

   bin/gs -sDEVICE=x11 -r90 -dMaxBitmap=3000000 examples/tiger.eps

Note this problem did not exist in 8.63.
Comment 1 Ray Johnston 2009-01-31 19:54:27 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
Comment 2 Ray Johnston 2009-01-31 19:57:03 UTC
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.
Comment 3 Till Kamppeter 2009-02-01 04:30:25 UTC
This patch fixes bug 690222.
Comment 4 Till Kamppeter 2009-02-01 04:30:50 UTC
*** Bug 690222 has been marked as a duplicate of this bug. ***
Comment 5 Till Kamppeter 2009-02-01 08:10:49 UTC
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.
Comment 6 Till Kamppeter 2009-02-01 08:21:14 UTC
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.
Comment 7 Ray Johnston 2009-02-01 10:16:55 UTC
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'.




Comment 8 Ray Johnston 2009-02-01 12:31:20 UTC
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.
Comment 9 Till Kamppeter 2009-02-01 15:34:54 UTC
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.
Comment 10 Ray Johnston 2009-02-01 18:04:04 UTC
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.
Comment 11 Ray Johnston 2009-02-01 21:48:02 UTC
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.
Comment 12 Ralph Giles 2009-02-01 23:56:03 UTC
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.
Comment 13 Ken Sharp 2009-02-02 00:19:40 UTC
In passing; this also fixes a problem I observed with pswrite where the
BoundingBox and HiResBoundingBox comments were always zero.
Comment 14 Ray Johnston 2009-02-02 00:59:41 UTC
Patches committed separately as Ralph requested in revs 9430 and 9431.