Support colors w/ alpha in SVG 'stop' elements and 'feFlood'

Support colors w/ alpha in SVG 'stop' elements and 'feFlood'

When a color w/ alpha is specified (via rgba(...)) as stop-color on
a SVG 'stop' element, or as flood-color (or equivalent) on 'feFlood',
then modulate the alpha-channel with the stop-opacity or flood-opacity
respectively to produce the final color to use.

BUG=331516
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=164690

[krit, 2014-01-07 17:46:15]

I would like the others look at the code first. Just a comment on the tests.

https://codereview.chromium.org/126033003/diff/1/LayoutTests/svg/custom/gradi...
File LayoutTests/svg/custom/gradient-rgba-stop-color.svg (right):

https://codereview.chromium.org/126033003/diff/1/LayoutTests/svg/custom/gradi...
LayoutTests/svg/custom/gradient-rgba-stop-color.svg:3: <stop
stop-color="rgba(255,0,0,0)"/>
I think it would be great if you would use 0.5 for alpha or a more stable value
that is between 0 and 1.

https://codereview.chromium.org/126033003/diff/1/LayoutTests/svg/filters/feFl...
File LayoutTests/svg/filters/feFlood-rgba-flood-color.svg (right):

https://codereview.chromium.org/126033003/diff/1/LayoutTests/svg/filters/feFl...
LayoutTests/svg/filters/feFlood-rgba-flood-color.svg:3: <feFlood
flood-color="rgba(255,0,0,0)"/>
ditto. Even though... it might be harder because of
color-interpolations-filter... but you could set it to sRGB and it is fine.

[Stephen Chennney, 2014-01-07 17:49:37]

Good fix.

Are the given RGBA colors not pre-multiplied? Given the code I assume that's the
case.

Could you please also add test cases for partial opacity, opacity == 0, alpha ==
128, etc. The results would all differ from the previous behavior, I think.

[pdr., 2014-01-08 05:26:38]

I like this as well.

My only contribution is a nitpick on the name: "modulate" is very generic.
Possibly "combineWithAlpha"?

[fs, 2014-01-08 08:22:47]

On 2014/01/07 17:49:37, Stephen Chenney wrote:
> Good fix.
> 
> Are the given RGBA colors not pre-multiplied? Given the code I assume that's
the
> case.

They are not pre-multiplied, no. (For several reasons is wouldn't make sense if
they were - like for instance that gradient interpolation [for SVG] should take
place in "non-premultiplied space" [component-wise/independent interpolation].)

> 
> Could you please also add test cases for partial opacity, opacity == 0, alpha
==
> 128, etc. The results would all differ from the previous behavior, I think.

Sure. (I hope this will address krit's comments/issues as well.)

pdr wrote:
> My only contribution is a nitpick on the name: "modulate" is very generic.
> Possibly "combineWithAlpha"?

Sure. (I was thinking of changing it to "modulateAlpha", but I can use your
suggestion.)

[fs, 2014-01-08 10:05:52]

Now with 400% more TCs... =)

[Stephen Chennney, 2014-01-08 15:48:24]

lgtm

[commit-bot: I haz the power, 2014-01-08 17:09:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fs@opera.com/126033003/240001

[commit-bot: I haz the power, 2014-01-08 17:30:45]

Change committed as 164690