Bug 41311 - Skia: Need to implement GraphicsContext::clipConvexPolygon()
Summary: Skia: Need to implement GraphicsContext::clipConvexPolygon()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Mike Reed
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-28 15:06 PDT by Beth Dakin
Modified: 2011-06-14 21:15 PDT (History)
10 users (show)

See Also:


Attachments
Patch (781.48 KB, patch)
2010-06-30 12:06 PDT, Adam Langley
no flags Details | Formatted Diff | Diff
implements clipConvexPolygon (3.35 KB, patch)
2011-03-03 11:22 PST, Mike Reed
no flags Details | Formatted Diff | Diff
change PLATFORM(SKIA) to USE(SKIA) (3.34 KB, patch)
2011-03-03 12:28 PST, Mike Reed
no flags Details | Formatted Diff | Diff
implement clipConvexPolygon but don't enable HAVE_PATH_BASED_BORDER_RADIUS_DRAWING yet. (3.01 KB, patch)
2011-03-04 11:26 PST, Mike Reed
no flags Details | Formatted Diff | Diff
Patch (3.10 KB, patch)
2011-04-29 06:29 PDT, Mike Reed
no flags Details | Formatted Diff | Diff
Patch (3.10 KB, patch)
2011-04-29 06:30 PDT, Mike Reed
no flags Details | Formatted Diff | Diff
Patch (2.66 KB, patch)
2011-05-23 09:01 PDT, Mike Reed
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2010-06-28 15:06:20 PDT
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.
Comment 1 James Robinson 2010-06-29 12:55:04 PDT
I'm guessing this clip should be anti-aliased, right?
Comment 2 Adam Langley 2010-06-29 13:02:35 PDT
(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.
Comment 3 Beth Dakin 2010-06-29 13:05:17 PDT
(In reply to comment #1)
> I'm guessing this clip should be anti-aliased, right?

Yes, I think so.
Comment 4 James Robinson 2010-06-29 13:08:20 PDT
Go for it.  I'm interested but busy with other things.
Comment 5 Adam Langley 2010-06-29 16:13:06 PDT
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
Comment 6 Peter Kasting 2010-06-29 16:16:15 PDT
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).
Comment 7 Beth Dakin 2010-06-29 16:17:33 PDT
(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.
Comment 8 Adam Langley 2010-06-29 16:24:36 PDT
(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.
Comment 9 Beth Dakin 2010-06-29 17:13:06 PDT
(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.
Comment 10 Adam Langley 2010-06-30 12:06:25 PDT
Created attachment 60140 [details]
Patch
Comment 11 Adam Langley 2010-06-30 12:11:42 PDT
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?
Comment 12 Beth Dakin 2010-06-30 12:28:32 PDT
(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.
Comment 13 Adam Langley 2010-06-30 12:35:07 PDT
(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).
Comment 14 Beth Dakin 2010-06-30 12:40:37 PDT
(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.
Comment 15 Mike Reed 2011-03-03 11:22:35 PST
Created attachment 84596 [details]
implements clipConvexPolygon
Comment 16 Stephen White 2011-03-03 11:51:07 PST
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.
Comment 17 Mike Reed 2011-03-03 12:28:27 PST
Created attachment 84606 [details]
change PLATFORM(SKIA) to USE(SKIA)
Comment 18 James Robinson 2011-03-03 13:33:13 PST
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?
Comment 19 James Robinson 2011-03-03 14:18:02 PST
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.
Comment 20 Mike Reed 2011-03-04 11:26:52 PST
Created attachment 84785 [details]
implement clipConvexPolygon but don't enable HAVE_PATH_BASED_BORDER_RADIUS_DRAWING yet.
Comment 21 James Robinson 2011-03-29 11:43:47 PDT
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?).
Comment 22 Dirk Schulze 2011-04-26 17:06:52 PDT
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
Comment 23 Ojan Vafai 2011-04-27 08:12:09 PDT
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.
Comment 24 Mike Reed 2011-04-27 08:22:05 PDT
sorry. I didn't expect a review+ to still want a change. Will upload it shortly.
Comment 25 Ojan Vafai 2011-04-27 09:37:08 PDT
(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.
Comment 26 Mike Reed 2011-04-27 10:04:08 PDT
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();
Comment 27 Ojan Vafai 2011-04-27 10:14:58 PDT
(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.
Comment 28 Mike Reed 2011-04-29 06:29:00 PDT
Created attachment 91670 [details]
Patch
Comment 29 Mike Reed 2011-04-29 06:30:32 PDT
Created attachment 91671 [details]
Patch
Comment 30 Mike Reed 2011-04-29 06:31:08 PDT
had to reinstall patch locally, and then upload with preincrement fix.
Comment 31 Ojan Vafai 2011-04-29 08:57:28 PDT
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?
Comment 32 Mike Reed 2011-04-29 09:17:07 PDT
That's correct -- I have tested it locally, but it will not be exercised in the build yet.
Comment 33 WebKit Commit Bot 2011-04-29 09:48:11 PDT
Comment on attachment 91671 [details]
Patch

Clearing flags on attachment: 91671

Committed r85330: <http://trac.webkit.org/changeset/85330>
Comment 34 WebKit Commit Bot 2011-04-29 09:48:20 PDT
All reviewed patches have been landed.  Closing bug.
Comment 35 James Robinson 2011-04-29 12:29:49 PDT
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.
Comment 36 James Robinson 2011-04-29 12:32:45 PDT
Reverted r85330 for reason:

Caused

Committed r85350: <http://trac.webkit.org/changeset/85350>
Comment 38 James Robinson 2011-04-29 12:35:44 PDT
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
Comment 39 James Robinson 2011-04-29 12:42:50 PDT
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.
Comment 40 Mike Reed 2011-04-29 12:48:50 PDT
Will investigate (1) why it got called, and (2) if in fact the failures are just the additional call to respect shouldAntiAlias.
Comment 41 James Robinson 2011-05-18 16:15:07 PDT
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!
Comment 42 Mike Reed 2011-05-23 09:01:23 PDT
Created attachment 94432 [details]
Patch
Comment 43 Mike Reed 2011-05-23 09:02:09 PDT
removed the antialiasing call, since that triggered the massive baseline failure.
Comment 44 James Robinson 2011-05-23 12:35:50 PDT
Comment on attachment 94432 [details]
Patch

Great, thanks Mike!
Comment 45 WebKit Commit Bot 2011-05-23 14:50:13 PDT
Comment on attachment 94432 [details]
Patch

Clearing flags on attachment: 94432

Committed r87099: <http://trac.webkit.org/changeset/87099>
Comment 46 WebKit Commit Bot 2011-05-23 14:50:29 PDT
All reviewed patches have been landed.  Closing bug.