Closed Bug 1662019 Opened 4 years ago Closed 4 years ago

Improve scroll generation management

Categories

(Core :: Panning and Zooming, task)

task

Tracking

()

RESOLVED FIXED
84 Branch
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:

  1. Have each APZC instance track the generation of the most recent applied ScrollPositionUpdate in a member variable.
  2. 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)
  3. Move the sScrollGenerationCounter from nsGfxScrollFrame.cpp to ScrollPositionUpdate.cpp as it maybe should get incremented by the ScrollPositionUpdate constructor.
  4. 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
  5. Increase the scroll generation from a 32-bit value to a 64-bit value to guard against wraparound problems.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #0)

  1. Have each APZC instance track the generation of the most recent applied ScrollPositionUpdate in a member variable.
  2. 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.

Note in particular the struct keeps its own static (per-process) counter value,
and uses a uint64_t.

Depends on D97034

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

Pushed by kgupta@mozilla.staktrace.com: https://hg.mozilla.org/integration/autoland/rev/280e26de1068 Create a dedicated struct for the scroll generation. r=tnikkel https://hg.mozilla.org/integration/autoland/rev/4424cf5d56cd Replace the uint32_t scroll generation with ScrollGeneration. r=tnikkel https://hg.mozilla.org/integration/autoland/rev/e9b3009540c6 Encapsulate the ScrollGeneration::New calls a bit more. r=tnikkel
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: