Bug 119082

Summary: Implement canvas blending tests to validate operators between different types of layers
Product: WebKit Reporter: Mihai Tica <mitica>
Component: CanvasAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, mihnea, roger_fong, WebkitBugTracker
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 100069    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Mihai Tica 2013-07-25 05:31:11 PDT
Implement canvas blending test that should validate the following scenarios:
Validate that all blending operators work as expected for the following layer combinations:

1. image over color
2. color over image
3. image over gradient
4. gradient over image
5. image over pattern
6. pattern over image
7. color over gradient
8. gradient over color
9. color over pattern
10. pattern over color
11. gradient over pattern
12. pattern over gradient
13. image over image
14. pattern over pattern
15. gradient over gradient
16. color over color
Comment 1 Mihai Tica 2013-07-25 07:34:03 PDT
Created attachment 207459 [details]
Patch
Comment 2 Mihai Tica 2013-07-25 07:36:55 PDT
Apparently, the soft light blend mode result is somewhat different from what is computed using the formula at http://www.adobe.com/content/dam/Adobe/en/devnet/pdf/pdfs/pdf_reference_archives/blend_modes.pdf, hence it was skipped.
Also, the nonseparate blend modes were all skipped, since they don't seem to be working.
Comment 3 Andrei Bucur 2013-07-25 08:27:45 PDT
Comment on attachment 207459 [details]
Patch

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

Suggestion: the expected test results can't be reference HTMLs with <img> tags inside for every square?
A nit: please use spaces instead of tabs for indentation.

> LayoutTests/fast/canvas/canvas-blending-helpers.js:45
> +    // Soft light seems to be using a slightly different formula.

A different formula in the WebKit implementation? If yes, can you add a FIXME and a bug number for easier tracking?

> LayoutTests/fast/canvas/canvas-blending-helpers.js:152
> +// The actual expected return value is commented.

Bugged in the implementation? If yes, can you add a FIXME and a bug number for easier tracking?
Comment 4 Mihai Tica 2013-07-30 06:04:59 PDT
Canvas blending should work in the following scenarios:

1. globalAlpha is set 
2. drawing with paths 
3. transforms are operated on the context 
4. text is drawn in the canvas 
5. shadow is set 
6. clipping is performed 

Adding test for these use cases.
Comment 5 Mihai Tica 2013-07-30 06:06:20 PDT
Created attachment 207729 [details]
Patch
Comment 6 Mihai Tica 2013-07-30 06:08:13 PDT
Thanks for the review!

(In reply to comment #3)
> (From update of attachment 207459 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=207459&action=review
> 
> Suggestion: the expected test results can't be reference HTMLs with <img> tags inside for every square?
Well, that would be just hiding pixel tests. Since different platforms might generate different png results, I' d rather not try this approach.
> A nit: please use spaces instead of tabs for indentation.
Done.
> 
> > LayoutTests/fast/canvas/canvas-blending-helpers.js:45
> > +    // Soft light seems to be using a slightly different formula.
> 
> A different formula in the WebKit implementation? If yes, can you add a FIXME and a bug number for easier tracking?
Done.
> 
> > LayoutTests/fast/canvas/canvas-blending-helpers.js:152
> > +// The actual expected return value is commented.
> 
> Bugged in the implementation? If yes, can you add a FIXME and a bug number for easier tracking?
Done.
Comment 7 Dirk Schulze 2013-07-30 06:28:17 PDT
Comment on attachment 207729 [details]
Patch

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.
Comment 8 Mihai Tica 2013-08-01 00:13:52 PDT
Created attachment 207900 [details]
Patch
Comment 9 Mihai Tica 2013-08-01 00:14:58 PDT
I rewrote most of the test using the model initially used by Rik, and the one that was used for the alpha test.
Can you please take another look?
Comment 10 Mihai Tica 2013-08-02 05:19:12 PDT
Created attachment 208005 [details]
Patch
Comment 11 Mihai Tica 2013-08-02 05:21:00 PDT
(In reply to comment #10)
> Created an attachment (id=208005) [details]
> Patch

Ok, there shouldn't be any more pixel tests with this patch, can you please have another look? Thanks!
Comment 12 Dirk Schulze 2013-08-02 11:16:30 PDT
Comment on attachment 208005 [details]
Patch

Great test suite! Awesome work. r=me.
Comment 13 WebKit Commit Bot 2013-08-02 11:41:41 PDT
Comment on attachment 208005 [details]
Patch

Clearing flags on attachment: 208005

Committed r153658: <http://trac.webkit.org/changeset/153658>
Comment 14 WebKit Commit Bot 2013-08-02 11:41:43 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Roger Fong 2013-08-02 16:09:27 PDT
Were these tests meant to work on Windows as well? / are they port agnostic results?
http://build.webkit.org/results/Apple%20Win%207%20Release%20(Tests)/r153658%20(37319)/results.html
Comment 16 Mihai Tica 2013-08-03 07:31:20 PDT
Huw(In reply to comment #15)
> Were these tests meant to work on Windows as well? / are they port agnostic results?
> http://build.webkit.org/results/Apple%20Win%207%20Release%20(Tests)/r153658%20(37319)/results.html

Hue, saturation, luminosity and color blend modes don't work on mac, but as these tests show, they seem to be functional on Windows.

I'll take some time in one of the following days to investigate this issue and find a solution for the failing win tests.