[Webkit-unassigned] [Bug 112327] Reorganize image animation starting

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Apr 18 09:03:38 PDT 2013


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





--- Comment #17 from Nandor Huszka <hnandor at inf.u-szeged.hu>  2013-04-18 09:01:54 PST ---
(In reply to comment #13)
> I'm not sure I agree with this approach in general. It doesn't play nicely with threaded scrolling on Mac, for example; unless we test for restarting image animations while scrolling, we'll not see animated images when the threaded scrolling is happening.

I'm not familiar with threaded scrolling. Do you mean we should take care of image animations in layout tests after scrolling? Please describe this problem with more details.

If this approach is completely no-go, what do you think about the one that Antti mentioned, would that cooperates well with threaded scrolling?

> > Source/WebCore/page/FrameView.cpp:4094
> > +        stoppedAnimationBoundingBox = stoppedAnimation->absoluteBoundingBoxRect();
> 
> absoluteBoundingBoxRect() can be a fairly expensive call. I'm a bit concerned about the performance implications.
> 
> > Source/WebCore/page/FrameView.cpp:4110
> > +        animationToResumeBoundingBox = animationToResume->absoluteBoundingBoxRect();
> 
> Why compute it twice?

It's true, the rect of animations could be stored. I think animations' bounding boxes could be gathered in an other way in order to not to increase run time significantly.

> > Source/WebCore/page/FrameView.cpp:4153
> > +IntRect FrameView::computeScrolledViewPort(float deltaX, float deltaY)
> Isn't this the same as visibleContentRect() or similar?

In FrameView::wheelEvent visibleContentRect gives back the previous viewport at this codepoint. However later (in RenderObject::willRenderImage) we should not only check whether animation is visible now, but whether it will become visible because of scrolling.

(In reply to comment #14)
> The basic idea of not animating on speculative tiles is good. This just needs a good universal and well-performing mechanism for restarting them when they come to view.

The tiled backing store approach works well, the only difficulty with it is we can't access TBS elegantly, just by piercing a lot of layer or by using a hack. Do you think is it rewarding to improve this instead of the current approach?

> I wonder if it would be better at some point to make GIF animations more like other animations (driven by self-repeating timer) rather than doing the restart-timer-on-paint trick? The main benefit of that approach is the automatic stopping and restarting but if we need viewport checks and explicit restarting anyway I'm not sure if it is really helpful.

If we choose this, we could use image's timer to decide whether their animation should step a frame. There could be an AnimationController interface that is implemented by the ImageAnimationController and the old AnimationController (that should be renamed to CSSAnimationController, because it deals with CSS/SVG animation as Simon mentioned in bug 108864).
Am I understand it correctly? A detailed description would give much help to me here.

> > Source/WebCore/page/FrameView.h:383
> > +    bool shouldRenderAnimation(RenderObject*);
> > +    void animateLater(RenderObject*);
> > +    void removeSavedAnimation(RenderObject*);
> 
> FrameView is bloated. It would be better to factor this functionality to a class (ImageAnimationController?)

Okay, I'll move them to a new class.

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