Attachment 342590 Details for Bug 186336 – Patch

[patch] Patch

bug-186336-20180612134144.patch (text/plain), 12.52 KB, created by Said Abou-Hallawa on 2018-06-12 13:41:44 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Said Abou-Hallawa

Created: 2018-06-12 13:41:44 PDT

Size: 12.52 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 232736)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,35 @@
>+2018-06-12  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 NOBODY (OOPS!).
>+
>+        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
>+        had to be generalized for large and animated images. The logic for making 
>+        the image decoding takes the required duration was moved from BitmapImage
>+        to the decoding thread in the ImageSource.
>+
>+        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):
>+        * rendering/RenderLayer.h:
>+
> 2018-06-11  Tim Horton  <timothy_horton@apple.com>
> 
>         Link drag image is inconsistently unreadable in dark mode
>Index: Source/WebCore/platform/graphics/BitmapImage.cpp
>===================================================================
>--- Source/WebCore/platform/graphics/BitmapImage.cpp	(revision 232736)
>+++ Source/WebCore/platform/graphics/BitmapImage.cpp	(working copy)
>@@ -340,7 +340,7 @@ bool BitmapImage::canUseAsyncDecodingFor
> 
> 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 Bitmap
>             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: Source/WebCore/platform/graphics/BitmapImage.h
>===================================================================
>--- Source/WebCore/platform/graphics/BitmapImage.h	(revision 232736)
>+++ Source/WebCore/platform/graphics/BitmapImage.h	(working copy)
>@@ -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: Source/WebCore/platform/graphics/ImageSource.cpp
>===================================================================
>--- Source/WebCore/platform/graphics/ImageSource.cpp	(revision 232736)
>+++ Source/WebCore/platform/graphics/ImageSource.cpp	(working copy)
>@@ -329,6 +329,10 @@ void ImageSource::startAsyncDecodingQueu
>         while (protectedFrameRequestQueue->dequeue(frameRequest)) {
>             TraceScope tracingScope(AsyncImageDecodeStart, AsyncImageDecodeEnd);
> 
>+            MonotonicTime startingTime;
>+            if (protectedThis->frameDecodingDurationForTesting() > 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)
>@@ -337,6 +341,12 @@ void ImageSource::startAsyncDecodingQueu
>                 LOG(Images, "ImageSource::%s - %p - url: %s [decoding for frame %ld has failed]", __FUNCTION__, protectedThis.ptr(), sourceURL.utf8().data(), frameRequest.index);
>                 continue;
>             }
>+            
>+            if (protectedThis->frameDecodingDurationForTesting() > 0_s) {
>+                auto delta = protectedThis->frameDecodingDurationForTesting() - (MonotonicTime::now() - startingTime);
>+                if (delta > 0_s)
>+                    sleep(delta);
>+            }
> 
>             // 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 {
>Index: Source/WebCore/platform/graphics/ImageSource.h
>===================================================================
>--- Source/WebCore/platform/graphics/ImageSource.h	(revision 232736)
>+++ Source/WebCore/platform/graphics/ImageSource.h	(working copy)
>@@ -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: Source/WebCore/rendering/RenderLayer.cpp
>===================================================================
>--- Source/WebCore/rendering/RenderLayer.cpp	(revision 232736)
>+++ Source/WebCore/rendering/RenderLayer.cpp	(working copy)
>@@ -4350,7 +4350,7 @@ void RenderLayer::paintLayerContents(Gra
>         if (paintingInfo.paintBehavior & PaintBehaviorSnapshotting)
>             paintBehavior |= PaintBehaviorSnapshotting;
>         
>-        if (paintingInfo.paintBehavior & PaintBehaviorTileFirstPaint)
>+        if ((paintingInfo.paintBehavior & PaintBehaviorTileFirstPaint) && isRoot())
>             paintBehavior |= PaintBehaviorTileFirstPaint;
> 
>         if (paintingInfo.paintBehavior & PaintBehaviorExcludeSelection)
>Index: Source/WebCore/rendering/RenderLayer.h
>===================================================================
>--- Source/WebCore/rendering/RenderLayer.h	(revision 232736)
>+++ Source/WebCore/rendering/RenderLayer.h	(working copy)
>@@ -186,6 +186,8 @@ public:
>         return curr;
>     }
>     
>+    bool isRoot() const { return !parent(); }
>+    
>     const LayoutPoint& location() const { return m_topLeft; }
>     void setLocation(const LayoutPoint& p) { m_topLeft = p; }
> 
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 232736)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,14 @@
>+2018-06-12  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 NOBODY (OOPS!).
>+
>+        * fast/images/async-image-composited-show-expected.html: Added.
>+        * fast/images/async-image-composited-show.html: Added.
>+
> 2018-06-11  Said Abou-Hallawa  <sabouhallawa@apple.com>
> 
>         Layout Test svg/dom/animated-tearoff-list-remove-target.html is a flaky timeout
>Index: LayoutTests/fast/images/async-image-composited-show-expected.html
>===================================================================
>--- LayoutTests/fast/images/async-image-composited-show-expected.html	(nonexistent)
>+++ LayoutTests/fast/images/async-image-composited-show-expected.html	(working copy)
>@@ -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>
>Index: LayoutTests/fast/images/async-image-composited-show.html
>===================================================================
>--- LayoutTests/fast/images/async-image-composited-show.html	(nonexistent)
>+++ LayoutTests/fast/images/async-image-composited-show.html	(working copy)
>@@ -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>

Actions: View | Formatted Diff | Diff

Attachments on bug 186336: 342025 | 342178 | 342179 | 342590 | 342614