Bug 90101 - [CSS Shaders] Parse mix function
Summary: [CSS Shaders] Parse mix function
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Max Vujovic
URL:
Keywords:
Depends on:
Blocks: 71392 93011
  Show dependency treegraph
 
Reported: 2012-06-27 15:04 PDT by Max Vujovic
Modified: 2012-08-06 17:00 PDT (History)
13 users (show)

See Also:


Attachments
Patch (52.54 KB, patch)
2012-07-13 14:35 PDT, Max Vujovic
no flags Details | Formatted Diff | Diff
Patch (57.03 KB, patch)
2012-07-13 14:57 PDT, Max Vujovic
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-08 (331.23 KB, application/zip)
2012-07-13 16:40 PDT, WebKit Review Bot
no flags Details
Archive of layout-test-results from gce-cr-linux-07 (381.44 KB, application/zip)
2012-07-13 17:54 PDT, WebKit Review Bot
no flags Details
Patch (273.00 KB, patch)
2012-07-23 15:37 PDT, Max Vujovic
no flags Details | Formatted Diff | Diff
Patch (272.99 KB, patch)
2012-07-23 15:45 PDT, Max Vujovic
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-02 (450.92 KB, application/zip)
2012-07-23 17:06 PDT, WebKit Review Bot
no flags Details
Archive of layout-test-results from gce-cr-linux-06 (731.47 KB, application/zip)
2012-07-23 18:01 PDT, WebKit Review Bot
no flags Details
Patch (241.41 KB, patch)
2012-07-24 17:15 PDT, Max Vujovic
mvujovic: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-05 (1.28 MB, application/zip)
2012-07-24 18:32 PDT, WebKit Review Bot
no flags Details
Patch (170.41 KB, patch)
2012-08-02 10:13 PDT, Max Vujovic
krit: review-
krit: commit-queue-
Details | Formatted Diff | Diff
Patch (164.43 KB, patch)
2012-08-03 18:08 PDT, Max Vujovic
krit: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (165.48 KB, patch)
2012-08-06 14:07 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-06-27 15:04:08 PDT
The Filter Effects spec specifies the syntax for the mix function:

<fragment-shader>	none | <uri> | mix(<uri> [ <blend-mode> || <alpha-compositing> ]?)

This is part of the custom function:

custom(<vertex-shader> <fragment-shader>? [ , <vertex-mesh> ]? [ , <params> ]?)

(https://dvcs.w3.org/hg/FXTF/raw-file/tip/filters/index.html#FilterFunction)

For reference, the mix function processing model is described here: 
https://dvcs.w3.org/hg/FXTF/raw-file/tip/filters/index.html#processing-model-mix
Comment 1 Max Vujovic 2012-07-13 14:35:29 PDT
Created attachment 152334 [details]
Patch
Comment 2 Max Vujovic 2012-07-13 14:57:26 PDT
Created attachment 152343 [details]
Patch
Comment 3 Alexandru Chiculita 2012-07-13 16:35:12 PDT
Comment on attachment 152343 [details]
Patch

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

> LayoutTests/ChangeLog:8
> +        Add tests for custom filter mix function parsing.

Maybe you could add a link to the spec here.

> LayoutTests/css3/filters/script-tests/custom-filter-property-computed-style.js:-49
> -    

Nit: Avoid making empty changes.

> LayoutTests/css3/filters/script-tests/custom-filter-property-computed-style.js:89
> +{

nit: Move "{" on the previous line.

> LayoutTests/css3/filters/script-tests/custom-filter-property-computed-style.js:98
> +{

Ditto.

> LayoutTests/css3/filters/script-tests/custom-filter-property-parsing.js:91
> +{

Ditto.

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:878
> +            } else {
> +                if (program->fragmentShader())

Nit: It seems like you could merge these two in a single } else if {

> Source/WebCore/css/CSSParser.cpp:7383
> +    CSSParserValueList* argsList = value->function->args.get();

Nit: You can exit early when argList has more than 3 elements and avoid the mixFunction creation.

> Source/WebCore/css/CSSParser.cpp:7384
> +    int argNumber = 0;

argsList already has m_current. I think we can expose that instead of keeping a separate counter.

> Source/WebCore/css/CSSParser.cpp:7415
> +    if (!mixFunction->length())

This should not be possible if we check for the size of the argsList and return early.

> Source/WebCore/css/CSSParser.cpp:7463
> +    int argNumber = 0;

argList should expose the internal m_current position.

> Source/WebCore/css/CSSParser.cpp:7475
> +            hadAtLeastOneCustomShader = value;

What about returning early from here?
if (!value)
   return 0;

> Source/WebCore/css/WebKitCSSMixFunctionValue.h:40
> +class WebKitCSSMixFunctionValue : public CSSValueList {

Don't we need an IDL file for this object?
Comment 4 WebKit Review Bot 2012-07-13 16:40:17 PDT
Comment on attachment 152343 [details]
Patch

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

New failing tests:
css3/filters/custom/custom-filter-property-computed-style.html
css3/filters/custom/custom-filter-property-parsing-invalid.html
css3/filters/custom/custom-filter-property-parsing.html
Comment 5 WebKit Review Bot 2012-07-13 16:40:21 PDT
Created attachment 152371 [details]
Archive of layout-test-results from gce-cr-linux-08

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-08  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 6 WebKit Review Bot 2012-07-13 17:53:59 PDT
Comment on attachment 152343 [details]
Patch

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

New failing tests:
css3/filters/custom/custom-filter-property-computed-style.html
css3/filters/custom/custom-filter-property-parsing-invalid.html
css3/filters/custom/custom-filter-property-parsing.html
Comment 7 WebKit Review Bot 2012-07-13 17:54:03 PDT
Created attachment 152388 [details]
Archive of layout-test-results from gce-cr-linux-07

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-07  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 8 Max Vujovic 2012-07-23 15:37:39 PDT
Created attachment 153877 [details]
Patch

Updated patch with changes from Alex's review.
Comment 9 WebKit Review Bot 2012-07-23 15:40:54 PDT
Attachment 153877 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3..." exit_code: 1
Source/WebCore/platform/graphics/filters/CustomFilterProgram.h:86:  The parameter name "properties" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 40 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Max Vujovic 2012-07-23 15:45:00 PDT
Created attachment 153882 [details]
Patch

Small fix for style bot.
Comment 11 Max Vujovic 2012-07-23 15:47:52 PDT
Comment on attachment 152343 [details]
Patch

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

Thanks for the review, Alex. I've posted a new patch with the requested changes.

>> LayoutTests/ChangeLog:8
>> +        Add tests for custom filter mix function parsing.
> 
> Maybe you could add a link to the spec here.

Done.

>> LayoutTests/css3/filters/script-tests/custom-filter-property-computed-style.js:-49
>> -    
> 
> Nit: Avoid making empty changes.

Done. (My editor was stripping trailing whitespace)

>> LayoutTests/css3/filters/script-tests/custom-filter-property-computed-style.js:89
>> +{
> 
> nit: Move "{" on the previous line.

Done.

>> LayoutTests/css3/filters/script-tests/custom-filter-property-computed-style.js:98
>> +{
> 
> Ditto.

Done.

>> LayoutTests/css3/filters/script-tests/custom-filter-property-parsing.js:91
>> +{
> 
> Ditto.

Done.

>> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:878
>> +                if (program->fragmentShader())
> 
> Nit: It seems like you could merge these two in a single } else if {

Done.

>> Source/WebCore/css/CSSParser.cpp:7383
>> +    CSSParserValueList* argsList = value->function->args.get();
> 
> Nit: You can exit early when argList has more than 3 elements and avoid the mixFunction creation.

Done.

>> Source/WebCore/css/CSSParser.cpp:7384
>> +    int argNumber = 0;
> 
> argsList already has m_current. I think we can expose that instead of keeping a separate counter.

Done. I like that.

>> Source/WebCore/css/CSSParser.cpp:7415
>> +    if (!mixFunction->length())
> 
> This should not be possible if we check for the size of the argsList and return early.

Yes, I agree. Removed.

>> Source/WebCore/css/CSSParser.cpp:7463
>> +    int argNumber = 0;
> 
> argList should expose the internal m_current position.

Done.

>> Source/WebCore/css/CSSParser.cpp:7475
>> +            hadAtLeastOneCustomShader = value;
> 
> What about returning early from here?
> if (!value)
>    return 0;

Done.

>> Source/WebCore/css/WebKitCSSMixFunctionValue.h:40
>> +class WebKitCSSMixFunctionValue : public CSSValueList {
> 
> Don't we need an IDL file for this object?

Sure- It would be nice so that the object shows up in CSSOM as WebKitCSSMixFunctionValue instead of CSSValueList. I've added WebKitCSSMixFunctionValue.idl to the patch.
Comment 12 WebKit Review Bot 2012-07-23 17:06:16 PDT
Comment on attachment 153882 [details]
Patch

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

New failing tests:
compositing/checkerboard.html
compositing/backface-visibility/backface-visibility-hierarchical-transform.html
animations/3d/matrix-transform-type-animation.html
compositing/direct-image-compositing.html
compositing/backface-visibility/backface-visibility-non3d.html
compositing/backing/no-backing-for-clip-overlap.html
animations/additive-transform-animations.html
animations/3d/replace-filling-transform.html
compositing/backface-visibility/backface-visibility-3d.html
compositing/clip-change.html
compositing/layers-inside-overflow-scroll.html
compositing/animation/state-at-end-event-transform-layer.html
compositing/generated-content.html
compositing/animation/computed-style-during-delay.html
animations/3d/change-transform-in-end-event.html
animations/animation-direction-reverse-hardware.html
compositing/flat-with-transformed-child.html
animations/3d/transform-perspective.html
compositing/bounds-in-flipped-writing-mode.html
compositing/scrollbar-painting.html
animations/3d/transform-origin-vs-functions.html
compositing/clip-child-by-non-stacking-ancestor.html
compositing/backface-visibility/backface-visibility-image.html
compositing/compositing-visible-descendant.html
animations/animation-direction-reverse-fill-mode-hardware.html
compositing/overflow-trumps-transform-style.html
compositing/backing/no-backing-for-clip.html
animations/animation-direction-reverse-timing-functions-hardware.html
Comment 13 WebKit Review Bot 2012-07-23 17:06:28 PDT
Created attachment 153908 [details]
Archive of layout-test-results from gce-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 14 WebKit Review Bot 2012-07-23 18:01:33 PDT
Comment on attachment 153882 [details]
Patch

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

New failing tests:
compositing/checkerboard.html
compositing/backface-visibility/backface-visibility-hierarchical-transform.html
animations/3d/matrix-transform-type-animation.html
compositing/direct-image-compositing.html
compositing/backing/no-backing-for-clip-overlap.html
animations/additive-transform-animations.html
animations/3d/replace-filling-transform.html
compositing/backface-visibility/backface-visibility-3d.html
compositing/clip-change.html
compositing/layers-inside-overflow-scroll.html
compositing/animation/state-at-end-event-transform-layer.html
compositing/generated-content.html
compositing/animation/computed-style-during-delay.html
animations/3d/change-transform-in-end-event.html
animations/animation-direction-reverse-hardware.html
compositing/flat-with-transformed-child.html
animations/3d/transform-perspective.html
compositing/bounds-in-flipped-writing-mode.html
compositing/scrollbar-painting.html
animations/3d/transform-origin-vs-functions.html
compositing/clip-child-by-non-stacking-ancestor.html
compositing/backface-visibility/backface-visibility-image.html
compositing/compositing-visible-descendant.html
animations/animation-direction-reverse-fill-mode-hardware.html
compositing/overflow-trumps-transform-style.html
compositing/color-matching/image-color-matching.html
compositing/backing/no-backing-for-clip.html
compositing/backing/no-backing-for-perspective.html
Comment 15 WebKit Review Bot 2012-07-23 18:01:41 PDT
Created attachment 153926 [details]
Archive of layout-test-results from gce-cr-linux-06

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-06  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 16 Max Vujovic 2012-07-24 17:15:55 PDT
Created attachment 154184 [details]
Patch

Refactored the patch a little bit:

Renamed "CustomFilterProgramProperties" to "BlendingAndCompositingSettings". Created platform/graphics/BlendingAndCompositingSettings.h.

Removed the cr-linux expectations because they should be the same as chromium.
Comment 17 WebKit Review Bot 2012-07-24 18:32:47 PDT
Comment on attachment 154184 [details]
Patch

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

New failing tests:
compositing/checkerboard.html
compositing/backface-visibility/backface-visibility-hierarchical-transform.html
animations/3d/matrix-transform-type-animation.html
compositing/direct-image-compositing.html
compositing/backface-visibility/backface-visibility-non3d.html
animations/additive-transform-animations.html
animations/3d/replace-filling-transform.html
compositing/backface-visibility/backface-visibility-3d.html
compositing/clip-change.html
compositing/layers-inside-overflow-scroll.html
compositing/animation/state-at-end-event-transform-layer.html
compositing/generated-content.html
compositing/animation/computed-style-during-delay.html
animations/3d/change-transform-in-end-event.html
animations/animation-direction-reverse-hardware.html
compositing/flat-with-transformed-child.html
animations/3d/transform-perspective.html
compositing/bounds-in-flipped-writing-mode.html
compositing/scrollbar-painting.html
animations/3d/transform-origin-vs-functions.html
compositing/clip-child-by-non-stacking-ancestor.html
compositing/backface-visibility/backface-visibility-image.html
compositing/compositing-visible-descendant.html
animations/animation-direction-reverse-fill-mode-hardware.html
compositing/overflow-trumps-transform-style.html
compositing/backface-visibility/backface-visibility-simple.html
compositing/backface-visibility/backface-visibility-webgl.html
animations/animation-direction-reverse-timing-functions-hardware.html
Comment 18 WebKit Review Bot 2012-07-24 18:32:58 PDT
Created attachment 154212 [details]
Archive of layout-test-results from gce-cr-linux-05

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-05  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 19 Max Vujovic 2012-08-02 10:13:33 PDT
Created attachment 156115 [details]
Patch

Re-running the bots.

Increased ClassTypeBits in CSSValue.h from 5 to 6 to fit the new enum value WebKitCSSMixFunctionValueClass. I think this was the cause of previous cr-linux EWS failures in animations and compositing.

Removed the WebKitCSSMixFunctionValue.idl from this patch. The IDL should probably come in a follow-up patch since this one is already getting big.
Comment 20 Max Vujovic 2012-08-02 11:41:04 PDT
Comment on attachment 156115 [details]
Patch

Bots are green. Setting review flag.

I've split out the IDL for WebKitCSSMixFunctionValue into bug 93011.
Comment 21 Dirk Schulze 2012-08-02 15:37:42 PDT
Comment on attachment 156115 [details]
Patch

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

I am not very happy about the duplication of code that we get. See inline comments.

> Source/WebCore/css/CSSParser.cpp:7433
> +    case CSSValueClear:
> +    case CSSValueSrc:
> +    case CSSValueDst:
> +    case CSSValueSrcOver:
> +    case CSSValueDstOver:
> +    case CSSValueSrcIn:
> +    case CSSValueDstIn:
> +    case CSSValueSrcOut:
> +    case CSSValueDstOut:
> +    case CSSValueSrcAtop:
> +    case CSSValueDstAtop:
> +    case CSSValueXor:
> +    case CSSValuePlus:
> +        return true;

The spec changed to take the long version instead of the shorten one.

> Source/WebCore/css/CSSParser.cpp:7503
> +    // blend-mode: normal | multiply | screen | overlay | darken | lighten | color-dodge |
> +    //             color-burn | hard-light | soft-light | difference | exclusion | hue |
> +    //             saturation | color | luminosity
> +    // alpha-compositing: clear | src | dst | src-over | dst-over | src-in | dst-in |
> +    //                    src-out | dst-out | src-atop | dst-atop | xor | plus

comment needs to be updated as well

> Source/WebCore/css/CSSPrimitiveValueMappings.h:3479
> +template<> inline CSSPrimitiveValue::CSSPrimitiveValue(EAlphaCompositingMode alphaCompositingMode)
> +    : CSSValue(PrimitiveClass)

needs update as well.

> Source/WebCore/css/CSSPrimitiveValueMappings.h:3525
> +template<> inline CSSPrimitiveValue::operator EAlphaCompositingMode() const

This is partly redundant to CSSPrimitiveValue::operator CompositeOperator(). I would like to avoid that. But I see the difficulty. They are not supported by Canvas not by background-composite. Can these value still support a subset of these and it is checked during the parsing of these properties?

> Source/WebCore/css/CSSValueKeywords.in:970
> +src
> +dst
> +src-over
> +dst-over
> +src-in
> +dst-in
> +src-out
> +dst-out
> +src-atop
> +dst-atop
> +// xor
> +plus

Long version of the name please.

> Source/WebCore/platform/graphics/BlendingAndCompositingSettings.h:33
> +#if ENABLE(CSS_SHADERS)

I expect it should work with other features as well. Also we use some of these values on other places already. Can we make sure that they are used on all places? I am also not sure if the bundling is that useful. Even if blend and compositing mode go together, background-compositing just needs EAlphaCompositing.
Comment 22 Max Vujovic 2012-08-02 20:42:46 PDT
Comment on attachment 156115 [details]
Patch

Thanks for review, Dirk! My responses are inline.

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

>> Source/WebCore/css/CSSParser.cpp:7433
>> +        return true;
> 
> The spec changed to take the long version instead of the shorten one.

So it has :)

>> Source/WebCore/css/CSSParser.cpp:7503
>> +    //                    src-out | dst-out | src-atop | dst-atop | xor | plus
> 
> comment needs to be updated as well

Agreed.

>> Source/WebCore/css/CSSPrimitiveValueMappings.h:3479
>> +    : CSSValue(PrimitiveClass)
> 
> needs update as well.

Agreed.

>> Source/WebCore/css/CSSPrimitiveValueMappings.h:3525
>> +template<> inline CSSPrimitiveValue::operator EAlphaCompositingMode() const
> 
> This is partly redundant to CSSPrimitiveValue::operator CompositeOperator(). I would like to avoid that. But I see the difficulty. They are not supported by Canvas not by background-composite. Can these value still support a subset of these and it is checked during the parsing of these properties?

Now that the Compositing spec has changed from short names to long names (e.g. src-over to source-over), we do have the opportunity to share some code, but it seems a little tricky.

Here are the values for CompositeOperator. The ones in [] are not used in the Compositing spec. The others map directly to the Compositing Spec.

CompositeClear
[CompositeCopy]
CompositeSourceOver
CompositeSourceIn
CompositeSourceOut
CompositeSourceAtop
CompositeDestinationOver
CompositeDestinationIn
CompositeDestinationOut
CompositeDestinationAtop
CompositeXOR
[CompositePlusDarker]
[CompositePlusLighter]
[CompositeDifference]

The following are missing from CompositeOperator, but defined in the Compositing spec:
"plus" (CompositePlusLighter has the same effect.)
"source" (CompositeCopy has the same effect.)
"destination" (There is no analogous CompositeOperator for this.)

I'm not sure how to reuse CompositorOperator cleanly while respecting these differences. Here are some options:

(1) Do we introduce CompositePlus, CompositeSource, and CompositeDestination?
Then, (CompositePlus, CompositePlusLighter) and (CompositeSource, CompositeCopy) are semantic duplicates.
We'll also need to support these at the platform level. CompositePlus and CompositePlusLighter will need to fall through to the same case in the platform code.
We'll have to figure out what to do with CompositeDestination on Mac because I don't see a kCGBlendModeDestination in the docs [1], but that's a separate issue.

(2) Do we introduce CompositeDestination, and map (CSSValueSource => CompositeCopy) and (CSSValuePlus => CompositePlusLighter)?
Then, we have a problem in CSSPrimitiveValueMappings because we need a one to many mapping for CompositeOperator. For -webkit-background-composite, we want (CompositeCopy => CSSValueCopy), but for the alpha-compositing property from the spec, we want (CompositeCopy => CSSValueSource).

What are your thoughts?

[1] http://developer.apple.com/library/ios/#DOCUMENTATION/GraphicsImaging/Reference/CGContext/Reference/reference.html

>> Source/WebCore/css/CSSValueKeywords.in:970
>> +plus
> 
> Long version of the name please.

Agreed.

>> Source/WebCore/platform/graphics/BlendingAndCompositingSettings.h:33
>> +#if ENABLE(CSS_SHADERS)
> 
> I expect it should work with other features as well. Also we use some of these values on other places already. Can we make sure that they are used on all places? I am also not sure if the bundling is that useful. Even if blend and compositing mode go together, background-compositing just needs EAlphaCompositing.

I see your point. Bundling blending and compositing is convenient for shaders. However, it doesn't seem as useful for the Blending and Compositing spec, since blending and compositing are separate properties there. I'll see what I can do about this.
Comment 23 Max Vujovic 2012-08-03 18:08:13 PDT
Created attachment 156495 [details]
Patch

Updated the patch based on Dirk's review. Running the bots.

In this patch:

- I removed the short-form CSS values for alpha compositing that I had added before. Instead, I reused the long-form ones used by -webkit-background-composite that match the spec. I put a FIXME to add "lighter" and "destination", since these are being discussed on www-style right now.
- I moved the blend modes to GraphicsTypes.h, right after the CompositeOperator enum.
- I removed the BlendingAndCompositingSettings.h file. I renamed the BlendingAndCompositingSettings struct to CustomFilterProgramMixSettings, and moved it to CustomFilterProgram.h. Now, it's specific to Shaders, since we determined that bundling blending and compositing is useful for Shaders but not for Blending and Compositing.
Comment 24 Max Vujovic 2012-08-04 17:44:59 PDT
Comment on attachment 156495 [details]
Patch

Bots are green. Setting r?.
Comment 25 Dirk Schulze 2012-08-06 11:08:11 PDT
Comment on attachment 156495 [details]
Patch

Looks great. r=me.
Comment 26 WebKit Review Bot 2012-08-06 12:14:21 PDT
Comment on attachment 156495 [details]
Patch

Rejecting attachment 156495 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
bkit', '--debug', '--chromium', '--update-chromium']" exit_code: 2
ValuePool.o
cc1plus: warnings being treated as errors
Source/WebCore/css/CSSValue.cpp: In member function 'void WebCore::CSSValue::reportMemoryUsage(WebCore::MemoryObjectInfo*) const':
Source/WebCore/css/CSSValue.cpp:161: error: enumeration value 'WebKitCSSMixFunctionValueClass' not handled in switch
make: *** [out/Debug/obj.target/webcore_remaining/Source/WebCore/css/CSSValue.o] Error 1
make: *** Waiting for unfinished jobs....

Full output: http://queues.webkit.org/results/13451001
Comment 27 Max Vujovic 2012-08-06 14:07:13 PDT
Created attachment 156759 [details]
Patch

Rebased patch for landing. Added WebKitCSSMixFunctionValue::reportDescendantMemoryUsage.
Comment 28 Dirk Schulze 2012-08-06 14:10:58 PDT
Comment on attachment 156759 [details]
Patch

r=me
Comment 29 WebKit Review Bot 2012-08-06 17:00:51 PDT
Comment on attachment 156759 [details]
Patch

Clearing flags on attachment: 156759

Committed r124820: <http://trac.webkit.org/changeset/124820>
Comment 30 WebKit Review Bot 2012-08-06 17:00:59 PDT
All reviewed patches have been landed.  Closing bug.