Bug 80199 – [chromium] Make compositeAndReadback and damage tracking play nicely together

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Shawn Singh]

Issue reported in: http://code.google.com/p/chromium/issues/detail?id=116055

compositeAndReadback causes the composited rendering path to proceed, including damage tracking, but in single-threaded mode, compositeAndReadback does not actually affect the front buffer (which is the purpose of damage tracking).  As a result, damage tracking gets out-of-sync with what really changed on the front buffer, and some repaints do not actually show up on the screen when partial swap is enabled.

Jonathan, when you have a chance, can you please try to reproduce with --enable-threaded-compositing?  (remember, HUD and rect visualization won't show up in multi threaded mode yet)  The error should be un-reproducible, because as I understand, swapBuffers is actually performed on compositeAndReadback in multi-threaded mode.  In my own linux osmesa attempts, I could not reproduce the problem in multi-threaded mode, and I could not reproduce the problem when I hacked a swapBuffers in single-threaded mode.

There are 3 options I see to fix this problem.  Nat can you please suggest which fix you think is best?  I think option #3 is correct, and the other two are hacky.  Thanks in advance.
  1. do a swapBuffers on compositeAndReadback for single threaded mode.
  2. bypass damage tracking in composite-and-readback single threaded mode.
  3. (seems like the right thing to do...) change damage tracking to *accumulate* dirtiness until swapbuffers is actually performed.  Eventually, when we can support per-surface scissoring, we can reset the dirty-accumulation after each surface is updated.

Let me know then I will proceed ... thanks!

[Shawn Singh]

Created attachment 130436 [details]
Patch

The fix is trivial, but testing it is not so obvious.  Testing this fix requires observing front-buffer changes. As I understand, this could be done via chromium browser gpu pixel tests, but it won't happen in layout tests or webkit_unit_tests.  Is it worth going through that trouble?  ... also, I think context lost issues are already addressed by the existing code in compositeAndReadback (exits early if doComposite returns false).

[Shawn Singh]

For clarity - Nat and I had discussed offline, and for now we're considering option #1.  Thanks

[Nat Duca]

Comment on attachment 130436 [details]
Patch

LGTM but copy the check for context lost at :85 up to :82 before calling swap. That has an assertion inside it that will pop if you call it with a lost context.

[Shawn Singh]

Created attachment 130442 [details]
contextLost() check placed before swap

[James Robinson]

Comment on attachment 130442 [details]
contextLost() check placed before swap

Seems good, R=me

[Shawn Singh]

Comment on attachment 130442 [details]
contextLost() check placed before swap

Cool, thanks for reviewing.

[WebKit Review Bot]

Comment on attachment 130442 [details]
contextLost() check placed before swap

Clearing flags on attachment: 130442

Committed r109965: <http://trac.webkit.org/changeset/109965>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.