Attachment 293975 Details for Bug 161566 – Patch

[patch] Patch

bug-161566-20161104191809.patch (text/plain), 15.59 KB, created by Said Abou-Hallawa on 2016-11-04 19:22:12 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Said Abou-Hallawa

Created: 2016-11-04 19:22:12 PDT

Size: 15.59 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 208412)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,39 @@
>+2016-11-04  Said Abou-Hallawa  <sabouhallawa@apple.com>
>+
>+        Change the decoding for some animated images to be asynchronous
>+        https://bugs.webkit.org/show_bug.cgi?id=161566
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Request the next frame before firing the animation timer. The asynchronous
>+        image decoding work queue notifies the BitmapImage when the frame finishes
>+        decoding. If the timer fires before the frame is decoded, no repaint will
>+        be requested. Only when the image frame is ready, the animation will be
>+        advanced and the image will be repainted.
>+
>+        * loader/cache/CachedImage.cpp:
>+        (WebCore::CachedImage::load): Cache the image settings in CachedImage.
>+        (WebCore::CachedImage::createImage): No need to pass allowSubsampling to BitmapImage. It can be retrieved through Image::imageObserver().
>+        * loader/cache/CachedImage.h: Cache the settings: allowSubsampling and allowAsyncImageDecoding through m_loader.
>+        * platform/graphics/BitmapImage.cpp:
>+        (WebCore::BitmapImage::draw): Store the current SubsamplingLevel to be used when requesting decoding the image of the next frame.
>+        (WebCore::BitmapImage::startAnimation): Request asynchronous image decoding for the next frame if required.
>+        (WebCore::BitmapImage::advanceAnimation): Call internalAdvanceAnimation() if the frame image is not being decoded.
>+        (WebCore::BitmapImage::internalAdvanceAnimation): This is the old body of advanceAnimation().
>+        (WebCore::BitmapImage::stopAnimation): Stop the asynchronous image decoding if it is started.
>+        (WebCore::BitmapImage::newFrameNativeImageAvailableAtIndex): This function is called from the async image decoding work queue when finishing decoding a native image frame.
>+        * platform/graphics/BitmapImage.h: 
>+        * platform/graphics/ImageFrameCache.cpp:
>+        (WebCore::ImageFrameCache::clearMetadata): Delete unreferenced member.
>+        * platform/graphics/ImageFrameCache.h: Ditto.
>+        * platform/graphics/ImageObserver.h: Add virtual functions for allowSubsampling and allowAsyncImageDecoding.
>+        * platform/graphics/ImageSource.cpp:
>+        (WebCore::ImageSource::maximumSubsamplingLevel): Move checking allowSubsampling() to the caller BitmapImage::draw().
>+        * platform/graphics/ImageSource.h: Remove the setting allowSubsampling(); it can be retrieved from imageObserver().
>+        (WebCore::ImageSource::setAllowSubsampling): Deleted.
>+        * rendering/RenderImage.cpp:
>+        (WebCore::RenderImage::~RenderImage): Stop the animation of an image when deleting its renderer.
>+
> 2016-11-04  Yusuke Suzuki  <utatane.tea@gmail.com>
> 
>         [DOMJIT] Add DOMJIT::Signature annotation to Document::getElementById
>Index: Source/WebCore/loader/cache/CachedImage.cpp
>===================================================================
>--- Source/WebCore/loader/cache/CachedImage.cpp	(revision 208401)
>+++ Source/WebCore/loader/cache/CachedImage.cpp	(working copy)
>@@ -89,6 +89,11 @@ void CachedImage::load(CachedResourceLoa
>         CachedResource::load(loader);
>     else
>         setLoading(false);
>+
>+    if (m_loader) {
>+        m_allowSubsampling = m_loader->frameLoader()->frame().settings().imageSubsamplingEnabled();
>+        m_allowAsyncImageDecoding = m_loader->frameLoader()->frame().settings().asyncImageDecodingEnabled();
>+    }
> }
> 
> void CachedImage::setBodyDataFrom(const CachedResource& resource)
>@@ -325,10 +330,8 @@ inline void CachedImage::createImage()
>         auto svgImage = SVGImage::create(*this, url());
>         m_svgImageCache = std::make_unique<SVGImageCache>(svgImage.ptr());
>         m_image = WTFMove(svgImage);
>-    } else {
>+    } else
>         m_image = BitmapImage::create(this);
>-        downcast<BitmapImage>(*m_image).setAllowSubsampling(m_loader && m_loader->frameLoader()->frame().settings().imageSubsamplingEnabled());
>-    }
> 
>     if (m_image) {
>         // Send queued container size requests.
>Index: Source/WebCore/loader/cache/CachedImage.h
>===================================================================
>--- Source/WebCore/loader/cache/CachedImage.h	(revision 208401)
>+++ Source/WebCore/loader/cache/CachedImage.h	(working copy)
>@@ -118,6 +118,8 @@ private:
>     bool stillNeedsLoad() const override { return !errorOccurred() && status() == Unknown && !isLoading(); }
> 
>     // ImageObserver
>+    bool allowSubsampling() const override { return m_allowSubsampling; }
>+    bool allowAsyncImageDecoding() const override { return m_allowAsyncImageDecoding; }
>     void decodedSizeChanged(const Image*, long long delta) override;
>     void didDraw(const Image*) override;
> 
>@@ -136,6 +138,14 @@ private:
>     std::unique_ptr<SVGImageCache> m_svgImageCache;
>     bool m_isManuallyCached { false };
>     bool m_shouldPaintBrokenImage { true };
>+
>+    // The default value of m_allowSubsampling should be the same as defaultImageSubsamplingEnabled in Settings.cpp
>+#if PLATFORM(IOS)
>+    bool m_allowSubsampling { true };
>+#else
>+    bool m_allowSubsampling { false };
>+#endif
>+    bool m_allowAsyncImageDecoding { true };
> };
> 
> } // namespace WebCore
>Index: Source/WebCore/platform/graphics/BitmapImage.cpp
>===================================================================
>--- Source/WebCore/platform/graphics/BitmapImage.cpp	(revision 208401)
>+++ Source/WebCore/platform/graphics/BitmapImage.cpp	(working copy)
>@@ -150,10 +150,10 @@ void BitmapImage::draw(GraphicsContext& 
>     }
> 
>     float scale = subsamplingScale(context, destRect, srcRect);
>-    SubsamplingLevel subsamplingLevel = m_source.subsamplingLevelForScale(scale);
>-    LOG(Images, "BitmapImage %p draw - subsamplingLevel %d at scale %.4f", this, static_cast<int>(subsamplingLevel), scale);
>+    m_currentSubsamplingLevel = allowSubsampling() ? m_source.subsamplingLevelForScale(scale) : SubsamplingLevel::Default;
>+    LOG(Images, "BitmapImage %p draw - subsamplingLevel %d at scale %.4f", this, static_cast<int>(m_currentSubsamplingLevel), scale);
> 
>-    auto image = frameImageAtIndex(m_currentFrame, subsamplingLevel, &context);
>+    auto image = frameImageAtIndex(m_currentFrame, m_currentSubsamplingLevel, &context);
>     if (!image) // If it's too early we won't have an image yet.
>         return;
> 
>@@ -228,6 +228,11 @@ void BitmapImage::startAnimation()
>     if (m_frameTimer || !shouldAnimate() || frameCount() <= 1)
>         return;
> 
>+    // Don't start a new animation until we draw the frame that is currently being decoded.
>+    size_t nextFrame = (m_currentFrame + 1) % frameCount();
>+    if (frameIsBeingDecodedAtIndex(nextFrame))
>+        return;
>+
>     if (m_currentFrame >= frameCount() - 1) {
>         // Don't advance past the last frame if we haven't decoded the whole image
>         // yet and our repetition count is potentially unset. The repetition count
>@@ -249,7 +254,6 @@ void BitmapImage::startAnimation()
>     }
> 
>     // Don't advance the animation to an incomplete frame.
>-    size_t nextFrame = (m_currentFrame + 1) % frameCount();
>     if (!m_source.isAllDataReceived() && !frameIsCompleteAtIndex(nextFrame))
>         return;
> 
>@@ -262,6 +266,9 @@ void BitmapImage::startAnimation()
>     // Setting 'm_desiredFrameStartTime' to 'time' means we are late; otherwise we are early.
>     m_desiredFrameStartTime = std::max(time, m_desiredFrameStartTime + frameDurationAtIndex(m_currentFrame));
> 
>+    if (allowAsyncImageDecoding() && m_source.isAsyncDecodingRequired())
>+        m_source.requestFrameAsyncDecodingAtIndex(nextFrame, m_currentSubsamplingLevel);
>+
>     ASSERT(!m_frameTimer);
>     startTimer(m_desiredFrameStartTime - time);
> }
>@@ -270,7 +277,16 @@ void BitmapImage::advanceAnimation()
> {
>     clearTimer();
> 
>+    size_t nextFrame = (m_currentFrame + 1) % frameCount();
>+    if (!frameIsBeingDecodedAtIndex(nextFrame))
>+        internalAdvanceAnimation();
>+}
>+
>+void BitmapImage::internalAdvanceAnimation()
>+{
>     m_currentFrame = (m_currentFrame + 1) % frameCount();
>+    ASSERT(!frameIsBeingDecodedAtIndex(m_currentFrame));
>+
>     destroyDecodedDataIfNecessary(false);
> 
>     if (imageObserver())
>@@ -282,6 +298,7 @@ void BitmapImage::stopAnimation()
>     // This timer is used to animate all occurrences of this image. Don't invalidate
>     // the timer unless all renderers have stopped drawing.
>     clearTimer();
>+    m_source.stopAsyncDecodingQueue();
> }
> 
> void BitmapImage::resetAnimation()
>@@ -296,6 +313,15 @@ void BitmapImage::resetAnimation()
>     destroyDecodedDataIfNecessary(true);
> }
> 
>+void BitmapImage::newFrameNativeImageAvailableAtIndex(size_t index)
>+{
>+    UNUSED_PARAM(index);
>+    ASSERT(index == (m_currentFrame + 1) % frameCount());
>+
>+    if (canAnimate() && !m_frameTimer)
>+        internalAdvanceAnimation();
>+}
>+
> void BitmapImage::dump(TextStream& ts) const
> {
>     Image::dump(ts);
>Index: Source/WebCore/platform/graphics/BitmapImage.h
>===================================================================
>--- Source/WebCore/platform/graphics/BitmapImage.h	(revision 208401)
>+++ Source/WebCore/platform/graphics/BitmapImage.h	(working copy)
>@@ -29,6 +29,7 @@
> 
> #include "Image.h"
> #include "Color.h"
>+#include "ImageObserver.h"
> #include "ImageOrientation.h"
> #include "ImageSource.h"
> #include "IntSize.h"
>@@ -80,8 +81,6 @@ public:
>     IntSize sizeRespectingOrientation() const { return m_source.sizeRespectingOrientation(); }
>     Color singlePixelSolidColor() const override { return m_source.singlePixelSolidColor(); }
> 
>-    void setAllowSubsampling(bool allowSubsampling) { m_source.setAllowSubsampling(allowSubsampling); }
>-
>     bool frameIsBeingDecodedAtIndex(size_t index) const { return m_source.frameIsBeingDecodedAtIndex(index); }
>     bool frameIsCompleteAtIndex(size_t index) const { return m_source.frameIsCompleteAtIndex(index); }
>     bool frameHasAlphaAtIndex(size_t index) const { return m_source.frameHasAlphaAtIndex(index); }
>@@ -131,6 +130,9 @@ protected:
> 
>     NativeImagePtr frameImageAtIndex(size_t, SubsamplingLevel = SubsamplingLevel::Default, const GraphicsContext* = nullptr);
> 
>+    bool allowSubsampling() const { return imageObserver() && imageObserver()->allowSubsampling(); }
>+    bool allowAsyncImageDecoding() const { return imageObserver() && imageObserver()->allowAsyncImageDecoding(); }
>+
>     // Called to invalidate cached data. When |destroyAll| is true, we wipe out
>     // the entire frame buffer cache and tell the image source to destroy
>     // everything; this is used when e.g. we want to free some room in the image
>@@ -155,12 +157,14 @@ protected:
>     bool canAnimate();
>     void startAnimation() override;
>     void advanceAnimation();
>+    void internalAdvanceAnimation();
> 
>     // It may look unusual that there is no start animation call as public API. This is because
>     // we start and stop animating lazily. Animation begins whenever someone draws the image. It will
>     // automatically pause once all observers no longer want to render the image anywhere.
>     void stopAnimation() override;
>     void resetAnimation() override;
>+    void newFrameNativeImageAvailableAtIndex(size_t) override;
> 
>     // Handle platform-specific data
>     void invalidatePlatformData();
>@@ -182,6 +186,7 @@ private:
>     mutable ImageSource m_source;
> 
>     size_t m_currentFrame { 0 }; // The index of the current frame of animation.
>+    SubsamplingLevel m_currentSubsamplingLevel { SubsamplingLevel::Default };
>     std::unique_ptr<Timer> m_frameTimer;
>     RepetitionCount m_repetitionsComplete { RepetitionCountNone }; // How many repetitions we've finished.
>     double m_desiredFrameStartTime { 0 }; // The system time at which we hope to see the next call to startAnimation().
>Index: Source/WebCore/platform/graphics/ImageFrameCache.cpp
>===================================================================
>--- Source/WebCore/platform/graphics/ImageFrameCache.cpp	(revision 208401)
>+++ Source/WebCore/platform/graphics/ImageFrameCache.cpp	(working copy)
>@@ -336,7 +336,6 @@ void ImageFrameCache::clearMetadata()
> {
>     m_frameCount = Nullopt;
>     m_singlePixelSolidColor = Nullopt;
>-    m_maximumSubsamplingLevel = Nullopt;
> }
> 
> template<typename T, T (ImageDecoder::*functor)() const>
>Index: Source/WebCore/platform/graphics/ImageFrameCache.h
>===================================================================
>--- Source/WebCore/platform/graphics/ImageFrameCache.h	(revision 208401)
>+++ Source/WebCore/platform/graphics/ImageFrameCache.h	(working copy)
>@@ -162,7 +162,6 @@ private:
>     // Image metadata which is calculated from the first ImageFrame.
>     Optional<IntSize> m_size;
>     Optional<IntSize> m_sizeRespectingOrientation;
>-    Optional<SubsamplingLevel> m_maximumSubsamplingLevel;
>     Optional<Color> m_singlePixelSolidColor;
> };
>     
>Index: Source/WebCore/platform/graphics/ImageObserver.h
>===================================================================
>--- Source/WebCore/platform/graphics/ImageObserver.h	(revision 208401)
>+++ Source/WebCore/platform/graphics/ImageObserver.h	(working copy)
>@@ -37,7 +37,10 @@ class ImageObserver {
> protected:
>     virtual ~ImageObserver() {}
> public:
>+    virtual bool allowSubsampling() const = 0;
>+    virtual bool allowAsyncImageDecoding() const = 0;
>     virtual void decodedSizeChanged(const Image*, long long delta) = 0;
>+
>     virtual void didDraw(const Image*) = 0;
> 
>     virtual void animationAdvanced(const Image*) = 0;
>Index: Source/WebCore/platform/graphics/ImageSource.cpp
>===================================================================
>--- Source/WebCore/platform/graphics/ImageSource.cpp	(revision 208401)
>+++ Source/WebCore/platform/graphics/ImageSource.cpp	(working copy)
>@@ -186,7 +186,7 @@ SubsamplingLevel ImageSource::maximumSub
>     if (m_maximumSubsamplingLevel)
>         return m_maximumSubsamplingLevel.value();
> 
>-    if (!m_allowSubsampling || !isDecoderAvailable() || !m_decoder->frameAllowSubsamplingAtIndex(0))
>+    if (!isDecoderAvailable() || !m_decoder->frameAllowSubsamplingAtIndex(0))
>         return SubsamplingLevel::Default;
> 
>     // FIXME: this value was chosen to be appropriate for iOS since the image
>Index: Source/WebCore/platform/graphics/ImageSource.h
>===================================================================
>--- Source/WebCore/platform/graphics/ImageSource.h	(revision 208401)
>+++ Source/WebCore/platform/graphics/ImageSource.h	(working copy)
>@@ -98,7 +98,6 @@ public:
> 
>     SubsamplingLevel maximumSubsamplingLevel();
>     SubsamplingLevel subsamplingLevelForScale(float);
>-    void setAllowSubsampling(bool allowSubsampling) { m_allowSubsampling = allowSubsampling; }
>     NativeImagePtr createFrameImageAtIndex(size_t, SubsamplingLevel = SubsamplingLevel::Default);
> 
> private:
>@@ -113,13 +112,6 @@ private:
> 
>     Optional<SubsamplingLevel> m_maximumSubsamplingLevel;
> 
>-    // The default value of m_allowSubsampling should be the same as defaultImageSubsamplingEnabled in Settings.cpp
>-#if PLATFORM(IOS)
>-    bool m_allowSubsampling { true };
>-#else
>-    bool m_allowSubsampling { false };
>-#endif
>-
> #if PLATFORM(IOS)
>     // FIXME: We should expose a setting to enable/disable progressive loading so that we can remove the PLATFORM(IOS)-guard.
>     double m_progressiveLoadChunkTime { 0 };
>Index: Source/WebCore/rendering/RenderImage.cpp
>===================================================================
>--- Source/WebCore/rendering/RenderImage.cpp	(revision 208401)
>+++ Source/WebCore/rendering/RenderImage.cpp	(working copy)
>@@ -143,6 +143,9 @@ RenderImage::RenderImage(Document& docum
> 
> RenderImage::~RenderImage()
> {
>+    auto* image = imageResource().image().get();
>+    if (image && !image->isNull())
>+        image->stopAnimation();
>     imageResource().shutdown();
> }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 161566: 287852 | 287855 | 287856 | 287920 | 287921 | 287991 | 288078 | 288144 | 288145 | 288158 | 291739 | 291794 | 291828 | 293975 | 294047 | 294088 | 294091 | 294092 | 294096 | 294120 | 294170 | 294190 | 294208 | 294272