Open
Bug 1259593
Opened 8 years ago
Updated 2 years ago
Displayport base rect can get clipped to empty
Categories
(Core :: Panning and Zooming, defect, P5)
Tracking
()
REOPENED
mozilla48
People
(Reporter: kats, Unassigned)
References
Details
(Keywords: perf, Whiteboard: [gfx-noted])
Attachments
(2 files, 1 obsolete file)
If you go to https://github.com/mozilla/gecko-dev/blob/master/layout/base/FrameLayerBuilder.cpp, scroll the inner subframe so it layerizes, and then scroll the outer page fast, you can end up in a state where the inner frame gets an empty displayport base rect. It's transient but undesirable, as it can result in unnecessary checkerboarding. I had a discussion with tnikkel about this (attached). We discovered that we get into this situation when the displayport we're painting doesn't intersect the main-thread's notion of what's visible. This can happen (currently) under two conditions: one is that velocity bias pushes the displayport out in the direction of scrolling so far that it no longer intersects the scroll position. The other is that the peek-messages code gets a future displayport that is far ahead of the current scroll position. In both of these cases, the intersection of the dirty rect, scroll port, and transformed root composition bounds ends up empty, because the dirty rect (which derives from the displayport) uses these "future scroll positions" whereas the transformed root composition bounds does not. Some possible approaches to fixing this are discussed at the end of the IRC conversation.
Comment 1•8 years ago
|
||
Just to add one last thought to the simple solution from the log. Instead of transforming the root comp bounds and intersecting with displayPortBase, we should transform the root comp bounds, then limit displayPortBase to the same width/height of the transform root comp bounds, taking an equal amount from the top and bottom/left and right.
Reporter | ||
Updated•8 years ago
|
Assignee: nobody → bugmail.mozilla
Reporter | ||
Comment 2•8 years ago
|
||
Attachment #8736393 -
Flags: review?(tnikkel)
Updated•8 years ago
|
Attachment #8736393 -
Flags: review?(tnikkel) → review+
Reporter | ||
Comment 3•8 years ago
|
||
Same patch, with r=tnikkel
Attachment #8736393 -
Attachment is obsolete: true
Attachment #8736881 -
Flags: review+
Comment 5•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/abe2031a710e
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Reporter | ||
Updated•8 years ago
|
status-firefox46:
--- → wontfix
status-firefox47:
--- → affected
Reporter | ||
Comment 6•8 years ago
|
||
I think patch actually made things worse somehow, leading to issues like bug 1259593. I'm going to back it out.
Reporter | ||
Comment 7•8 years ago
|
||
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/bd68133cf771
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter | ||
Comment 8•8 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #6) > I think patch actually made things worse somehow, leading to issues like bug > 1259593. I'm going to back it out. This should say "... like bug 1262151"
Reporter | ||
Comment 9•8 years ago
|
||
After thinking about this some more I think we definitely need to intersect mScrollPort with the transform root composition bounds. If we don't do that, then we don't account for the size change that occurs as the subframe moves out of the visible portion of the parent (e.g. scrolling down from y=0 to y=100 on [1] should increase the height of the subframe's base rect by 100). Right now the end result that we compute is the intersection of the above with the dirty rect, which has the "future scroll position" applied. I think that intersection doesn't make sense because we are intersecting things with temporally different scroll positions; instead we want to do a MoveInsideAndClamp. IOW aDirtyRect->MoveInsideAndClamp(mScrollPort.Intersect(rootCompBounds)) seems to fix this issue for me and as far as I can tell doesn't incur any badness. [1] https://github.com/mozilla/gecko-dev/blob/master/layout/base/FrameLayerBuilder.cpp
Reporter | ||
Comment 10•8 years ago
|
||
That being said, I'm not confident that it doesn't incur any badness. It's true that this would result in the base rect including parts of the dirty rect that it wasn't before, so it's quite possible that we'll do something bad. I'm going to let this bug sit for a while since there's no user-visible impact of it as far as I can tell, it's just something I noticed that results in inefficient behaviour.
Priority: -- → P5
Reporter | ||
Updated•8 years ago
|
Attachment #8736881 -
Flags: checkin-
Reporter | ||
Updated•8 years ago
|
Assignee: bugmail.mozilla → nobody
Reporter | ||
Updated•8 years ago
|
Reporter | ||
Updated•8 years ago
|
status-firefox49:
--- → wontfix
status-firefox50:
--- → affected
Reporter | ||
Updated•8 years ago
|
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•