Bug 41311 – Skia: Need to implement GraphicsContext::clipConvexPolygon()

• Component: CSS
• Status: RESOLVED FIXED

[Beth Dakin]

http://trac.webkit.org/changeset/62035 introduced a new method of drawing border-radius using paths. Right now, this new code is only enabled for some platforms. To enable the new and much improved code for Skia, GraphicsContext::clipConvexPolygon() needs to be implemented, and then Skia should be added to the list of platforms that set #define HAVE_PATH_BASED_BORDER_RADIUS_DRAWING in RenderObject.h

I would like to note that Skia already has a function implemented called GraphicsContext::drawConvexPolygon(). So hopefully it is straightforward to use some of that same logic for clipping instead of drawing.

[James Robinson]

I'm guessing this clip should be anti-aliased, right?

[Adam Langley]

(In reply to comment #1)
> I'm guessing this clip should be anti-aliased, right?

I would guess so, yes. It would have to be a layer with outside clearing, like our rounded corners. If you want to take this patch, please speak up, otherwise I'll probably do it this week.

[Beth Dakin]

(In reply to comment #1)
> I'm guessing this clip should be anti-aliased, right?

Yes, I think so.

[James Robinson]

Go for it.  I'm interested but busy with other things.

[Adam Langley]

The patch is pretty trivial (see below). I'll do the ChangeLog and layout tests song-and-dance tomorrow.

We still suck at some rounded corners. However, I spent an few hours today convincing myself (again) that it's just not possible to do a good job with our layer-based clipping. (At least, not without using a lot of memory.)




Index: platform/graphics/skia/GraphicsContextSkia.cpp
===================================================================
--- platform/graphics/skia/GraphicsContextSkia.cpp      (revision 62103)
+++ platform/graphics/skia/GraphicsContextSkia.cpp      (working copy)
@@ -485,8 +485,13 @@
 
     if (numPoints <= 1)
         return;
-    
-    // FIXME: IMPLEMENT!!
+
+    SkPath path;
+    path.moveTo(points[0].x(), points[0].y());
+    for (size_t i = 1; i < numPoints; i++)
+        path.lineTo(points[i].x(), points[i].y());
+    path.setFillType(SkPath::kInverseWinding_FillType);
+    platformContext()->clipPathAntiAliased(path);
 }
 
 // This method is only used to draw the little circles used in lists.
Index: rendering/RenderObject.h
===================================================================
--- rendering/RenderObject.h    (revision 62103)
+++ rendering/RenderObject.h    (working copy)
@@ -37,7 +37,7 @@
 #include "TransformationMatrix.h"
 #include <wtf/UnusedParam.h>
 
-#if PLATFORM(CG)
+#if PLATFORM(CG) || PLATFORM(CHROMIUM)
 #define HAVE_PATH_BASED_BORDER_RADIUS_DRAWING 1
 #endif

[Peter Kasting]

Looks like, much like with CG's implementation, ours might be able to share the first portion with the existing drawConvexPolygon() implementation (which does a few other checks and conversions).

[Beth Dakin]

(In reply to comment #5)
> The patch is pretty trivial (see below). I'll do the ChangeLog and layout tests song-and-dance tomorrow.
> 

Awesome! I'm excited to see a patch so quickly for this!

> We still suck at some rounded corners. However, I spent an few hours today convincing myself (again) that it's just not possible to do a good job with our layer-based clipping. (At least, not without using a lot of memory.)

It's true that it's not terribly difficult to find something that still looks buggy when you play with the interactive test case. I have filed a few bugs that remain with the new implementation; let me know if you want links. I hope to fix some of them soon. I doubt we will fix ALL edge cases, but we have already fixed a huge number of bugs with the new implementation, so I think we're okay for now.

[Adam Langley]

(In reply to comment #7)
> It's true that it's not terribly difficult to find something that still looks buggy when you play with the interactive test case. I have filed a few bugs that remain with the new implementation; let me know if you want links. I hope to fix some of them soon. I doubt we will fix ALL edge cases, but we have already fixed a huge number of bugs with the new implementation, so I think we're okay for now.

One thing that I did notice. If you look at the image in:

http://trac.webkit.org/browser/trunk/LayoutTests/platform/mac/fast/borders/borderRadiusDashed01-expected.png?rev=62035

The bottom rightmost, red dash has a faint line across it. It's something that we see in Chrome also. I would guess that it's the result of anti-aliased paths not joining correctly. (Coincidently, that's why Chrome don't do anti-aliased clipping for canvas.) I don't know what to do about it however.

[Beth Dakin]

(In reply to comment #8)
> (In reply to comment #7)
> > It's true that it's not terribly difficult to find something that still looks buggy when you play with the interactive test case. I have filed a few bugs that remain with the new implementation; let me know if you want links. I hope to fix some of them soon. I doubt we will fix ALL edge cases, but we have already fixed a huge number of bugs with the new implementation, so I think we're okay for now.
> 
> One thing that I did notice. If you look at the image in:
> 
> http://trac.webkit.org/browser/trunk/LayoutTests/platform/mac/fast/borders/borderRadiusDashed01-expected.png?rev=62035
> 
> The bottom rightmost, red dash has a faint line across it. It's something that we see in Chrome also. I would guess that it's the result of anti-aliased paths not joining correctly. (Coincidently, that's why Chrome don't do anti-aliased clipping for canvas.) I don't know what to do about it however.

Oooh, very interesting. I will file a bug for this.

[Adam Langley]

Created attachment 60140 [details]
Patch

[Adam Langley]

Comment on attachment 60140 [details]
Patch

Am actually removing the review? flag from this patch because I think we're worse off with this change. Partly it's because this change tickles our "works only because of luck" anti-aliased clipping a different way and doesn't work as well, and partly because the joins are very visable (which I also see with CG).

Maybe we should hold off on this until we have immediate-mode anti-aliased clipping in Skia?

[Beth Dakin]

(In reply to comment #11)
> (From update of attachment 60140 [details])
> Am actually removing the review? flag from this patch because I think we're worse off with this change. Partly it's because this change tickles our "works only because of luck" anti-aliased clipping a different way and doesn't work as well, and partly because the joins are very visable (which I also see with CG).
> 
> Maybe we should hold off on this until we have immediate-mode anti-aliased clipping in Skia?

It's also an option not to anti-alias the clip. Honestly, I am considering turning off anti-aliased clipping for this in CG because of the corner joins problem anyway. I'm still trying to wrap my head around all of the edge cases. (I'm building up a bunch of tests now.) But even if we decided to keep it on in CG and you turned it off in Skia then your border-radius drawing would still be MUCH closer to CG-WebKit's trunk than it will be if you don't implement this function at all.

[Adam Langley]

(In reply to comment #12)
> It's also an option not to anti-alias the clip.

You mean the polygon clip only? With this patch we also lose anti-aliasing of the inside of the rounded corners (probably because we can't anti-alias clipOut).

[Beth Dakin]

(In reply to comment #13)
> (In reply to comment #12)
> > It's also an option not to anti-alias the clip.
> 
> You mean the polygon clip only? With this patch we also lose anti-aliasing of the inside of the rounded corners (probably because we can't anti-alias clipOut).

Oh, I see. I did mean polygon clip only.  We still have anti-aliasing inside the rounded corners with CG. I didn't realize there were clipOut issues with Skia.

[Mike Reed]

Created attachment 84596 [details]
implements clipConvexPolygon

[Stephen White]

Comment on attachment 84596 [details]
implements clipConvexPolygon

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

> WebCore/ChangeLog:5
> +        Implement clipConvexPolygon for PLATFORM(SKIA)

I think PLATFORM(SKIA) should now be USE(SKIA), as of http://trac.webkit.org/changeset/79578.

> WebCore/rendering/RenderObject.h:40
> +#if PLATFORM(CG) || PLATFORM(CAIRO) || PLATFORM(QT) || PLATFORM(SKIA)

Ibid.

[Mike Reed]

Created attachment 84606 [details]
change PLATFORM(SKIA) to USE(SKIA)

[James Robinson]

Well that's pretty simple!  Has skia changed since the last time this bug was pinged such that antialiasing artifacts on path joins are no longer an issue?

[James Robinson]

Comment on attachment 84606 [details]
change PLATFORM(SKIA) to USE(SKIA)

Mike said he'd look into the graphical glitches that nixed this patch in the first place.  Clearing flags until we know if this will render border radius correctly.

[Mike Reed]

Created attachment 84785 [details]
implement clipConvexPolygon but don't enable HAVE_PATH_BASED_BORDER_RADIUS_DRAWING yet.

[James Robinson]

Comment on attachment 84785 [details]
implement clipConvexPolygon but don't enable HAVE_PATH_BASED_BORDER_RADIUS_DRAWING yet.

This patch looks good, but is the new codepath used at all without the macro set?  I'm a little wary of checking in code that can never actually run (after all, how will we know if it breaks?).

[Dirk Schulze]

Comment on attachment 84785 [details]
implement clipConvexPolygon but don't enable HAVE_PATH_BASED_BORDER_RADIUS_DRAWING yet.

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

LGTM. r=me

> WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:455
> +    for (size_t i = 1; i < numPoints; i++) {

use ++i instead of i++ here

[Ojan Vafai]

Comment on attachment 84785 [details]
implement clipConvexPolygon but don't enable HAVE_PATH_BASED_BORDER_RADIUS_DRAWING yet.

Please post a new patch addressing Dirk's last comment. Once you are a committer, it would be OK for you to make the change locally and commit since he r+'ed the patch. Until then, you'll need to post a new patch here for the commit-queue.

[Mike Reed]

sorry. I didn't expect a review+ to still want a change. Will upload it shortly.

[Ojan Vafai]

(In reply to comment #24)
> sorry. I didn't expect a review+ to still want a change. Will upload it shortly.

No worries. It's common practice in WebKit for the cases where you're happy with a patch, but ideally would like to see a few nits addressed before it's committed. That way you don't need to go through a whole new round of review just for a few nits.

[Mike Reed]

For future reference, does webkit always preincrement loop variables? I ask, having looked for it in the style guide. The for-loops I see there all post-increment.

e.g.

for (int i = 0; i < 10; i++)
    doSomething();

[Ojan Vafai]

(In reply to comment #26)
> For future reference, does webkit always preincrement loop variables? I ask, having looked for it in the style guide. The for-loops I see there all post-increment.
> 
> e.g.
> 
> for (int i = 0; i < 10; i++)
>     doSomething();

It's a defacto style rule. We should change the examples in the style guide.

[Mike Reed]

Created attachment 91670 [details]
Patch

[Mike Reed]

Created attachment 91671 [details]
Patch

[Mike Reed]

had to reinstall patch locally, and then upload with preincrement fix.

[Ojan Vafai]

Comment on attachment 91671 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        No new tests. covered by existing layout tests

Technically this doesn't affect tests because it's behind a flag we don't have on right now, right?

[Mike Reed]

That's correct -- I have tested it locally, but it will not be exercised in the build yet.

[WebKit Commit Bot]

Comment on attachment 91671 [details]
Patch

Clearing flags on attachment: 91671

Committed r85330: <http://trac.webkit.org/changeset/85330>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[James Robinson]

This changed border rendering for 500 tests on linux.  Since I do not think any change in behavior was intended I'm going to revert to try to figure things out.

[James Robinson]

Reverted r85330 for reason:

Caused

Committed r85350: <http://trac.webkit.org/changeset/85350>

[James Robinson]

Example: css2.1/t0805-c5516-brdr-c-00-a.html used to render like this:
http://svn.webkit.org/repository/webkit/trunk/LayoutTests/platform/chromium-linux/css2.1/t0805-c5516-brdr-c-00-a-expected.png

but with this patch rendered like this:
http://build.chromium.org/f/chromium/layout_test_results/Webkit_Linux/results/layout-test-results/css2.1/t0805-c5516-brdr-c-00-a-actual.png

diff:

http://build.chromium.org/f/chromium/layout_test_results/Webkit_Linux/results/layout-test-results/css2.1/t0805-c5516-brdr-c-00-a-diff.png

[James Robinson]

List of tests affected on the chromium linux canary:

tables/mozilla/bugs/bug51140.html,css2.1/t0805-c5516-brdr-c-00-a.html,css2.1/t0805-c5516-ibrdr-c-00-a.html,css2.1/t0805-c5517-brdr-s-00-c.html,css2.1/t0805-c5522-brdr-00-b.html,css2.1/t0805-c5522-ibrdr-00-a.html,editing/deleting/5026848-3.html,editing/deleting/5032066.html,editing/deleting/5099303.html,editing/deleting/5115601.html,editing/deleting/5126166.html,editing/deleting/5206311-1.html,editing/deleting/5206311-2.html,editing/deleting/5433862-2.html,editing/deleting/5483370.html,editing/execCommand/5481523.html,editing/execCommand/create-list-with-hr.html,editing/execCommand/find-after-replace.html,editing/execCommand/insertHorizontalRule.html,editing/execCommand/paste-1.html,editing/execCommand/paste-2.html,editing/pasteboard/3976872.html,editing/pasteboard/4076267-2.html,editing/pasteboard/4076267-3.html,editing/pasteboard/4076267.html,editing/pasteboard/5156401-1.html,editing/pasteboard/5387578.html,editing/selection/4402375.html,editing/selection/4776665.html,editing/selection/4818145.html,editing/selection/4889598.html,editing/selection/4895428-1.html,editing/selection/4895428-2.html,editing/selection/4895428-4.html,editing/selection/4960137.html,editing/selection/4975120.html,editing/selection/5057506-2.html,editing/selection/5057506.html,editing/selection/6476.html,editing/selection/7152-1.html,editing/selection/7152-2.html,editing/selection/click-start-of-line.html,editing/selection/iframe.html,editing/selection/image-before-linebreak.html,editing/selection/inline-table.html,editing/selection/mixed-editability-3.html,editing/selection/mixed-editability-4.html,editing/selection/mixed-editability-5.html,editing/selection/mixed-editability-8.html,editing/selection/mixed-editability-9.html,editing/selection/move-by-line-001.html,editing/selection/select-all-iframe.html,fast/css/MarqueeLayoutTest.html,fast/css/acid2-pixel.html,fast/css/apple-prefix.html,fast/css/border-radius-outline-offset.html,fast/css/fieldset-display-row.html,fast/css/find-next-layer.html,fast/css/first-letter-detach.html,fast/css/h1-in-section-elements.html,fast/css/hover-subselector.html,fast/css/input-search-padding.html,fast/css/list-outline.html,fast/css/outline-auto-location.html,fast/css/resize-corner-tracking.html,fast/css/rtl-ordering.html,fast/css/universal-hover-quirk.html,fast/dom/34176.html,fast/dom/attr_dead_doc.html,fast/dom/clone-node-dynamic-style.html,fast/dom/css-inline-style-important.html,fast/dom/isindex-001.html,fast/dom/isindex-002.html,fast/dom/row-inner-text.html,fast/forms/006.html,fast/forms/007.html,fast/forms/button-default-title.html,fast/forms/button-generated-content.html,fast/forms/button-inner-block-reuse.html,fast/forms/button-style-color.html,fast/forms/caret-rtl.html,fast/forms/fieldset-align.html,fast/forms/fieldset-with-float.html,fast/forms/float-before-fieldset.html,fast/forms/floating-textfield-relayout.html,fast/forms/image-border.html,fast/forms/input-appearance-bkcolor.html,fast/forms/input-disabled-color.html,fast/forms/input-value.html,fast/forms/isindex-placeholder.html,fast/forms/radio_checked_dynamic.html,fast/forms/search-vertical-alignment.html,fast/forms/select-baseline.html,fast/forms/select-dirty-parent-pref-widths.html,fast/forms/targeted-frame-submission.html,fast/forms/text-style-color.html,http/tests/loading/simple-subframe.html,http/tests/local/file-url-sent-as-referer.html,http/tests/misc/acid2-pixel.html,http/tests/misc/iframe404.html,http/tests/misc/location-replace-crossdomain.html,svg/custom/inline-svg-in-xhtml.xml,svg/custom/repaint-moving-svg-and-div.xhtml,svg/custom/svg-float-border-padding.xml,svg/dynamic-updates/SVGFEDropShadowElement-dom-shadow-color-attr.html,svg/dynamic-updates/SVGFEDropShadowElement-dom-shadow-opacity-attr.html,svg/dynamic-updates/SVGFEDropShadowElement-svgdom-shadow-color-prop.html,svg/dynamic-updates/SVGFEDropShadowElement-svgdom-shadow-opacity-prop.html,tables/mozilla/bugs/bug10009.html,tables/mozilla/bugs/bug10036.html,tables/mozilla/bugs/bug10039.html,tables/mozilla/bugs/bug101201.html,tables/mozilla/bugs/bug1055-1.html,tables/mozilla/bugs/bug10565.html,tables/mozilla/bugs/bug1067-2.html,tables/mozilla/bugs/bug106795.html,tables/mozilla/bugs/bug106816.html,tables/mozilla/bugs/bug108340.html,tables/mozilla/bugs/bug109043.html,tables/mozilla/bugs/bug110566.html,tables/mozilla/bugs/bug113235-1.html,tables/mozilla/bugs/bug113235-3.html,tables/mozilla/bugs/bug113424.html,tables/mozilla/bugs/bug11384q.html,tables/mozilla/bugs/bug11384s.html,tables/mozilla/bugs/bug1188.html,tables/mozilla/bugs/bug11944.html,tables/mozilla/bugs/bug119786.html,tables/mozilla/bugs/bug12008.html,tables/mozilla/bugs/bug1220.html,tables/mozilla/bugs/bug1224.html,tables/mozilla/bugs/bug12268.html,tables/mozilla/bugs/bug12384.html,tables/mozilla/bugs/bug1261.html,tables/mozilla/bugs/bug12709.html,tables/mozilla/bugs/bug128229.html,tables/mozilla/bugs/bug12908-1.html,tables/mozilla/bugs/bug12910.html,tables/mozilla/bugs/bug1302.html,tables/mozilla/bugs/bug131020-2.html,tables/mozilla/bugs/bug131020.html,tables/mozilla/bugs/bug13118.html,tables/mozilla/bugs/bug13169.html,tables/mozilla/bugs/bug13196.html,tables/mozilla/bugs/bug133756-2.html,tables/mozilla/bugs/bug133948.html,tables/mozilla/bugs/bug13484.html,tables/mozilla/bugs/bug137388-1.html,tables/mozilla/bugs/bug137388-2.html,tables/mozilla/bugs/bug137388-3.html,tables/mozilla/bugs/bug139524-4.html,tables/mozilla/bugs/bug14323.html,tables/mozilla/bugs/bug1474.html,tables/mozilla/bugs/bug149275-1.html,tables/mozilla/bugs/bug149275-2.html,tables/mozilla/bugs/bug14929.html,tables/mozilla/bugs/bug15247.html,tables/mozilla/bugs/bug15933.html,tables/mozilla/bugs/bug16252.html,tables/mozilla/bugs/bug17168.html,tables/mozilla/bugs/bug175455-4.html,tables/mozilla/bugs/bug17548.html,tables/mozilla/bugs/bug17587.html,tables/mozilla/bugs/bug1800.html,tables/mozilla/bugs/bug1802.html,tables/mozilla/bugs/bug1802s.html,tables/mozilla/bugs/bug1809.html,tables/mozilla/bugs/bug1818-1.html,tables/mozilla/bugs/bug1818-2.html,tables/mozilla/bugs/bug1818-3.html,tables/mozilla/bugs/bug1818-4.html,tables/mozilla/bugs/bug1828.html,tables/mozilla/bugs/bug18359.html,tables/mozilla/bugs/bug18440.html,tables/mozilla/bugs/bug18558.html,tables/mozilla/bugs/bug18955.html,tables/mozilla/bugs/bug19061-1.html,tables/mozilla/bugs/bug19061-2.html,tables/mozilla/bugs/bug19356.html,tables/mozilla/bugs/bug194024.html,tables/mozilla/bugs/bug19599.html,tables/mozilla/bugs/bug2050.html,tables/mozilla/bugs/bug2065.html,tables/mozilla/bugs/bug21299.html,tables/mozilla/bugs/bug21918.html,tables/mozilla/bugs/bug219693-1.html,tables/mozilla/bugs/bug219693-2.html,tables/mozilla/bugs/bug220536.html,tables/mozilla/bugs/bug221784-1.html,tables/mozilla/bugs/bug221784-2.html,tables/mozilla/bugs/bug22246-2.html,tables/mozilla/bugs/bug22246-2a.html,tables/mozilla/bugs/bug22246-3.html,tables/mozilla/bugs/bug22246-3a.html,tables/mozilla/bugs/bug222846.html,tables/mozilla/bugs/bug2267.html,tables/mozilla/bugs/bug2296.html,tables/mozilla/bugs/bug23235.html,tables/mozilla/bugs/bug24503.html,tables/mozilla/bugs/bug24627.html,tables/mozilla/bugs/bug24661.html,tables/mozilla/bugs/bug2479-3.html,tables/mozilla/bugs/bug24880.html,tables/mozilla/bugs/bug25004.html,tables/mozilla/bugs/bug25086.html,tables/mozilla/bugs/bug2516.html,tables/mozilla/bugs/bug25663.html,tables/mozilla/bugs/bug2585.html,tables/mozilla/bugs/bug26553.html,tables/mozilla/bugs/bug2684.html,tables/mozilla/bugs/bug27038-2.html,tables/mozilla/bugs/bug2757.html,tables/mozilla/bugs/bug2773.html,tables/mozilla/bugs/bug278385.html,tables/mozilla/bugs/bug27993-1.html,tables/mozilla/bugs/bug28928.html,tables/mozilla/bugs/bug29058-1.html,tables/mozilla/bugs/bug29058-3.html,tables/mozilla/bugs/bug29157.html,tables/mozilla/bugs/bug29326.html,tables/mozilla/bugs/bug29429.html,tables/mozilla/bugs/bug2962.html,tables/mozilla/bugs/bug2981-2.html,tables/mozilla/bugs/bug30273.html,tables/mozilla/bugs/bug30332-1.html,tables/mozilla/bugs/bug30332-2.html,tables/mozilla/bugs/bug30418.html,tables/mozilla/bugs/bug30559.html,tables/mozilla/bugs/bug30692.html,tables/mozilla/bugs/bug3103.html,tables/mozilla/bugs/bug3191.html,tables/mozilla/bugs/bug32205-2.html,tables/mozilla/bugs/bug32205-3.html,tables/mozilla/bugs/bug32205-5.html,tables/mozilla/bugs/bug3260.html,tables/mozilla/bugs/bug3263.html,tables/mozilla/bugs/bug32841.html,tables/mozilla/bugs/bug3309-2.html,tables/mozilla/bugs/bug33137.html,tables/mozilla/bugs/bug34176.html,tables/mozilla/bugs/bug3454.html,tables/mozilla/bugs/bug35662.html,tables/mozilla/bugs/bug3718.html,tables/mozilla/bugs/bug38916.html,tables/mozilla/bugs/bug39209.html,tables/mozilla/bugs/bug3977.html,tables/mozilla/bugs/bug40828.html,tables/mozilla/bugs/bug4093.html,tables/mozilla/bugs/bug41890.html,tables/mozilla/bugs/bug42187.html,tables/mozilla/bugs/bug42443.html,tables/mozilla/bugs/bug4284.html,tables/mozilla/bugs/bug4385.html,tables/mozilla/bugs/bug43854-1.html,tables/mozilla/bugs/bug4427.html,tables/mozilla/bugs/bug4429.html,tables/mozilla/bugs/bug44505.html,tables/mozilla/bugs/bug44523.html,tables/mozilla/bugs/bug4501.html,tables/mozilla/bugs/bug4520.html,tables/mozilla/bugs/bug4523.html,tables/mozilla/bugs/bug45486.html,tables/mozilla/bugs/bug4576.html,tables/mozilla/bugs/bug46268-1.html,tables/mozilla/bugs/bug46268-2.html,tables/mozilla/bugs/bug46268-3.html,tables/mozilla/bugs/bug46268-5.html,tables/mozilla/bugs/bug46268.html,tables/mozilla/bugs/bug46368-1.html,tables/mozilla/bugs/bug46368-2.html,tables/mozilla/bugs/bug46623-1.html,tables/mozilla/bugs/bug46623-2.html,tables/mozilla/bugs/bug46924.html,tables/mozilla/bugs/bug46944.html,tables/mozilla/bugs/bug4739.html,tables/mozilla/bugs/bug47432.html,tables/mozilla/bugs/bug48028-1.html,tables/mozilla/bugs/bug48028-2.html,tables/mozilla/bugs/bug50695-1.html,tables/mozilla/bugs/bug50695-2.html,tables/mozilla/bugs/bug51037.html,tables/mozilla/bugs/bug5188.html,tables/mozilla/bugs/bug53690-1.html,tables/mozilla/bugs/bug53690-2.html,tables/mozilla/bugs/bug53891.html,tables/mozilla/bugs/bug54450.html,tables/mozilla/bugs/bug55694.html,tables/mozilla/bugs/bug56201.html,tables/mozilla/bugs/bug56563.html,tables/mozilla/bugs/bug57300.html,tables/mozilla/bugs/bug57378.html,tables/mozilla/bugs/bug57828.html,tables/mozilla/bugs/bug5797.html,tables/mozilla/bugs/bug5798.html,tables/mozilla/bugs/bug5799.html,tables/mozilla/bugs/bug5835.html,tables/mozilla/bugs/bug5838.html,tables/mozilla/bugs/bug58402-1.html,tables/mozilla/bugs/bug59354.html,tables/mozilla/bugs/bug60013.html,tables/mozilla/bugs/bug60749.html,tables/mozilla/bugs/bug60804.html,tables/mozilla/bugs/bug60807.html,tables/mozilla/bugs/bug60992.html,tables/mozilla/bugs/bug6184.html,tables/mozilla/bugs/bug6304.html,tables/mozilla/bugs/bug647.html,tables/mozilla/bugs/bug67915-1.html,tables/mozilla/bugs/bug68998.html,tables/mozilla/bugs/bug69187.html,tables/mozilla/bugs/bug69382-1.html,tables/mozilla/bugs/bug69382-2.html,tables/mozilla/bugs/bug7112-1.html,tables/mozilla/bugs/bug7112-2.html,tables/mozilla/bugs/bug7121-1.html,tables/mozilla/bugs/bug727.html,tables/mozilla/bugs/bug7342.html,tables/mozilla/bugs/bug7471.html,tables/mozilla/bugs/bug7714.html,tables/mozilla/bugs/bug78162.html,tables/mozilla/bugs/bug81934.html,tables/mozilla/bugs/bug82946-1.html,tables/mozilla/bugs/bug8381.html,tables/mozilla/bugs/bug86708.html,tables/mozilla/bugs/bug88035-1.html,tables/mozilla/bugs/bug88035-2.html,tables/mozilla/bugs/bug8858.html,tables/mozilla/bugs/bug8950.html,tables/mozilla/bugs/bug9123-1.html,tables/mozilla/bugs/bug9123-2.html,tables/mozilla/bugs/bug92143.html,tables/mozilla/bugs/bug92647-2.html,tables/mozilla/bugs/bug9271-1.html,tables/mozilla/bugs/bug9271-2.html,tables/mozilla/bugs/bug965.html,tables/mozilla/bugs/bug97138.html,tables/mozilla/bugs/bug98196.html,tables/mozilla/bugs/bug9879-1.html,tables/mozilla/bugs/bug99923.html,tables/mozilla/bugs/bug99948.html,tables/mozilla/marvin/body_col.html,tables/mozilla/marvin/col_span.html,tables/mozilla/marvin/colgroup_align_center.html,tables/mozilla/marvin/colgroup_align_justify.html,tables/mozilla/marvin/colgroup_align_left.html,tables/mozilla/marvin/colgroup_align_right.html,tables/mozilla/marvin/colgroup_span.html,tables/mozilla/marvin/colgroup_valign_baseline.html,tables/mozilla/marvin/colgroup_valign_bottom.html,tables/mozilla/marvin/colgroup_valign_middle.html,tables/mozilla/marvin/colgroup_valign_top.html,tables/mozilla/marvin/colgroup_width_pct.html,tables/mozilla/marvin/colgroup_width_px.html,tables/mozilla/marvin/table_frame_border.html,tables/mozilla/marvin/table_frame_box.html,tables/mozilla/marvin/table_overflow_hidden_td.html,tables/mozilla/marvin/table_overflow_td_dynamic_deactivate.html,tables/mozilla/marvin/table_row_align_center.html,tables/mozilla/marvin/table_row_align_left.html,tables/mozilla/marvin/table_row_align_right.html,tables/mozilla/marvin/tables_bgcolor_aqua.html,tables/mozilla/marvin/tables_bgcolor_aqua_rgb.html,tables/mozilla/marvin/tables_bgcolor_black.html,tables/mozilla/marvin/tables_bgcolor_black_rgb.html,tables/mozilla/marvin/tables_bgcolor_blue.html,tables/mozilla/marvin/tables_bgcolor_blue_rgb.html,tables/mozilla/marvin/tables_bgcolor_fuchsia.html,tables/mozilla/marvin/tables_bgcolor_fuchsia_rgb.html,tables/mozilla/marvin/tables_bgcolor_gray.html,tables/mozilla/marvin/tables_bgcolor_gray_rgb.html,tables/mozilla/marvin/tables_bgcolor_green.html,tables/mozilla/marvin/tables_bgcolor_green_rgb.html,tables/mozilla/marvin/tables_bgcolor_lime.html,tables/mozilla/marvin/tables_bgcolor_lime_rgb.html,tables/mozilla/marvin/tables_bgcolor_maroon.html,tables/mozilla/marvin/tables_bgcolor_maroon_rgb.html,tables/mozilla/marvin/tables_bgcolor_navy.html,tables/mozilla/marvin/tables_bgcolor_navy_rgb.html,tables/mozilla/marvin/tables_bgcolor_olive.html,tables/mozilla/marvin/tables_bgcolor_olive_rgb.html,tables/mozilla/marvin/tables_bgcolor_purple.html,tables/mozilla/marvin/tables_bgcolor_purple_rgb.html,tables/mozilla/marvin/tables_bgcolor_red.html,tables/mozilla/marvin/tables_bgcolor_red_rgb.html,tables/mozilla/marvin/tables_bgcolor_silver.html,tables/mozilla/marvin/tables_bgcolor_silver_rgb.html,tables/mozilla/marvin/tables_bgcolor_teal.html,tables/mozilla/marvin/tables_bgcolor_teal_rgb.html,tables/mozilla/marvin/tables_bgcolor_white.html,tables/mozilla/marvin/tables_bgcolor_white_rgb.html,tables/mozilla/marvin/tables_bgcolor_yellow.html,tables/mozilla/marvin/tables_bgcolor_yellow_rgb.html,tables/mozilla/marvin/tables_border_1.html,tables/mozilla/marvin/tables_border_2.html,tables/mozilla/marvin/tables_border_3.html,tables/mozilla/marvin/tables_caption_align_bot.html,tables/mozilla/marvin/tables_caption_align_top.html,tables/mozilla/marvin/tables_cellpadding.html,tables/mozilla/marvin/tables_cellpadding_pct.html,tables/mozilla/marvin/tables_class.html,tables/mozilla/marvin/tables_id.html,tables/mozilla/marvin/tables_row_th_nowrap.html,tables/mozilla/marvin/tables_td_align_center.html,tables/mozilla/marvin/tables_td_align_left.html,tables/mozilla/marvin/tables_td_align_right.html,tables/mozilla/marvin/tables_td_colspan.html,tables/mozilla/marvin/tables_td_height.html,tables/mozilla/marvin/tables_td_nowrap.html,tables/mozilla/marvin/tables_td_rowspan.html,tables/mozilla/marvin/tables_td_width.html,tables/mozilla/marvin/tables_th_align_center.html,tables/mozilla/marvin/tables_th_align_left.html,tables/mozilla/marvin/tables_th_align_right.html,tables/mozilla/marvin/tables_th_colspan.html,tables/mozilla/marvin/tables_th_height.html,tables/mozilla/marvin/tables_th_rowspan.html,tables/mozilla/marvin/tables_th_width.html,tables/mozilla/marvin/tbody_align_center.html,tables/mozilla/marvin/tbody_align_char.html,tables/mozilla/marvin/tbody_align_justify.html,tables/mozilla/marvin/tbody_align_left.html,tables/mozilla/marvin/tbody_align_right.html,tables/mozilla/marvin/tbody_char.html,tables/mozilla/marvin/tbody_valign_baseline.html,tables/mozilla/marvin/tbody_valign_bottom.html,tables/mozilla/marvin/tbody_valign_middle.html,tables/mozilla/marvin/tbody_valign_top.html,tables/mozilla/marvin/td_valign_baseline.html,tables/mozilla/marvin/td_valign_bottom.html,tables/mozilla/marvin/td_valign_middle.html,tables/mozilla/marvin/td_valign_top.html,tables/mozilla/marvin/tfoot_align_center.html,tables/mozilla/marvin/tfoot_align_char.html,tables/mozilla/marvin/tfoot_align_justify.html,tables/mozilla/marvin/tfoot_align_left.html,tables/mozilla/marvin/tfoot_align_right.html,tables/mozilla/marvin/tfoot_char.html,tables/mozilla/marvin/tfoot_valign_baseline.html,tables/mozilla/marvin/tfoot_valign_bottom.html,tables/mozilla/marvin/tfoot_valign_middle.html,tables/mozilla/marvin/tfoot_valign_top.html,tables/mozilla/marvin/th_valign_baseline.html,tables/mozilla/marvin/th_valign_bottom.html,tables/mozilla/marvin/th_valign_middle.html,tables/mozilla/marvin/th_valign_top.html,tables/mozilla/marvin/thead_align_center.html,tables/mozilla/marvin/thead_align_char.html,tables/mozilla/marvin/thead_align_justify.html,tables/mozilla/marvin/thead_align_left.html,tables/mozilla/marvin/thead_align_right.html,tables/mozilla/marvin/thead_char.html,tables/mozilla/marvin/thead_valign_baseline.html,tables/mozilla/marvin/thead_valign_bottom.html,tables/mozilla/marvin/thead_valign_middle.html,tables/mozilla/marvin/thead_valign_top.html,tables/mozilla/marvin/tr_bgcolor_aqua_rgb.html,tables/mozilla/marvin/tr_bgcolor_black.html,tables/mozilla/marvin/tr_bgcolor_black_rgb.html,tables/mozilla/marvin/tr_bgcolor_blue.html,tables/mozilla/marvin/tr_bgcolor_blue_rgb.html,tables/mozilla/marvin/tr_bgcolor_fuchsia.html,tables/mozilla/marvin/tr_bgcolor_fuchsia_rgb.html,tables/mozilla/marvin/tr_bgcolor_gray.html,tables/mozilla/marvin/tr_bgcolor_gray_rgb.html,tables/mozilla/marvin/tr_bgcolor_green.html,tables/mozilla/marvin/tr_bgcolor_green_rgb.html,tables/mozilla/marvin/tr_bgcolor_lime.html,tables/mozilla/marvin/tr_bgcolor_lime_rgb.html,tables/mozilla/marvin/tr_bgcolor_maroon.html,tables/mozilla/marvin/tr_bgcolor_maroon_rgb.html,tables/mozilla/marvin/tr_bgcolor_navy.html,tables/mozilla/marvin/tr_bgcolor_navy_rgb.html,tables/mozilla/marvin/tr_bgcolor_olive.html,tables/mozilla/marvin/tr_bgcolor_olive_rgb.html,tables/mozilla/marvin/tr_bgcolor_purple.html,tables/mozilla/marvin/tr_bgcolor_purple_rgb.html,tables/mozilla/marvin/tr_bgcolor_red.html,tables/mozilla/marvin/tr_bgcolor_red_rgb.html,tables/mozilla/marvin/tr_bgcolor_silver.html,tables/mozilla/marvin/tr_bgcolor_silver_rgb.html,tables/mozilla/marvin/tr_bgcolor_teal.html,tables/mozilla/marvin/tr_bgcolor_teal_rgb.html,tables/mozilla/marvin/tr_bgcolor_white.html,tables/mozilla/marvin/tr_bgcolor_white_rgb.html,tables/mozilla/marvin/tr_bgcolor_yellow.html,tables/mozilla/marvin/tr_bgcolor_yellow_rgb.html,tables/mozilla/marvin/tr_valign_baseline.html,tables/mozilla/marvin/tr_valign_bottom.html,tables/mozilla/marvin/tr_valign_middle.html,tables/mozilla/marvin/tr_valign_top.html,tables/mozilla/marvin/x_caption_align_bottom.xml,tables/mozilla/marvin/x_caption_align_top.xml,tables/mozilla/marvin/x_caption_class.xml,tables/mozilla/marvin/x_caption_id.xml,tables/mozilla/marvin/x_caption_style.xml,tables/mozilla/marvin/x_col_align_center.xml,tables/mozilla/marvin/x_col_align_char.xml,tables/mozilla/marvin/x_col_align_justify.xml,tables/mozilla/marvin/x_col_align_left.xml,fast/dom/prototype-inheritance.html

[James Robinson]

Oops, that last reply kind of breaks the layout of this page.  Hit the (-) link on the comment to get rid of the ugly horizontal scrollbar.

Also that list of failures is a bit too large - there are some editing and other failures mixed in there unrelated to this bug, but most of the failures are due to this.  Looks like some bit of code is mishandling the antialiasing bit.

[Mike Reed]

Will investigate (1) why it got called, and (2) if in fact the failures are just the additional call to respect shouldAntiAlias.

[James Robinson]

Hi Mike - have you had a chance to take another look at this yet?  I'd really like to turn HAVE(PATH_BASED_BORDER_RADIUS_DRAWING) on for skia ASAP as it fixes a lot of rendering glitches we have today.  We'll definitely need to enable this before we can enable skia for chromium mac.  Let me know if you need any help sorting this out!

[Mike Reed]

Created attachment 94432 [details]
Patch

[Mike Reed]

removed the antialiasing call, since that triggered the massive baseline failure.

[James Robinson]

Comment on attachment 94432 [details]
Patch

Great, thanks Mike!

[WebKit Commit Bot]

Comment on attachment 94432 [details]
Patch

Clearing flags on attachment: 94432

Committed r87099: <http://trac.webkit.org/changeset/87099>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.