Closed
Bug 603885
Opened 14 years ago
Closed 14 years ago
Fix bad cases caused by swap-and-invalidate (implement swap-and-readback)
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
fennec | 2.0b3+ | --- |
People
(Reporter: cjones, Assigned: cjones)
References
()
Details
Attachments
(8 files)
(deleted),
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
roc
:
review+
vlad
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
joe
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
joe
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•14 years ago
|
||
This will be ready in time for b2, but blocking-bN is fine.
tracking-fennec: --- → ?
Updated•14 years ago
|
tracking-fennec: ? → 2.0b3+
Assignee | ||
Comment 2•14 years ago
|
||
Assignee: nobody → jones.chris.g
Attachment #483396 -
Flags: review?(roc)
Assignee | ||
Comment 3•14 years ago
|
||
Attachment #483397 -
Flags: superreview?(vladimir)
Attachment #483397 -
Flags: review?(roc)
Assignee | ||
Comment 4•14 years ago
|
||
Attachment #483398 -
Flags: review?(joe)
Assignee | ||
Comment 5•14 years ago
|
||
Attachment #483399 -
Flags: review?(joe)
Assignee | ||
Comment 6•14 years ago
|
||
Attachment #483400 -
Flags: review?(roc)
Assignee | ||
Comment 7•14 years ago
|
||
Attachment #483401 -
Flags: review?(roc)
Assignee | ||
Comment 8•14 years ago
|
||
Attachment #483402 -
Flags: review?(roc)
Assignee | ||
Comment 9•14 years ago
|
||
Attachment #483403 -
Flags: review?(roc)
Updated•14 years ago
|
Attachment #483398 -
Flags: review?(joe) → review+
Updated•14 years ago
|
Attachment #483399 -
Flags: review?(joe) → review+
Attachment #483396 -
Flags: review?(roc) → review+
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.
Attachment #483397 -
Flags: review?(roc) → review+
Attachment #483400 -
Flags: review?(roc) → review+
Attachment #483401 -
Flags: review?(roc) → review+
Attachment #483402 -
Flags: review?(roc) → review+
Attachment #483403 -
Flags: review?(roc) → review+
Assignee | ||
Comment 11•14 years ago
|
||
We previously used it in PPluginInstance, but no longer. "Optional" suits me fine.
Assignee | ||
Comment 12•14 years ago
|
||
Vlad: Review ping.
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.
Attachment #483397 -
Flags: superreview?(vladimir) → superreview+
Assignee | ||
Comment 14•14 years ago
|
||
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.
Updated•14 years ago
|
Status: NEW → ASSIGNED
Whiteboard: [fennec-checkin-postb2][has-patch]
Assignee | ||
Comment 15•14 years ago
|
||
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
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
Whiteboard: [fennec-checkin-postb2][has-patch]
You need to log in
before you can comment on or make changes to this bug.
Description
•