After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 552845 - Use DamageReportRawRectangles
Use DamageReportRawRectangles
Status: RESOLVED WONTFIX
Product: gtk+
Classification: Platform
Component: Backend: X11
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2008-09-18 22:37 UTC by Owen Taylor
Modified: 2016-01-24 06:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to switch to DamageReportRawRectangles (2.64 KB, patch)
2008-09-18 22:38 UTC, Owen Taylor
none Details | Review
Formal proof of correctness (3.02 KB, text/plain)
2008-12-19 20:44 UTC, Owen Taylor
  Details

Description Owen Taylor 2008-09-18 22:37:57 UTC
The current integration of damage reporting and the anti-expose queue doesn't work properly with DamageReportBoundingBox because you can get:

 - Window modified, damage event reported
 - GTK+ redraws parent window and queues anti-expose
 - Window modified again, no damage event reported (bounding box didn't change)
 - Damage event received by GTK+ and discarded because of the anti-expose

Simple fix is to switch to DamageeReportRawRectangles. The main downside is that if the redirected window isn't using double-buffering you could conceivably get many, many events being sent. But if the redirected window isn't using double-buffering, then you'll also get intermediate redraws and flashing, so I think this can basically be ignored.
Comment 1 Owen Taylor 2008-09-18 22:38:34 UTC
Created attachment 118976 [details] [review]
Patch to switch to DamageReportRawRectangles
Comment 2 Allison Karlitskaya (desrt) 2008-09-19 23:17:13 UTC
I agree that this is probably the best thing to do (without making modifications to the X server).  It kinda sucks, though :/

It may be worth opening a discussion with some of the Xorg guys about this.  They seem to be willing to accept proposals for improvement to these sorts of things.
Comment 3 Owen Taylor 2008-09-20 13:20:20 UTC
It's hard to imagine what the X server should do better; for clients that do proper double buffering, we want damage reported instantly and with the correct serials for when the damage occurs.

All I can really imagine is something like:

 If more than 1000 rectangles of damage are reported for a client over
 a 1 second period, then the client is put (permanently?) in the "penalty box" 
 and damage for that client is accumulated and reported in the server's
 block handler.

Which sounds hard to implement and prone to false alarms.
Comment 4 Allison Karlitskaya (desrt) 2008-09-20 18:30:30 UTC
On second thought, (just for discussion/ideas/brainstorming) let's look at the other side: what are we doing wrong?

We're telling the X server that a damaged region has been "repaired" when actually we've done no such thing (due to anti-expose).  Unfortunately, this is actually the -normal- code path, the the repair really does need to be reported.  In any case, though, it's definitely wrong to call DamageSubtract() if we're not actually doing any updates.

So, what if we did XDamageSubtract at the time that we're actually doing the repairs (ie: when we invalidate the parent area, or from the same context that's calling the expose event handler of the parent, just before the handler is called)?

The anti-expose handler would take down the damage event that we'll get "later" (even though it's already in-flight) as a result of the child drawing.  Any additional damage that was done in the mean time will be ignored as long as it happens before the expose event handler (which is fine, because then the handler would take care of it anyway).  Anything that happens later causes a redraw.

I think this would work and it would avoid having to use RawRectangles...  Discuss? :)
Comment 5 Matthias Clasen 2008-11-29 19:20:34 UTC
Any conclusion here ?
Comment 6 Owen Taylor 2008-12-19 20:43:36 UTC
Assume that we are implementing Ryan's idea by doing:

 A) Record anti-expose
 B) Subtract damage from subwindows
 C) Handle expose

Then the shortest argument I can come up with with why that's correct:

 1) If we draw to a pixel that's already in the damage region of a subwindow,
    then we haven't gotten to B) yet in processing the damage event that 
    added that pixel, so we are still going to do C).

 2) If we draw to a pixel that's not already in the damage region of a subwindow,
    then we'll generate a new damage event. That damage event can only be
    cancelled by an A) that is recorded happens after our drawing, so C) still 
    going to draw the pixel.

That's not really a fully satisfactory proof, since it doesn't get to the
reason A) needs to go before B). (To avoid cancelling damage from new drawing
that happens after B.) I'll attach something more cumbersome that covers
all the bases.

Anyways, I'm convinced of the correctness, and if Ryan wants to come up with
a patch, that would be a fine fix (not that the RawRectangles approach doesn't
work as well.)
Comment 7 Owen Taylor 2008-12-19 20:44:09 UTC
Created attachment 125028 [details]
Formal proof of correctness
Comment 8 Matthias Clasen 2016-01-24 06:38:05 UTC
sadly, the patch never happened