1550582 - Negative outline offset behavior broken in Nightly

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P3)

Description

[Brian Peiris]

Attachment: test.html

An outline-offset with a negative value renders four squares in the current Nightly (68.0a1 build 20190509033505) but renders as a solid block in Firefox Stable and Chrome Stable. See attached test case and images

Comment 1

[Brian Peiris]

Attachment: Broken render in firefox nightly

Comment 2

[Brian Peiris]

Attachment: Working render in firefox stable

Comment 3

[Brian Peiris]

mozregression bisect points to https://phabricator.services.mozilla.com/D16872

Comment 4

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Set relevant flags and move bug component to webrender based on comment 3.

Comment 5

[Jamie Nicol [:jnicol]]

In order to fix bug 1496540 I made it so that we clip the border corners so they do not overlap with their opposite edges.

The problem here is that the offset is -17px, whereas the width is 30px. And since 2 * 17 > 30, this means that /all/ of the corners overlap with their opposite edges. We end up clipping 4px off of every corner, which results in the gap in the middle. If the offset were -15px this wouldn't occur.

The fix should be fairly easy - in the case where the edges overlap with their opposite edges we calculate the midpoint, and then clip each corner to this midpoint rather than the opposite edge. In this case we would clip 2px from each corner and they would touch in the middle, rather than there be a gap or them overlapping.

Note that this doesn't affect any negative outline offset: only ones greater than the element size / 2. In case that affects whether we want this to block 67 / be uplifted.

Comment 6

[Jamie Nicol [:jnicol]]

Attachment: Bug 1550582 - Ensure the overlap is filled correctly when opposite border edges overlap. r?gw

To fix bug 1496540 it was made so that webrender clips border corner
segments so that they do not overlap with their opposing
edges. However, cases where opposing edges both overlap with
eachother (rather than just a corner overlapping with an edge), the
corners are clipped too far and a gap is left in the middle.

Additionally, no clipping was added to the edge segments. So rather
than there be a gap there is an area that is painted twice, which is
apparent if the colour is semi-transparent.

This fixes these issues by identifying when opposing edges overlap and
calculating the midpoint, then clipping the edges and corners to that
midpoint instead.

Comment 7

[Jamie Nicol [:jnicol]]

Attachment: Bug 1550582 - Add wrench reftest r?gw

And rename the old overlapping corners testcase to make clearer one is
for a corner overlapping with an edge and the new one is for the
edges (and therefore multiple corners) overlapping.

Depends on D30814

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/93299787ec39
Ensure the overlap is filled correctly when opposite border edges overlap. r=gw
https://hg.mozilla.org/integration/autoland/rev/f613e818e435
Add wrench reftest r=gw

Comment 9

[Jamie Nicol [:jnicol]]

Jessie, Jeff, do you want me to request beta uplift for this?

Comment 10

[Jessie [:jbonisteel] pls NI]

(In reply to Jamie Nicol [:jnicol] from comment #9)

Jessie, Jeff, do you want me to request beta uplift for this?

Yes please!

Comment 11

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/93299787ec39
https://hg.mozilla.org/mozilla-central/rev/f613e818e435

Comment 12

[Jamie Nicol [:jnicol]]

Comment on attachment 9064312 [details]
Bug 1550582 - Ensure the overlap is filled correctly when opposite border edges overlap. r?gw

Beta/Release Uplift Approval Request

• User impact if declined: Incorrect rendering of borders/outlines where the edges overlap with eachother
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very small and localized change which slightly modifies the size of drawn border segments in certain conditions, using existing code paths.
• String changes made/needed:

Comment 13

[Brian Peiris]

Thanks for the quick turnaround!

Comment 14

[Pascal Chevrel:pascalc (PTO until April 26)]

Jamie, this is RC week, did you mean to ask for an uplift into the 67.0 release (approval-mozilla-release)?

Comment 15

[Jamie Nicol [:jnicol]]

Yes I did indeed

Comment 16

[Jamie Nicol [:jnicol]]

Comment on attachment 9064312 [details]
Bug 1550582 - Ensure the overlap is filled correctly when opposite border edges overlap. r?gw

Beta/Release Uplift Approval Request

• User impact if declined: Incorrect rendering of borders/outlines where the edges overlap with eachother
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very small and localized change which slightly modifies the size of drawn border segments in certain conditions, using existing code paths.
• String changes made/needed:

Comment 17

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9064312 [details]
Bug 1550582 - Ensure the overlap is filled correctly when opposite border edges overlap. r?gw

Fix correction regression in Webrender, low risk patch with tests, approved for RC2.

Comment 18

[Julien Cristau [:jcristau] (back April 22)]

https://hg.mozilla.org/releases/mozilla-release/rev/5f21e6f900c8
https://hg.mozilla.org/releases/mozilla-release/rev/499809263177