RESOLVED FIXED Bug 93870
[CSS Shaders] Implement multiply, screen, darken, lighten, difference, exclusion blend modes.
https://bugs.webkit.org/show_bug.cgi?id=93870
Summary [CSS Shaders] Implement multiply, screen, darken, lighten, difference, exclus...
Max Vujovic
Reported 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.
Attachments
Patch (11.81 KB, patch)
2012-08-31 18:47 PDT, Max Vujovic
webkit.review.bot: commit-queue-
Patch (12.27 KB, patch)
2012-09-04 11:01 PDT, Max Vujovic
no flags
Patch (12.25 KB, patch)
2012-09-04 11:40 PDT, Max Vujovic
no flags
Max Vujovic
Comment 1 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
Max Vujovic
Comment 2 2012-08-31 18:47:23 PDT
WebKit Review Bot
Comment 3 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
Max Vujovic
Comment 4 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.
Alexandru Chiculita
Comment 5 2012-09-04 11:09:11 PDT
Alexandru Chiculita
Comment 6 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
Max Vujovic
Comment 7 2012-09-04 11:40:11 PDT
Max Vujovic
Comment 8 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.
Dirk Schulze
Comment 9 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?
Max Vujovic
Comment 10 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.
Max Vujovic
Comment 11 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.
WebKit Review Bot
Comment 12 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>
WebKit Review Bot
Comment 13 2012-09-04 15:53:06 PDT
All reviewed patches have been landed. Closing bug.
Adrian von Gegerfelt
Comment 14 2013-05-09 02:13:09 PDT
If we have -webkit-filter: blur(); -webkit-filter: sephia(); etc why not -webkit-filter: [multiply|screen|darken|...];
Max Vujovic
Comment 15 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.
Note You need to log in before you can comment on or make changes to this bug.