Elements with a combination of 'border' and 'border-radius' are drawn incorrectly #20922

MartinRogalla · 2018-06-05T22:48:04Z

Excuse my narcism here for posting my own profile picture, but I was checking out my own website in servo. What I noticed is that my profile picture that is supposed to be round, get's drawn incorrectly:

Google Chrome:

Servo:

When I try to create a 'circular' element with the following attributes:

border: 3px solid #fff;
border-radius: 100px; /* note, border-radius >= 75px for the element do be 'circular' */
width: 150px;
height: 150px;

To me it looks like the image gets square borders and then is rounded, instead of the other way around. I'm interested in contributing to servo and would love to pick this issue up if you think it's doable.

jdm · 2018-06-05T22:52:28Z

servo/components/layout/display_list/background.rs

Line 714 in 11edbfc

pub fn build_border_radius(

is the code that is responsible for creating a border radius value that is added to display list items. It would be worth creating a minimal testcase so it's easier to trace the code that ends up reaching there.

MartinRogalla · 2018-06-05T22:54:03Z

Perfect, will do. Thanks!

MartinRogalla · 2018-06-06T16:09:30Z

The following document should render a purple circle with a pink border:

<html>
<head>
    <link rel=match href=border_radius_border_solid_combination.html>
</head>
<body>
    <div style="background: #1f1f2e; border-radius: 50%; border: 10px solid #ff3399; width: 250px; height: 250px;"></div>
</body>
</html>

Expected output:

Actual output:

This is only happening when the border is greater than 3px. If it is 3px or less, it renders as expected.

Display-list Dump

Nothing in the display-lists caught my eye that would explain why it's incorrectly rendering. I will keep looking.

jdm · 2018-06-06T16:35:38Z

It may be that our display list is correct and webrender is interpreting it incorrectly. @gw3583 does this ring any bells?

MartinRogalla · 2018-06-06T17:07:47Z

I think the issue might be here: webrender/src/util.rs extract_inner_rect_impl. I'll have another look in a bit and if needed will try to build the webrenderer and use that to find where it's going wrong.

gw3583 · 2018-06-06T21:09:14Z

It doesn't ring any bells. It's certainly possible it could be a bug in WR, although I don't think we've seen anything like this in Gecko thus far. One option would be to build wrench (the standalone WR reftest tool) and see if you can reproduce it in wrench by modifying some of the border reftests (for example https://github.com/servo/webrender/tree/master/wrench/reftests/border).

gw3583 · 2018-06-06T21:09:36Z

If you can reproduce it there, that would rule out Servo being the issue.

MartinRogalla · 2018-06-06T21:38:32Z

Perfect, I'll try that. Thanks!

MartinRogalla · 2018-06-07T15:18:56Z

Running the border-tests using:

cd wrench && cargo run -- reftest reftests/border/

in WebRender outputs failing tests (both on 'master' & on 'auto'):

REFTEST INFO | 15 passing, 5 failing
Reftests with unexpected results:
        reftests/border/border-clamp-corner-radius.yaml == reftests/border/border-clamp-corner-radius.png
        reftests/border/border-suite.yaml == reftests/border/border-suite.png
        reftests/border/border-suite-2.yaml == reftests/border/border-suite-2.png
        reftests/border/border-suite-3.yaml == reftests/border/border-suite-3.png
        reftests/border/degenerate-curve.yaml == reftests/border/degenerate-curve.png

Is this to be expected (as in tests written but not yet implemented), or should these reftests always pass?

I think this is indeed a WR issue. See servo/webrender/issues/2754

gw3583 · 2018-06-07T21:47:11Z

For the reftests, you'll need to use the headless script to run them - this will ensure that they are drawn with a software rasterizer for consistency between machines:

./script/headless.py reftest reftests/border/

dralley · 2020-04-06T18:15:25Z

The test snippet from #20922 (comment) is fixed, but the original website link still renders incorrectly.

jdm · 2020-04-06T18:25:00Z

<style>
#avatar {
        border: 3px solid #000;
        border-radius: 100px;
        width: 150px;
        height: 150px;
}
</style>
<img id="avatar" src="https://martinrogalla.com/img/martin-rogalla-bw.jpg">

Firefox:

Servo:

jdm · 2020-04-06T18:25:42Z

Switching the img to a div shows the expected circular background color with the borders appearing over top as expected.

dralley · 2020-04-08T14:58:43Z

Do you have idea where the code that controls this would be located?

dralley · 2020-04-09T02:44:25Z

@jdm ^

jdm · 2020-04-13T16:36:10Z

https://github.com/servo/servo/blob/master/components/layout/display_list/builder.rs#L811-L813 is the code for adding images to the display list [1]
https://github.com/servo/servo/blob/master/components/layout/display_list/builder.rs#L1061-L1063 is the code for adding borders to the display list
https://github.com/servo/servo/blob/master/components/layout/display_list/builder.rs#L1697-L1704 is the code that should ultimate end up calling [1], and note how it's after a call to [2]

Of course, it's not clear to me if ultimately the problem is that:

we're drawing the image over the borders
our border calculation code is incorrect
our border-radius calculation code is incorrect
something else??

dralley · 2020-04-13T22:24:55Z

So, 2 things:

It looks like the size is exactly the same and the border width is the same.
If you look closely, the visible parts of the border have completely straight edges. It kinda looks like the picture + border is being drawn square and then the whole thing clipped to fit a circle, rather than the image clipped into a circle and then a round border applied. (edit: which I'm somehow just noticing is exactly what the original reporter said...)

Does that help pin down the problem?

Other differences, irrelevant to the current problem but might be worthwhile to think about:

Firefox seems to add a tiny white edge between the border and the image so that you can distinguish the two when they have very similar colors
The Servo-rendered image has a lot more pixelation

dralley · 2020-04-14T01:46:14Z

Found a duplicate (well, probably duplicate) issue, it's a lot more obvious with the example they use: #22818

dralley · 2020-04-14T14:59:36Z

Layout 2020, meanwhile, looks like this:

dralley · 2020-04-17T03:53:41Z

Ignore #20922 (comment), I'm wrong, it's definitely drawing a round border and then drawing the rectangular image on top of that. It's obvious if you toss a panic in before the image gets drawn.

All the dimensions are correct when I measure with GIMP (as far as I know, I have very little web dev experience). It seems like the image stops drawing at the outer edge of the border rather than the inner edge, or it's on top when it should not be.

I assume the issue is probably somewhere in here? https://github.com/servo/servo/blob/master/components/layout/display_list/builder.rs#L1720

jdm · 2020-04-17T16:12:36Z

The CSS specification defines an order in which elements are painted in https://www.w3.org/TR/CSS2/zindex.html#painting-order. Given this, I believe that painting borders before the image content is correct. Since changing the border-radius to 0 in the testcase shows the border correctly, this suggests to me that the calculations for border-radius interact poorly with the calculations for where to draw the border, which suggests that the error is in https://github.com/servo/servo/blob/master/components/layout/display_list/builder.rs#L1731-L1739 or https://github.com/servo/servo/blob/master/components/layout/display_list/builder.rs#L574-L590


        Fix combination of border and border-radius being drawn incorrectly

fixes servo#20922


        Fix combination of border and border-radius being drawn incorrectly

fixes servo#20922


        Auto merge of #26215 - dralley:fix-image-border, r=<try>

Fix combination of border and border-radius being drawn incorrectly fixes #20922 - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #20922  - [x] There are tests for these changes OR - [ ] These changes do not require tests because ___


        Auto merge of #26215 - dralley:fix-image-border, r=<try>

Fix combination of border and border-radius being drawn incorrectly fixes #20922 Manual testcase in the attached issue - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #20922  - [x] There are tests for these changes OR - [ ] These changes do not require tests because ___


        Auto merge of #26215 - dralley:fix-image-border, r=jdm

Fix combination of border and border-radius being drawn incorrectly fixes #20922 Manual testcase in the attached issue - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #20922  - [x] There are tests for these changes OR - [ ] These changes do not require tests because ___


        Auto merge of #26215 - dralley:fix-image-border, r=jdm

Fix combination of border and border-radius being drawn incorrectly fixes #20922 Manual testcase in the attached issue - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #20922  - [x] There are tests for these changes OR - [ ] These changes do not require tests because ___

MartinRogalla changed the title ~~Elements with border and border-radius causes draw incorrectly~~ Elements with a combination of 'border' and 'border-radius' are drawn incorrectly Jun 5, 2018

jdm added the A-gfx/displaylist label Jun 5, 2018

qezz mentioned this issue Jul 2, 2018

Bottom left border radius isn't drawn correctly (on macOS 10.12) servo/webrender#2862

Closed

jdm added the C-has-manual-testcase label Apr 6, 2020

jdm mentioned this issue Apr 14, 2020

Images with padding and border-radius are not cropped correctly #22818

Closed

jdm added the A-layout/2020 label Apr 14, 2020

dralley added a commit to dralley/servo that referenced this issue Apr 17, 2020

Fix combination of border and border-radius being drawn incorrectly

f678610

fixes servo#20922

dralley added a commit to dralley/servo that referenced this issue Apr 17, 2020

Fix combination of border and border-radius being drawn incorrectly

84ffa73

fixes servo#20922

dralley mentioned this issue Apr 17, 2020

Fix combination of border and border-radius being drawn incorrectly #26215

Merged

4 of 5 tasks complete

bors-servo closed this in 232df23 May 2, 2020

servo / servo

Elements with a combination of 'border' and 'border-radius' are drawn incorrectly #20922

Elements with a combination of 'border' and 'border-radius' are drawn incorrectly #20922

MartinRogalla commented Jun 5, 2018

jdm commented Jun 5, 2018

MartinRogalla commented Jun 5, 2018

MartinRogalla commented Jun 6, 2018 •

edited

jdm commented Jun 6, 2018

MartinRogalla commented Jun 6, 2018 •

edited

gw3583 commented Jun 6, 2018

gw3583 commented Jun 6, 2018

MartinRogalla commented Jun 6, 2018

MartinRogalla commented Jun 7, 2018 •

edited

gw3583 commented Jun 7, 2018

dralley commented Apr 6, 2020

jdm commented Apr 6, 2020

jdm commented Apr 6, 2020

dralley commented Apr 8, 2020 •

edited

dralley commented Apr 9, 2020

jdm commented Apr 13, 2020

dralley commented Apr 13, 2020 •

edited

dralley commented Apr 14, 2020 •

edited

dralley commented Apr 14, 2020

dralley commented Apr 17, 2020 •

edited

jdm commented Apr 17, 2020

servo / servo

Join GitHub today

Elements with a combination of 'border' and 'border-radius' are drawn incorrectly #20922

Elements with a combination of 'border' and 'border-radius' are drawn incorrectly #20922

Comments

MartinRogalla commented Jun 5, 2018

jdm commented Jun 5, 2018

MartinRogalla commented Jun 5, 2018

MartinRogalla commented Jun 6, 2018 • edited

jdm commented Jun 6, 2018

MartinRogalla commented Jun 6, 2018 • edited

gw3583 commented Jun 6, 2018

gw3583 commented Jun 6, 2018

MartinRogalla commented Jun 6, 2018

MartinRogalla commented Jun 7, 2018 • edited

gw3583 commented Jun 7, 2018

dralley commented Apr 6, 2020

jdm commented Apr 6, 2020

jdm commented Apr 6, 2020

dralley commented Apr 8, 2020 • edited

dralley commented Apr 9, 2020

jdm commented Apr 13, 2020

dralley commented Apr 13, 2020 • edited

dralley commented Apr 14, 2020 • edited

dralley commented Apr 14, 2020

dralley commented Apr 17, 2020 • edited

jdm commented Apr 17, 2020

MartinRogalla commented Jun 6, 2018 •

edited

MartinRogalla commented Jun 6, 2018 •

edited

MartinRogalla commented Jun 7, 2018 •

edited

dralley commented Apr 8, 2020 •

edited

dralley commented Apr 13, 2020 •

edited

dralley commented Apr 14, 2020 •

edited

dralley commented Apr 17, 2020 •

edited