[Webkit-unassigned] [Bug 97859] [CSS Shaders] Implement all composite operators.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=97859





--- Comment #12 from Max Vujovic <mvujovic at adobe.com>  2012-10-01 13:08:51 PST ---
(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!

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.