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

Pop WebRender reference frames in layout 2020 #26066

Closed
cbrewster opened this issue Mar 31, 2020 · 2 comments
Closed

Pop WebRender reference frames in layout 2020 #26066

cbrewster opened this issue Mar 31, 2020 · 2 comments

Comments

@cbrewster
Copy link
Member

@cbrewster cbrewster commented Mar 31, 2020

Followup of #26063

I'm not familiar with layout 2020, but I think all we need to do is call pop_reference_frame at the end of the function if the following returns true:

let established_reference_frame =
self.build_reference_frame_if_necessary(builder, &border_rect);

cc @mrobinson @dralley

@mrobinson
Copy link
Member

@mrobinson mrobinson commented Mar 31, 2020

I think that pop_reference_frame is only necessary to adjust mapping of stacking context coordinates to reference frame coordinates. Since Layout 2020 only uses reference frame coordinates, it shouldn't be necessary to include this in the display list.

On the other hand, now that Layout 2020 is using reference frame coordinates everywhere, it raises the question of whether or not Layout 2013 and Gecko should as well. This would have the benefit of allowing the removal of the coordinate space mapper from scene_building.rs and removing the PopReferenceFrame display item. Spatial nodes are allowed to have children in various places in the display list, not just hierarchically so I have some doubts about the coordinate mapping anyhow. Thoughts @dralley?

The refactor that you are suggesting probably makes sense either way.

@dralley
Copy link
Contributor

@dralley dralley commented Mar 31, 2020

Ah, sorry, I deleted that comment after thinking about it some more. Old-layout passed around intermediate objects and I didn't immediately recognize that push_reference_frame was not using that strategy, and that the function call required a bunch of bits of intermediate information. Once I recognized that, the refactor idea didn't make quite as much sense, or at least would require a bit more work.

I can't make any judgements on big-picture strategy things.. I have maybe 20 hours of combined time looking at any of this code :)

bors-servo added a commit that referenced this issue Mar 31, 2020
Ensure reference frame is popped in layout 2020

closes: #26066

Should be equivalent to #26063 for layout2020

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [x] These changes fix #26066 (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___
bors-servo added a commit that referenced this issue Mar 31, 2020
Ensure reference frame is popped in layout 2020

closes: #26066

Should be equivalent to #26063 for layout2020

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [x] These changes fix #26066 (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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