Bug 93870 - [CSS Shaders] Implement multiply, screen, darken, lighten, difference, exclusion blend modes.
Summary: [CSS Shaders] Implement multiply, screen, darken, lighten, difference, exclus...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Max Vujovic
URL: https://dvcs.w3.org/hg/FXTF/rawfile/t...
Keywords:
Depends on: 93869
Blocks: 71392 98504
  Show dependency treegraph
 
Reported: 2012-08-13 11:14 PDT by Max Vujovic
Modified: 2013-05-13 09:31 PDT (History)
6 users (show)

See Also:


Attachments
Patch (11.81 KB, patch)
2012-08-31 18:47 PDT, Max Vujovic
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (12.27 KB, patch)
2012-09-04 11:01 PDT, Max Vujovic
no flags Details | Formatted Diff | Diff
Patch (12.25 KB, patch)
2012-09-04 11:40 PDT, Max Vujovic
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Max Vujovic 2012-08-13 11:14:10 PDT
Implement the multiply blend mode in the CSS Shaders mix function. For example:
-webkit-filter: custom(none mix(shader.fs multiply));

This will multiply the special css_MixColor symbol in the fragment shader with the DOM element texture.
Comment 1 Max Vujovic 2012-08-31 16:02:15 PDT
Repurposing this bug to implement not just the multiply blend mode, but all of the following:

- multiply 
- screen
- darken
- lighten
- difference
- exclusion

These are all straightforward to implement (one line of code each). The more interesting part is how to test them in an organized fashion since there are 16 blend modes total in the spec [1]. I'm going to implement several of them together to show how the testing is going to look.

[1] https://dvcs.w3.org/hg/FXTF/rawfile/tip/compositing/index.html
Comment 2 Max Vujovic 2012-08-31 18:47:23 PDT
Created attachment 161807 [details]
Patch
Comment 3 WebKit Review Bot 2012-08-31 22:42:07 PDT
Comment on attachment 161807 [details]
Patch

Attachment 161807 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13719447

New failing tests:
css3/filters/custom/custom-filter-blend-modes.html
Comment 4 Max Vujovic 2012-09-04 11:01:50 PDT
Created attachment 162063 [details]
Patch

The color calculations are sometimes off by 1 due to floating point error. I've tweaked the reference colors to account for this.
Comment 5 Alexandru Chiculita 2012-09-04 11:09:11 PDT
Added the link to compositing spec: https://dvcs.w3.org/hg/FXTF/rawfile/tip/compositing/index.html
Comment 6 Alexandru Chiculita 2012-09-04 11:21:30 PDT
Comment on attachment 162063 [details]
Patch

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

Looks good. Just minor comments.

> LayoutTests/css3/filters/custom/custom-filter-blend-modes-expected.html:6
> +        This is the reference file for the test.

Add a comment about how the colors were computed. Maybe just say that the test file explains it.

> LayoutTests/css3/filters/custom/custom-filter-blend-modes-expected.html:10
> +        if (window.testRunner) {

You don't need these ones in the ref test.

> LayoutTests/css3/filters/custom/custom-filter-blend-modes-expected.html:79
> +    <!-- Be careful not to exceed 600px in height, since DumpRenderTree compares 800x600 snapshots of the page. -->

I suppose you don't need this comment here. Do you expect more blending tests to be added to this file?

> LayoutTests/css3/filters/custom/custom-filter-blend-modes.html:181
> +    <!-- Be careful not to exceed 600px in height, since DumpRenderTree compares 800x600 snapshots of the page. -->

ditto
Comment 7 Max Vujovic 2012-09-04 11:40:11 PDT
Created attachment 162072 [details]
Patch
Comment 8 Max Vujovic 2012-09-04 11:42:28 PDT
Comment on attachment 162063 [details]
Patch

Thanks for the review, Alex.

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

>> LayoutTests/css3/filters/custom/custom-filter-blend-modes-expected.html:6
>> +        This is the reference file for the test.
> 
> Add a comment about how the colors were computed. Maybe just say that the test file explains it.

Done. Added a comment saying the test file explains it.

>> LayoutTests/css3/filters/custom/custom-filter-blend-modes-expected.html:10
>> +        if (window.testRunner) {
> 
> You don't need these ones in the ref test.

Done. Thanks!

>> LayoutTests/css3/filters/custom/custom-filter-blend-modes-expected.html:79
>> +    <!-- Be careful not to exceed 600px in height, since DumpRenderTree compares 800x600 snapshots of the page. -->
> 
> I suppose you don't need this comment here. Do you expect more blending tests to be added to this file?

I reworded it to say: "If you add more blend modes to this page, be careful not to...". That should be more clear.

>> LayoutTests/css3/filters/custom/custom-filter-blend-modes.html:181
>> +    <!-- Be careful not to exceed 600px in height, since DumpRenderTree compares 800x600 snapshots of the page. -->
> 
> ditto

Done.
Comment 9 Dirk Schulze 2012-09-04 13:43:13 PDT
Comment on attachment 162072 [details]
Patch

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

Awesome reduction with the expressions! r=me. Just one question inline.

> Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp:191
> +        expression = "Cb + Cs - 2.0 * Cb * Cs";

Usually the style is 2, can Cb and Cs be integer?
Comment 10 Max Vujovic 2012-09-04 13:48:41 PDT
(In reply to comment #9)
> (From update of attachment 162072 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=162072&action=review
> 
> Awesome reduction with the expressions! r=me. Just one question inline.
> 
> > Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp:191
> > +        expression = "Cb + Cs - 2.0 * Cb * Cs";
> 
> Usually the style is 2, can Cb and Cs be integer?

No, GLSL requires 2.0 instead of 2, otherwise it's a syntax error.
Comment 11 Max Vujovic 2012-09-04 13:49:29 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > (From update of attachment 162072 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=162072&action=review
> > 
> > Awesome reduction with the expressions! r=me. Just one question inline.
> > 
> > > Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp:191
> > > +        expression = "Cb + Cs - 2.0 * Cb * Cs";
> > 
> > Usually the style is 2, can Cb and Cs be integer?
> 
> No, GLSL requires 2.0 instead of 2, otherwise it's a syntax error.

And Cb and Cs are always going to be floats.
Comment 12 WebKit Review Bot 2012-09-04 15:53:02 PDT
Comment on attachment 162072 [details]
Patch

Clearing flags on attachment: 162072

Committed r127517: <http://trac.webkit.org/changeset/127517>
Comment 13 WebKit Review Bot 2012-09-04 15:53:06 PDT
All reviewed patches have been landed.  Closing bug.
Comment 14 Adrian von Gegerfelt 2013-05-09 02:13:09 PDT
If we have

-webkit-filter: blur();
-webkit-filter: sephia();
etc

why not

-webkit-filter: [multiply|screen|darken|...];
Comment 15 Max Vujovic 2013-05-13 09:31:32 PDT
(In reply to comment #14)
> If we have
> 
> -webkit-filter: blur();
> -webkit-filter: sephia();
> etc
> 
> why not
> 
> -webkit-filter: [multiply|screen|darken|...];

I think you are looking for the functionality defined in the CSS Compositing and Blending spec:
https://dvcs.w3.org/hg/FXTF/rawfile/tip/compositing/index.html#mix-blend-mode

That spec is currently early in implementation.