| Summary: | Skia: Need to implement GraphicsContext::clipConvexPolygon() | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Beth Dakin <bdakin> | ||||||||||||||||
| Component: | CSS | Assignee: | Mike Reed <reed> | ||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||
| Severity: | Normal | CC: | agl, arv, bdakin, benwells, commit-queue, jamesr, ojan, pkasting, reed, senorblanco | ||||||||||||||||
| Priority: | P2 | ||||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||||
| Hardware: | PC | ||||||||||||||||||
| OS: | OS X 10.5 | ||||||||||||||||||
| Attachments: |
|
||||||||||||||||||
|
Description
Beth Dakin
2010-06-28 15:06:20 PDT
I'm guessing this clip should be anti-aliased, right? (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. (In reply to comment #1) > I'm guessing this clip should be anti-aliased, right? Yes, I think so. Go for it. I'm interested but busy with other things. 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
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). (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. (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. (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. Created attachment 60140 [details]
Patch
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?
(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. (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). (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. Created attachment 84596 [details]
implements clipConvexPolygon
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. Created attachment 84606 [details]
change PLATFORM(SKIA) to USE(SKIA)
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 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.
Created attachment 84785 [details]
implement clipConvexPolygon but don't enable HAVE_PATH_BASED_BORDER_RADIUS_DRAWING yet.
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 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 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.
sorry. I didn't expect a review+ to still want a change. Will upload it shortly. (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. 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();
(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. Created attachment 91670 [details]
Patch
Created attachment 91671 [details]
Patch
had to reinstall patch locally, and then upload with preincrement fix. 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? That's correct -- I have tested it locally, but it will not be exercised in the build yet. Comment on attachment 91671 [details] Patch Clearing flags on attachment: 91671 Committed r85330: <http://trac.webkit.org/changeset/85330> All reviewed patches have been landed. Closing bug. 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. Reverted r85330 for reason: Caused Committed r85350: <http://trac.webkit.org/changeset/85350> 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 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 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. Will investigate (1) why it got called, and (2) if in fact the failures are just the additional call to respect shouldAntiAlias. 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! Created attachment 94432 [details]
Patch
removed the antialiasing call, since that triggered the massive baseline failure. Comment on attachment 94432 [details]
Patch
Great, thanks Mike!
Comment on attachment 94432 [details] Patch Clearing flags on attachment: 94432 Committed r87099: <http://trac.webkit.org/changeset/87099> All reviewed patches have been landed. Closing bug. |
