Bug 108508 – Enable CANVAS_PATH flag

• Status: RESOLVED
• Resolution: FIXED
• Component: Canvas

[Dirk Schulze]

This is a placeholder bug for logging all tests that need to be unskipped after enabling the CANVAS_PATH flag.

[Dirk Schulze]

Created attachment 189232 [details]
Test building

[Dirk Schulze]

Created attachment 189236 [details]
Second attempt

Not for review.

[Dirk Schulze]

Created attachment 189245 [details]
Patch

[Laszlo Gombos]

Dirk, would you mind setting it to 1 as well in Source/WTF/wtf/FeatureDefines.h ?

[Dirk Schulze]

(In reply to comment #4)
> Dirk, would you mind setting it to 1 as well in Source/WTF/wtf/FeatureDefines.h ?

Yes of course.

[Dirk Schulze]

Created attachment 189247 [details]
Patch

[Build Bot]

Comment on attachment 189247 [details]
Patch

Attachment 189247 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16657131

New failing tests:
fast/css/sticky/inline-sticky-abspos-child.html
canvas/philip/tests/2d.path.arc.scale.2.html
fast/canvas/canvas-composite-alpha.html
canvas/philip/tests/2d.path.stroke.skew.html
fast/canvas/webgl/read-pixels-test.html
fast/dom/constructed-objects-prototypes.html
canvas/philip/tests/2d.path.arcTo.scale.html
fast/css/sticky/sticky-both-sides.html
canvas/philip/tests/2d.path.arcTo.transformation.html
canvas/philip/tests/2d.path.stroke.scale1.html
fast/css/sticky/inline-sticky.html

[Dirk Schulze]

(In reply to comment #7)
> (From update of attachment 189247 [details])
> Attachment 189247 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://queues.webkit.org/results/16657131
> 
> New failing tests:
> fast/css/sticky/inline-sticky-abspos-child.html
> canvas/philip/tests/2d.path.arc.scale.2.html
> fast/canvas/canvas-composite-alpha.html
> canvas/philip/tests/2d.path.stroke.skew.html
> fast/canvas/webgl/read-pixels-test.html
> fast/dom/constructed-objects-prototypes.html
> canvas/philip/tests/2d.path.arcTo.scale.html
> fast/css/sticky/sticky-both-sides.html
> canvas/philip/tests/2d.path.arcTo.transformation.html
> canvas/philip/tests/2d.path.stroke.scale1.html
> fast/css/sticky/inline-sticky.html

I can not verify these problems. I run the tests multiple times on my mac build and all these tests passed every single time. I will watch the bots.

[Dirk Schulze]

Committed r143505: <http://trac.webkit.org/changeset/143505>

[Csaba Osztrogonác]

(In reply to comment #9)
> Committed r143505: <http://trac.webkit.org/changeset/143505>

It broke the Qt minimal build:
In file included from /ramdisk/qt-linux-release-minimal/build/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:45:0:
/ramdisk/qt-linux-release-minimal/build/Source/WebCore/html/canvas/DOMPath.h: In static member function 'static WTF::PassRefPtr<WebCore::DOMPath> WebCore::DOMPath::create(const WTF::String&)':
/ramdisk/qt-linux-release-minimal/build/Source/WebCore/html/canvas/DOMPath.h:45:101: error: no matching function for call to 'WebCore::DOMPath::DOMPath(const WTF::String&)'
/ramdisk/qt-linux-release-minimal/build/Source/WebCore/html/canvas/DOMPath.h:45:101: note: candidates are:
/ramdisk/qt-linux-release-minimal/build/Source/WebCore/html/canvas/DOMPath.h:60:5: note: WebCore::DOMPath::DOMPath(WebCore::DOMPath*)
/ramdisk/qt-linux-release-minimal/build/Source/WebCore/html/canvas/DOMPath.h:60:5: note:   no known conversion for argument 1 from 'const WTF::String' to 'WebCore::DOMPath*'
/ramdisk/qt-linux-release-minimal/build/Source/WebCore/html/canvas/DOMPath.h:55:5: note: WebCore::DOMPath::DOMPath(const WebCore::Path&)
/ramdisk/qt-linux-release-minimal/build/Source/WebCore/html/canvas/DOMPath.h:55:5: note:   no known conversion for argument 1 from 'const WTF::String' to 'const WebCore::Path&'
/ramdisk/qt-linux-release-minimal/build/Source/WebCore/html/canvas/DOMPath.h:54:5: note: WebCore::DOMPath::DOMPath()
/ramdisk/qt-linux-release-minimal/build/Source/WebCore/html/canvas/DOMPath.h:54:5: note:   candidate expects 0 arguments, 1 provided
/ramdisk/qt-linux-release-minimal/build/Source/WebCore/html/canvas/DOMPath.h:42:14: note: WebCore::DOMPath::DOMPath(const WebCore::DOMPath&)
/ramdisk/qt-linux-release-minimal/build/Source/WebCore/html/canvas/DOMPath.h:42:14: note:   no known conversion for argument 1 from 'const WTF::String' to 'const WebCore::DOMPath&'


I cc Qt guys, maybe you can help fixing it.

[Oliver Hunt]

(In reply to comment #9)
> Committed r143505: <http://trac.webkit.org/changeset/143505>

Looks like a number of tests started failing after this -- presumably some interaction between the canvas path logic and regular path stuff?

[Dirk Schulze]

(In reply to comment #10)
> (In reply to comment #9)
> > Committed r143505: <http://trac.webkit.org/changeset/143505>
> I cc Qt guys, maybe you can help fixing it.

Definitely. It would be interesting to know why this did not happen on the EWS bots. Both, WK1 and WK2 Qt passed.

[Dirk Schulze]

(In reply to comment #11)
> (In reply to comment #9)
> > Committed r143505: <http://trac.webkit.org/changeset/143505>
> 
> Looks like a number of tests started failing after this -- presumably some interaction between the canvas path logic and regular path stuff?

It actually does not touch anything of the current logic for drawing paths. If it is related to this patch, I wonder if one of the new tests causes flakiness. I run the test suite multiple times with the patch applied and never got any problems. I don't know how to investigate so.

Do you have the link to the results of the failing tests?

[Dirk Schulze]

(In reply to comment #13)
> (In reply to comment #11)
> > (In reply to comment #9)
> > > Committed r143505: <http://trac.webkit.org/changeset/143505>
> > 
> > Looks like a number of tests started failing after this -- presumably some interaction between the canvas path logic and regular path stuff?
> 
> It actually does not touch anything of the current logic for drawing paths. If it is related to this patch, I wonder if one of the new tests causes flakiness. I run the test suite multiple times with the patch applied and never got any problems. I don't know how to investigate so.
> 
> Do you have the link to the results of the failing tests?

From the last build bots result I get the following crashing tests:

  fast/canvas/canvas-currentPath.html [ Failure ]
  fast/canvas/canvas-path-constructors.html [ Failure ]
  fast/dom/constructed-objects-prototypes.html [ Failure ]

http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK1%20(Tests)/r143533%20(5686)/fast/canvas/canvas-currentPath-pretty-diff.html

From the results it looks like the attribute currentPath is not working on the bots properly. I am building from scratch again right now and investigate. The last test needs a rebaseline.

Do I miss more failing tests?

[Dirk Schulze]

(In reply to comment #10)
> (In reply to comment #9)
> > Committed r143505: <http://trac.webkit.org/changeset/143505>
> I cc Qt guys, maybe you can help fixing it.

That build does not support SVG. I guarded everything but forgot one thing:

#if ENABLE(SVG)
    DOMPath(const String& pathData)
        : CanvasPathMethods()
    {
        buildPathFromString(pathData, m_path);
    }
#endif

should be

    DOMPath(const String& pathData)
        : CanvasPathMethods()
    {
#if ENABLE(SVG)
        buildPathFromString(pathData, m_path);
#else
        UNUSED_PARAM(pathData);
#endif
    }

I do not have a clean build right now. But will upload a patch ASAP.

[Dirk Schulze]

Committed r143554: <http://trac.webkit.org/changeset/143554>

[Dirk Schulze]

(In reply to comment #16)
> Committed r143554: <http://trac.webkit.org/changeset/143554>

Landed a fix for Qt and rebaselined the Ctor test.

The other two tests work absolutely fine for me. Maybe a completely clean build might fix this problem? It looks like the currentPath attribute on CanvasRenderingContext2D is not enabled on the bots.

[Allan Sandfeld Jensen]

Reopening to attach new patch.

[Allan Sandfeld Jensen]

Created attachment 190712 [details]
Patch

[Tor Arne Vestbø]

Comment on attachment 190712 [details]
Patch

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

> Tools/ChangeLog:4
> +        âhttps://bugs.webkit.org/show_bug.cgi?id=108508

?

> LayoutTests/ChangeLog:4
> +        âhttps://bugs.webkit.org/show_bug.cgi?id=108508

?

[Allan Sandfeld Jensen]

Landed in https://trac.webkit.org/r144300

[Ádám Kallai]

fast/canvas/canvas-currentPath.html test fails on Qt.
Diff is here:

--- /ramdisk/qt-linux-64-release/build/layout-test-results/fast/canvas/canvas-currentPath-expected.txt
+++ /ramdisk/qt-linux-64-release/build/layout-test-results/fast/canvas/canvas-currentPath-actual.txt
@@ -62,7 +62,7 @@
 Transform CTM in the process of adding segments to context path. Check that currentPath's path object archive these transformations.
 PASS ctx.isPointInPath(49,49) is true
 PASS ctx.isPointInPath(99,99) is true
-PASS ctx.isPointInPath(149,149) is false
+FAIL ctx.isPointInPath(149,149) should be false. Was true.
 PASS ctx.isPointInPath(199,199) is true
 PASS ctx.isPointInPath(249,249) is true
 Clear current path on object and check that it is cleaned up.
@@ -75,7 +75,7 @@
 Apply path back to context path.
 PASS ctx.isPointInPath(49,49) is true
 PASS ctx.isPointInPath(99,99) is true
-PASS ctx.isPointInPath(149,149) is false
+FAIL ctx.isPointInPath(149,149) should be false. Was true.
 PASS ctx.isPointInPath(199,199) is true
 PASS ctx.isPointInPath(249,249) is true
 PASS successfullyParsed is true

Could you check it please?

[Allan Sandfeld Jensen]

(In reply to comment #22)
> fast/canvas/canvas-currentPath.html test fails on Qt.
<snip>
> Could you check it please?
It fails the same way here. Looks like a wrong rounding or something similar small. I would make the test as failing.