Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove StackingContext::scroll_policy #2747

Merged
merged 1 commit into from May 10, 2018

Conversation

@mrobinson
Copy link
Member

mrobinson commented May 10, 2018

This can be handled in clients now that they have access to reference
frame ciip ids. Removing fixed positioning support from WebRender
simplifies the code greatly and also opens the way to explicit creation
of reference frames in the API. It will also allow us to get rid of the
hacky replacements code during display list flattening.

This also requires exposing the root reference frame to Wrench tests in
order to enable them retain the fixed positioning feature. This change
exposes new aliases to wrench "root-scroll-node" and
"root-reference-frame" in order to make this clearer. These nodes
correspond to the ids 1 and 0 respectively and Wrench will halt if it
encounters these used for other nodes.


This change is Reviewable

@mrobinson
Copy link
Member Author

mrobinson commented May 10, 2018

r? @glennw or anyone

@mrobinson
Copy link
Member Author

mrobinson commented May 10, 2018

Since Gecko only uses ScrollPolicy:Scrollable, the only change necessary there should be to remove this argument from DisplayListBuilder::push_stacking_context. There should be no behavior change.

This can be handled in clients now that they have access to reference
frame ciip ids. Removing fixed positioning support from WebRender
simplifies the code greatly and also opens the way to explicit creation
of reference frames in the API. It will also allow us to get rid of the
hacky replacements code during display list flattening.

This also requires exposing the root reference frame to Wrench tests in
order to enable them retain the fixed positioning feature. This change
exposes new aliases to wrench "root-scroll-node" and
"root-reference-frame" in order to make this clearer. These nodes
correspond to the ids 1 and 0 respectively and Wrench will halt if it
encounters these used for other nodes.
@mrobinson mrobinson force-pushed the mrobinson:remove-fixed-stacking-contexts branch from 1d60f4f to f779ed6 May 10, 2018
@gw3583
Copy link
Collaborator

gw3583 commented May 10, 2018

Reviewed 48 of 48 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@gw3583
Copy link
Collaborator

gw3583 commented May 10, 2018

@bors-servo
Copy link
Contributor

bors-servo commented May 10, 2018

📌 Commit f779ed6 has been approved by gw3583

@bors-servo
Copy link
Contributor

bors-servo commented May 10, 2018

Testing commit f779ed6 with merge 4e7850e...

bors-servo added a commit that referenced this pull request May 10, 2018
Remove StackingContext::scroll_policy

This can be handled in clients now that they have access to reference
frame ciip ids. Removing fixed positioning support from WebRender
simplifies the code greatly and also opens the way to explicit creation
of reference frames in the API. It will also allow us to get rid of the
hacky replacements code during display list flattening.

This also requires exposing the root reference frame to Wrench tests in
order to enable them retain the fixed positioning feature. This change
exposes new aliases to wrench "root-scroll-node" and
"root-reference-frame" in order to make this clearer. These nodes
correspond to the ids 1 and 0 respectively and Wrench will halt if it
encounters these used for other nodes.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2747)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 10, 2018

☀️ Test successful - status-appveyor, status-taskcluster
Approved by: gw3583
Pushing 4e7850e to master...

@bors-servo bors-servo merged commit f779ed6 into servo:master May 10, 2018
4 checks passed
4 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
code-review/reviewable 48 files reviewed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.