Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upHave transforms and filters be CBs for all descendants in layout_2020 #25862
Conversation
| current_space_and_clip: wr::SpaceAndClipInfo, | ||
|
|
||
| /// The id of the nearest ancestor reference frame for this `DisplayListBuilder`. | ||
| nearest_reference_frame: wr::SpatialId, |
This comment has been minimized.
This comment has been minimized.
pcwalton
Feb 28, 2020
Contributor
What is a reference frame defined as? A quick Google search doesn't pull up that term in a CSS spec.
This comment has been minimized.
This comment has been minimized.
mrobinson
Mar 2, 2020
Author
Member
The reference frame is a concept from WebRender. Essentially it is the thing that does transformations and starts new coordinate systems.
| // https://www.w3.org/TR/css-transforms-1/#containing-block-for-all-descendants | ||
| // | ||
| // We use the abbreviation `cbfad` here only because `containing block for all descendants` | ||
| // is so unwieldy. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
nox
Feb 28, 2020
Member
Personally I think containing_block_for_all_descendants is way better, and not even the longest identifier in layout 2020.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
mrobinson
Mar 2, 2020
Author
Member
Okay. I've changed this to be containing_block_for_all_descendants everywhere.
| cbis - start - pb.inline_sum() - margin.inline_sum() | ||
| }, | ||
| Anchor::End(end) => cbis - end - pb.inline_sum() - margin.inline_sum(), | ||
| let for_cbfad = for_nearest_cbfad; |
This comment has been minimized.
This comment has been minimized.
pcwalton
Feb 28, 2020
Contributor
This isn't changed other than the replacement of PositioningContext::for_positioned with PositioningContext::create_and_layout_positioned, right?
This comment has been minimized.
This comment has been minimized.
mrobinson
Mar 2, 2020
Author
Member
That's correct. Unfortunately this caused a reflow of the entire block.
| _ => return self.get_position().z_index.integer_or(0), | ||
| } | ||
|
|
||
| 0 |
This comment has been minimized.
This comment has been minimized.
pcwalton
Feb 28, 2020
Contributor
nit: would probably be simpler and more idiomatic to simply write:
match self.get_box().position {
ComputedPosition::Static => 0,
_ => self.get_position().z_index.integer_or(0),
}
or even
if self.get_box().position == ComputedPosition::Static {
0
} else {
self.get_position().z_index.integer_or(0)
}
This comment has been minimized.
This comment has been minimized.
| !self.get_box().transform.0.is_empty() || self.get_box().perspective != Perspective::None | ||
| } | ||
|
|
||
| /// Get the effective z-index of this fragment. Z-indices only apply to positioned element |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
mrobinson
force-pushed the
mrobinson:transforms-containing-block
branch
from
d1b1c90
to
4ec2d6a
|
Okay. Thanks for the reviews. There's a new version of the branch if you'd all like to take a look. |
|
@bors-servo r=pcwalton,nox |
|
|
…ton,nox Have transforms and filters be CBs for all descendants in layout_2020 This is a feature that was never properly implemented in the previous layout system. We still need to preserve their in-tree order in the display list though. <!-- Please describe your changes on the following line: --> --- <!-- 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 - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #___ (GitHub issue number if applicable) <!-- Either: --> - [x] There are tests for these changes OR - [ ] These changes do not require tests because ___ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
|
|
This is a feature that was never properly implemented in the previous layout system. We still need to preserve their in-tree order in the display list though.
mrobinson
force-pushed the
mrobinson:transforms-containing-block
branch
from
4ec2d6a
to
8de5569
|
@bors-servo r=pcwalton,nox The earlier version of the branch had added a bogus assertion. I've removed this. |
|
|
…ton,nox Have transforms and filters be CBs for all descendants in layout_2020 This is a feature that was never properly implemented in the previous layout system. We still need to preserve their in-tree order in the display list though. <!-- Please describe your changes on the following line: --> --- <!-- 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 - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #___ (GitHub issue number if applicable) <!-- Either: --> - [x] There are tests for these changes OR - [ ] These changes do not require tests because ___ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
|
|
|
@bors-servo retry |
|
|
mrobinson commentedFeb 28, 2020
This is a feature that was never properly implemented in the previous
layout system. We still need to preserve their in-tree order in the
display list though.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errors