RE: FW: Fix 687189 for bbox_forward_add_rect infiniterecursion

Igor,

>Hmm, I'm not sure that this goes right.
>I would prefer to apply an agebraic consideration.
>
>In the beginning we've got bbox-->target .
>After bbox_create_compositor we get bbox-->compositor-->bbox-->target.
>IMO it is superfluous, because a compositor doesn't change a bounding box.
(Does it ?)

The overprint compositor probably does not change the bounding
box.  That assumes that the trivial case of overprinting with no
separation planes is ignored.  However the compositor mechanism
is also used for PCL/XL raster ops and this definitely can
affect the bounding box.

I admit that it is not obvious to me why the multiple bounding
box devices are ever needed,  I did not design the x11 and x11alpha
devices but I assume that bounding box device is some how used
for determining the size of regions that need to be sent on to
the x11 handler (external to Ghostscript) for updating the display.
To me, this implies the need for only one bounding box device.

However, I am assuming that the original designer of the x11
and x11alpha devices (probably Peter) knew what he was doing and
that there is some need for the multiple bounding box devices in
some case.

Since the bounding box device imposes little in the way of overhead
and there are other tasks that have value to our users and we have
finite resources, I see little justification for an effort to
further optimize this code.  I will change my mind about the
previous statement if someone can show that that the extra bounding
box degrades x11 performance significantly.  Prior to this fix,
there were hundreds of bounding box devices being chained together
for some files and this did produce noticeable slow down.  But I
have not seen evidence that one or two is significant.


Dan



-----Original Message-----
From: Igor V. Melichev [mailto:[email protected]]
Sent: Tuesday, April 13, 2004 12:33 AM
To: [email protected]; Gs-Code-Review
Subject: Re: [gs-code-review] FW: Fix 687189 for bbox_forward_add_rect
infiniterecursion


Hmm, I'm not sure that this goes right.
I would prefer to apply an agebraic consideration.

In the beginning we've got bbox-->target .
After bbox_create_compositor we get bbox-->compositor-->bbox-->target.
IMO it is superfluous, because a compositor doesn't change a bounding box.
(Does it ?)
If it doesn't, I'm unclear why do we need the second bbox.
I would go with compositor-->bbox-->target, and the number of anomalies
(such as page erasing) should decrease.

Another way (which  looks even more attractive) is bbox_create_compositor
to convert bbox-->target into bbox-->compositor-->target.
This requires a little more access rights for gx_device::create_compositor,
but again we reduce anomalies.

In any case, since a bbox device and a compositor device are commutative
(i.e. any1-->bbox-->compositor-->any2 should give same external effect as
any1-->compositor-->bbox-->any2), then any chain
any1-->+{bbox-->compositor}-->any2
is equivalent to any1-->bbox-->compositor-->any2 and to
any1-->compositor-->bbox-->any2
and implementations of bbox should take care of this law,
and they may use this law for a simplification.

Igor.

----- Original Message -----
From: "Dan Coby" <[email protected]>
To: "Gs-Code-Review" <[email protected]>
Sent: Tuesday, April 13, 2004 12:58 AM
Subject: [gs-code-review] FW: Fix 687189 for bbox_forward_add_rect
infiniterecursion


>
> Fix 687189 for bbox_forward_add_rect infinite recursion.
> This problem caused segment faults or apparent hangs with
> the x11alpha device.  This fix also works for 656414
> segfault when using x11alpha
>
> DETAILS:
>
> The problem was due to the bounding box device creating
> more and more bounding box devices in the device chain
> when the overprint compositor was being used.  The x11alpha
> device uses the bounding box device.  The overprint logic
> uses the 'create compositor' logic when a change is made
> in the status of the overprint or overprint mode parameters.
> However a new compositor is not created each time.  Instead
> the overprint device simply updates its parameters.  The
> bounding box device has its own create compositor.  This
> routine would call its target device's create compositor
> routine and then wraps a new bounding box device around the
> target's compositor.  The combination of many create
> compositor calls from the overprint logic and the fact that
> the bounding box device adds a new bounding box device
> could result in hundreds of bounding devices in the chain.
>
> The fix consists of checking if the bounding box's target
> device actually created a new compositor device.  Only when
> a new compositor was added is a new bounding box also added
> to the chain.
>
> [End log message]
>
> diff -u -r1.15 gdevbbox.c
> --- a/src/gdevbbox.c    14 Oct 2003 12:43:18 -0000      1.15
> +++ b/src/gdevbbox.c    8 Apr 2004 03:27:41 -0000
> @@ -1170,8 +1170,11 @@
>         int code = (*dev_proc(target, create_compositor))
>             (target, &cdev, pcte, pis, memory);
>
> -       if (code < 0)
> +       /* If the target did not create a new compositor then we are done.
> */
> +       if (code < 0 || target == cdev) {
> +           *pcdev = dev;
>             return code;
> +       }
>         bbcdev = gs_alloc_struct_immovable(memory, gx_device_bbox,
>                                            &st_device_bbox,
>                                            "bbox_create_compositor");
>
>
> This problem was noticed with the following test files in
> addition to the original file in the problem report.  This
> fix corrects the original problem.  However another problem
> is visible with some of the files.  The files marked with an
> 'X' erase the page after drawing the entire page.  The files
> marked with an 'XX' erase the page one or more times while
> drawing the page.
>
> 1_2001.pdf X
> Altona-Testsuite_p2_S_x3.pdf
> Altona.Page_3.2002-09-27.pdf X
> BEST8-99-Path.fh7.pdf X
> BW0696FOLD1FRONT.pdf
> CAIB_highres_page4.pdf
> CIDembedded.pdf
> Fixed_Original.pdf X
> HeiseiMinStd.pdf
> ICPconcept.pdf
> Openhuis_pdf_zw.pdf XX
> Original.pdf X
> S2_Digitalproof-Forum_circles.pdf X
> S2_Digitalproof-Forum_x3k.pdf
> Testform.v1.0.2.pdf XX
> adesso8.pdf
> file.pdf X
> messenger.pdf X
> messenger16.pdf X
> non-sepqxd2distiller.pdf X
> test.pdf
> p02.pdf X
> plt_paper_503.eps XX
>
> Due to the difference is the symptoms, I am assuming that this
> is a different problem (possibly related).  I have verified that
> with this fix, the chains of multiple bounding box devices is
> not created with these files.  I am creating a new bug report for
> this latter problem.
>
>
> Dan
>
> _______________________________________________
> gs-code-review mailing list
> [email protected]
> http://www.ghostscript.com/mailman/listinfo/gs-code-review
>