603885 - Fix bad cases caused by swap-and-invalidate (implement swap-and-readback)

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

STR
 (1) Load URL

Expected results can be seen in desktop FF: the page takes a looong time to load (initial paint), but afterwards the yellow box moves around at a constant rate.

Actual results: in fennec, the yellow box moves around quickly at first but thereafter at a decreasing rate.

This is a contrived example, but highlights the big flaw in swap-and-invalidate: successive invalidated regions have to accumulate.  It's pretty easy to hit this case on "real" sites: if one focuses an input field, pans around so as to force prefetch-region updates, then goes back to the input field, we end up invalidating the union of all previously invalidated regions.  This brings back the "character input slow" bug, because we're invalidating more than just the input field.  This also hurts our toroidal-buffer optimization which makes updating after scrolls more expensive.

The fix is to swap back/front then read back the updated region from new-front to new-back.  Toroidal buffers make this a bit more complicated, but it's not too bad.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

This will be ready in time for b2, but blocking-bN is fine.

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 1: Move separate a method decl/def and reindent a long block

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 2: Set up the framework for swap-and-readback

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 3: Allow allocating gfxSharedImageSurface with unsafe Shmem

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 4: Use unsafe-shmem-backed gfxSharedImageSurface for IPC shadow layers

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 5: Move the code for getting a gfxContext for a quadrant update into its own method

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 6: Implement swap-and-readback

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 7: Remove front-buffer pre-fill code that is now subsumed by swap-and-readback (which also gets the pre-fill off the critical path for repainting)

Comment 9

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 8: Add logging for buffer creation and readback operations

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 483397 [details] [diff] [review]
part 2: Set up the framework for swap-and-readback

Do we use this Maybe convention elsewhere? Seems to me "Optional" would be clearer.

Comment 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

We previously used it in PPluginInstance, but no longer.  "Optional" suits me fine.

Comment 12

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Vlad: Review ping.

Comment 13

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 483397 [details] [diff] [review]
part 2: Set up the framework for swap-and-readback

Sorry, was traveling -- looks fine, but what's null_t() ?  If it's just nsnull, would prefer nsnull for the convention, but not very strongly.

Comment 14

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

null_t is a type that means "nothing", nsnull is a value of type void*.  null_t can be used in IPDL union types but nsnull can't, because it's a value.

Comment 15

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

http://hg.mozilla.org/mozilla-central/rev/dc526c027743
http://hg.mozilla.org/mozilla-central/rev/bffc3f0cf949
http://hg.mozilla.org/mozilla-central/rev/4f4736cb9e70
http://hg.mozilla.org/mozilla-central/rev/3c824e9712ac
http://hg.mozilla.org/mozilla-central/rev/b5086952fc37
http://hg.mozilla.org/mozilla-central/rev/d6b5d3e3ffbf
http://hg.mozilla.org/mozilla-central/rev/f2b5fc8dc7a3
http://hg.mozilla.org/mozilla-central/rev/1e1c3519b5a9