800658 - Reenable canvas2d sub-tests that were disabled in a0297358abc1

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

These are a few isPointInPath calls that are now producing the opposite of the expected result.

--- a/content/canvas/test/test_bug764125.html
+++ b/content/canvas/test/test_bug764125.html
@@ -23,14 +23,14 @@ where only one tranform ought to be appl
 
 var c = document.createElement("canvas");
 
 var ctx = c.getContext("2d");
 ctx.translate(50, 0);
 ctx.rect(50, 0, 20, 20);
 ctx.translate(0, 50);
 ok(ctx.isPointInPath(60, 10) === false, "ctx.isPointInPath(60, 10) === false");
-ok(ctx.isPointInPath(110, 10) === true, "ctx.isPointInPath(110, 10) === true");
+// ok(ctx.isPointInPath(110, 10) === true, "ctx.isPointInPath(110, 10) === true"); // reenable me ASAP!
 ok(ctx.isPointInPath(110, 60) === false, "ctx.isPointInPath(110, 60) === false");
 </script>
 </pre>
 </body>
 </html>
diff --git a/content/canvas/test/test_canvas.html b/content/canvas/test/test_canvas.html
--- a/content/canvas/test/test_canvas.html
+++ b/content/canvas/test/test_canvas.html
@@ -13374,20 +13374,20 @@ ok(ctx.isPointInPath(50, 10) === true, "
 function test_2d_path_isPointInPath_transform_1() {
 
 var canvas = document.getElementById('c409');
 var ctx = canvas.getContext('2d');
 
 ctx.translate(50, 0);
 ctx.rect(0, 0, 20, 20);
 ok(ctx.isPointInPath(-40, 10) === false, "ctx.isPointInPath(-40, 10) === false");
-ok(ctx.isPointInPath(10, 10) === false, "ctx.isPointInPath(10, 10) === false");
+// ok(ctx.isPointInPath(10, 10) === false, "ctx.isPointInPath(10, 10) === false"); // reenable me ASAP!
 ok(ctx.isPointInPath(49, 10) === false, "ctx.isPointInPath(49, 10) === false");
-ok(ctx.isPointInPath(51, 10) === true, "ctx.isPointInPath(51, 10) === true");
-ok(ctx.isPointInPath(69, 10) === true, "ctx.isPointInPath(69, 10) === true");
+// ok(ctx.isPointInPath(51, 10) === true, "ctx.isPointInPath(51, 10) === true"); // reenable me ASAP!
+// ok(ctx.isPointInPath(69, 10) === true, "ctx.isPointInPath(69, 10) === true"); // reenable me ASAP!
 ok(ctx.isPointInPath(71, 10) === false, "ctx.isPointInPath(71, 10) === false");
 

First step: bisect when this regressed.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

test_canvas.html was failing elsewhere as well. Disabled more tests.
https://hg.mozilla.org/integration/mozilla-inbound/rev/022c39b4aae6

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

The first Nightly with the regression was that of Oct. 3.

Regression window:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=85dd8e346102&tochange=635fcc11d2b1

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Online testcase:
http://philip.html5.org/tests/canvas/suite/tests/2d.path.isPointInPath.transform.2.html

Comment 4

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Fixes contains point on Cairo. Needs comparing against other backends.

Comment 5

[Nick Cameron [:nrc]]

Testing the patch: without the test case fails with Cairo and passes with D2D; with the patch it passes with Cairo, but fails with D2D. From previous discussions with Bas, we think the canvas implementation is probably OK, but that the Cairo DrawTarget might need changing, but I need to page this back in before I can really say that about this case with any confidence.

Comment 6

[Nick Cameron [:nrc]]

What does Skia do?

Comment 7

[Nick Cameron [:nrc]]

The problem seems to be with aCommitTransform in EnsureUserSpacePath(), that is, it is no longer necessary. I assume this is because that behaviour is now implemented properly in Cairo paths, probably as a result of bug 795135 (although I would not put money on that).

Comment 8

[Nick Cameron [:nrc]]

Attachment: patch - remove aCommitTransform

Comment 9

[Nick Cameron [:nrc]]

Attachment: reenable the tests

Comment 10

[Nick Cameron [:nrc]]

try push: https://tbpl.mozilla.org/?tree=Try&rev=32214be26c9c

Comment 11

[Nick Cameron [:nrc]]

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=38c868aa3acd

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/69b764d11dcc
https://hg.mozilla.org/mozilla-central/rev/38c868aa3acd