Bug 89620

Summary: fast/box-decoration-break/box-decoration-break-rendering.html failing on mac bots
Product: WebKit Reporter: Jon Lee <jonlee>
Component: Tools / TestsAssignee: Alexis Menard (darktears) <menard>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, menard, simon.fraser, webkit-bug-importer, webkit.review.bot
Priority: P2 Keywords: InRadar, LayoutTestFailure, MakingBotsRed
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.7   
URL: http://build.webkit.org/results/Apple%20Lion%20Release%20WK1%20(Tests)/r120838%20(351)/results.html
Attachments:
Description Flags
Rendering of the test case.
none
Test case rendering on Mac OS Lion
none
Ref file rendering on Mac OS Lion
none
Image diff
none
Patch none

Comment 1 Radar WebKit Bug Importer 2012-06-21 10:26:54 PDT
<rdar://problem/11719206>
Comment 2 Jon Lee 2012-06-21 10:32:13 PDT
This was not a regression, this was part of a new test added in r120835.
Comment 3 Jon Lee 2012-06-21 10:34:41 PDT
Skipped in r120940
Comment 4 Simon Fraser (smfr) 2012-06-21 10:45:21 PDT
Alexis, please take a look.
Comment 5 Alexis Menard (darktears) 2012-06-21 11:32:27 PDT
(In reply to comment #4)
> Alexis, please take a look.

I will as soon as I get my hands on a Mac (monday).

As http://build.webkit.org/results/Apple%20Lion%20Release%20WK1%20(Tests)/r120838%20(351)/fast/box-decoration-break/box-decoration-break-rendering-diffs.html

it's hard to see any difference. I will try to figure out what is going on.

Funny enough (or not) http://build.webkit.org/results/Apple%20Lion%20Release%20WK1%20(Tests)/r120838%20(351)/fast/box-decoration-break/box-decoration-break-rendering-diff.png shows differences for the slice case, which is the default one of box-decoration-break, i.e the behavior we had before, so it doesn't use the new code path at all.
Comment 6 Alexis Menard (darktears) 2012-06-25 13:49:19 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > Alexis, please take a look.
> 
> I will as soon as I get my hands on a Mac (monday).
> 
> As http://build.webkit.org/results/Apple%20Lion%20Release%20WK1%20(Tests)/r120838%20(351)/fast/box-decoration-break/box-decoration-break-rendering-diffs.html
> 
> it's hard to see any difference. I will try to figure out what is going on.
> 
> Funny enough (or not) http://build.webkit.org/results/Apple%20Lion%20Release%20WK1%20(Tests)/r120838%20(351)/fast/box-decoration-break/box-decoration-break-rendering-diff.png shows differences for the slice case, which is the default one of box-decoration-break, i.e the behavior we had before, so it doesn't use the new code path at all.

I looked at the differences and unlike all other ports Mac behave differently when it comes to border-radius.

If you look the attached image what the ref file is trying to reproduce is the right and left "cut" border-radius/border/shadow which happen when we layout a box in multiple lines. This is the box-decoration-break: slice case, which is the default case we always had in the past.

I declare the ref test like this :

#rightCutHighlight {
        display: inline;
        border-top: 2px solid red;
        border-right: 0px solid red;
        border-left: 2px solid red;
        border-bottom: 2px solid red;
        border-top-left-radius: 4px;
        border-top-right-radius: 0px;
        border-bottom-right-radius: 0px;
        border-bottom-left-radius: 4px;
        background: yellow;
        box-shadow: 0px 1px 3px dimgrey;
    }

which reproduce the same effect as the cut, at least semantically.

While this render exactly the same as the reference on Chromium, Qt, and GTK the radius renders differently on Mac. The anti-aliasing (or maybe the drawing itself) is 1 or 2 pixels different from the original file.

I noticed that if I change border-right: 0px solid red; to border-right: 2px solid white; I have no visual difference as well as no image difference. Therefore it seems that the drawing of the radius for some reason is influenced by the total size of the box (which I believe it shouldn't).

That's all for now. I will try to figure out how the radius is drawn in the Mac port.
Comment 7 Alexis Menard (darktears) 2012-06-25 13:50:06 PDT
Created attachment 149345 [details]
Rendering of the test case.
Comment 8 Alexis Menard (darktears) 2012-06-25 13:58:36 PDT
Created attachment 149349 [details]
Test case rendering on Mac OS Lion
Comment 9 Alexis Menard (darktears) 2012-06-25 13:59:55 PDT
Created attachment 149350 [details]
Ref file rendering on Mac OS Lion
Comment 10 Alexis Menard (darktears) 2012-06-25 14:00:51 PDT
Created attachment 149351 [details]
Image diff
Comment 11 Alexis Menard (darktears) 2012-06-26 14:11:51 PDT
(In reply to comment #10)
> Created an attachment (id=149351) [details]
> Image diff

So a bit more investigation leads me to :

RenderBoxModelObject::paintBorder()

where the ref test uses a different code path than the test case.

The test uses the fast path for drawing all solid edges and the ref test fallback to the other code path : paintBorderSides(). This itself explains why they paint differently (forcing the slow path all the time make the test pass). Now most of this code is cross ports and only call the graphic context with paths and various parameters. I need to debug further why on Mac the graphic context draws differently the slow path and the fast path.

The fast path is basically using path with a rounded rectangle and the clip make the "cut" effect.
The "slow" path draws borders one by one and in the ref test I specify only three borders.

I still think it's a Mac bug somehow as I would expect the radius to render exactly the same rather I use one path or the other (all ports seems to render the same). I will try to go deeper to find out why it is different.
Comment 12 Alexis Menard (darktears) 2012-06-29 07:36:57 PDT
Created attachment 150176 [details]
Patch
Comment 13 WebKit Review Bot 2012-07-02 12:56:53 PDT
Comment on attachment 150176 [details]
Patch

Clearing flags on attachment: 150176

Committed r121703: <http://trac.webkit.org/changeset/121703>
Comment 14 WebKit Review Bot 2012-07-02 12:56:59 PDT
All reviewed patches have been landed.  Closing bug.