Improve scroll generation management
Categories
(Core :: Panning and Zooming, task)
Tracking
()
Tracking | Status | |
---|---|---|
firefox84 | --- | fixed |
People
(Reporter: kats, Assigned: kats)
References
(Blocks 1 open bug)
Details
Attachments
(3 files)
Things to do with respect to scroll generation management:
- Have each APZC instance track the generation of the most recent applied ScrollPositionUpdate in a member variable.
- Remove the scroll generation from the FrameMetrics as it shouldn't be needed there any more. (Keep the copy on the RepaintRequest, but populate it from the APZC member variable)
- Move the sScrollGenerationCounter from nsGfxScrollFrame.cpp to ScrollPositionUpdate.cpp as it maybe should get incremented by the ScrollPositionUpdate constructor.
- Extract a helper method in nsGfxScrollFrame to actually append a ScrollPositionUpdate instance into the mScrollUpdates array. Have this helper method also update the nsGfxScrollframe's mScrollGeneration from the ScrollPositionUpdate, rather than the other way around
- Increase the scroll generation from a 32-bit value to a 64-bit value to guard against wraparound problems.
Assignee | ||
Comment 1•4 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #0)
- Have each APZC instance track the generation of the most recent applied ScrollPositionUpdate in a member variable.
- Remove the scroll generation from the FrameMetrics as it shouldn't be needed there any more. (Keep the copy on the RepaintRequest, but populate it from the APZC member variable)
This seems a bit harder to do now, because of the mechanism in bug 1664838 comment 10. Specifically it seems like we still need a way to send a scroll generation from the main thread to APZ without any ScrollPositionUpdate instances, because the SPUs may have been sent previously and not processed by the APZ side. Maybe if we do some surgery on the repaint request side of things, so that we don't need this check then we might be able to do this still.
Assignee | ||
Comment 3•4 years ago
|
||
Note in particular the struct keeps its own static (per-process) counter value,
and uses a uint64_t.
Depends on D97034
Assignee | ||
Comment 4•4 years ago
|
||
Depends on D97035
Assignee | ||
Comment 5•4 years ago
|
||
Instead of the scrollframe manually calling ScrollGeneration::New this has the
ScrollPositionUpdate do it automatically. Just makes things a little more
encapsulated.
Note that this patch does contain a small functional change, which is that if
ScrollFrameHelper::ScrollToImpl is called with ScrollOrigin::apz, it will not
increment mScrollGeneration whereas it used to before. As far as I can tell
this should be a harmless change.
Depends on D97036
Comment 7•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/280e26de1068
https://hg.mozilla.org/mozilla-central/rev/4424cf5d56cd
https://hg.mozilla.org/mozilla-central/rev/e9b3009540c6
Description
•