[webkit-reviews] review denied: [Bug 119082] Implement canvas blending tests to validate operators between different types of layers : [Attachment 207729] Patch

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied Mihai Tica <mitica at adobe.com>'s
request for review:
Bug 119082: Implement canvas blending tests to validate operators between
different types of layers
https://bugs.webkit.org/show_bug.cgi?id=119082

Attachment 207729: Patch
https://bugs.webkit.org/attachment.cgi?id=207729&action=review

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=207729&action=review


> LayoutTests/ChangeLog:4
> +	   Validate that all blending operators work as expected for the
following layer combinations:

Which layer combinations do you mean? You didn't list them. I assume you
actually do not want to blend layers, but want to test blending in general in
Canvas, right? IIRC I reviewed a bunch of tests from Rik already. What new
value do these tests have?

> LayoutTests/fast/canvas/canvas-blending-clipping-expected.html:35
> +    <script type="text/javascript">
> +	   var canvasElements = document.getElementsByTagName("canvas");
> +	   for (var i = 0; i < separateBlendmodes.length; ++i) {
> +	       var context = canvasElements[i].getContext("2d");
> +	       context.beginPath();
> +	       context.moveTo(3, 3);
> +	       context.lineTo(3, 7);
> +	       context.lineTo(7, 7);
> +	       context.lineTo(7, 3);
> +	       context.lineTo(3, 3);
> +	       context.clip();
> +
> +	       separateBlendColorsAndDrawInContext([129/255, 1, 129/255, 1],
[1, 129/255, 129/255, 1],
> +		   context, separateBlendFunctions[separateBlendmodes[i]]);
> +	   }

You are actually testing normal blending within Canvas. You create a bunch of
Canvas elements and do a pixel test. I would strongly suggest to convert all
these tests to ref tests.

* Create just one Canvas element and one context,
* Draw your Source,
* Set the blend mode
* Draw your Destiantion
* Use getImageData and compare the colors with real values (take a small delta
for different platforms)
* Clear the canvas
* Start the next test in the same canvas

You also do not need the layout structure. You can disable DRT and just
concentrate on a bunch of pass messages.

Take a look at some tests in the fast/canvas directory for inspirations on how
to write canvas tests without pixel results.
Again, Rik created a bunch of test cases and has a script to auto generate them
partly. Maybe you contact him as well.