Closed Bug 436296 Opened 16 years ago Closed 12 years ago

SVG SMIL: implement "animateColor"

Categories

(Core :: SVG, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: codedread, Unassigned)

References

()

Details

Attachments

(2 files, 2 obsolete files)

Simple test to animateColor on fill, which does not yet work in the
hg repository for SMIL at
http://hg.mozilla.org/users/dholbert_mozilla.com/smil-patches/

See Bug 216462, Comment #58 for more details.
Rather than overloading the "URL" field to point to the hg repository, I'm trying out a whiteboard syntax suggested by timeless, for dealing with hg branches: "[hgurl:c0]", which means "this is a bug on a specific hg repo, whose url is in comment 0".

(alternately, we could just put the whole hg URL in the whiteboard, but hg urls are probably too long for that)
OS: Windows XP → All
Hardware: PC → All
Whiteboard: [hgurl:c0]
Depends on: 216462
Summary: SVG SMIL: Cannot animate color → SVG SMIL: implement "animateColor"
What does the separate 'animateColor' element give over and above the 'animate' element? It adds to the learning curve for authors and implementation/maintenance burden of implementers, and it's not clear to me why it's needed. The 'color-interpolation' property could be supported on 'animate'.
In non-SVG smil-animation, <animate> is can only interpolate numbers[1], and so <animateColor> adds the ability to animate colors.

However, in SVG 1.1, <animate> is extended to explicitly support color-animation[2], so you're right -- <animateColor> doesn't seem to add anything for SVG.

[1] http://www.w3.org/TR/smil-animation/#animateElement
[2] http://www.w3.org/TR/SVG11/animate.html#Animatable
Whiteboard: [hgurl:c0]
So I think we should just honor 'color-interpolation' on <animate> or on the target instead of implementing <animateColor>.

More specifically, I think:

 1) if 'color-interpolation' is set on the <animate> element, interpolate in
    that color space

 2) else, if the user specifies a 'to'/'by' color using 'hsl()' or 'hsla()'
    function syntax, then animate in HSL space

 3) else, if 'color-interpolation' is *explicitly* set on the target element,
    interpolate in that color space

 4) else animate in sRGB space

I think (2) is especially important. If a user writes:

  <animate attributeName="fill" from="hsl(0,100%,50%)" to="hsl(120,100%,50%)" />

then their intent is almost certainly to animate around the hue wheel from red to green rather than through sRGB space. We should support them by making it just work.

I'm not sure how well this will align with CSS transitions/animations. CC'ing dbaron and roc.
Blocks: svg11tests
(In reply to comment #4)
>  2) else, if the user specifies a 'to'/'by' color using 'hsl()' or 'hsla()'
>     function syntax, then animate in HSL space

I've heard comments that animating in HSL space actually tends to produce bad results.  Do we really want to do this?

>  3) else, if 'color-interpolation' is *explicitly* set on the target element,
>     interpolate in that color space

Do we really need to distinguish setting on the animate element vs. the target element?  Why isn't looking at it on one or the other enough?

>  4) else animate in sRGB space

That's the default value of 'color-interpolation'; I don't think it needs to be an "else".
Most times animating in HSL space certainly would produce bad results, but I've wanted to do that before and have heard the same from someone else.

Distinguishing between setting 'color-interpolation' on the animate element vs. the target element can be useful. For example, you may want to have a gradient with two stops where you interpolate from red to black using the default sRGB, but then you may want to animate the red stop around the hue ring.
(In reply to comment #5)
> (In reply to comment #4)
> >  2) else, if the user specifies a 'to'/'by' color using 'hsl()' or 'hsla()'
> >     function syntax, then animate in HSL space
> 
> I've heard comments that animating in HSL space actually tends to produce bad
> results.  Do we really want to do this?

Whether HSL-space is a good idea or not, I agree with Jonathan that if the author's using hsl/hsla it's clearly what they want.

I agree that we should avoid imposing <animateColor> on the Web.
animateColor is a killer feature for doing fast morphing in Opera. This is presently my most desired Firefox enhancement. Thanks, keep up the great work.
The point is that <animate> will work just as well as <animateColor>, so <animateColor> isn't needed.
For compatibility with other browsers, with some existing documents, and to be spec’-compliant, <animateColor/> must work like <animate/> for the colors. So why not make <animateColor/> an exact clone of <animate/>?
To be clear, there are a few key differences between <animateColor> and <animate> that would prevent us from just directly cloning <animate> for spec-compliance.

 (1) animateColor is only supposed to work for color-valued attrs & properties
(This restriction is easy to address, of course.)

 (2a) SVG 1.1 says that if an <animateColor> has 'color-interpolation' or 'color-rendering' set, it's supposed to honor those properties for its animation. References:
  http://www.w3.org/TR/SVG11/animate.html#AnimateColorElement
  http://www.w3.org/TR/SVG11/painting.html#ColorInterpolationProperty
  http://www.w3.org/TR/SVG11/painting.html#ColorRenderingProperty

 (2b) SVG Tiny 1.2 makes no mention of the 'color-interpolation' property, but it does still recognize 'color-rendering', and it specifically says that this property applies to <animateColor>.  Reference:
  http://www.w3.org/TR/SVGTiny12/painting.html#ColorRenderingProperty

Differences 2a/2b above would require some code-reworking, and I'm not convinced that they're useful.  (To make that work, we need to know, at interpolation-time, whether we should honor 'color-interpolation/rendering' on the _target_ element vs on the _animation_ element.  So basically, we'd need to pass down an "is this animateColor or something else" flag, all the way down to our interpolation code.)

Without Differences 2a/2b, <animateColor> is just a restricted version of <animate>, which also isn't particularly useful.

The interbrowser-compatibility & "working with existing documents" arguments sway me a little, but the spec-compliance argument doesn't.  This appears to be an area where the spec mandates something that's not useful.  I'd prefer that we lobby to change the spec.
Version: Other Branch → Trunk
(In reply to comment #11)
> To be clear, there are a few key differences between <animateColor> and
> <animate> that would prevent us from just directly cloning <animate> for
> spec-compliance.
> 
>  (1) animateColor is only supposed to work for color-valued attrs & properties
> (This restriction is easy to address, of course.)
> 
>  (2a) SVG 1.1 says that if an <animateColor> has 'color-interpolation' or
> 'color-rendering' set, it's supposed to honor those properties for its
> animation. References:
>   http://www.w3.org/TR/SVG11/animate.html#AnimateColorElement
>   http://www.w3.org/TR/SVG11/painting.html#ColorInterpolationProperty
>   http://www.w3.org/TR/SVG11/painting.html#ColorRenderingProperty
> 
>  (2b) SVG Tiny 1.2 makes no mention of the 'color-interpolation' property, but
> it does still recognize 'color-rendering', and it specifically says that this
> property applies to <animateColor>.  Reference:
>   http://www.w3.org/TR/SVGTiny12/painting.html#ColorRenderingProperty
> 
> Differences 2a/2b above would require some code-reworking, and I'm not
> convinced that they're useful.  (To make that work, we need to know, at
> interpolation-time, whether we should honor 'color-interpolation/rendering' on
> the _target_ element vs on the _animation_ element.  So basically, we'd need to
> pass down an "is this animateColor or something else" flag, all the way down to
> our interpolation code.)
> 
> Without Differences 2a/2b, <animateColor> is just a restricted version of
> <animate>, which also isn't particularly useful.
> 
> The interbrowser-compatibility & "working with existing documents" arguments
> sway me a little, but the spec-compliance argument doesn't.  This appears to be
> an area where the spec mandates something that's not useful.  I'd prefer that
> we lobby to change the spec.

How does it currently work in other browsers?
Attached patch patch (obsolete) — Splinter Review
This patch basically treats animateColor as a colour-limited animate. Safari, Opera and Chrome all seem to have bitten the bullet and implemented and various authors seem to be using it even though they could have used animate in every case I've seen e.g. http://srufaculty.sru.edu/david.dailey/svg/SVGAnimations.htm so perhaps it's just too much effort to get the genie back in the bottle.

The mistake that SVG made was probably to extend animate to colours as in other cases it just aligns directly with SMIL and that does have a reson for the distinction since there animate doesn't do colours.

FWIW we pass

http://www.w3.org/Graphics/SVG/Test/20061213/htmlObjectHarness/full-animate-elem-23-t.html
http://www.w3.org/Graphics/SVG/Test/20061213/htmlObjectHarness/full-animate-elem-30-t.html
http://www.w3.org/Graphics/SVG/Test/20061213/htmlObjectHarness/full-animate-elem-84-t.html
http://www.w3.org/Graphics/SVG/Test/20061213/htmlObjectHarness/full-animate-elem-85-t.html

with this but not

http://www.w3.org/Graphics/SVG/Test/20061213/htmlObjectHarness/full-pservers-grad-19-b.html

not sure why atm.
I'm tempted to take this but maybe we should actually ship our SMIL support without animateColor first and see how authors respond? If enough switch from <animateColor> to <animate>, we still might be able to kill off <animateColor> and get <animateColor> removed from the SVG test suite.
It would be great if the current, maintained test suite was used, not the old 2006 version:

http://dev.w3.org/SVG/profiles/1.1F2/test/harness/htmlObject/animate-elem-23-t.html
http://dev.w3.org/SVG/profiles/1.1F2/test/harness/htmlObject/animate-elem-30-t.html
http://dev.w3.org/SVG/profiles/1.1F2/test/harness/htmlObject/animate-elem-84-t.html
http://dev.w3.org/SVG/profiles/1.1F2/test/harness/htmlObject/animate-elem-85-t.html

(btw SVG WG extended animate to colours since SMIL asserts that a list of vectors can be animated, and that is what colour is, so we responded to criticism that it was not possible to animate this way and that the restriction was arbitrary and unwarranted).
Comment on attachment 448297 [details] [diff] [review]
patch

>+  // animateColor can only animate color values
>+  if (atom == nsGkAtoms::fill ||
>+      atom == nsGkAtoms::font_style ||
>+      atom == nsGkAtoms::color ||
>+      atom == nsGkAtoms::stop_color ||
>+      atom == nsGkAtoms::flood_color ||
>+      atom == nsGkAtoms::lighting_color) {
>+    return atom;

This needs a s/font_style/stroke/, right? :)
Attached patch right (obsolete) — Splinter Review
Attachment #448297 - Attachment is obsolete: true
(In reply to comment #17)
> Created attachment 448430 [details] [diff] [review]
> right

This patch is rather severely bit-rotted.
Attached patch unbitrottedSplinter Review
Attachment #448430 - Attachment is obsolete: true
(In reply to comment #18)
> This patch is rather severely bit-rotted.

We're not planning to land this for 4.0, maybe not ever in fact. But just this once since I still have it :-)
I think I understand the rationale for this as being separate from the more general case of being able to animate anything.  Animating color does not effect layout, so applications that do not support animating things that effect layout might be able to support animating color.  The problem here is that color is not the only CSS property that could be animated that does not effect layout so it is still sort a brain-damaged part of the specification.
I don't think that's actually the rationale for why <animateColor> exists in the spec(s).

First question: Why is animateColor in the smil-animation spec?
Answer: In the smil-animation spec, <animate> doesn't support interpolation of colors values.  (In fact, it only supports interpolation of unitless numeric values.) [1]  This makes <animate> quite simple, but it means that another element (<animateColor>) is required in order to support animation of colors.

So, second question: Why is animateColor in the SVG spec?
Answer: because it imported the element (with everything else) from smil-animation.

However, <animateColor> is an unnecessary element **in SVG**, because the SVG spec extended <animate> to support interpolating more types of values, including colors (and list-of-[type], and numeric values with units)[2].  And once <animate> has been thus extended, <animateColor> becomes unnecessary and extraneous.

[1] http://www.w3.org/TR/smil-animation/#AnimationFunctionValueDetails

[2] http://www.w3.org/TR/SVG/animate.html#AnimationAttributesAndProperties
(see table halfway down this section ^)
(In reply to comment #22)
> I don't think that's actually the rationale for why <animateColor> exists in
> the spec(s).
> 
> First question: Why is animateColor in the smil-animation spec?
> Answer: In the smil-animation spec, <animate> doesn't support interpolation of
> colors values.  (In fact, it only supports interpolation of unitless numeric
> values.) [1]  This makes <animate> quite simple, but it means that another
> element (<animateColor>) is required in order to support animation of colors.

Isn't animation of color just animating unitless numerical values?
(In reply to comment #23)
> (In reply to comment #22)
> > I don't think that's actually the rationale for why <animateColor> exists in
> > the spec(s).
> > 
> > First question: Why is animateColor in the smil-animation spec?
> > Answer: In the smil-animation spec, <animate> doesn't support interpolation of
> > colors values.  (In fact, it only supports interpolation of unitless numeric
> > values.) [1]  This makes <animate> quite simple, but it means that another
> > element (<animateColor>) is required in order to support animation of colors.
> 
> Isn't animation of color just animating unitless numerical values?

OIC you are saying they are things specified as unitless numerical values, so the fact that animating color just ends up being the animation of multiple unitless numerical values does not mean it does not need a different way to specify it.
(In reply to comment #23)
> Isn't animation of color just animating unitless numerical values?

No -- color values are not unitless numerical values.  They may *have* unitless numeric values *in* them, but they necessarily have other text (e.g. "rgb()", "#", or even "blue"), too.

Interpolating between actual unitless numeric values (e.g. from="1" to="10") is simple. Interpolating between colors (e.g. from="rgb(50,50,50)" to="#FFAABB") clearly requires more parsing & logic.

(I didn't quite understand Comment 24, but I think this might be what you were basically saying there.)
(In reply to comment #25)
> (In reply to comment #23)
> > Isn't animation of color just animating unitless numerical values?
> 
> No -- color values are not unitless numerical values.  They may *have* unitless
> numeric values *in* them, but they necessarily have other text (e.g. "rgb()",
> "#", or even "blue"), too.
> 
> Interpolating between actual unitless numeric values (e.g. from="1" to="10") is
> simple. Interpolating between colors (e.g. from="rgb(50,50,50)" to="#FFAABB")
> clearly requires more parsing & logic.
> 
> (I didn't quite understand Comment 24, but I think this might be what you were
> basically saying there.)

Actually, that is what I realized and was trying to say in my previous comment.  I guess I didn't express it well.
Chris Lilley found a Wikipedia page that uses animateColor, so I suppose we'll have to implement this.
Can't someone just edit the file so that it uses animate rather than animateColor?
Also updated the article referencing the file:
http://commons.wikimedia.org/wiki/Commons:Stroke_Order_Project/SVG
I guess the question is whether that's the only such file on the Internet.
I use it several times in my art gallery (wasted time to explore hundreds of PHP-scripts to find maybe 50 to 100 that use animateColor). I explained it as well in a wikibook about SVG and in my SVG tutorial including examples. 
Because it was never a problem in other viewers and intended to animate
colours, it should be no surprise, that sometimes authors really use it to animate colours ;o)
Concerning my art gallery, this gap will be one of two main reasons, 
why I have to recommend visitors of the art gallery not to use a Gecko 2.0/Firefox4 :-(

I think it is now a little bit late to discuss about the sense of animateColor, because it is already used for about 10 years.

In march for example I started to test Gecko 2 in my SVG test suite - and one
of the reasons, why the statistical result is soooo bad for Gecko is, that
obviously all tests fail, that use animateColor - and because it was never a
problem for other viewers I use it in the same amount as animate or
animateTransform to explore the support of animation features.
SVG 1.1 Second edition has deprecated animateColor.

  The use of ‘animateColor’ is deprecated, since all of its functionality can be achieved simply by using ‘animate’ to target properties that can take color values. The ‘animateColor’ element may be dropped from a future version of the SVG specification.
http://www.w3.org/TR/SVG11/animate.html#AnimateColorElement

I'm resolving this as WONTFIX.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
I think, the idea of dpecrecated features is, that implementors should implement
them to avoid problems for authors and authors may avoid them in new documents.

Currently authors have to note for existing SVG, that the audience should not
use Gecko based viewers, because they fail to display the documents correctly -
what is different from most other older viewers with animation capabilities.
For them, there was never a specific problem with animateColor, especially 
because it is much simpler to implement animateColor that animate, because the
possible value range is much more restricted - and indeed, I think, currently
no viewer has a completely correct implementation for animate for fill or stroke, but for animateColor it is much better due to its clever restrictions
on the allowed values.

Because the element exists and is already used for more than 10 years, it is
no meaningful argument, that one can use other methods to achieve the same
effect, just because this method is already used...
If dropped for SVG 2.0, it will be ok of course not to interprete it for 
SVG 2.0 documents. But currently and for the next years, those 2.0 documents
simply do not exist, but there are a lot of SVG 1.1.1 documents around, and
for this variant, the element exists both in the specification and in documents.
I agree with Olaf, deprecating a feature in the spec. is mostly for authors to be aware of and start finding alternatives, IMO not an argument for implementers to avoid implementing a feature, especially when 1) it's apparently simple to implement (AFAIK, it's even already underway) and 2) it will causing unexpected results if/when Gecko states SMIL is fully supported.

For example, I recently stumbled across this during development of a a quick way to determine if SMIL is available or if FakeSmile [1] should be injected to the page (idea is to suggest enhancements for FakeSmile itself later). Somehow surprisingly, currently FakeSmile detection is enabling JavaScript together with the native implementation (useless CPU usage and unexpected jumpy behavior in some cases!), as it relies on the SVG 1.1 feature string [2]. I had to develop further validations, one per each supported element, in order to avoid current Firefox versions from activating the library... :-|

So I'd ask if Gecko will say animation [2] is supported without supporting the animateColor element (which can lead to some broken content) or if feature string detection for animation will never work for Gecko, given the intent of not fixing this.

IMHO, I'd vote for animateColor to be available together with flipping the feature string status, and leaving it's removal for SVG 2.0.

[1] http://leunen.me/fakesmile/
[2] http://www.w3.org/TR/SVG11/feature#SVG-animation
(In reply to Helder "Lthere" Magalhães from comment #37)
> So I'd ask if Gecko will say animation [2] is supported without supporting
> the animateColor element (which can lead to some broken content) or if
> feature string detection for animation will never work for Gecko, given the
> intent of not fixing this.

Polite ping.
(In reply to Robert Longson from comment #39)
> http://longsonr.wordpress.com/2012/07/29/hyperlinking-semantics-for-svg-
> animation/

Thanks for the heads up, Robert! :-)  I've probably missed this in bug mail and/or in the release notes - pace has increased quite a bit... ;-)

Based on the feedback and using Jeff's test [1], apparently Gecko now (checked with Firefox 16) exposes the following feature strings:
* http://www.w3.org/TR/SVG11/feature#Animation (1.1)
* http://www.w3.org/TR/SVG11/feature#SVGDOM-animation (1.1)
* org.w3c.dom.svg.animation (1.0)

Some popular ones aren't currently supported, for example:
* http://www.w3.org/TR/SVG11/feature#SVG-animation (1.1)
* org.w3c.svg.animation (1.0)
Apparently they have additional requirements not yet met. Therefore I'd suggest people to stick with the 'feature#Animation' from the '1.1' feature set (as already hinted by Robert), as I've quickly confirmed it works as expected in all major implementations (OK in Firefox 16, Chrome 22, Safari 5.1, Batik 1.8; NOK IE 9, which doesn't support SMIL). Combine with 'org.w3c.svg.animation' from the '1.0' feature set if you need to support the "ancient" ASV 3/6 (plug-in for older IE versions).

[1] http://www.codedread.com/svgtest.svg
You need to log in before you can comment on or make changes to this bug.