[Webkit-unassigned] [Bug 98504] [CSS Shaders] Implement overlay, color-dodge, color-burn, hard-light, soft-light blend modes.

[bugzilla-daemon at webkit.org]

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


Dean Jackson <dino at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #168290|review?                     |review-
               Flag|                            |




--- Comment #12 from Dean Jackson <dino at apple.com>  2012-10-11 15:48:00 PST ---
(From update of attachment 168290)
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."

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)

> 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?

> LayoutTests/css3/filters/custom/custom-filter-blend-modes.html:213
> +    .color-burn-expected {
> +        /*
> +            ColorBurn:
> +
> +            Co = if(Cs > 0)
> +                     1 - min(1, (1 - Cb) / Cs)
> +                 else
> +                     0
> +
> +            r = 0
> +            g = 1 - min(1, (1 - 0.3) / 0.6) = 0
> +            b = 1 - min(1, (1 - 0.5) / 0.5) = 0
> +        */
> +        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.

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