Closed
Bug 804801
Opened 10 years ago
Closed 8 years ago
Simplify mValidRegion after painting
Categories
(Firefox for Android Graveyard :: Toolbar, defect)
Tracking
(Not tracked)
RESOLVED
INVALID
People
(Reporter: BenWa, Unassigned)
References
Details
Attachments
(1 file)
994 bytes,
patch
|
cwiiis
:
review+
|
Details | Diff | Splinter Review |
I've seen mValidRegion get very complex and this happens very frequently with progressive drawing. Here's a typical 16 part mValidRegion that is identical to mVisibleRegion after the paint: I/Gecko (29153): mValidRegion [513,768,2305,1280; 513,1280,1024,1536; 1024,1280,1792,3072; 1792,1280,2305,1536; 513,1536,1024,1792; 1792,1536,2305,1792; 513,1792,1024,2048; 1792,1792,2305,2048; 513,2048,1024,2304; 1792,2048,2305,2304; 513,2304,1024,2560; 1792,2304,2305,2560; 513,2560,1024,2816; 1792,2560,2305,2816; 513,2816,1024,3072; 1792,2816,2305,3072] I/Gecko (29153): mVisibleRegion [513,768,2305,3072] The trick is simple, assign mVisibleRegion to mValidRegion after a progressive paint to simplify the region.
Attachment #674413 -
Flags: review?(chrislord.net)
Reporter | ||
Updated•10 years ago
|
Attachment #674413 -
Attachment is patch: true
Comment 1•10 years ago
|
||
Comment on attachment 674413 [details] [diff] [review] patch Review of attachment 674413 [details] [diff] [review]: ----------------------------------------------------------------- Well caught - nit about the comment, but nicely done. ::: gfx/layers/basic/BasicTiledThebesLayer.cpp @@ +358,5 @@ > // The transaction is completed, store the last scroll offset. > mLastScrollOffset = aScrollOffset; > + NS_ASSERTION(mValidRegion.IsEqual(mVisibleRegion), > + "valid region doesn't match visible region after progressive paint."); > + // mVisibleRegion is equal to mValidRegion but is a simpler reduce so assigning it here Maybe just because it's late, but 'is a simpler reduce' doesn't read well to me - s/reduce/region/? or reduction? Or perhaps just // mVisibleRegion is equal to mValidRegion, but simpler, so assign it to the // latter to avoid unnecessarily complex regions.
Attachment #674413 -
Flags: review?(chrislord.net) → review+
Reporter | ||
Comment 2•10 years ago
|
||
Pushed to try since I added an assert and I'm not running under debug to catch it. https://tbpl.mozilla.org/?tree=Try&rev=a8585e6b5622
Comment 3•10 years ago
|
||
Is this patch still valid? Should it be landed?
Updated•8 years ago
|
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → INVALID
Updated•2 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•