Closed Bug 1411627 Opened 2 years ago Closed 2 years ago

WR API for position:sticky needs to support painting at nonzero scroll offsets

Categories

(Core :: Graphics: WebRender, enhancement, P1)

Other Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox57 --- unaffected
firefox58 --- unaffected

People

(Reporter: kats, Assigned: kats)

References

Details

(Whiteboard: [wr-mvp] [gfx-noted])

Attachments

(3 files)

Right now the WR implementation for position:sticky assumes that all the scrollframes are at zero offsets when the display list is generated. This means, for instance, if something is sticky with a top margin, then async scrolling can only ever move that item downwards. Therefore WR does things like [1] where it bakes in a max(0, ...) to prevent negative adjustments.

However, in reality we send display lists to gecko with scrollframes at nonzero scroll offsets. So if we async scroll to a smaller scroll offset, we tell WR to scroll to what it considers a negative scroll offset (because it's negative relative to the display list that we painted at). This means that items that are sticky with a top margin actually can move upwards with async scrolling, and the WR code doesn't allow that.

I believe the way to fix this is not hard-code the 0 value into the code at [1] but instead have the caller provide it via the API just like it provides max_offset.

[1] http://searchfox.org/mozilla-central/rev/1285ae3e810e2dbf2652c49ee539e3199fcfe820/gfx/webrender/src/clip_scroll_node.rs#449
Priority: P3 → P2
Whiteboard: [gfx-noted][wr-mvp][triage] → [wr-mvp] [gfx-noted]
Summary: WR API for positon:sticky needs to support painting at nonzero scroll offsets → WR API for position:sticky needs to support painting at nonzero scroll offsets
Assignee: nobody → bugmail
Status: NEW → ASSIGNED
Priority: P2 → P1
I cleaned up the patches and added another reftest. Here's a final try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=12867e3d82bc3db7418108ad848f22d647ee3a6c

I submitted the WR patch upstream as servo/webrender#1977. Once that is merged I'll put the rest of the patches up for review.
Depends on: 1413178
Comment on attachment 8925959 [details]
Bug 1411627 - Stop sending sticky clips to WR for scrollframes with no ASR.

https://reviewboard.mozilla.org/r/197178/#review202400
Attachment #8925959 - Flags: review?(mstange) → review+
Comment on attachment 8925960 [details]
Bug 1411627 - Send the applied offset for sticky frames to WR.

https://reviewboard.mozilla.org/r/197180/#review202402
Attachment #8925960 - Flags: review?(mstange) → review+
Comment on attachment 8925961 [details]
Bug 1411627 - Add reftests.

https://reviewboard.mozilla.org/r/197182/#review202404
Attachment #8925961 - Flags: review?(mstange) → review+
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a165d1dcc904
Stop sending sticky clips to WR for scrollframes with no ASR. r=mstange
https://hg.mozilla.org/integration/autoland/rev/857a0e87e490
Send the applied offset for sticky frames to WR. r=mstange
https://hg.mozilla.org/integration/autoland/rev/6ca404d46fc2
Add reftests. r=mstange
https://hg.mozilla.org/mozilla-central/rev/a165d1dcc904
https://hg.mozilla.org/mozilla-central/rev/857a0e87e490
https://hg.mozilla.org/mozilla-central/rev/6ca404d46fc2
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.