Bug 70471 – Seaming on border corners with mixed colour alpha borders

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Ben Wells]

Created attachment 111721 [details]
Testcase - div on right shows seaming

If you have a border edge which has some transparency against another edge which also has transparency but a different color, seaming is evident. This doesn't happen with dashed or dotted borders. Also, it doesn't happen with skia.

[Simon Fraser (smfr)]

Might depend on whether we draw with antialiasing or not.

[Ben Wells]

Yes, with dashed / dotted we clip out the polygon (which uses anti-aliasing), with the others we just use drawConvexPolygon with the border side shape. I have a simple fix for this to clip out if there is transparency but need to do some more testing first.

Skia draws polygons slightly differently when antialiasing is off (less pixels are drawn), which is why it doesn't happen with skia.

[Ben Wells]

Created attachment 111871 [details]
Patch for comment (needs test)

[Ben Wells]

Comment on attachment 111871 [details]
Patch for comment (needs test)

This patch fixes the problem by using anti-aliasing on mitred edges (via the edge clip polygon) in some more cases.

An alternative (and simpler) fix would be to make the top and bottom edge polygons slightly smaller to avoid overdrawing, however that will introduce gaps on skia.

[WebKit Review Bot]

Comment on attachment 111871 [details]
Patch for comment (needs test)

Attachment 111871 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10178657

New failing tests:
fast/borders/mixed-border-styles.html

[Simon Fraser (smfr)]

Comment on attachment 111871 [details]
Patch for comment (needs test)

View in context: https://bugs.webkit.org/attachment.cgi?id=111871&action=review

Looks OK but could use a test.

> Source/WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

Seems like you could have a pixel test for this.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:1409
> +        bool clipAdjacentSide1 = colorNeedsAntiAliasAtCorner(side, adjacentSide1, edges) && mitreAdjacentSide1;
> +        bool clipAdjacentSide2 = colorNeedsAntiAliasAtCorner(side, adjacentSide1, edges) && mitreAdjacentSide2;

You're calling colorNeedsAntiAliasAtCorner(side, adjacentSide1, edges) twice here.

[Ben Wells]

Created attachment 115098 [details]
Patch with test

[Simon Fraser (smfr)]

Comment on attachment 115098 [details]
Patch with test

View in context: https://bugs.webkit.org/attachment.cgi?id=115098&action=review

> Source/WebCore/rendering/RenderBoxModelObject.cpp:1527
> +        bool clipForStyle = styleRequiresClipPolygon(edgeToRender.style) && (mitreAdjacentSide1 || mitreAdjacentSide2);
> +        bool clipAdjacentSide1 = colorNeedsAntiAliasAtCorner(side, adjacentSide1, edges) && mitreAdjacentSide1;
> +        bool clipAdjacentSide2 = colorNeedsAntiAliasAtCorner(side, adjacentSide2, edges) && mitreAdjacentSide2;
> +        bool shouldClip = clipForStyle || clipAdjacentSide1 || clipAdjacentSide2;

If you avoided the local variables, the test could short-circuit, which would be slightly more efficient (if not quite as readable).

> LayoutTests/fast/borders/border-mixed-alpha.html:11
> +    div {
> +        width: 20px;
> +        height: 20px;
> +        display: inline-block;
> +        border-width: 40px;
> +        border-color: rgba(0, 255, 0, 0.5);
> +        border-right-color: rgba(0, 0, 255, 0.5);
> +    }
> +    .dashed { border-style: dashed; }

Can you make everything bigger here, so that the test fills the image (without overflowing).

[Ben Wells]

Created attachment 115296 [details]
Patch with better test

[Ben Wells]

Comment on attachment 115296 [details]
Patch with better test

I've made the test result fill much more of the space.

The variables which could be removed to enable short-circuit evaluation are all used later; if the test short circuits they will be needed so any savings will be lost.

[WebKit Review Bot]

Comment on attachment 115296 [details]
Patch with better test

Clearing flags on attachment: 115296

Committed r100528: <http://trac.webkit.org/changeset/100528>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.