1713730 - 'border-image-repeat: space' renders incorrectly when a non-initial 'border-image-width' value is specified

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P3)

Description

[Mats Palmgren (inactive)]

Attachment: Testcase #1

STEPS TO REPRODUCE

1. load the attached testcase

ACTUAL RESULTS
The borders of the first two boxes are rendered incorrectly. They should render exactly the same as the third box.

EXPECTED RESULTS
All three boxes render exactly the same as the third box currently does.

ADDITIONAL INFORMATION
The third box has border-image-width: 1 (the initial value) which should be used as a factor of border-width which is explicitly specified as 27px, so the used value for border-image-width is 27px.

The first box has an explicit border-image-width: 27px.
The second box has an explicit border-image-width: auto which means we should use the border-image-slice value instead which is specified (for all three boxes) to be 27px.
So the border-image-width used value should be 27px for these two boxes too.

Spec:
https://drafts.csswg.org/css-backgrounds-3/#the-border-image-width

Fwiw, all three boxes look the same in Chrome, but note that they have implemented border-image-repeat: space incorrectly. Spec:
https://drafts.csswg.org/css-backgrounds-3/#valdef-background-repeat-space

BTW, I printed out the numbers we calculate here:
https://searchfox.org/mozilla-central/rev/d3b116e554eb3546a95851e81f7ddc53d5b7e096/layout/painting/nsCSSRenderingBorders.cpp#3845-3854
and they appear to be correct at this point.

Comment 1

[Mats Palmgren (inactive)]

The problem is that nsImageRenderer::DrawBorderImageComponent calls nsLayoutUtils::DrawBackgroundImage which uses aAnchor. That's not what we want for border images. Should be a pretty simple fix...

Comment 2

[Mats Palmgren (inactive)]

https://treeherder.mozilla.org/jobs?repo=try&revision=98159785bc4145f4f7a4b461f4c5010703e4be5b

Comment 3

[Mats Palmgren (inactive)]

Attachment: Bug 1713730 - Make 'border-image-repeat: space' paint correctly. r=dholbert

This splits DrawBackgroundImage into two functions, one that takes
an anchor (for painting backgrounds) and one that does not (for
painting borders). The underlying painting implementation is
still shared. Then make the border image painting code call
nsLayoutUtils::DrawBorderImage that doesn't use an anchor.

Comment 4

[Daniel Holbert [:dholbert]]

(In reply to Mats Palmgren (:mats) from comment #0)

ACTUAL RESULTS
The borders of the first two boxes are rendered incorrectly. They should render exactly the same as the third box.

EXPECTED RESULTS
All three boxes render exactly the same as the third box currently does.

FWIW, I get EXPECTED RESULTS in current Nightly; I don't see any differences between the first two boxes vs. the third. But if I use full-page zoom to zoom in, then I do see some clipped/overlapping diamonds in the borders on the first two boxes. So I'm guessing this is mostly an issue for HiDPI and similar scenarios (like full-page zoom).

Comment 5

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #4)

if I use full-page zoom to zoom in, then I do see some clipped/overlapping diamonds in the borders on the first two boxes.

(and just for good measure / to be sure I wasn't seeing something different: I confirmed that the attached patch does fix this full-page-zoom-specific issue that I'm seeing)

Comment 6

[Mats Palmgren (inactive)]

(In reply to Daniel Holbert [:dholbert] from comment #4)

FWIW, I get EXPECTED RESULTS in current Nightly; I don't see any differences between the first two boxes vs. the third. But if I use full-page zoom to zoom in, then I do see some clipped/overlapping diamonds in the borders on the first two boxes. So I'm guessing this is mostly an issue for HiDPI and similar scenarios (like full-page zoom).

Interesting, I didn't even realize that. My main screen indeed has a CSS/dev-pixel ratio != 1.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c629f163190c
Make 'border-image-repeat: space' paint correctly.  r=dholbert

Comment 8

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/29371 for changes under testing/web-platform/tests

Comment 9

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/c629f163190c

Comment 10

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot