TileFirstPaint strategy for async image decoding should be disabled for non root...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Jun 2018 19:10:16 +0000 (19:10 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Jun 2018 19:10:16 +0000 (19:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186336
<rdar://problem/40808099>

Patch by Said Abou-Hallawa <sabouhallawa@apple.com> on 2018-06-13
Reviewed by Simon Fraser.

Source/WebCore:

When showing a composited RenderLayer for the first time, the images in
this layer have to be decoded synchronously to avoid unwanted flashing.

To create a layout test for this patch, FrameDecodingDurationForTesting
needs to be generalized for large and animated images. The decoding thread
now forces the decoding time to be at least equal to
FrameDecodingDurationForTesting.

Test: fast/images/async-image-composited-show.html

* platform/graphics/BitmapImage.cpp:
(WebCore::BitmapImage::shouldUseAsyncDecodingForAnimatedImages const):
(WebCore::BitmapImage::internalStartAnimation):
(WebCore::BitmapImage::advanceAnimation):
* platform/graphics/BitmapImage.h:
* platform/graphics/ImageSource.cpp:
(WebCore::ImageSource::startAsyncDecodingQueue):
* platform/graphics/ImageSource.h:
(WebCore::ImageSource::setFrameDecodingDurationForTesting):
(WebCore::ImageSource::frameDecodingDurationForTesting const):
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::paintLayerContents):

LayoutTests:

* fast/images/async-image-composited-show-expected.html: Added.
* fast/images/async-image-composited-show.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@232802 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/images/async-image-composited-show-expected.html [new file with mode: 0644]
LayoutTests/fast/images/async-image-composited-show.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/BitmapImage.cpp
Source/WebCore/platform/graphics/BitmapImage.h
Source/WebCore/platform/graphics/ImageSource.cpp
Source/WebCore/platform/graphics/ImageSource.h
Source/WebCore/rendering/RenderLayer.cpp

index 502db04..3603901 100644 (file)
@@ -1,3 +1,14 @@
+2018-06-13  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        TileFirstPaint strategy for async image decoding should be disabled for non root RenderLayers
+        https://bugs.webkit.org/show_bug.cgi?id=186336
+        <rdar://problem/40808099>
+
+        Reviewed by Simon Fraser.
+
+        * fast/images/async-image-composited-show-expected.html: Added.
+        * fast/images/async-image-composited-show.html: Added.
+
 2018-06-13  Carlos Alberto Lopez Perez  <clopez@igalia.com>
 
         [GTK] Mark some flaky tests and expected failures.
diff --git a/LayoutTests/fast/images/async-image-composited-show-expected.html b/LayoutTests/fast/images/async-image-composited-show-expected.html
new file mode 100644 (file)
index 0000000..3c48106
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        .container {
+            position: relative;
+            width: 400px;
+            height: 400px;
+            background-color: green;
+        }
+    </style>
+</head>
+<body>
+    <div class="container"></div>
+</body>
+</html>
diff --git a/LayoutTests/fast/images/async-image-composited-show.html b/LayoutTests/fast/images/async-image-composited-show.html
new file mode 100644 (file)
index 0000000..38777ca
--- /dev/null
@@ -0,0 +1,52 @@
+<style>
+    .container {
+        position: relative;
+        width: 400px;
+        height: 400px;
+        background-color: #ff0000;
+    }
+    .composited {
+        position: absolute;
+        top: 0;
+        left: 0;
+        width: 100%;
+        height: 100%;
+        background-repeat: no-repeat;
+        z-index: 0;
+    }
+    body {
+        opacity: 0;
+        background: rgba(0, 0, 0, 0);
+    }
+    body.background-loaded {
+        opacity: 1;
+    }
+</style>
+<body>
+    <div class="container">
+        <div class="composited"></div>
+    </div>
+    <script>
+        if (window.testRunner) {
+            internals.clearMemoryCache();
+            internals.settings.setLargeImageAsyncDecodingEnabled(true);
+            testRunner.waitUntilDone();
+        }
+
+        var image = new Image();
+        image.onload = function() {
+            // Force very long async image decoding for this image.
+            if (window.internals)
+                internals.setImageFrameDecodingDuration(image, 5000);
+
+            // Change the background of the element.
+            let element = document.querySelector(".composited");
+            element.style.backgroundImage = 'url(' + image.src + ')';
+
+            // Show the body element and end the test.
+            document.body.classList.add("background-loaded");
+            testRunner.notifyDone();
+        };
+        image.src = "resources/green-400x400.png";
+    </script>
+</body>
index 007491d..2a9a325 100644 (file)
@@ -1,3 +1,34 @@
+2018-06-13  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        TileFirstPaint strategy for async image decoding should be disabled for non root RenderLayers
+        https://bugs.webkit.org/show_bug.cgi?id=186336
+        <rdar://problem/40808099>
+
+        Reviewed by Simon Fraser.
+
+        When showing a composited RenderLayer for the first time, the images in 
+        this layer have to be decoded synchronously to avoid unwanted flashing.
+
+        To create a layout test for this patch, FrameDecodingDurationForTesting
+        needs to be generalized for large and animated images. The decoding thread
+        now forces the decoding time to be at least equal to 
+        FrameDecodingDurationForTesting.
+
+        Test: fast/images/async-image-composited-show.html
+
+        * platform/graphics/BitmapImage.cpp:
+        (WebCore::BitmapImage::shouldUseAsyncDecodingForAnimatedImages const):
+        (WebCore::BitmapImage::internalStartAnimation):
+        (WebCore::BitmapImage::advanceAnimation):
+        * platform/graphics/BitmapImage.h:
+        * platform/graphics/ImageSource.cpp:
+        (WebCore::ImageSource::startAsyncDecodingQueue):
+        * platform/graphics/ImageSource.h:
+        (WebCore::ImageSource::setFrameDecodingDurationForTesting):
+        (WebCore::ImageSource::frameDecodingDurationForTesting const):
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::paintLayerContents):
+
 2018-06-13  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         CSS "background-color" style no longer affects natively rendered text fields
index 0e6dc15..bf36de0 100644 (file)
@@ -340,7 +340,7 @@ bool BitmapImage::canUseAsyncDecodingForLargeImages() const
 
 bool BitmapImage::shouldUseAsyncDecodingForAnimatedImages() const
 {
-    return canAnimate() && m_allowAnimatedImageAsyncDecoding && (shouldUseAsyncDecodingForAnimatedImagesForTesting() || m_source->canUseAsyncDecoding());
+    return canAnimate() && m_allowAnimatedImageAsyncDecoding && (shouldUseAsyncDecodingForTesting() || m_source->canUseAsyncDecoding());
 }
 
 void BitmapImage::clearTimer()
@@ -449,7 +449,6 @@ BitmapImage::StartAnimationStatus BitmapImage::internalStartAnimation()
             LOG(Images, "BitmapImage::%s - %p - url: %s [requesting async decoding for nextFrame = %ld]", __FUNCTION__, this, sourceURL().string().utf8().data(), nextFrame);
         }
 
-        m_desiredFrameDecodeTimeForTesting = time + std::max(m_frameDecodingDurationForTesting, 0_s);
         if (m_clearDecoderAfterAsyncFrameRequestForTesting)
             m_source->resetData(data());
     }
@@ -463,17 +462,6 @@ void BitmapImage::advanceAnimation()
 {
     clearTimer();
 
-    // Pretend as if decoding nextFrame has taken m_frameDecodingDurationForTesting from
-    // the time this decoding was requested.
-    if (shouldUseAsyncDecodingForAnimatedImagesForTesting()) {
-        MonotonicTime time = MonotonicTime::now();
-        // Start a timer with the remaining time from now till the m_desiredFrameDecodeTime.
-        if (m_desiredFrameDecodeTimeForTesting > std::max(time, m_desiredFrameStartTime)) {
-            startTimer(m_desiredFrameDecodeTimeForTesting - time);
-            return;
-        }
-    }
-
     // Don't advance to nextFrame unless its decoding has finished or was not required.
     size_t nextFrame = (m_currentFrame + 1) % frameCount();
     if (!frameIsBeingDecodedAndIsCompatibleWithOptionsAtIndex(nextFrame, DecodingOptions(DecodingMode::Asynchronous)))
index 199079e..f7eef03 100644 (file)
@@ -102,8 +102,8 @@ public:
     ImageOrientation orientationForCurrentFrame() const { return frameOrientationAtIndex(currentFrame()); }
     bool canAnimate() const;
 
-    bool shouldUseAsyncDecodingForAnimatedImagesForTesting() const { return m_frameDecodingDurationForTesting > 0_s; }
-    void setFrameDecodingDurationForTesting(Seconds duration) { m_frameDecodingDurationForTesting = duration; }
+    bool shouldUseAsyncDecodingForTesting() const { return m_source->frameDecodingDurationForTesting() > 0_s; }
+    void setFrameDecodingDurationForTesting(Seconds duration) { m_source->setFrameDecodingDurationForTesting(duration); }
     bool canUseAsyncDecodingForLargeImages() const;
     bool shouldUseAsyncDecodingForAnimatedImages() const;
     void setClearDecoderAfterAsyncFrameRequestForTesting(bool value) { m_clearDecoderAfterAsyncFrameRequestForTesting = value; }
@@ -216,8 +216,6 @@ private:
     MonotonicTime m_desiredFrameStartTime; // The system time at which we hope to see the next call to startAnimation().
 
     std::unique_ptr<Vector<Function<void()>, 1>> m_decodingCallbacks;
-    Seconds m_frameDecodingDurationForTesting;
-    MonotonicTime m_desiredFrameDecodeTimeForTesting;
 
     bool m_animationFinished { false };
 
index e73e79d..488f945 100644 (file)
@@ -325,10 +325,15 @@ void ImageSource::startAsyncDecodingQueue()
     // We need to protect this, m_decodingQueue and m_decoder from being deleted while we are in the decoding loop.
     decodingQueue().dispatch([protectedThis = makeRef(*this), protectedDecodingQueue = makeRef(decodingQueue()), protectedFrameRequestQueue = makeRef(frameRequestQueue()), protectedDecoder = makeRef(*m_decoder), sourceURL = sourceURL().string().isolatedCopy()] {
         ImageFrameRequest frameRequest;
+        Seconds minDecodingDuration = protectedThis->frameDecodingDurationForTesting();
 
         while (protectedFrameRequestQueue->dequeue(frameRequest)) {
             TraceScope tracingScope(AsyncImageDecodeStart, AsyncImageDecodeEnd);
 
+            MonotonicTime startingTime;
+            if (minDecodingDuration > 0_s)
+                startingTime = MonotonicTime::now();
+
             // Get the frame NativeImage on the decoding thread.
             NativeImagePtr nativeImage = protectedDecoder->createFrameImageAtIndex(frameRequest.index, frameRequest.subsamplingLevel, frameRequest.decodingOptions);
             if (nativeImage)
@@ -338,6 +343,10 @@ void ImageSource::startAsyncDecodingQueue()
                 continue;
             }
 
+            // Pretend as if the decoding takes minDecodingDuration.
+            if (minDecodingDuration > 0_s)
+                sleep(minDecodingDuration - (MonotonicTime::now() - startingTime));
+
             // Update the cached frames on the main thread to avoid updating the MemoryCache from a different thread.
             callOnMainThread([protectedThis = protectedThis.copyRef(), protectedQueue = protectedDecodingQueue.copyRef(), protectedDecoder = protectedDecoder.copyRef(), sourceURL = sourceURL.isolatedCopy(), nativeImage = WTFMove(nativeImage), frameRequest] () mutable {
                 // The queue may have been closed if after we got the frame NativeImage, stopAsyncDecodingQueue() was called.
index fd466b1..0901012 100644 (file)
@@ -81,6 +81,8 @@ public:
     void stopAsyncDecodingQueue();
     bool hasAsyncDecodingQueue() const { return m_decodingQueue; }
     bool isAsyncDecodingQueueIdle() const;
+    void setFrameDecodingDurationForTesting(Seconds duration) { m_frameDecodingDurationForTesting = duration; }
+    Seconds frameDecodingDurationForTesting() const { return m_frameDecodingDurationForTesting; }
 
     // Image metadata which is calculated either by the ImageDecoder or directly
     // from the NativeImage if this class was created for a memory image.
@@ -181,6 +183,7 @@ private:
     RefPtr<FrameRequestQueue> m_frameRequestQueue;
     FrameCommitQueue m_frameCommitQueue;
     RefPtr<WorkQueue> m_decodingQueue;
+    Seconds m_frameDecodingDurationForTesting;
 
     // Image metadata.
     std::optional<EncodedDataStatus> m_encodedDataStatus;
index 8bfafe6..e107e44 100644 (file)
@@ -4350,7 +4350,7 @@ void RenderLayer::paintLayerContents(GraphicsContext& context, const LayerPainti
         if (paintingInfo.paintBehavior & PaintBehaviorSnapshotting)
             paintBehavior |= PaintBehaviorSnapshotting;
         
-        if (paintingInfo.paintBehavior & PaintBehaviorTileFirstPaint)
+        if ((paintingInfo.paintBehavior & PaintBehaviorTileFirstPaint) && isRenderViewLayer())
             paintBehavior |= PaintBehaviorTileFirstPaint;
 
         if (paintingInfo.paintBehavior & PaintBehaviorExcludeSelection)