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 upApply border-radius to img, canvas, iframe #17589
Conversation
highfive
commented
Jul 2, 2017
|
Heads up! This PR modifies the following files:
|
| let mut adjusted_clip = ClippingRegion::max(); | ||
| if !border_radii.is_square() { | ||
| adjusted_clip.intersect_with_rounded_rect(clip, &border_radii); | ||
| }; |
This comment has been minimized.
This comment has been minimized.
| @@ -1834,6 +1841,8 @@ impl FragmentDisplayListBuilding for Fragment { | |||
| let stacking_relative_content_box = | |||
| self.stacking_relative_content_box(stacking_relative_border_box); | |||
|
|
|||
| let clipping_region = clip_for_border_radius(&self.style, &stacking_relative_content_box); | |||
This comment has been minimized.
This comment has been minimized.
emilio
Jul 2, 2017
Member
Why stacking_relative_border_box? I'd have expected this to use clip.
Also, what about the other uses for the clip rect? Should they account for border-radius too?
This comment has been minimized.
This comment has been minimized.
streichgeorg
Jul 2, 2017
•
Author
Contributor
I tried clip and it doesn't work, but it works with stacking_relative_content_box so I just used that. I don't really have any oversight over this code so I don't know why that is.
|
Probably @glennw can review this faster than I, too :) |
|
@bors-servo try |
Apply border-radius to img, canvas, iframe <!-- Please describe your changes on the following line: --> Added some code that adjusts the clipping region of ```img```, ```canvas```, ```iframe``` elements for the ```border-radius``` property. --- <!-- 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 #17510 (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. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17589) <!-- Reviewable:end -->
|
|
|
I won't be able to do any work next week, so if there are any changes required I'll do them on the next weekend. |
|
You will need to run |
|
|
|
r? @glennw |
|
This looks good to me, but I'd like @mrobinson to check over it too. |
|
r? @mrobinson |
|
Oof! I was working on this one in parallel (https://github.com/mrobinson/servo/tree/rounded-corners). The reason it isn't working for iframes is that I fixed this in WebRender and I was waiting for the WebRender update to post my PR. There's also an issue here with the size of the clip. We really want to use the inner region. I'm really sorry for not noticing this PR sooner. |
|
Oh, I probably should have looked around a bit more before working on this. I guess this issue is a lot more complex than I thought it was, so I'll just close this PR. |
streichgeorg commentedJul 2, 2017
•
edited by jdm
Added some code that adjusts the clipping region of
img,canvas,iframeelements for theborder-radiusproperty../mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is