Closed Bug 1550582 Opened 4 months ago Closed 3 months ago

Negative outline offset behavior broken in Nightly

Categories

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

Desktop
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox-esr60 --- unaffected
firefox67 --- fixed
firefox68 --- fixed

People

(Reporter: bpeiris, Assigned: jnicol)

References

(Regression)

Details

(Keywords: regression)

Attachments

(5 files)

Attached file 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

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

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

Component: Layout: Block and Inline → Graphics: WebRender
Flags: needinfo?(jnicol)
Priority: -- → P3
Regressed by: 1496540

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.

Assignee: nobody → jnicol
Flags: needinfo?(jnicol)

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.

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

Pushed by jnicol@mozilla.com:
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

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

Flags: needinfo?(jmuizelaar)
Flags: needinfo?(jbonisteel)

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

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

Yes please!

Flags: needinfo?(jbonisteel)
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

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:
Attachment #9064312 - Flags: approval-mozilla-beta?
Attachment #9064313 - Flags: approval-mozilla-beta?

Thanks for the quick turnaround!

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

Flags: needinfo?(jnicol)

Yes I did indeed

Flags: needinfo?(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:
Attachment #9064312 - Flags: approval-mozilla-beta? → approval-mozilla-release?
Attachment #9064313 - Flags: approval-mozilla-release?
Attachment #9064313 - Flags: approval-mozilla-beta?
Flags: needinfo?(jmuizelaar)

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.

Attachment #9064312 - Flags: approval-mozilla-release? → approval-mozilla-release+
Attachment #9064313 - Flags: approval-mozilla-release? → approval-mozilla-release+
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.