Changeset 215900 in webkit

Timestamp:

Apr 27, 2017 3:57:32 PM (7 years ago)

Author:

commit-queue@webkit.org

Message:

REGRESSION(r213764): Async decoding of animated images is disabled for ImageDocument
​https://bugs.webkit.org/show_bug.cgi?id=170333

Patch by Said Abou-Hallawa <​sabouhallawa@apple.com> on 2017-04-27
Reviewed by Simon Fraser.

Source/WebCore:

The way the image drawing settings are transfered from the Settings to
BitmapImage is problematic. The drawing settings are retrieved from the
CachedImageObserver which store them in the constructor only if the
CachedImage as a loader. In the case of ImageDocument, there isn't loader
set in CachedImage so the settings of ImageDocument are not set. Also
the CachedImage can be used after loading by another document which may
have a different drawing settings.

The fix is to make BitmapImage reads the drawing settings every time it
is drawn as a foreground or background image in a RenderElement.

• html/canvas/CanvasRenderingContext2D.cpp:

(WebCore::CanvasRenderingContext2D::drawImage):

• loader/cache/CachedImage.cpp:

(WebCore::CachedImage::CachedImageObserver::CachedImageObserver):

• loader/cache/CachedImage.h:
• platform/graphics/BitmapImage.cpp:

(WebCore::BitmapImage::setDrawingSettings):
(WebCore::BitmapImage::draw):
(WebCore::BitmapImage::shouldUseAsyncDecodingForLargeImages): I was
trying to disable the async image decoding temporarily but this way will
even prevent testing it until it is enabled. Disable it through WK1 and
WK2 preferences.
(WebCore::BitmapImage::shouldUseAsyncDecodingForAnimatedImages):
(WebCore::BitmapImage::advanceAnimation):

• platform/graphics/BitmapImage.h:
• platform/graphics/ImageObserver.h:
• rendering/RenderBoxModelObject.cpp:

(WebCore::RenderBoxModelObject::paintFillLayerExtended):

• rendering/RenderImage.cpp:

(WebCore::RenderImage::paintIntoRect):

Source/WebKit/mac:

Disbale the async decoding for large images for now.

• WebView/WebView.mm:

(-[WebView _preferencesChanged:]):

Source/WebKit2:

Disbale the async decoding for large images for now.

• WebProcess/WebPage/WebPage.cpp:

(WebKit::WebPage::updatePreferences):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/html/canvas/CanvasRenderingContext2D.cpp (modified) (1 diff)
• WebCore/loader/cache/CachedImage.cpp (modified) (1 diff)
• WebCore/loader/cache/CachedImage.h (modified) (2 diffs)
• WebCore/platform/graphics/BitmapImage.cpp (modified) (8 diffs)
• WebCore/platform/graphics/BitmapImage.h (modified) (4 diffs)
• WebCore/platform/graphics/ImageObserver.h (modified) (2 diffs)
• WebCore/rendering/RenderBoxModelObject.cpp (modified) (2 diffs)
• WebCore/rendering/RenderImage.cpp (modified) (1 diff)
• WebKit/mac/ChangeLog (modified) (1 diff)
• WebKit/mac/WebView/WebView.mm (modified) (1 diff)
• WebKit2/ChangeLog (modified) (1 diff)
• WebKit2/WebProcess/WebPage/WebPage.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-04-27  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        REGRESSION(r213764): Async decoding of animated images is disabled for ImageDocument
+        https://bugs.webkit.org/show_bug.cgi?id=170333
+
+        Reviewed by Simon Fraser.
+
+        The way the image drawing settings are transfered from the Settings to
+        BitmapImage is problematic. The drawing settings are retrieved from the
+        CachedImageObserver which store them in the constructor only if the
+        CachedImage as a loader. In the case of ImageDocument, there isn't loader
+        set in CachedImage so the settings of ImageDocument are not set. Also
+        the CachedImage can be used after loading by another document which may
+        have a different drawing settings.
+
+        The fix is to make BitmapImage reads the drawing settings every time it 
+        is drawn as a foreground or background image in a RenderElement.
+
+        * html/canvas/CanvasRenderingContext2D.cpp:
+        (WebCore::CanvasRenderingContext2D::drawImage):
+        * loader/cache/CachedImage.cpp:
+        (WebCore::CachedImage::CachedImageObserver::CachedImageObserver):
+        * loader/cache/CachedImage.h:
+        * platform/graphics/BitmapImage.cpp:
+        (WebCore::BitmapImage::setDrawingSettings):
+        (WebCore::BitmapImage::draw):
+        (WebCore::BitmapImage::shouldUseAsyncDecodingForLargeImages): I was
+        trying to disable the async image decoding temporarily but this way will
+        even prevent testing it until it is enabled. Disable it through WK1 and
+        WK2 preferences.
+        (WebCore::BitmapImage::shouldUseAsyncDecodingForAnimatedImages):
+        (WebCore::BitmapImage::advanceAnimation):
+        * platform/graphics/BitmapImage.h:
+        * platform/graphics/ImageObserver.h:
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::paintFillLayerExtended):
+        * rendering/RenderImage.cpp:
+        (WebCore::RenderImage::paintIntoRect):
+
 2017-04-27  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp

@@ -1432 +1432 @@
     }
 
+    if (image->isBitmapImage())
+        downcast<BitmapImage>(*image).updateFromSettings(imageElement.document().settings());
+
     if (rectContainsCanvas(normalizedDstRect)) {
         c->drawImage(*image, normalizedDstRect, normalizedSrcRect, ImagePaintingOptions(op, blendMode));

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

@@ -353 +353 @@
     m_cachedImages.reserveInitialCapacity(1);
     m_cachedImages.append(&image);
-    if (auto* loader = image.loader()) {
-        m_allowSubsampling = loader->frameLoader()->frame().settings().imageSubsamplingEnabled();
-        m_allowLargeImageAsyncDecoding = loader->frameLoader()->frame().settings().largeImageAsyncDecodingEnabled();
-        m_allowAnimatedImageAsyncDecoding = loader->frameLoader()->frame().settings().animatedImageAsyncDecodingEnabled();
-        m_showDebugBackground = loader->frameLoader()->frame().settings().showDebugBorders();
-    }
 }
 

trunk/Source/WebCore/loader/cache/CachedImage.h

@@ -129 +129 @@
         // ImageObserver API
         URL sourceUrl() const override { return m_cachedImages[0]->url(); }
-        bool allowSubsampling() const final { return m_allowSubsampling; }
-        bool allowLargeImageAsyncDecoding() const override { return m_allowLargeImageAsyncDecoding; }
-        bool allowAnimatedImageAsyncDecoding() const override { return m_allowAnimatedImageAsyncDecoding; }
-        bool showDebugBackground() const final { return m_showDebugBackground; }
         void decodedSizeChanged(const Image*, long long delta) final;
         void didDraw(const Image*) final;
@@ -140 +136 @@
 
         Vector<CachedImage*> m_cachedImages;
-        // 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_allowLargeImageAsyncDecoding { false };
-        bool m_allowAnimatedImageAsyncDecoding { false };
-        bool m_showDebugBackground { false };
     };
 

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

@@ -34 +34 @@
 #include "IntRect.h"
 #include "Logging.h"
+#include "Settings.h"
 #include "TextStream.h"
 #include "Timer.h"
@@ -62 +63 @@
     invalidatePlatformData();
     stopAnimation();
+}
+
+void BitmapImage::updateFromSettings(const Settings& settings)
+{
+    m_allowSubsampling = settings.imageSubsamplingEnabled();
+    m_allowLargeImageAsyncDecoding = settings.largeImageAsyncDecodingEnabled();
+    m_allowAnimatedImageAsyncDecoding = settings.animatedImageAsyncDecodingEnabled();
+    m_showDebugBackground = settings.showDebugBorders();
 }
 
@@ -167 +176 @@
     IntSize sizeForDrawing = expandedIntSize(size() * scaleFactorForDrawing);
     
-    m_currentSubsamplingLevel = allowSubsampling() ? m_source.subsamplingLevelForScaleFactor(context, scaleFactorForDrawing) : SubsamplingLevel::Default;
+    m_currentSubsamplingLevel = m_allowSubsampling ? m_source.subsamplingLevelForScaleFactor(context, scaleFactorForDrawing) : SubsamplingLevel::Default;
     LOG(Images, "BitmapImage::%s - %p - url: %s [subsamplingLevel = %d scaleFactorForDrawing = (%.4f, %.4f)]", __FUNCTION__, this, sourceURL().string().utf8().data(), static_cast<int>(m_currentSubsamplingLevel), scaleFactorForDrawing.width(), scaleFactorForDrawing.height());
 
@@ -181 +190 @@
 
         if (!frameHasDecodedNativeImageCompatibleWithOptionsAtIndex(m_currentFrame, m_currentSubsamplingLevel, DecodingMode::Asynchronous)) {
-            if (showDebugBackground())
+            if (m_showDebugBackground)
                 fillWithSolidColor(context, destRect, Color(Color::yellow).colorWithAlpha(0.5), op);
             return;
@@ -192 +201 @@
         ASSERT_IMPLIES(status == StartAnimationStatus::DecodingActive, frameHasFullSizeNativeImageAtIndex(m_currentFrame, m_currentSubsamplingLevel));
 
-        if (status == StartAnimationStatus::DecodingActive && showDebugBackground()) {
+        if (status == StartAnimationStatus::DecodingActive && m_showDebugBackground) {
             fillWithSolidColor(context, destRect, Color(Color::yellow).colorWithAlpha(0.5), op);
             return;
@@ -199 +208 @@
         if (frameIsBeingDecodedAndIsCompatibleWithOptionsAtIndex(m_currentFrame, DecodingMode::Asynchronous)) {
             // FIXME: instead of showing the yellow rectangle and returning we need to wait for this the frame to finish decoding.
-            if (showDebugBackground()) {
+            if (m_showDebugBackground) {
                 fillWithSolidColor(context, destRect, Color(Color::yellow).colorWithAlpha(0.5), op);
                 LOG(Images, "BitmapImage::%s - %p - url: %s [waiting for async decoding to finish]", __FUNCTION__, this, sourceURL().string().utf8().data());
@@ -274 +283 @@
 bool BitmapImage::shouldUseAsyncDecodingForLargeImages()
 {
-    // FIXME: enable async image decoding after the flickering bug wk170640 is fixed.
-    // return !canAnimate() && allowLargeImageAsyncDecoding() && m_source.shouldUseAsyncDecoding();
-    return false;
+    return !canAnimate() && m_allowLargeImageAsyncDecoding && m_source.shouldUseAsyncDecoding();
 }
 
 bool BitmapImage::shouldUseAsyncDecodingForAnimatedImages()
 {
-    return canAnimate() && allowAnimatedImageAsyncDecoding() && (shouldUseAsyncDecodingForAnimatedImagesForTesting() || m_source.shouldUseAsyncDecoding());
+    return canAnimate() && m_allowAnimatedImageAsyncDecoding && (shouldUseAsyncDecodingForAnimatedImagesForTesting() || m_source.shouldUseAsyncDecoding());
 }
 
@@ -391 +398 @@
     else {
         // Force repaint if showDebugBackground() is on.
-        if (showDebugBackground())
+        if (m_showDebugBackground)
             imageObserver()->changedInRect(this);
         LOG(Images, "BitmapImage::%s - %p - url: %s [lateFrameCount = %ld nextFrame = %ld]", __FUNCTION__, this, sourceURL().string().utf8().data(), ++m_lateFrameCount, nextFrame);

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

@@ -50 +50 @@
 namespace WebCore {
 
+class Settings;
 class Timer;
 
@@ -66 +67 @@
 #endif
     virtual ~BitmapImage();
-    
+
+    void updateFromSettings(const Settings&);
+
     bool hasSingleSecurityOrigin() const override { return true; }
 
@@ -137 +140 @@
     NativeImagePtr frameImageAtIndex(size_t index) { return m_source.frameImageAtIndex(index); }
     NativeImagePtr frameImageAtIndexCacheIfNeeded(size_t, SubsamplingLevel = SubsamplingLevel::Default, const GraphicsContext* = nullptr);
-
-    bool allowSubsampling() const { return imageObserver() && imageObserver()->allowSubsampling(); }
-    bool allowLargeImageAsyncDecoding() const { return imageObserver() && imageObserver()->allowLargeImageAsyncDecoding(); }
-    bool allowAnimatedImageAsyncDecoding() const { return imageObserver() && imageObserver()->allowAnimatedImageAsyncDecoding(); }
-    bool showDebugBackground() const { return imageObserver() && imageObserver()->showDebugBackground(); }
 
     // Called to invalidate cached data. When |destroyAll| is true, we wipe out
@@ -206 +204 @@
     RepetitionCount m_repetitionsComplete { RepetitionCountNone }; // How many repetitions we've finished.
     MonotonicTime m_desiredFrameStartTime; // The system time at which we hope to see the next call to startAnimation().
-    bool m_animationFinished { false };
 
     Seconds m_frameDecodingDurationForTesting;
     MonotonicTime m_desiredFrameDecodeTimeForTesting;
+
+    bool m_animationFinished { false };
+
+    // 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_allowLargeImageAsyncDecoding { false };
+    bool m_allowAnimatedImageAsyncDecoding { false };
+    bool m_showDebugBackground { false };
+
     bool m_clearDecoderAfterAsyncFrameRequestForTesting { false };
+
 #if !LOG_DISABLED
     size_t m_lateFrameCount { 0 };

trunk/Source/WebCore/platform/graphics/ImageObserver.h

@@ -40 +40 @@
 public:
     virtual URL sourceUrl() const = 0;
-    virtual bool allowSubsampling() const = 0;
-    virtual bool allowLargeImageAsyncDecoding() const = 0;
-    virtual bool allowAnimatedImageAsyncDecoding() const = 0;
-    virtual bool showDebugBackground() const = 0;
     virtual void decodedSizeChanged(const Image*, long long delta) = 0;
 
@@ -49 +45 @@
 
     virtual void animationAdvanced(const Image*) = 0;
-
     virtual void changedInRect(const Image*, const IntRect* changeRect = nullptr) = 0;
 };

trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp

@@ -27 +27 @@
 #include "RenderBoxModelObject.h"
 
+#include "BitmapImage.h"
 #include "BorderEdge.h"
 #include "FloatRoundedRect.h"
@@ -878 +879 @@
             context.setDrawLuminanceMask(bgLayer.maskSourceType() == MaskLuminance);
 
+            if (is<BitmapImage>(image.get()))
+                downcast<BitmapImage>(*image).updateFromSettings(settings());
+
             auto interpolation = chooseInterpolationQuality(context, *image, &bgLayer, geometry.tileSize());
             context.drawTiledImage(*image, geometry.destRect(), toLayoutPoint(geometry.relativePhase()), geometry.tileSize(), geometry.spaceSize(), ImagePaintingOptions(compositeOp, bgLayer.blendMode(), DecodingMode::Asynchronous, ImageOrientationDescription(), interpolation));

trunk/Source/WebCore/rendering/RenderImage.cpp

@@ -581 +581 @@
 #endif
 
+    if (is<BitmapImage>(image))
+        downcast<BitmapImage>(*image).updateFromSettings(settings());
+
     ImageOrientationDescription orientationDescription(shouldRespectImageOrientation(), style().imageOrientation());
     context.drawImage(*img, rect, ImagePaintingOptions(compositeOperator, BlendModeNormal, DecodingMode::Asynchronous, orientationDescription, interpolation));

trunk/Source/WebKit/mac/ChangeLog

@@ +1 @@
+2017-04-27  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        REGRESSION(r213764): Async decoding of animated images is disabled for ImageDocument
+        https://bugs.webkit.org/show_bug.cgi?id=170333
+
+        Reviewed by Simon Fraser.
+
+        Disbale the async decoding for large images for now.
+
+        * WebView/WebView.mm:
+        (-[WebView _preferencesChanged:]):
+
 2017-04-27  Alex Christensen  <achristensen@webkit.org>
 

trunk/Source/WebKit/mac/WebView/WebView.mm

@@ -3085 +3085 @@
     settings.setShouldConvertInvalidURLsToBlank(shouldConvertInvalidURLsToBlank());
 
-    settings.setLargeImageAsyncDecodingEnabled([preferences largeImageAsyncDecodingEnabled]);
+    // FIXME: enable async image decoding after the flickering bug wk170640 is fixed.
+    // settings.setLargeImageAsyncDecodingEnabled([preferences largeImageAsyncDecodingEnabled]);
+    settings.setLargeImageAsyncDecodingEnabled(false);
     settings.setAnimatedImageAsyncDecodingEnabled([preferences animatedImageAsyncDecodingEnabled]);
 }

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2017-04-27  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        REGRESSION(r213764): Async decoding of animated images is disabled for ImageDocument
+        https://bugs.webkit.org/show_bug.cgi?id=170333
+
+        Reviewed by Simon Fraser.
+
+        Disbale the async decoding for large images for now.
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::updatePreferences):
+
 2017-04-27  Brent Fulgham  <bfulgham@apple.com>
 

trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp

@@ -3365 +3365 @@
     setForceAlwaysUserScalable(m_forceAlwaysUserScalable || store.getBoolValueForKey(WebPreferencesKey::forceAlwaysUserScalableKey()));
 #endif
-    settings.setLargeImageAsyncDecodingEnabled(store.getBoolValueForKey(WebPreferencesKey::largeImageAsyncDecodingEnabledKey()));
+    // FIXME: enable async image decoding after the flickering bug wk170640 is fixed.
+    // settings.setLargeImageAsyncDecodingEnabled(store.getBoolValueForKey(WebPreferencesKey::largeImageAsyncDecodingEnabledKey()));
+    settings.setLargeImageAsyncDecodingEnabled(false);
     settings.setAnimatedImageAsyncDecodingEnabled(store.getBoolValueForKey(WebPreferencesKey::animatedImageAsyncDecodingEnabledKey()));
     settings.setShouldSuppressKeyboardInputDuringProvisionalNavigation(store.getBoolValueForKey(WebPreferencesKey::shouldSuppressKeyboardInputDuringProvisionalNavigationKey()));