Changeset 214503 in webkit


Ignore:
Timestamp:
Mar 28, 2017, 4:11:35 PM (8 years ago)
Author:
Chris Dumez
Message:

Animated SVG images are not paused when outside viewport
​https://bugs.webkit.org/show_bug.cgi?id=170155
<rdar://problem/31288893>

Reviewed by Antti Koivisto.

Source/WebCore:

Make sure animated SVG images get paused when outside the viewport,
similarly to what was already done for animated GIF images. Also
make sure they are paused when they no longer have any renderers
using them.

Tests: svg/animations/animated-svg-image-outside-viewport-paused.html

svg/animations/animated-svg-image-removed-from-document-paused.html

  • loader/cache/CachedImage.cpp:

(WebCore::CachedImage::didAddClient):
Restart the animation whenever a new CachedImage client is added. This
will cause us the re-evaluate if the animation should run. The animation
will pause again if the new renderer is not inside the viewport.

(WebCore::CachedImage::animationAdvanced):
Add a flag to newImageAnimationFrameAvailable() so that the renderers can
let us know if we can pause the animation. Pause the animation if all no
renderer requires it (i.e. they are all outside the viewport, or there
are no renderers).

  • loader/cache/CachedImageClient.h:

(WebCore::CachedImageClient::newImageAnimationFrameAvailable):
By default, the CachedImageClients allow pausing. Only renderer will
potentially prevent pausing if they are inside the viewport.

  • platform/graphics/BitmapImage.cpp:

(WebCore::BitmapImage::isAnimating):

  • platform/graphics/BitmapImage.h:
  • platform/graphics/Image.h:

(WebCore::Image::isAnimating):
Add isAnimating() flag on Image for layout testing purposes.

  • rendering/RenderElement.cpp:

(WebCore::RenderElement::newImageAnimationFrameAvailable):
Set canPause flag to true if the renderer is not inside the viewport.

(WebCore::RenderElement::repaintForPausedImageAnimationsIfNeeded):
Call startAnimation() if the renderer is now visible to resume SVG
animations. Repainting is enough for GIF animations but not for SVG
animations, we have to explicitly resume them.

  • rendering/RenderElement.h:
  • rendering/RenderView.cpp:

(WebCore::RenderView::addRendererWithPausedImageAnimations):
(WebCore::RenderView::removeRendererWithPausedImageAnimations):
(WebCore::RenderView::resumePausedImageAnimationsIfNeeded):

  • rendering/RenderView.h:

Store CachedImages with the renderers that have paused animations.
This is required for SVG where we need to explicitly resume the
animation on the CachedImage when the renderer becomes visible
again. Having access to the Image will also allow us to do smarter
visibility checks in RenderElement's shouldRepaintForImageAnimation(),
in the future.

  • svg/SVGSVGElement.cpp:

(WebCore::SVGSVGElement::hasActiveAnimation):

  • svg/SVGSVGElement.h:

Add hasActiveAnimation() method.

  • svg/graphics/SVGImage.cpp:

(WebCore::SVGImage::startAnimation):
Check that animations are paused before starting them. This avoid
jumping due to unnecessary calls to rootElement->setCurrentTime(0).

(WebCore::SVGImage::isAnimating):
Add isAnimating() method for layout tests purposes.

  • svg/graphics/SVGImage.h:
  • svg/graphics/SVGImageClients.h:

Call animationAdvanced() on the observer instead of the generic
changedInRect() when the SVGImage is animating. This way, we go
through the same code path as GIF animations and we end up calling
CachedImage::animationAdvanced() which calls newImageAnimationFrameAvailable()
on RenderElement, which determines if the animation should keep
running or not.

  • testing/Internals.cpp:

(WebCore::Internals::isImageAnimating):

  • testing/Internals.h:
  • testing/Internals.idl:

Add layout testing infrastructure.

LayoutTests:

Add layout test coverage.

  • platform/mac-wk1/TestExpectations:
  • svg/animations/animated-svg-image-outside-viewport-paused-expected.txt: Added.
  • svg/animations/animated-svg-image-outside-viewport-paused.html: Added.
  • svg/animations/animated-svg-image-removed-from-document-paused-expected.txt: Added.
  • svg/animations/animated-svg-image-removed-from-document-paused.html: Added.
  • svg/animations/resources/smilAnimation.svg: Added.
Location:
trunk
Files:
5 added
21 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r214501 r214503  
     12017-03-28  Chris Dumez  <cdumez@apple.com>
     2
     3        Animated SVG images are not paused when outside viewport
     4        https://bugs.webkit.org/show_bug.cgi?id=170155
     5        <rdar://problem/31288893>
     6
     7        Reviewed by Antti Koivisto.
     8
     9        Add layout test coverage.
     10
     11        * platform/mac-wk1/TestExpectations:
     12        * svg/animations/animated-svg-image-outside-viewport-paused-expected.txt: Added.
     13        * svg/animations/animated-svg-image-outside-viewport-paused.html: Added.
     14        * svg/animations/animated-svg-image-removed-from-document-paused-expected.txt: Added.
     15        * svg/animations/animated-svg-image-removed-from-document-paused.html: Added.
     16        * svg/animations/resources/smilAnimation.svg: Added.
     17
    1182017-03-28  Antti Koivisto  <antti@apple.com>
    219

  • trunk/LayoutTests/platform/mac-wk1/TestExpectations

    r214423 r214503  
    124124fast/images/animated-gif-window-resizing.html [ Skip ]
    125125fast/images/animated-gif-zooming.html [ Skip ]
     126svg/animations/animated-svg-image-outside-viewport-paused.html [ Skip ]
     127svg/animations/animated-svg-image-removed-from-document-paused.html [ Skip ]
    126128
    127129# WK1 uses the native scrollview for scrolling by page.

  • trunk/Source/WebCore/ChangeLog

    r214501 r214503  
     12017-03-28  Chris Dumez  <cdumez@apple.com>
     2
     3        Animated SVG images are not paused when outside viewport
     4        https://bugs.webkit.org/show_bug.cgi?id=170155
     5        <rdar://problem/31288893>
     6
     7        Reviewed by Antti Koivisto.
     8
     9        Make sure animated SVG images get paused when outside the viewport,
     10        similarly to what was already done for animated GIF images. Also
     11        make sure they are paused when they no longer have any renderers
     12        using them.
     13
     14        Tests: svg/animations/animated-svg-image-outside-viewport-paused.html
     15               svg/animations/animated-svg-image-removed-from-document-paused.html
     16
     17        * loader/cache/CachedImage.cpp:
     18        (WebCore::CachedImage::didAddClient):
     19        Restart the animation whenever a new CachedImage client is added. This
     20        will cause us the re-evaluate if the animation should run. The animation
     21        will pause again if the new renderer is not inside the viewport.
     22
     23        (WebCore::CachedImage::animationAdvanced):
     24        Add a flag to newImageAnimationFrameAvailable() so that the renderers can
     25        let us know if we can pause the animation. Pause the animation if all no
     26        renderer requires it (i.e. they are all outside the viewport, or there
     27        are no renderers).
     28
     29        * loader/cache/CachedImageClient.h:
     30        (WebCore::CachedImageClient::newImageAnimationFrameAvailable):
     31        By default, the CachedImageClients allow pausing. Only renderer will
     32        potentially prevent pausing if they are inside the viewport.
     33
     34        * platform/graphics/BitmapImage.cpp:
     35        (WebCore::BitmapImage::isAnimating):
     36        * platform/graphics/BitmapImage.h:
     37        * platform/graphics/Image.h:
     38        (WebCore::Image::isAnimating):
     39        Add isAnimating() flag on Image for layout testing purposes.
     40
     41        * rendering/RenderElement.cpp:
     42        (WebCore::RenderElement::newImageAnimationFrameAvailable):
     43        Set canPause flag to true if the renderer is not inside the viewport.
     44
     45        (WebCore::RenderElement::repaintForPausedImageAnimationsIfNeeded):
     46        Call startAnimation() if the renderer is now visible to resume SVG
     47        animations. Repainting is enough for GIF animations but not for SVG
     48        animations, we have to explicitly resume them.
     49
     50        * rendering/RenderElement.h:
     51        * rendering/RenderView.cpp:
     52        (WebCore::RenderView::addRendererWithPausedImageAnimations):
     53        (WebCore::RenderView::removeRendererWithPausedImageAnimations):
     54        (WebCore::RenderView::resumePausedImageAnimationsIfNeeded):
     55        * rendering/RenderView.h:
     56        Store CachedImages with the renderers that have paused animations.
     57        This is required for SVG where we need to explicitly resume the
     58        animation on the CachedImage when the renderer becomes visible
     59        again. Having access to the Image will also allow us to do smarter
     60        visibility checks in RenderElement's shouldRepaintForImageAnimation(),
     61        in the future.
     62
     63        * svg/SVGSVGElement.cpp:
     64        (WebCore::SVGSVGElement::hasActiveAnimation):
     65        * svg/SVGSVGElement.h:
     66        Add hasActiveAnimation() method.
     67
     68        * svg/graphics/SVGImage.cpp:
     69        (WebCore::SVGImage::startAnimation):
     70        Check that animations are paused before starting them. This avoid
     71        jumping due to unnecessary calls to rootElement->setCurrentTime(0).
     72
     73        (WebCore::SVGImage::isAnimating):
     74        Add isAnimating() method for layout tests purposes.
     75
     76        * svg/graphics/SVGImage.h:
     77        * svg/graphics/SVGImageClients.h:
     78        Call animationAdvanced() on the observer instead of the generic
     79        changedInRect() when the SVGImage is animating. This way, we go
     80        through the same code path as GIF animations and we end up calling
     81        CachedImage::animationAdvanced() which calls newImageAnimationFrameAvailable()
     82        on RenderElement, which determines if the animation should keep
     83        running or not.
     84
     85        * testing/Internals.cpp:
     86        (WebCore::Internals::isImageAnimating):
     87        * testing/Internals.h:
     88        * testing/Internals.idl:
     89        Add layout testing infrastructure.
     90
    1912017-03-28  Antti Koivisto  <antti@apple.com>
    292

  • trunk/Source/WebCore/loader/cache/CachedImage.cpp

    r210697 r214503  
    118118        static_cast<CachedImageClient&>(client).imageChanged(this);
    119119
     120    if (m_image)
     121        m_image->startAnimation();
     122
    120123    CachedResource::didAddClient(client);
    121124}
    … …  
    515518    if (!image || image != m_image)
    516519        return;
     520
     521    bool shouldPauseAnimation = true;
     522
    517523    CachedResourceClientWalker<CachedImageClient> clientWalker(m_clients);
    518     while (CachedImageClient* client = clientWalker.next())
    519         client->newImageAnimationFrameAvailable(*this);
     524    while (CachedImageClient* client = clientWalker.next()) {
     525        bool canPause = false;
     526        client->newImageAnimationFrameAvailable(*this, canPause);
     527        if (!canPause)
     528            shouldPauseAnimation = false;
     529    }
     530
     531    if (shouldPauseAnimation)
     532        m_image->stopAnimation();
    520533}
    521534

  • trunk/Source/WebCore/loader/cache/CachedImageClient.h

    r208646 r214503  
    4141
    4242    // Called when GIF animation progresses.
    43     virtual void newImageAnimationFrameAvailable(CachedImage& image) { imageChanged(&image); }
     43    virtual void newImageAnimationFrameAvailable(CachedImage& image, bool& canPause) { imageChanged(&image); canPause = true; }
    4444};
    4545

  • trunk/Source/WebCore/platform/graphics/BitmapImage.cpp

    r214450 r214503  
    407407}
    408408
     409bool BitmapImage::isAnimating() const
     410{
     411    return !!m_frameTimer;
     412}
     413
    409414void BitmapImage::stopAnimation()
    410415{

  • trunk/Source/WebCore/platform/graphics/BitmapImage.h

    r214450 r214503  
    170170    void advanceAnimation();
    171171    void internalAdvanceAnimation();
     172    bool isAnimating() const final;
    172173
    173174    // It may look unusual that there is no start animation call as public API. This is because

  • trunk/Source/WebCore/platform/graphics/Image.h

    r214450 r214503  
    132132    virtual void resetAnimation() {}
    133133    virtual void newFrameNativeImageAvailableAtIndex(size_t) { }
     134    virtual bool isAnimating() const { return false; }
    134135   
    135136    // Typically the CachedImage that owns us.

  • trunk/Source/WebCore/rendering/RenderElement.cpp

    r214443 r214503  
    14951495}
    14961496
    1497 void RenderElement::newImageAnimationFrameAvailable(CachedImage& image)
     1497void RenderElement::newImageAnimationFrameAvailable(CachedImage& image, bool& canPause)
    14981498{
    14991499    auto& frameView = view().frameView();
    15001500    auto visibleRect = frameView.windowToContents(frameView.windowClipRect());
    15011501    if (!shouldRepaintForImageAnimation(*this, visibleRect)) {
    1502         // FIXME: It would be better to pass the image along with the renderer
    1503         // so that we can be smarter about detecting if the image is inside the
    1504         // viewport in repaintForPausedImageAnimationsIfNeeded().
    1505         view().addRendererWithPausedImageAnimations(*this);
     1502        view().addRendererWithPausedImageAnimations(*this, image);
     1503        canPause = true;
    15061504        return;
    15071505    }
    … …  
    15091507}
    15101508
    1511 bool RenderElement::repaintForPausedImageAnimationsIfNeeded(const IntRect& visibleRect)
     1509bool RenderElement::repaintForPausedImageAnimationsIfNeeded(const IntRect& visibleRect, CachedImage& cachedImage)
    15121510{
    15131511    ASSERT(m_hasPausedImageAnimations);
    … …  
    15161514
    15171515    repaint();
     1516
     1517    if (auto* image = cachedImage.image())
     1518        image->startAnimation();
    15181519
    15191520    // For directly-composited animated GIFs it does not suffice to call repaint() to resume animation. We need to mark the image as changed.

  • trunk/Source/WebCore/rendering/RenderElement.h

    r214443 r214503  
    196196    virtual void visibleInViewportStateChanged();
    197197
    198     bool repaintForPausedImageAnimationsIfNeeded(const IntRect& visibleRect);
     198    bool repaintForPausedImageAnimationsIfNeeded(const IntRect& visibleRect, CachedImage&);
    199199    bool hasPausedImageAnimations() const { return m_hasPausedImageAnimations; }
    200200    void setHasPausedImageAnimations(bool b) { m_hasPausedImageAnimations = b; }
    … …  
    318318    void invalidateCachedFirstLineStyle();
    319319
    320     void newImageAnimationFrameAvailable(CachedImage&) final;
     320    void newImageAnimationFrameAvailable(CachedImage&, bool& canPause) final;
    321321
    322322    bool getLeadingCorner(FloatPoint& output, bool& insideFixed) const;

  • trunk/Source/WebCore/rendering/RenderView.cpp

    r214443 r214503  
    13961396}
    13971397
    1398 void RenderView::addRendererWithPausedImageAnimations(RenderElement& renderer)
    1399 {
    1400     if (renderer.hasPausedImageAnimations()) {
    1401         ASSERT(m_renderersWithPausedImageAnimation.contains(&renderer));
    1402         return;
    1403     }
     1398void RenderView::addRendererWithPausedImageAnimations(RenderElement& renderer, CachedImage& image)
     1399{
     1400    ASSERT(!renderer.hasPausedImageAnimations() || m_renderersWithPausedImageAnimation.contains(&renderer));
     1401
    14041402    renderer.setHasPausedImageAnimations(true);
    1405     m_renderersWithPausedImageAnimation.add(&renderer);
     1403    auto& images = m_renderersWithPausedImageAnimation.ensure(&renderer, [] {
     1404        return Vector<CachedImage*>();
     1405    }).iterator->value;
     1406    if (!images.contains(&image))
     1407        images.append(&image);
    14061408}
    14071409
    … …  
    14151417}
    14161418
     1419void RenderView::removeRendererWithPausedImageAnimations(RenderElement& renderer, CachedImage& image)
     1420{
     1421    ASSERT(renderer.hasPausedImageAnimations());
     1422
     1423    auto it = m_renderersWithPausedImageAnimation.find(&renderer);
     1424    ASSERT(it != m_renderersWithPausedImageAnimation.end());
     1425
     1426    auto& images = it->value;
     1427    if (!images.contains(&image))
     1428        return;
     1429
     1430    if (images.size() == 1)
     1431        removeRendererWithPausedImageAnimations(renderer);
     1432    else
     1433        images.removeFirst(&image);
     1434}
     1435
    14171436void RenderView::resumePausedImageAnimationsIfNeeded(IntRect visibleRect)
    14181437{
    1419     Vector<RenderElement*, 10> toRemove;
    1420     for (auto* renderer : m_renderersWithPausedImageAnimation) {
    1421         if (renderer->repaintForPausedImageAnimationsIfNeeded(visibleRect))
    1422             toRemove.append(renderer);
    1423     }
    1424     for (auto& renderer : toRemove)
    1425         removeRendererWithPausedImageAnimations(*renderer);
     1438    Vector<std::pair<RenderElement*, CachedImage*>, 10> toRemove;
     1439    for (auto& it : m_renderersWithPausedImageAnimation) {
     1440        auto* renderer = it.key;
     1441        for (auto* image : it.value) {
     1442            if (renderer->repaintForPausedImageAnimationsIfNeeded(visibleRect, *image))
     1443                toRemove.append(std::make_pair(renderer, image));
     1444        }
     1445    }
     1446    for (auto& pair : toRemove)
     1447        removeRendererWithPausedImageAnimations(*pair.first, *pair.second);
    14261448}
    14271449

  • trunk/Source/WebCore/rendering/RenderView.h

    r213897 r214503  
    230230    void unregisterForVisibleInViewportCallback(RenderElement&);
    231231    void resumePausedImageAnimationsIfNeeded(IntRect visibleRect);
    232     void addRendererWithPausedImageAnimations(RenderElement&);
     232    void addRendererWithPausedImageAnimations(RenderElement&, CachedImage&);
    233233    void removeRendererWithPausedImageAnimations(RenderElement&);
     234    void removeRendererWithPausedImageAnimations(RenderElement&, CachedImage&);
    234235
    235236    class RepaintRegionAccumulator {
    … …  
    390391#endif
    391392
    392     HashSet<RenderElement*> m_renderersWithPausedImageAnimation;
     393    HashMap<RenderElement*, Vector<CachedImage*>> m_renderersWithPausedImageAnimation;
    393394    HashSet<RenderElement*> m_visibleInViewportRenderers;
    394395    Vector<RefPtr<RenderWidget>> m_protectedRenderWidgets;

  • trunk/Source/WebCore/rendering/style/StyleCachedImage.cpp

    r210758 r214503  
    3030#include "CachedImage.h"
    3131#include "RenderElement.h"
     32#include "RenderView.h"
    3233
    3334namespace WebCore {
    … …  
    187188        return;
    188189    ASSERT(renderer);
     190
     191    if (renderer->hasPausedImageAnimations())
     192        renderer->view().removeRendererWithPausedImageAnimations(*renderer, *m_cachedImage);
     193
    189194    m_cachedImage->removeClient(*renderer);
    190195}

  • trunk/Source/WebCore/svg/SVGSVGElement.cpp

    r214327 r214503  
    508508}
    509509
     510bool SVGSVGElement::hasActiveAnimation() const
     511{
     512    return m_timeContainer->isActive();
     513}
     514
    510515float SVGSVGElement::getCurrentTime() const
    511516{

  • trunk/Source/WebCore/svg/SVGSVGElement.h

    r208828 r214503  
    8686    void unpauseAnimations();
    8787    bool animationsPaused() const;
     88    bool hasActiveAnimation() const;
    8889
    8990    float getCurrentTime() const;

  • trunk/Source/WebCore/svg/graphics/SVGImage.cpp

    r214450 r214503  
    375375{
    376376    SVGSVGElement* rootElement = this->rootElement();
    377     if (!rootElement)
     377    if (!rootElement || !rootElement->animationsPaused())
    378378        return;
    379379    rootElement->unpauseAnimations();
    … …  
    392392{
    393393    stopAnimation();
     394}
     395
     396bool SVGImage::isAnimating() const
     397{
     398    SVGSVGElement* rootElement = this->rootElement();
     399    if (!rootElement)
     400        return false;
     401    return rootElement->hasActiveAnimation();
    394402}
    395403

  • trunk/Source/WebCore/svg/graphics/SVGImage.h

    r214450 r214503  
    6464    void stopAnimation() final;
    6565    void resetAnimation() final;
     66    bool isAnimating() const final;
    6667
    6768#if USE(CAIRO)

  • trunk/Source/WebCore/svg/graphics/SVGImageClients.h

    r208668 r214503  
    3030
    3131#include "EmptyClients.h"
     32#include "SVGImage.h"
    3233
    3334namespace WebCore {
    … …  
    5354    {
    5455        // If m_image->m_page is null, we're being destructed, don't fire changedInRect() in that case.
    55         if (m_image && m_image->imageObserver() && m_image->m_page)
    56             m_image->imageObserver()->changedInRect(m_image, &r);
     56        if (!m_image || !m_image->m_page)
     57            return;
     58
     59        auto* imageObserver = m_image->imageObserver();
     60        if (!imageObserver)
     61            return;
     62
     63        if (m_image->isAnimating())
     64            imageObserver->animationAdvanced(m_image);
     65        else
     66            imageObserver->changedInRect(m_image, &r);
    5767    }
    5868   

  • trunk/Source/WebCore/testing/Internals.cpp

    r214322 r214503  
    745745}
    746746
     747bool Internals::isImageAnimating(HTMLImageElement& element)
     748{
     749    auto* cachedImage = element.cachedImage();
     750    if (!cachedImage)
     751        return false;
     752
     753    auto* image = cachedImage->image();
     754    if (!image)
     755        return false;
     756
     757    return image->isAnimating();
     758}
     759
    747760void Internals::setClearDecoderAfterAsyncFrameRequestForTesting(HTMLImageElement& element, bool value)
    748761{

  • trunk/Source/WebCore/testing/Internals.h

    r214322 r214503  
    116116    void setImageFrameDecodingDuration(HTMLImageElement&, float duration);
    117117    void resetImageAnimation(HTMLImageElement&);
     118    bool isImageAnimating(HTMLImageElement&);
    118119    void setClearDecoderAfterAsyncFrameRequestForTesting(HTMLImageElement&, bool);
    119120

  • trunk/Source/WebCore/testing/Internals.idl

    r214322 r214503  
    245245    void setImageFrameDecodingDuration(HTMLImageElement element, unrestricted float duration);
    246246    void resetImageAnimation(HTMLImageElement element);
     247    boolean isImageAnimating(HTMLImageElement element);
    247248    void setClearDecoderAfterAsyncFrameRequestForTesting(HTMLImageElement element, boolean value);
    248249
Note: See TracChangeset for help on using the changeset viewer.