White line with dotted/dashed border-radiused with zoom

RESOLVED FIXED in Firefox 50

Status

()

Core
Layout
RESOLVED FIXED
a year ago
10 months ago

People

(Reporter: Zéfling, Assigned: arai)

Tracking

50 Branch
mozilla50
Unspecified
Linux
Points:
---

Firefox Tracking Flags

(firefox49 unaffected, firefox50 fixed)

Details

(Whiteboard: [mozfr-community])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

a year ago
Created attachment 8769343 [details]
Firefox-50-radius-zoom.png

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20160708030216

Steps to reproduce:

When the page is zoomed, they are errors of the delimitation painting. 
See the screenshot.

With is page : https://bug382721.bmoattachments.org/attachment.cgi?id=8654683
(Reporter)

Updated

a year ago
Whiteboard: [mozfr-community]
(Reporter)

Updated

a year ago
Component: Untriaged → Layout
OS: Unspecified → Linux
Product: Firefox → Core
(Assignee)

Comment 1

a year ago
corner and other parts are now rendered separately, and I think the gap comes from rounding error or anti-aliasing or something.
will investigate this.
Status: UNCONFIRMED → NEW
Ever confirmed: true
See Also: → bug 382721
(Assignee)

Comment 2

a year ago
oops, ceil is applied there.
I should apply it to other parts.

> https://dxr.mozilla.org/mozilla-central/rev/23dc78b7b57e9f91798ea44c242a04e112c37db0/layout/base/nsCSSRenderingBorders.cpp#285
>     (*aDimsRet)[C_TL] = Size(ceil(std::max(leftWidth, aRadii[C_TL].width)),
>                              ceil(std::max(topWidth, aRadii[C_TL].height)));
>     (*aDimsRet)[C_TR] = Size(ceil(std::max(rightWidth, aRadii[C_TR].width)),
>                              ceil(std::max(topWidth, aRadii[C_TR].height)));
>     (*aDimsRet)[C_BR] = Size(ceil(std::max(rightWidth, aRadii[C_BR].width)),
>                              ceil(std::max(bottomWidth, aRadii[C_BR].height)));
>     (*aDimsRet)[C_BL] = Size(ceil(std::max(leftWidth, aRadii[C_BL].width)),
>                              ceil(std::max(bottomWidth, aRadii[C_BL].height)));
(Assignee)

Updated

a year ago
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
(Assignee)

Comment 3

a year ago
testing patch
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f8fa64545c70
(Assignee)

Comment 4

a year ago
Created attachment 8769436 [details] [diff] [review]
Remove a gap between dashed side and dashed corner if border radius is not an integer.

The end point of dashed side uses `ceil`, so added `ceil` to the end point of dashed corner.

Green on try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f8fa64545c70
Attachment #8769436 - Flags: review?(jmuizelaar)
Attachment #8769436 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 5

a year ago
Thank you for reviewing :D
status-firefox49: --- → unaffected
status-firefox50: --- → affected
(Assignee)

Comment 6

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a2e0ea4065264c7a738a57c4110b8b13632894a6
Bug 1285658 - Remove a gap between dashed side and dashed corner if border radius is not an integer. r=jrmuizel

Comment 7

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a2e0ea406526
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
(Reporter)

Comment 8

a year ago
Created attachment 8769992 [details]
With the last nightly

It's better but not perfect.
Attachment #8769343 - Attachment is obsolete: true
(Assignee)

Updated

a year ago
Status: RESOLVED → REOPENED
Flags: needinfo?(arai.unmht)
Resolution: FIXED → ---
(Assignee)

Comment 9

a year ago
Apparently, there are some error happens while calculating border corner size.

https://dxr.mozilla.org/mozilla-central/rev/88bebcaca249aeaca9197382e89d35b02be8292e/layout/base/nsCSSRenderingBorders.cpp#197
>   mInnerRect = mOuterRect;
>   mInnerRect.Deflate(
>       Margin(mBorderStyles[0] != NS_STYLE_BORDER_STYLE_NONE ? mBorderWidths[0] : 0,
>              mBorderStyles[1] != NS_STYLE_BORDER_STYLE_NONE ? mBorderWidths[1] : 0,
>              mBorderStyles[2] != NS_STYLE_BORDER_STYLE_NONE ? mBorderWidths[2] : 0,
>              mBorderStyles[3] != NS_STYLE_BORDER_STYLE_NONE ? mBorderWidths[3] : 0));

https://dxr.mozilla.org/mozilla-central/rev/88bebcaca249aeaca9197382e89d35b02be8292e/layout/base/nsCSSRenderingBorders.cpp#270
>   Float leftWidth = aInnerRect.X() - aOuterRect.X();
>   Float topWidth = aInnerRect.Y() - aOuterRect.Y();
>   Float rightWidth = aOuterRect.Width() - aInnerRect.Width() - leftWidth;
>   Float bottomWidth = aOuterRect.Height() - aInnerRect.Height() - topWidth;

There, each value is following:
  mOuterRect: x: 19.636364, y: 19.636364, width: 130.909088, height: 130.909088
  mInnerRect: x: 71.636368, y: 71.636368, width: 26.909088, height: 26.909088
  mBorderWidths: 52.000000, 52.000000, 52.000000, 52.000000
  leftWidth: 52.000004
  topWidth: 52.000004

the 0.000004 px difference there is increased by `ceil` call, and results in 1 pixel gap.
maybe we should use mBorderWidths[0] instead of leftWidth etc calculated from mInnerRect that may have error.
Flags: needinfo?(arai.unmht)
(Assignee)

Comment 10

a year ago
about overlap, it's caused by ceil applied to both corner's size.
not sure what's the best solution, but we might need to decrement either side.
(Assignee)

Comment 11

a year ago
might be better just drawing dashed corner and side at once.
(Assignee)

Comment 12

a year ago
the overlap happens also with double style.
I'll file a dedicated bug for the overlap.
(Assignee)

Comment 13

a year ago
Filed bug 1286465 for overlap.

will fix gap here.
See Also: → bug 1286465
(Assignee)

Comment 14

a year ago
Created attachment 8770819 [details] [diff] [review]
Part 2: Use less-erroneous value while calculating border corner dimensions.

Previously we ware calculating border corner dimensions from outerRect and innerRect, but innerRect was calculated from outerRect and borderWidth, and border corner dimensions can be calculated directly from borderWidth.
So just using borderWidth should be less-erronous than using innerRect.
Attachment #8770819 - Flags: review?(jmuizelaar)
Attachment #8770819 - Flags: review?(jmuizelaar) → review+

Comment 15

a year ago
Pushed by arai_a@mac.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5e5a4c74d4c
Part 2: Use less-erroneous value while calculating border corner dimensions. r=jrmuizel

Comment 16

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e5e5a4c74d4c
Status: REOPENED → RESOLVED
Last Resolved: a year agoa year ago
Resolution: --- → FIXED

Updated

10 months ago
Depends on: 1297427
You need to log in before you can comment on or make changes to this bug.