Bug 38836 - SMIL Animation does not starts
Summary: SMIL Animation does not starts
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL: http://svg.kvalitne.cz/mix/spirit2.svg
Keywords:
Depends on:
Blocks: 41761
  Show dependency treegraph
 
Reported: 2010-05-10 02:05 PDT by marek
Modified: 2012-06-23 18:42 PDT (History)
4 users (show)

See Also:


Attachments
Two files inside, one working, one not, diff is pretty easy to see... (6.14 KB, application/zip)
2010-05-10 02:05 PDT, marek
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description marek 2010-05-10 02:05:53 PDT
Created attachment 55529 [details]
Two files inside, one working, one not, diff is pretty easy to see...

If there is SMIL animation inside of mask element, it does not not starts at all.

http://svg.kvalitne.cz/mix/spirit2.svg

But by simple adding one more animate element to the document, see

http://svg.kvalitne.cz/mix/spirit.svg

it is working as expected. I consider this to be a bug, since it is working properly in other browsers.
The only difference is presence of one more animate element...
Comment 1 Alexey Proskuryakov 2010-05-10 14:38:55 PDT
Confirmed with r58908.
Comment 2 Robin Cao 2010-06-01 04:50:55 PDT
The root cause of this bug is: the masker is not updated during animation.
This problem can only reproduce with 'animateTransform' and 'animateMotion'. Other animation elements use a different mechanism: 
Everything is routed through 'setAttribute' calls, then the masker gets invalidated in attribute changed callback SVGStyledElement::svgAttributeChanged'.

I have a quick fix for this bug.
Add the following code to the end of SVGAnimateTransformElement::applyResultsToTarget()

    if (targetElement->isStyled())
        static_cast<SVGStyledElement*>(targetElement)->invalidateResourcesInAncestorChain();

It works, but i'm not sure if this is the correct solution.

Dirk, do you have any suggestions?
Comment 3 Dirk Schulze 2010-06-01 05:21:31 PDT
(In reply to comment #2)
> The root cause of this bug is: the masker is not updated during animation.
> This problem can only reproduce with 'animateTransform' and 'animateMotion'. Other animation elements use a different mechanism: 
> Everything is routed through 'setAttribute' calls, then the masker gets invalidated in attribute changed callback SVGStyledElement::svgAttributeChanged'.
> 
> I have a quick fix for this bug.
> Add the following code to the end of SVGAnimateTransformElement::applyResultsToTarget()
> 
>     if (targetElement->isStyled())
>         static_cast<SVGStyledElement*>(targetElement)->invalidateResourcesInAncestorChain();
> 
> It works, but i'm not sure if this is the correct solution.
> 
> Dirk, do you have any suggestions?

Well, isn't it better to do it like other elements (SVGRectElement, SVGGElement, ...)? It would look like this:

     RenderObject* renderer = targetElement->renderer();
     if (targetElement->isStyled()) {
        renderer->setNeedsTransformUpdate();
        renderer->setNeedsLayout(true);
    } else
        renderer->setNeedsLayout(true);

This is untested, but telling the renderer to just update the transform is maybe faster. And it's my opinion, that the code shouldn't handle resources manually here.

Niko may know it better, since he changed layouting lately.
Comment 4 Dirk Schulze 2010-06-01 05:40:30 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > The root cause of this bug is: the masker is not updated during animation.
> > This problem can only reproduce with 'animateTransform' and 'animateMotion'. Other animation elements use a different mechanism: 
> > Everything is routed through 'setAttribute' calls, then the masker gets invalidated in attribute changed callback SVGStyledElement::svgAttributeChanged'.
> > 
> > I have a quick fix for this bug.
> > Add the following code to the end of SVGAnimateTransformElement::applyResultsToTarget()
> > 
> >     if (targetElement->isStyled())
> >         static_cast<SVGStyledElement*>(targetElement)->invalidateResourcesInAncestorChain();
> > 
> > It works, but i'm not sure if this is the correct solution.
> > 
> > Dirk, do you have any suggestions?
> 
> Well, isn't it better to do it like other elements (SVGRectElement, SVGGElement, ...)? It would look like this:
> 
>      RenderObject* renderer = targetElement->renderer();
>      if (targetElement->isStyled()) {
>         renderer->setNeedsTransformUpdate();
>         renderer->setNeedsLayout(true);
>     } else
>         renderer->setNeedsLayout(true);
> 
> This is untested, but telling the renderer to just update the transform is maybe faster. And it's my opinion, that the code shouldn't handle resources manually here.
> 
> Niko may know it better, since he changed layouting lately.

Sorry, looked at SVGAnimateTransformElement again. We're already calling relayouting. It looks like you're right with invalidateResourcesInAncestorChain(), but you also need to call registerFromResources(renderer), see SVGStyledElement::svgAttributeChanged.
I still believe that this should be done in layout() on the renderer, but it's ok to handle it here for the moment.
Comment 5 Philip Rogers 2012-06-23 18:42:07 PDT
This has been fixed. Possibly with the recent SMIL changes.