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.

Sign up for GitHub

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

Jump to bottom

inner_radii needs to normalize radii to avoid overlapping curves #29685

Closed
Loirooriol opened this issue Apr 29, 2023 · 0 comments · Fixed by #29686
Closed

inner_radii needs to normalize radii to avoid overlapping curves #29685

Loirooriol opened this issue Apr 29, 2023 · 0 comments · Fixed by #29686
Labels
A-layout/2020 https://github.com/servo/servo/wiki/Layout-2020

Comments

@Loirooriol
Copy link
Contributor

Loirooriol commented Apr 29, 2023 •
edited

inner_radii is very naive:

servo/components/layout_2020/display_list/mod.rs

Lines 669 to 682 in 4339b3b

fn inner_radii(mut radii: wr::BorderRadius, offsets: units::LayoutSideOffsets) -> wr::BorderRadius {
radii.top_left.width -= offsets.left;
radii.bottom_left.width -= offsets.left;
radii.top_right.width -= offsets.right;
radii.bottom_right.width -= offsets.right;
radii.top_left.height -= offsets.top;
radii.top_right.height -= offsets.top;
radii.bottom_left.height -= offsets.bottom;
radii.bottom_right.height -= offsets.bottom;
radii
}

The problem is with overlapping radii: https://drafts.csswg.org/css-backgrounds-3/#corner-overlap

Corner curves must not overlap: When the sum of any two adjacent border radii exceeds the size of the border box, UAs must proportionally reduce the used values of all border radii until none of them overlap.

For example:

<!DOCTYPE html>
<div style="
  width: 100px;
  height: 100px;
  border: 10px solid blue;
  border-radius: 100px 50px 50px 50px / 300px 50px 50px 300px;
  background: linear-gradient(cyan, cyan) padding-box, red border-box;
"></div>

The border area is 120x120, so border-radius behaves like 20px 10px 10px 10px / 60px 10px 10px 60px.
Then when we shrink by 10px, the padding should use 10px 0 0 0 / 50px 0 0 50px.
However, since inner_radii doesn't normalize, it produces 90px 40px 40px 40px / 290px 40px 40px 290px.
When normalized to fit into 100x100, that becomes 50px calc(200px/9) calc(200px/9) calc(200px/9) / calc(1450px/9) calc(200px/9) calc(200px/9) calc(1450px/9), which is considerably different:

There should be no red.
@Loirooriol Loirooriol added the A-layout/2020 https://github.com/servo/servo/wiki/Layout-2020 label Apr 29, 2023
@Loirooriol Loirooriol mentioned this issue Apr 29, 2023
Merged
5 tasks
bors-servo added a commit that referenced this issue Apr 29, 2023
@bors-servo
Auto merge of #29686 - Loirooriol:normalize-border-radius, r=<try>
e11c448 
Normalize corner radii that add up to > 100%

WebRender already seems to be doing this normalization, but this is needed by inner_radii in order to properly compute the reduced radii when background-clip is content-box or padding-box.

This will also be needed for expanding the radii for box-shadow or outline.

<!-- 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
- [X] These changes fix #29685 (GitHub issue number if applicable)

<!-- Either: -->
- [ ] 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 added a commit that referenced this issue Apr 29, 2023
@bors-servo
Auto merge of #29686 - Loirooriol:normalize-border-radius, r=<try>
94eb279 
Normalize corner radii that add up to > 100%

WebRender already seems to be doing this normalization, but this is needed by inner_radii in order to properly compute the reduced radii when background-clip is content-box or padding-box.

This will also be needed for expanding the radii for box-shadow or outline.

Test: css/css-backgrounds/background-rounded-image-clip-002.html

<!-- 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
- [X] These changes fix #29685 (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 added a commit that referenced this issue Apr 29, 2023
@bors-servo
Auto merge of #29686 - Loirooriol:normalize-border-radius, r=<try>
9331199 
Normalize corner radii that add up to > 100%

WebRender already seems to be doing this normalization, but this is needed by inner_radii in order to properly compute the reduced radii when background-clip is content-box or padding-box.

This will also be needed for expanding the radii for box-shadow or outline.

Test: css/css-backgrounds/background-rounded-image-clip-002.html

<!-- 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
- [X] These changes fix #29685 (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 added a commit that referenced this issue Apr 29, 2023
@bors-servo
Auto merge of #29686 - Loirooriol:normalize-border-radius, r=<try>
90976be 
Normalize corner radii that add up to > 100%

WebRender already seems to be doing this normalization, but this is needed by inner_radii in order to properly compute the reduced radii when background-clip is content-box or padding-box.

This will also be needed for expanding the radii for box-shadow or outline.

Test: css/css-backgrounds/background-rounded-image-clip-002.html

<!-- 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
- [X] These changes fix #29685 (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 added a commit that referenced this issue Apr 30, 2023
@bors-servo
Auto merge of #29686 - Loirooriol:normalize-border-radius, r=<try>
83a6b36 
Normalize corner radii that add up to > 100%

WebRender already seems to be doing this normalization, but this is needed by inner_radii in order to properly compute the reduced radii when background-clip is content-box or padding-box.

This will also be needed for expanding the radii for box-shadow or outline.

Test: css/css-backgrounds/background-rounded-image-clip-002.html

<!-- 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
- [X] These changes fix #29685 (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 added a commit that referenced this issue Apr 30, 2023
@bors-servo
Auto merge of #29686 - Loirooriol:normalize-border-radius, r=<try>
1c42d7e 
Normalize corner radii that add up to > 100%

WebRender already seems to be doing this normalization, but this is needed by inner_radii in order to properly compute the reduced radii when background-clip is content-box or padding-box.

This will also be needed for expanding the radii for box-shadow or outline.

Test: css/css-backgrounds/background-rounded-image-clip-002.html

<!-- 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
- [X] These changes fix #29685 (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 added a commit that referenced this issue Apr 30, 2023
@bors-servo
Auto merge of #29686 - Loirooriol:normalize-border-radius, r=<try>
51681e2 
Normalize corner radii that add up to > 100%

WebRender already seems to be doing this normalization, but this is needed by inner_radii in order to properly compute the reduced radii when background-clip is content-box or padding-box.

This will also be needed for expanding the radii for box-shadow or outline.

Test: css/css-backgrounds/background-rounded-image-clip-002.html

<!-- 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
- [X] These changes fix #29685 (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 added a commit that referenced this issue May 1, 2023
@bors-servo
Auto merge of #29686 - Loirooriol:normalize-border-radius, r=<try>
b1eff54 
Normalize corner radii that add up to > 100%

WebRender already seems to be doing this normalization, but this is needed by inner_radii in order to properly compute the reduced radii when background-clip is content-box or padding-box.

This will also be needed for expanding the radii for box-shadow or outline.

Test: css/css-backgrounds/background-rounded-image-clip-002.html

<!-- 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
- [X] These changes fix #29685 (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 added a commit that referenced this issue May 1, 2023
@bors-servo
Auto merge of #29686 - Loirooriol:normalize-border-radius, r=mrobinson
5c5c424 
Normalize corner radii that add up to > 100%

WebRender already seems to be doing this normalization, but this is needed by inner_radii in order to properly compute the reduced radii when background-clip is content-box or padding-box.

This will also be needed for expanding the radii for box-shadow or outline.

Test: css/css-backgrounds/background-rounded-image-clip-002.html

<!-- 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
- [X] These changes fix #29685 (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. -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-layout/2020 https://github.com/servo/servo/wiki/Layout-2020
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Normalize corner radii that add up to > 100% Loirooriol/servo
1 participant
@Loirooriol