Bug 97859 – [CSS Shaders] Implement all composite operators except destination and lighter.

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Dongseong Hwang]

Implement the composite operators in the CSS Shaders mix function. For example:
-webkit-filter: custom(none mix(shader.fs normal source-over));

This will composite the special css_MixColor symbol in the fragment shader with the DOM element texture.

[Dongseong Hwang]

I'll submit a patch today.

[Dongseong Hwang]

Created attachment 166171 [details]
Patch

[Dongseong Hwang]

Created attachment 166180 [details]
Patch

[Dongseong Hwang]

(In reply to comment #3)
> Created an attachment (id=166180) [details]
> Patch

css3/filters/custom/custom-filter-composite-operators.html failed because rgba(75%, 37.5%, 50%, 0.8) can not be represented by 8-bit accurately. So I baselined the result color values using the same way in Bug 93870.

[Build Bot]

Comment on attachment 166180 [details]
Patch

Attachment 166180 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14068157

New failing tests:
css3/filters/custom/custom-filter-composite-operators.html
css3/filters/custom/custom-filter-property-parsing-invalid.html

[WebKit Review Bot]

Comment on attachment 166180 [details]
Patch

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

New failing tests:
css3/filters/custom/custom-filter-composite-operators.html
css3/filters/custom/custom-filter-property-parsing-invalid.html

[Dongseong Hwang]

Created attachment 166202 [details]
Patch

[WebKit Review Bot]

Comment on attachment 166202 [details]
Patch

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

New failing tests:
css3/filters/custom/custom-filter-composite-operators.html

[Build Bot]

Comment on attachment 166202 [details]
Patch

Attachment 166202 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14059386

New failing tests:
css3/filters/custom/custom-filter-composite-operators.html

[Max Vujovic]

Comment on attachment 166202 [details]
Patch

Thanks for the patch! Looking good. I have a couple of comments:

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

> Source/WebCore/ChangeLog:3
> +        [CSS Shaders] Implement all composite operators.

You haven't implemented "destination" from the Compositing and Blending spec [1], so this title should probably say "Implement all composite operators except destination".

We'll need to figure out what to do with destination eventually, but it doesn't have to be in this bug :).
We'll need a new CompositeOperator, CompositeDestination. Unrelated to the shaders implementation, there's no kCGBlendModeDestination in CoreGraphics [2].

[1]: https://dvcs.w3.org/hg/FXTF/rawfile/tip/compositing/index.html#alpha-compositing 
[2]: Search for "CGBlendMode" in http://developer.apple.com/library/ios/#documentation/GraphicsImaging/Reference/CGContext/Reference/reference.html

> Source/WebCore/css/CSSParser.cpp:-7467
> -    // FIXME: Add CSSValueDestination and CSSValueLighter when the Compositing spec updates.

You haven't implemented "destination", so this FIXME needs to be updated to say that. It shouldn't be removed quite yet.

> Source/WebCore/css/CSSParser.cpp:7467
> +    return ident >= CSSValueClear && ident <= CSSValuePlusLighter && ident != CSSValuePlusDarker;

@achicu, @dino: Can one of you comment on the following issue?

According to the Compositing and Blending spec [1], "plus-lighter" is not a valid CSS value. You want "lighter".

However, we can't just add the "lighter" CSS value and map it to CompositePlusLighter. The CSS value "plus-lighter" for -webkit-background-composite is already mapped to CompositePlusLighter in CSSPrimitiveValueMappings.h [3].

We have a couple of options for resolving this:
(1) Change the CSS value "plus-lighter" to "lighter" and break -webkit-background-composite. Then, we should probably also change "plus-darker" to "darker".
(2) Keep the CSS value "plus-lighter". Add the CSS value "lighter". Add CompositeOperatorLighter. Then, we have two synonymous CompositeOperators, CompositeOperatorPlusLighter and CompositeOperatorLighter. Platform code might have to deal with these two CompositeOperators, which is not ideal.
(3) Create separate CSSValue <-> CompositeOperator mappings for -webkit-background-composite and shaders/CSS compositing. For shaders, we would have CSSValueLighter <-> CompositeOperatorPlusLighter. For -webkit-background-composite, we would have CSSValuePlusLighter <-> CompositeOperatorPlusLighter. I'm not sure how to implement this nicely, though.

[3]: Search for "template<> inline CSSPrimitiveValue::operator CompositeOperator() const" in CSSPrimitiveValueMappings.h.

> Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp:251
>      case CompositeClear:

The expressions look good.

> LayoutTests/css3/filters/custom/custom-filter-composite-operators.html:5
> +    <!-- This test passes if each horizontal pair of squares are exactly the same color. -->

@achicu, @dino: Could one of you chime in on this following point? I wouldn't want to make Huang rewrite his tests if it's just my opinion.

This type of test is particularly good for testing blend modes, which define how to mix two RGB colors. However, I'm not sure it's the best way to test alpha compositing operators.

Conventionally, alpha compositing operators are related to which parts of the resulting image come from the source (css_MixColor) and which parts come from the destination (the element texture), based on the alpha channel.

That's why the spec [4] shows examples that composite an offset blue square surrounded by transparency with a differently offset yellow square surrounded by transparency. If you look at our existing compositing test for "source-atop" [5], you'll see we use that technique. I would prefer that we use that same technique to test all of the compositing operators. It makes it easy to visually verify the correctness of the test, since it looks exactly like the classic compositing images from the spec. Also, each test case verifies multiple different combinations of alpha values for each composite operator, instead of just one combination per composite operator.
 
I do like that you used a ref test instead of a pixel test, and I like that you put all of the composite operators in this single ref test. I suggest you keep the same overall structure, where each row is a test case, and the left side and right side of each row must match. However, instead of single-color squares, each "square" should look like one of compositing result images from the spec. In the test file, the left image can be rendered with shaders, and the right image can be rendered with divs. In the reference file, both sides can be rendered with divs. 

[4]: https://dvcs.w3.org/hg/FXTF/rawfile/tip/compositing/index.html#porterduffcompositingoperators_clear
[5]: LayoutTests/css3/filters/custom/custom-filter-composite-source-atop.html

> LayoutTests/css3/filters/custom/custom-filter-composite-operators.html:47
> +            Fa = αb; Fb = 1 â αs

I'm seeing non-ASCII characters in these expressions on my machine, such as a "plus equals" sign [6].

[6]: http://www.w3.org/TR/MathML2/glyphs/02A/U02A72.png

> LayoutTests/css3/filters/custom/custom-filter-composite-operators.html:279
> +        -webkit-filter: custom(none mix(url('../resources/mix-color.fs') normal plus-lighter), mix_color 0.0 0.6 0.5 0.5);

Same as before. "plus-lighter" is not a valid CSS compositing operator according to the spec.

> LayoutTests/css3/filters/custom/custom-filter-property-parsing-invalid-expected.txt:-82
> -Mix function with alpha compositing mode 'plus-lighter', which should only apply to -webkit-background-composite : custom(none mix(url(shader) plus-lighter))

Please keep this text expectation in place.

> LayoutTests/css3/filters/script-tests/custom-filter-property-parsing-invalid.js:-46
> -testInvalidFilterRule("Mix function with alpha compositing mode 'plus-lighter', which should only apply to -webkit-background-composite", "custom(none mix(url(shader) plus-lighter))");

Please keep this test case in place.

[Dongseong Hwang]

(In reply to comment #10)
> (From update of attachment 166202 [details])
> Thanks for the patch! Looking good. I have a couple of comments:

Thanks for your review!

> You haven't implemented "destination", so this FIXME needs to be updated to say that. It shouldn't be removed quite yet.
> 
> > Source/WebCore/css/CSSParser.cpp:7467
> > +    return ident >= CSSValueClear && ident <= CSSValuePlusLighter && ident != CSSValuePlusDarker;

I handled plus-lighter because CompositePlusLighter enum was in CustomFilterValidatedProgram.cpp
As reading your comments, plus-lighter should be postponed like destination. Do you think it is better that I file another bug about plus-lighter and destination?

> 
> According to the Compositing and Blending spec [1], "plus-lighter" is not a valid CSS value. You want "lighter".

I did not recognize the difference between "plus-lighter" and "lighter" due to the lack of knowledge. I see now.

> Conventionally, alpha compositing operators are related to which parts of the resulting image come from the source (css_MixColor) and which parts come from the destination (the element texture), based on the alpha channel.
> 
> That's why the spec [4] shows examples that composite an offset blue square surrounded by transparency with a differently offset yellow square surrounded by transparency. If you look at our existing compositing test for "source-atop" [5], you'll see we use that technique.

I agree with your opinion. I'll try to make new test like "source-atop" test. However, I can submit new patch after about 5 days because Korea is on Thanksgiving from today.

By the way, cr-linux and mac bots failed this test after I adjusted the pixels from GTK results. cr-linux and mac bots had failed also the test in the second patch that I adjusted the pixels from Qt results. I think this kind of tests will be flaky when other ports unskip them. I'm curious whether other tests in css3/filters/custom have similar problems. 

> > LayoutTests/css3/filters/script-tests/custom-filter-property-parsing-invalid.js:-46
> > -testInvalidFilterRule("Mix function with alpha compositing mode 'plus-lighter', which should only apply to -webkit-background-composite", "custom(none mix(url(shader) plus-lighter))");
> 
> Please keep this test case in place.

I'll rollback CSSParcer to not handle 'plus-lighter' and rollback this test also.

[Max Vujovic]

(In reply to comment #11)
> (In reply to comment #10)
> > (From update of attachment 166202 [details] [details])
> > Thanks for the patch! Looking good. I have a couple of comments:
> 
> Thanks for your review!
> 
> > You haven't implemented "destination", so this FIXME needs to be updated to say that. It shouldn't be removed quite yet.
> > 
> > > Source/WebCore/css/CSSParser.cpp:7467
> > > +    return ident >= CSSValueClear && ident <= CSSValuePlusLighter && ident != CSSValuePlusDarker;
> 
> I handled plus-lighter because CompositePlusLighter enum was in CustomFilterValidatedProgram.cpp
> As reading your comments, plus-lighter should be postponed like destination. Do you think it is better that I file another bug about plus-lighter and destination?

Yes, that sounds like a good idea. Destination and lighter are tricky enough to be their own bugs.

> 
> > 
> > According to the Compositing and Blending spec [1], "plus-lighter" is not a valid CSS value. You want "lighter".
> 
> I did not recognize the difference between "plus-lighter" and "lighter" due to the lack of knowledge. I see now.

No problem. These are really subtle :)

> 
> > Conventionally, alpha compositing operators are related to which parts of the resulting image come from the source (css_MixColor) and which parts come from the destination (the element texture), based on the alpha channel.
> > 
> > That's why the spec [4] shows examples that composite an offset blue square surrounded by transparency with a differently offset yellow square surrounded by transparency. If you look at our existing compositing test for "source-atop" [5], you'll see we use that technique.
> 
> I agree with your opinion. I'll try to make new test like "source-atop" test. However, I can submit new patch after about 5 days because Korea is on Thanksgiving from today.

Ok, great. Thanks for doing that- I know it's a lot more work. You can take your time, since we don't have any dependent bugs on this one. Enjoy your vacation :)

> 
> By the way, cr-linux and mac bots failed this test after I adjusted the pixels from GTK results. cr-linux and mac bots had failed also the test in the second patch that I adjusted the pixels from Qt results. I think this kind of tests will be flaky when other ports unskip them. I'm curious whether other tests in css3/filters/custom have similar problems. 

The other tests might have problems. Using rgb colors with 8-bit values instead of floats helped me in custom-filter-blend-modes.html.

Hopefully, after you change the test, you won't have as many problems because you just have to compare two simple colors (blue and yellow).

> 
> > > LayoutTests/css3/filters/script-tests/custom-filter-property-parsing-invalid.js:-46
> > > -testInvalidFilterRule("Mix function with alpha compositing mode 'plus-lighter', which should only apply to -webkit-background-composite", "custom(none mix(url(shader) plus-lighter))");
> > 
> > Please keep this test case in place.
> 
> I'll rollback CSSParcer to not handle 'plus-lighter' and rollback this test also.

Sounds good. Thanks!

[Dongseong Hwang]

Created attachment 167061 [details]
Patch

[Dongseong Hwang]

(In reply to comment #12)
> Yes, that sounds like a good idea. Destination and lighter are tricky enough to be their own bugs.

I filed Bug 98391.

> Ok, great. Thanks for doing that- I know it's a lot more work. You can take your time, since we don't have any dependent bugs on this one. Enjoy your vacation :)

Thank you for your kindness :) I made new test similar to css3/filters/custom/custom-filter-composite-source-atop.html as you suggested.

> > I'll rollback CSSParcer to not handle 'plus-lighter' and rollback this test also.
> 
> Sounds good. Thanks!

When destination and lighter are implemented, CSSParcer will be changed.

Thanks.

[Dongseong Hwang]

Created attachment 167067 [details]
Patch

[Dongseong Hwang]

(In reply to comment #15)
> Created an attachment (id=167067) [details]
> Patch

Fixed typo in comments and Changelog.

[Max Vujovic]

Comment on attachment 167067 [details]
Patch

Looking great! Thanks for the updates.

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

> Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp:310
> +            return vec4((as * Fa * Cs + ab * Fb * Cb) / ao, ao);

This change is unrelated to the compositing modes, so I would take it out. I think you should land bug 98396 first, since you're removing this change in there. That is, refactor first, and then add new functionality.

> LayoutTests/css3/filters/custom/custom-filter-composite-operators.html:4
> +    <title>Tests that custom filter composite operators compute color values correctly.</title>

This test looks great! Thanks for making it. Do you think it can be a ref test instead of a pixel test?

[Dongseong Hwang]

Created attachment 167183 [details]
Patch

[Dongseong Hwang]

(In reply to comment #17)
> This change is unrelated to the compositing modes, so I would take it out. I think you should land bug 98396 first, since you're removing this change in there. That is, refactor first, and then add new functionality.

Absolutely!

> This test looks great! Thanks for making it. Do you think it can be a ref test instead of a pixel test?

I made it because of your help. Unfortunately, it seems not to be an alternative method of a pixel test IMHO. In my experience of layout tests, only canvas element has an alternative method because the canvas has getImageData API. I wish listening other forks' opinions too.

[Dean Jackson]

Comment on attachment 167183 [details]
Patch

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

> Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp:294
>      case CompositePlusLighter:
>          notImplemented();
>          return String();
> +        // FIXME: destination and lighter were not implemented yet.

I don't think we need this comment. It's clear that they are not implemented.

> LayoutTests/css3/filters/custom/custom-filter-composite-operators.html:4
> +    <title>Tests that custom filter composite operators compute color values correctly.</title>

As Max originally suggested, it would be great if this could be a reftest, with the expected value shown next to each subtest. I wonder if we can be accurate enough by placing colored divs to replicate the blending. Anyway, this is ok for now.

[Dongseong Hwang]

Created attachment 167957 [details]
Patch

[Dongseong Hwang]

(In reply to comment #20)
> I don't think we need this comment. It's clear that they are not implemented.

Done.

> As Max originally suggested, it would be great if this could be a reftest, with the expected value shown next to each subtest. I wonder if we can be accurate enough by placing colored divs to replicate the blending. Anyway, this is ok for now.

I agree.


Thanks for your review.

Could you check r+ c+ one more time? You already checked r+. I'm not commiter, so I always depend on commit-queue-bot.

[WebKit Review Bot]

Comment on attachment 167957 [details]
Patch

Clearing flags on attachment: 167957

Committed r131100: <http://trac.webkit.org/changeset/131100>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Adam Barth]

This patch caused an ASSERT:

crash log for DumpRenderTree (pid 1352):
STDOUT: <empty>
STDERR: ASSERTION FAILED: m_samplerLocation != -1
STDERR: /Volumes/data/b/build/slave/webkit-mac-latest-dbg/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../platform/graphics/filters/CustomFilterCompiledProgram.cpp(139) : void WebCore::CustomFilterCompiledProgram::initializeParameterLocations(WebCore::CustomFilterProgramType)
STDERR: 1   0x24a96b14 WebCore::CustomFilterCompiledProgram::initializeParameterLocations(WebCore::CustomFilterProgramType)
STDERR: 2   0x24a9624b WebCore::CustomFilterCompiledProgram::CustomFilterCompiledProgram(WTF::PassRefPtr<WebCore::GraphicsContext3D>, WTF::String const&, WTF::String const&, WebCore::CustomFilterProgramType)
STDERR: 3   0x24a9603c WebCore::CustomFilterCompiledProgram::CustomFilterCompiledProgram(WTF::PassRefPtr<WebCore::GraphicsContext3D>, WTF::String const&, WTF::String const&, WebCore::CustomFilterProgramType)
STDERR: 4   0x24a984cb WebCore::CustomFilterCompiledProgram::create(WTF::PassRefPtr<WebCore::GraphicsContext3D>, WTF::String const&, WTF::String const&, WebCore::CustomFilterProgramType)
STDERR: 5   0x24a97944 WebCore::CustomFilterValidatedProgram::compiledProgram()
STDERR: 6   0x24aaad86 WebCore::FECustomFilter::initializeContext()
STDERR: 7   0x24aaab65 WebCore::FECustomFilter::prepareForDrawing(WebCore::FECustomFilter::CustomFilterDrawType)
STDERR: 8   0x24aa941f WebCore::FECustomFilter::applyShader()
STDERR: 9   0x24aa932e WebCore::FECustomFilter::platformApplySoftware()
STDERR: 10  0x24ac4396 WebCore::FilterEffect::apply()
STDERR: 11  0x25a367ab WebCore::FilterEffectRenderer::apply()
STDERR: 12  0x25a36f64 WebCore::FilterEffectRendererHelper::applyFilterEffect()
STDERR: 13  0x25bea756 WebCore::RenderLayer::paintLayerContents(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, WebCore::FractionalLayoutSize const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int)
STDERR: 14  0x25be921b WebCore::RenderLayer::paintLayerContentsAndReflection(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, WebCore::FractionalLayoutSize const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int)
STDERR: 15  0x25be852c WebCore::RenderLayer::paintLayer(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, WebCore::FractionalLayoutSize const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int)
STDERR: 16  0x25beb635 WebCore::RenderLayer::paintList(WTF::Vector<WebCore::RenderLayer*, 0ul>*, WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int)
STDERR: 17  0x25bea5fa WebCore::RenderLayer::paintLayerContents(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, WebCore::FractionalLayoutSize const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int)
STDERR: 18  0x25be921b WebCore::RenderLayer::paintLayerContentsAndReflection(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, WebCore::FractionalLayoutSize const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int)
STDERR: 19  0x25be852c WebCore::RenderLayer::paintLayer(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, WebCore::FractionalLayoutSize const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int)
STDERR: 20  0x25beb635 WebCore::RenderLayer::paintList(WTF::Vector<WebCore::RenderLayer*, 0ul>*, WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int)
STDERR: 21  0x25bea5fa WebCore::RenderLayer::paintLayerContents(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, WebCore::FractionalLayoutSize const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int)
STDERR: 22  0x25be921b WebCore::RenderLayer::paintLayerContentsAndReflection(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, WebCore::FractionalLayoutSize const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int)
STDERR: 23  0x25be852c WebCore::RenderLayer::paintLayer(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, WebCore::FractionalLayoutSize const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int)
STDERR: 24  0x25be7c5b WebCore::RenderLayer::paint(WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, unsigned int)
STDERR: 25  0x2588a73c WebCore::FrameView::paintContents(WebCore::GraphicsContext*, WebCore::IntRect const&)
STDERR: 26  0x2494fabc WebCore::ScrollView::paint(WebCore::GraphicsContext*, WebCore::IntRect const&)
STDERR: 27  0x23516b7e WebKit::PageWidgetDelegate::paint(WebCore::Page*, WebKit::PageOverlayList*, SkCanvas*, WebKit::WebRect const&, WebKit::PageWidgetDelegate::CanvasBackground)
STDERR: 28  0x23655e28 WebKit::WebViewImpl::paint(SkCanvas*, WebKit::WebRect const&, WebKit::WebWidget::PaintOptions)
STDERR: 29  0x22baa308 WebViewHost::paintRect(WebKit::WebRect const&)
STDERR: 30  0x22baa7c9 WebViewHost::paintInvalidatedRegion()
STDERR: 31  0x22b88493 TestShell::dump()
STDERR: [1352:-1602484928:365815664370:ERROR:process_util_posix.cc(144)] Received signal 11
STDERR: 	0   libbase.dylib                       0x2c05ef7f base::debug::StackTrace::StackTrace() + 63
STDERR: 	1   libbase.dylib                       0x2c05ef1b base::debug::StackTrace::StackTrace() + 43
STDERR: 	2   libbase.dylib                       0x2c13a407 base::(anonymous namespace)::StackDumpSignalHandler(int, __siginfo*, __darwin_ucontext*) + 295
STDERR: 	3   libSystem.B.dylib                   0x9539405b _sigtramp + 43
STDERR: 	4   ???                                 0xffffffff 0x0 + 4294967295
STDERR: 	5   libwebkit.dylib                     0x24a9624b WebCore::CustomFilterCompiledProgram::CustomFilterCompiledProgram(WTF::PassRefPtr<WebCore::GraphicsContext3D>, WTF::String const&, WTF::String const&, WebCore::CustomFilterProgramType) + 491
STDERR: 	6   libwebkit.dylib                     0x24a9603c WebCore::CustomFilterCompiledProgram::CustomFilterCompiledProgram(WTF::PassRefPtr<WebCore::GraphicsContext3D>, WTF::String const&, WTF::String const&, WebCore::CustomFilterProgramType) + 92
STDERR: 	7   libwebkit.dylib                     0x24a984cb WebCore::CustomFilterCompiledProgram::create(WTF::PassRefPtr<WebCore::GraphicsContext3D>, WTF::String const&, WTF::String const&, WebCore::CustomFilterProgramType) + 139
STDERR: 	8   libwebkit.dylib                     0x24a97944 WebCore::CustomFilterValidatedProgram::compiledProgram() + 356
STDERR: 	9   libwebkit.dylib                     0x24aaad86 WebCore::FECustomFilter::initializeContext() + 310
STDERR: 	10  libwebkit.dylib                     0x24aaab65 WebCore::FECustomFilter::prepareForDrawing(WebCore::FECustomFilter::CustomFilterDrawType) + 85
STDERR: 	11  libwebkit.dylib                     0x24aa941f WebCore::FECustomFilter::applyShader() + 175
STDERR: 	12  libwebkit.dylib                     0x24aa932e WebCore::FECustomFilter::platformApplySoftware() + 46
STDERR: 	13  libwebkit.dylib                     0x24ac4396 WebCore::FilterEffect::apply() + 438
STDERR: 	14  libwebkit.dylib                     0x25a367ab WebCore::FilterEffectRenderer::apply() + 75
STDERR: 	15  libwebkit.dylib                     0x25a36f64 WebCore::FilterEffectRendererHelper::applyFilterEffect() + 212
STDERR: 	16  libwebkit.dylib                     0x25bea756 WebCore::RenderLayer::paintLayerContents(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, WebCore::FractionalLayoutSize const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int) + 5382
STDERR: 	17  libwebkit.dylib                     0x25be921b WebCore::RenderLayer::paintLayerContentsAndReflection(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, WebCore::FractionalLayoutSize const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int) + 555
STDERR: 	18  libwebkit.dylib                     0x25be852c WebCore::RenderLayer::paintLayer(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, WebCore::FractionalLayoutSize const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int) + 2012
STDERR: 	19  libwebkit.dylib                     0x25beb635 WebCore::RenderLayer::paintList(WTF::Vector<WebCore::RenderLayer*, 0ul>*, WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int) + 389
STDERR: 	20  libwebkit.dylib                     0x25bea5fa WebCore::RenderLayer::paintLayerContents(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, WebCore::FractionalLayoutSize const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int) + 5034
STDERR: 	21  libwebkit.dylib                     0x25be921b WebCore::RenderLayer::paintLayerContentsAndReflection(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, WebCore::FractionalLayoutSize const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int) + 555
STDERR: 	22  libwebkit.dylib                     0x25be852c WebCore::RenderLayer::paintLayer(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, WebCore::FractionalLayoutSize const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int) + 2012
STDERR: 	23  libwebkit.dylib                     0x25beb635 WebCore::RenderLayer::paintList(WTF::Vector<WebCore::RenderLayer*, 0ul>*, WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int) + 389
STDERR: 	24  libwebkit.dylib                     0x25bea5fa WebCore::RenderLayer::paintLayerContents(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, WebCore::FractionalLayoutSize const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int) + 5034
STDERR: 	25  libwebkit.dylib                     0x25be921b WebCore::RenderLayer::paintLayerContentsAndReflection(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, WebCore::FractionalLayoutSize const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int) + 555
STDERR: 	26  libwebkit.dylib                     0x25be852c WebCore::RenderLayer::paintLayer(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, WebCore::FractionalLayoutSize const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >*, unsigned int) + 2012
STDERR: 	27  libwebkit.dylib                     0x25be7c5b WebCore::RenderLayer::paint(WebCore::GraphicsContext*, WebCore::FractionalLayoutRect const&, unsigned int, WebCore::RenderObject*, WebCore::RenderRegion*, unsigned int) + 299
STDERR: 	28  libwebkit.dylib                     0x2588a73c WebCore::FrameView::paintContents(WebCore::GraphicsContext*, WebCore::IntRect const&) + 1708
STDERR: 	29  libwebkit.dylib                     0x2494fabc WebCore::ScrollView::paint(WebCore::GraphicsContext*, WebCore::IntRect const&) + 1068
STDERR: 	30  libwebkit.dylib                     0x23516b7e WebKit::PageWidgetDelegate::paint(WebCore::Page*, WebKit::PageOverlayList*, SkCanvas*, WebKit::WebRect const&, WebKit::PageWidgetDelegate::CanvasBackground) + 462
STDERR: 	31  libwebkit.dylib                     0x23655e28 WebKit::WebViewImpl::paint(SkCanvas*, WebKit::WebRect const&, WebKit::WebWidget::PaintOptions) + 840
STDERR: 	32  DumpRenderTree                      0x22baa308 WebViewHost::paintRect(WebKit::WebRect const&) + 552
STDERR: 	33  DumpRenderTree                      0x22baa7c9 WebViewHost::paintInvalidatedRegion() + 745
STDERR: 	34  DumpRenderTree                      0x22b88493 TestShell::dump() + 2003
STDERR: 	35  DumpRenderTree                      0x22b87c92 TestShell::testFinished() + 82
STDERR: 	36  DumpRenderTree                      0x22b4465b DRTTestRunner::WorkQueue::processWorkSoon() + 187
STDERR: 	37  DumpRenderTree                      0x22b44cca DRTTestRunner::locationChangeDone() + 138
STDERR: 	38  DumpRenderTree                      0x22ba52d8 WebViewHost::locationChangeDone(WebKit::WebFrame*) + 88
STDERR: 	39  DumpRenderTree                      0x22ba5c63 WebViewHost::didFinishLoad(WebKit::WebFrame*) + 163
STDERR: 	40  DumpRenderTree                      0x22ba5cb9 non-virtual thunk to WebViewHost::didFinishLoad(WebKit::WebFrame*) + 41
STDERR: 	41  libwebkit.dylib                     0x234d3104 WebKit::FrameLoaderClientImpl::dispatchDidFinishLoad() + 148
STDERR: 	42  libwebkit.dylib                     0x25709ce9 WebCore::FrameLoader::checkLoadCompleteForThisFrame() + 1945
STDERR: 	43  libwebkit.dylib                     0x25700529 WebCore::FrameLoader::checkLoadComplete() + 425
STDERR: 	44  libwebkit.dylib                     0x256ffdd1 WebCore::FrameLoader::checkCompleted() + 433
STDERR: 	45  libwebkit.dylib                     0x256fffcb WebCore::FrameLoader::loadDone() + 43
STDERR: 	46  libwebkit.dylib                     0x257aa7bf WebCore::CachedResourceLoader::loadDone() + 127
STDERR: 	47  libwebkit.dylib                     0x25758008 WebCore::SubresourceLoader::releaseResources() + 216
STDERR: 	48  libwebkit.dylib                     0x2574e0a3 WebCore::ResourceLoader::didFinishLoading(double) + 99
STDERR: 	49  libwebkit.dylib                     0x25757adc WebCore::SubresourceLoader::didFinishLoading(double) + 716
STDERR: 	50  libwebkit.dylib                     0x2574eb0f WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle*, double) + 79
STDERR: 	51  libwebkit.dylib                     0x24ba2cda WebCore::ResourceHandleInternal::didFinishLoading(WebKit::WebURLLoader*, double) + 282
STDERR: 	52  libglue.dylib                       0x2ef8006e webkit_glue::WebURLLoaderImpl::Context::OnCompletedRequest(int, bool, std::string const&, base::TimeTicks const&) + 1134
STDERR: 	53  DumpRenderTree                      0x22e65c6d (anonymous namespace)::RequestProxy::NotifyCompletedRequest(int, std::string const&, base::TimeTicks const&) + 125
STDERR: 	54  DumpRenderTree                      0x22e66487 base::internal::RunnableAdapter<void ((anonymous namespace)::RequestProxy::*)(int, std::string const&, base::TimeTicks const&)>::Run((anonymous namespace)::RequestProxy*, int const&, std::string const&, base::TimeTicks const&) + 215
STDERR: 	55  DumpRenderTree                      0x22e66386 base::internal::InvokeHelper<false, void, base::internal::RunnableAdapter<void ((anonymous namespace)::RequestProxy::*)(int, std::string const&, base::TimeTicks const&)>, void ()((anonymous namespace)::RequestProxy* const&, int const&, std::string const&, base::TimeTicks const&)>::MakeItSo(base::internal::RunnableAdapter<void ((anonymous namespace)::RequestProxy::*)(int, std::string const&, base::TimeTicks const&)>, (anonymous namespace)::RequestProxy* const&, int const&, std::string const&, base::TimeTicks const&) + 150
STDERR: 	56  DumpRenderTree                      0x22e662c5 base::internal::Invoker<4, base::internal::BindState<base::internal::RunnableAdapter<void ((anonymous namespace)::RequestProxy::*)(int, std::string const&, base::TimeTicks const&)>, void ()((anonymous namespace)::RequestProxy*, int, std::string const&, base::TimeTicks const&), void ()((anonymous namespace)::RequestProxy*, int, std::string, base::TimeTicks)>, void ()((anonymous namespace)::RequestProxy*, int, std::string const&, base::TimeTicks const&)>::Run(base::internal::BindStateBase*) + 229
STDERR: 	57  libbase.dylib                       0x2c04ecdb base::Callback<void ()()>::Run() const + 75
STDERR: 	58  libbase.dylib                       0x2c0da147 MessageLoop::RunTask(base::PendingTask const&) + 1159
STDERR: 	59  libbase.dylib                       0x2c0da642 MessageLoop::DeferOrRunPendingTask(base::PendingTask const&) + 98
STDERR: 	60  libbase.dylib                       0x2c0da842 MessageLoop::DoWork() + 322
STDERR: 	61  libbase.dylib                       0x2c02d05b base::MessagePumpCFRunLoopBase::RunWork() + 107
STDERR: ax: bbadbeef, bx: 3, cx: 10eadfe0, dx: 10eadfe0
STDERR: di: 26283fe0, si: 26283f76, bp: bfffa2a8, sp: bfffa1b0, ss: 23, flags: 10282
STDERR: ip: 24a96b1e, cs: 1b, ds: 23, es: 23, fs: 0, gs: f

[WebKit Review Bot]

Re-opened since this is blocked by bug 99127

[Dongseong Hwang]

(In reply to comment #26)
> Re-opened since this is blocked by bug 99127

It seems this patch can not cause "ASSERTION FAILED: m_samplerLocation != -1", but I'll fix the assertion fail and then post this patch again.

[Adam Barth]

> It seems this patch can not cause "ASSERTION FAILED: m_samplerLocation != -1"

Reverting the patch appears to have made the assertion go away.

[Dongseong Hwang]

(In reply to comment #28)
> > It seems this patch can not cause "ASSERTION FAILED: m_samplerLocation != -1"
> 
> Reverting the patch appears to have made the assertion go away.

Ok. Could I know which test cases cause this crash. I could not reproduce crash using css1/filters/custom/*, although I tested using Qt.

[Adam Barth]

I'm sorry, I forgot to record that information.  :(

[Max Vujovic]

This assert means we can't find the sampler "css_u_texture" in the compiled program. Maybe the driver compiler optimizes away "css_u_texture" when the compositing mode is clear, since it sees a multiplication by zero?

[Dean Jackson]

(In reply to comment #31)
> This assert means we can't find the sampler "css_u_texture" in the compiled program. Maybe the driver compiler optimizes away "css_u_texture" when the compositing mode is clear, since it sees a multiplication by zero?

Yeah, good suggestion on where to start.

Thanks for rolling it out Adam.

[Max Vujovic]

(In reply to comment #32)
> (In reply to comment #31)
> > This assert means we can't find the sampler "css_u_texture" in the compiled program. Maybe the driver compiler optimizes away "css_u_texture" when the compositing mode is clear, since it sees a multiplication by zero?
> 
> Yeah, good suggestion on where to start.
> 
> Thanks for rolling it out Adam.

Yup, thanks Adam :)

[Dongseong Hwang]

(In reply to comment #30)
> I'm sorry, I forgot to record that information.  :(

Ok. Thanks. I reproduced.

[Dongseong Hwang]

(In reply to comment #31)
> This assert means we can't find the sampler "css_u_texture" in the compiled program. Maybe the driver compiler optimizes away "css_u_texture" when the compositing mode is clear, since it sees a multiplication by zero?

Yes, exactly! When the compositing mode is clear or copy, the driver compiler optimizes may remove "css_u_texture", so we may remove assertions.

[Dongseong Hwang]

Created attachment 168535 [details]
Patch

[Dongseong Hwang]

(In reply to comment #36)
> Created an attachment (id=168535) [details]
> Patch

I just removed ASSERTIONs in CustomFilterCompiledProgram::initializeParameterLocations().

It is ok because FECustomFilter checks if uniform and attribute are -1 before binding them.

1. For ASSERT(m_samplerLocation != -1), FECustomFilter binds m_samplerLocation uniform if m_samplerLocation is not -1.
void FECustomFilter::bindProgramAndBuffers(Platform3DObject inputTexture)
{
    ....    
    if (programNeedsInputTexture()) {
        ....
        m_context->activeTexture(GraphicsContext3D::TEXTURE0);
        m_context->uniform1i(m_compiledProgram->samplerLocation(), 0);
        m_context->bindTexture(GraphicsContext3D::TEXTURE_2D, inputTexture);
        ....
    }
    ....
}

bool FECustomFilter::programNeedsInputTexture() const
{
    ASSERT(m_compiledProgram.get());
    return m_compiledProgram->samplerLocation() != -1;
}

2. For ASSERT(m_internalTexCoordAttribLocation != -1), FECustomFilter binds m_internalTexCoordAttribLocation attribute if m_internalTexCoordAttribLocation is not -1.
void FECustomFilter::bindProgramAndBuffers(Platform3DObject inputTexture)
{
    ....
    bindVertexAttribute(m_compiledProgram->internalTexCoordAttribLocation(), TexAttribSize, TexAttribOffset);
    ....
}

void FECustomFilter::bindVertexAttribute(int attributeLocation, unsigned size, unsigned offset)
{
    if (attributeLocation != -1) {
        m_context->vertexAttribPointer(attributeLocation, size, GraphicsContext3D::FLOAT, false, m_mesh->bytesPerVertex(), offset);
        m_context->enableVertexAttribArray(attributeLocation);
    }
}

[Max Vujovic]

Comment on attachment 168535 [details]
Patch

Thanks for investigating the issue!

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

> Source/WebCore/ChangeLog:19
> +            Remove ASSERTION checking if glGetUniformlocation returns negative,

Nit: Capitalization "glGetUniformLocation".

> Source/WebCore/ChangeLog:24
> +            glGetuniformlocation returns -1. glGetAttribLocation ditto.

Ditto.

> Source/WebCore/platform/graphics/filters/CustomFilterCompiledProgram.cpp:-139
> -        ASSERT(m_samplerLocation != -1);

This assertion is nice because it can catch bugs in the shader rewriting, so I'm not sure we should remove it.

Perhaps we can move this assertion to:
CustomFilterValidatedProgram::compiledProgram()

And it can look like this:
ASSERT(m_samplerLocation != -1 || !needsInputTexture());

Calling a new method:
bool CustomFilterValidatedProgram::needsInputTexture() const
{
     return m_programInfo.programType == PROGRAM_TYPE_BLENDS_ELEMENT_TEXTURE
          && m_programInfo.compositeOperator != CompositeClear 
          && m_programInfo.compositeOperator != CompositeCopy;
}

I'm suggesting moving it CustomFilterValidatedProgram because CustomFilterCompiledProgram doesn't know about compositing operators. It only knows about the GLSL shader strings it should to compile.

What do you think?

[Dongseong Hwang]

(In reply to comment #38)
> (From update of attachment 168535 [details])
> Thanks for investigating the issue!
> This assertion is nice because it can catch bugs in the shader rewriting, so I'm not sure we should remove it.
> 
> I'm suggesting moving it CustomFilterValidatedProgram because CustomFilterCompiledProgram doesn't know about compositing operators. It only knows about the GLSL shader strings it should to compile.
> 
> What do you think?

Thanks for your explanation! You are right absolutely.

I wanted to not remove ASSERTIONs but I did not know how CustomFilterCompiledProgram can know about compositing operators.

I'll update soon.

[Dongseong Hwang]

Created attachment 169042 [details]
Patch

[Dongseong Hwang]

(In reply to comment #40)
> Created an attachment (id=169042) [details]
> Patch

Move assertions in CustomFilterCompiledProgram to CustomFilterValidatedProgram as Max suggested.

[Max Vujovic]

(In reply to comment #41)
> (In reply to comment #40)
> > Created an attachment (id=169042) [details] [details]
> > Patch
> 
> Move assertions in CustomFilterCompiledProgram to CustomFilterValidatedProgram as Max suggested.

Thanks! This looks good to me. Perhaps you can ask Dean for another look :)

[WebKit Review Bot]

Comment on attachment 169042 [details]
Patch

Clearing flags on attachment: 169042

Committed r132267: <http://trac.webkit.org/changeset/132267>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.