1285658 - White line with dotted/dashed border-radiused with zoom

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Zéfling]

Attachment: 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

Comment 1

[Tooru Fujisawa [:arai]]

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.

Comment 2

[Tooru Fujisawa [:arai]]

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)));

Comment 3

[Tooru Fujisawa [:arai]]

testing patch
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f8fa64545c70

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: 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

Comment 5

[Tooru Fujisawa [:arai]]

Thank you for reviewing :D

Comment 6

[Tooru Fujisawa [:arai]]

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

[Carsten Book [:Tomcat]]

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

Comment 8

[Zéfling]

Attachment: With the last nightly

It's better but not perfect.

Comment 9

[Tooru Fujisawa [:arai]]

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.

Comment 10

[Tooru Fujisawa [:arai]]

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.

Comment 11

[Tooru Fujisawa [:arai]]

might be better just drawing dashed corner and side at once.

Comment 12

[Tooru Fujisawa [:arai]]

the overlap happens also with double style.
I'll file a dedicated bug for the overlap.

Comment 13

[Tooru Fujisawa [:arai]]

Filed bug 1286465 for overlap.

will fix gap here.

Comment 14

[Tooru Fujisawa [:arai]]

Attachment: 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.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5e5a4c74d4c
Part 2: Use less-erroneous value while calculating border corner dimensions. r=jrmuizel

Comment 16

[Carsten Book [:Tomcat]]

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