create a minimal display port type that can be used for bug 1675547 (fission hit testing)
Categories
(Core :: Panning and Zooming, enhancement)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox87 | --- | fixed |
People
(Reporter: tnikkel, Assigned: tnikkel)
References
Details
Attachments
(5 files)
|
Assignee | |
Comment 1•3 years ago
|
||
We introduce a new type of display port, a minimal display port. It is controlled via a property on the content element. When the property is present any other display port specified on the element is ignored and instead the display port rect is computed by assuming 0 display port margins and no alignment (this reuses the existing code for display port suppression).
We then add code to set a minimal display port on every scroll frame that is painted that has WantAsyncScroll() when certain prefs are set (the prefs are disabled as of this patch though).
We then need to manage removing the minimal display port property when, before this patch, we would have created a regular display port. As well we need to add the minimal display port property when, before this patch, we would have removed a regular display port.
In order to do this I audited all sites where we set the display port rect and display port margins property. The changes to the code for handling the removal display ports happens in a later patch.
My audit found that all of the places we set a display port want to clear the minimal display port property except:
-UpdateSub/RoomFrame in APZCCallbackHelper
-UpdateDisplayPortMarginsForPendingMetrics in DisplayPortUtils
UpdateDisplayPortMarginsForPendingMetrics is basically a fast path of the UpdateSub/RoomFrame code. These are the places where we handle calls to RequestContentRepaint from apz. By adding an assert and running it through try server I found that UpdateSub/RoomFrame can create a display port in the following cases:
-a scroll info layer
-a scroll frame with !WantAsyncScroll() (the main thread never creates a display port for a scroll frame with !WantAsyncScroll()) (for example if the main thread creates a scroll id and sends over metadata via nsLayoutUtils::GetRootMetaData, and then the scroll rect changes, that will cause a RequestContentRepaint call)
-a few instances that don't fall into the above that happened on try server but didn't reproduce for me locally, so I don't know more about them.
It's not very important whether we clear the minimal display port property for these cases or not (the first two cases we don't async scroll the scroll frame at all, the last case seems quite rare).
Note that we intentionally do not change the existing behaviour of zero margin display ports set via SetZeroMarginDisplayPortOnAsyncScrollableAncestors as we are aiming for no behaviour changes with this patch (until we flip the pref). A later patch in a different bug handles changing these display ports over to minimal display ports.
|
|
Assignee | |
Comment 2•3 years ago
|
||
This patch is the result of auditing all places that look at the presence or absence of a display port to handle minimal display ports (HasDisplayPort, GetDisplayPort, etc).
Broadly speaking the places were in two categories:
- things related to painting, that want to consider minimal display ports as display ports for purposes of things like sending over metadata and separating out layers.
- things that care about async scrolling, and so actually want to have a properly sized display port.
Type 1) were not changed by this patch. Type 2) were changed to consider minimal display ports as not display ports.
Again, we are aiming to leave behaviour unchanged.
Depends on D103855
|
|
Assignee | |
Comment 3•3 years ago
|
||
Depends on D103856
|
|
Assignee | |
Comment 4•3 years ago
|
||
In the case that we have a painted minimal display port apzc knows about the scroll frame already, it just has them minimal amount of painted content. So we can tell apz right away. Note that the async transform for minimal dp's are still the identity so we'll still jank minimap dp's before the painted regular dp reaches the apzc.
Depends on D103857
|
|
Assignee | |
Comment 5•3 years ago
|
||
If the activate_all_scroll_frames pref is on we want to never remove a minimal display port, and when a regular display port expires we want to downgrade it to minimal.
If the pref is off it's the same as before except we want to also remove the minimal property when on expiry.
Depends on D103858
Pushed by tnikkel@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/544291ca47c1 Create a minimal display port type. r=botond https://hg.mozilla.org/integration/autoland/rev/8c822bc94990 Update code that looks for the presence of a display port for new minimal display ports. r=botond https://hg.mozilla.org/integration/autoland/rev/8badffb98c05 Prefer to jank minimal display ports until we can set a proper display port over checkerboarding them. r=botond https://hg.mozilla.org/integration/autoland/rev/22cf9fa5ef47 If in PrepareForSetTargetAPZCNotification we go from a minimal display port to a regular display port return the painted status of the existing (minimal) display port instead of always telling the caller to wait for a refresh. r=botond https://hg.mozilla.org/integration/autoland/rev/11bf30988746 Update display port expiry to handle minimal display ports. r=botond
|
||
Comment 7•3 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/544291ca47c1
https://hg.mozilla.org/mozilla-central/rev/8c822bc94990
https://hg.mozilla.org/mozilla-central/rev/8badffb98c05
https://hg.mozilla.org/mozilla-central/rev/22cf9fa5ef47
https://hg.mozilla.org/mozilla-central/rev/11bf30988746
|
|
Assignee | |
Comment 8•3 years ago
|
||
Filed bug 1691878 to more better test this code in some existing tests.
Description
•