RE: Proposed fix for 687421; page cleared in x11alpha

Raph,

I also have no idea about why gx_forward_get_page_device had logic
for returning the forwarding device if the target device was the
page device.  It does look a very strange piece of logic.

Does this change make any differences in the regression testing?


Dan


-----Original Message-----
From: [email protected]
[mailto:[email protected]]On Behalf Of Raph Levien
Sent: Thursday, September 16, 2004 3:58 PM
To: [email protected]
Subject: [gs-code-review] Proposed fix for 687421; page cleared in
x11alpha 


Hi code reviewers,

   I think I've nailed down the bug that was clearing the page when
doing overprints in the x11alpha device, but am lacking confidence
that my fix doesn't break something else.

Analysis of the bug

   The page is being cleared by a callout to %grestorepagedevice
(which, in turn does a .installpagedevice, which does the erasepage),
when z2grestore() determines from a call to restore_page_device() that
the change of page device is necessary. The logic of
restore_page_device() is a little complicated, but basically it's
trying to determine whether the page device has changed since the
corresponding gsave saved the graphics state. The _device_ changes
when overprinting inserts an overprint compositor device into the
chain, but I believe the page device should not change. From what I
can see, the correct logic is for forwarding devices to always forward
the request to obtain the page device, so restore_page_device() will
always detect the same page device at the end of the page.

   Jan seems to have noted this problem, at least in part, when he
implemented the overprint compositor. I note the following comment in
overprint_get_page_device(). My patch removes this comment, as I
believe the patch to gdevnfwd.c fixes the underlying issue.

 * The overprint device must never be confused with a page device. The
 * default procedure for forwarding devices, gx_forward_get_page_device,
 * will masquerade as a page device if its immediate target is a page
 * device. We are not certain why this is so, and it looks like an error
 * (see, for example, zcallinstall in zdevice2.c).

   The overprint device does forward the get_page_device request, but
the bbox device does not do so consistently, for two reasons. First,
the forwarding device has logic (as noted above) to "masquerade as a
page device" (as opposed to forwarding the request) if the target's
get_page_device returns a device equal to the target. Second, even
when the bbox device is configured as a forwarding device (as is the
case when invoked by x11), it uses the get_page_device routing from
the bbox device (which is a page device) rather than the forwarding
device.

The proposed patch

   The attached patch fixes these problems in a rather straightforward
way. It changes the logic in gx_forward_get_page_device() to always
forward the request. Also, the bbox device now installs this
forwarding version of get_page_device when configured in forwarding
mode.

Archaeology

   Why does gx_forward_get_page_device() have this somewhat tricky
logic to sometimes masquerade as a page device? Digging through
historical sources, I see this change note for the 4.80 release:

	- The forwarding implementation of get_page_device didn't do the
right thing.  (gdevnfwd.c)

   I believe the current HEAD version is identical to 4.80. The
previous version, 4.70, had this implementation:

gx_device *
gx_forward_get_page_device(gx_device *dev)
{	gx_device *tdev = fdev->target;
	return (tdev == 0 ? gx_default_get_page_device(dev) :
		(*dev_proc(tdev, get_page_device))(tdev));
}

   Presumably, the patch from 4.70 to 4.80 was added to fix some
problem. What was that problem? Are we likely to trip across it now,
or has the infrastructure of forwarding devices changed enough since
then that the problem has gone away?

Raph