[Webkit-unassigned] [Bug 98504] [CSS Shaders] Implement overlay, color-dodge, color-burn, hard-light, soft-light blend modes.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Oct 11 16:48:03 PDT 2012
https://bugs.webkit.org/show_bug.cgi?id=98504
--- Comment #14 from Huang Dongsung <luxtella at company100.net> 2012-10-11 16:48:44 PST ---
(In reply to comment #13)
> (In reply to comment #12)
> > (From update of attachment 168290 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=168290&action=review
> >
> > I think this looks great, but I have one minor concern that I'd like Max's feedback on: we're now doing more work on the simple compositing operations - calling per-component functions. I wonder if it would be worth only dropping to css_BlendComponent if necessary.
> >
> > Hmm... I see that Max did in fact make this suggestion "For the simpler blend modes that don't need to blend by component, we will overwrite this default value."
>
> Unless the driver compiler is very smart, it's more efficient if we avoid calling the per-component functions for the simpler blend modes.
>
> I didn't comment on this again since Huang's variant looks a little better. It's only managing one string for css_BlendComponent instead of two for css_BlendColor and css_BlendComponent. It's a small performance / complexity tradeoff.
>
> In this case, I'm leaning toward performance. Could you make that change please, Huang?
Of course, I'll change. I did miss those. sorry.
>
> >
> > In that case I will r-. It should be pretty easy to add this.
> >
> > > Source/WebCore/platform/graphics/filters/CustomFilterValidatedProgram.cpp:184
> > > // Cs: is the source color
> > > // Cb: is the backdrop color
> >
> > This comment is now incorrect. These are color components (although see general comment on the patch)
>
> If Huang makes the previous change, then it should be something like:
> // Cs: is the source color in css_BlendColor() and the source color component in css_BlendComponent()
> ...
Thanks!
>
> >
> > > LayoutTests/css3/filters/custom/custom-filter-blend-modes-expected.html:54
> > > + .hard-light-expected {
> > > + background-color: rgb(0%, 44%, 50%);
> > > + }
> >
> > Is there a reason why you used % here but numbers elsewhere?
>
> I think the % maps nicely to the hand calculations above, but sometimes the % causes color rounding problems and we have to use the 8-bit number variants. (I wish we didn't have to! It would be nice if we could specify a color tolerance per test.)
I tested this test on Chromium, Gtk and Qt. I approached native ways. If this test passed on all platforms, I leaved %. If not, I found numbers to make this test pass.
A color tolerance per test is holy grail. We really need it to reduce creating tests and maintenance cost for each platform.
>
> >
> > > LayoutTests/css3/filters/custom/custom-filter-blend-modes.html:213
> > > + background-color: rgb(0, 0, 1);
> >
> > I'm not sure why this is rgb(0, 0, 1) when you calculated 0, 0, 0.
> >
> > Also, this shows that we may need a more complex test for burn, since a completely black result doesn't really exercise much.
>
> Good point- perhaps we could use different input colors whose result isn't black.
Sadly, linux chromium failed when rgb(0, 0, 0).
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list