Charts on sixcolors.com flicker when zooming in

https://bugs.webkit.org/show_bug.cgi?id=256620 rdar://108930635 Reviewed by Simon Fraser. If 1. An image is in the current viewport, 2. It has the attribute decoding="async", 3. The layer has already painted at least once, 4. And the image frame is being re-decoded Then the decoding="async" attribute should be ignored to avoid flickering. RenderBoxModelObject::decodingModeForImageDraw() is re-factored such that caces which return DecodingMode::Synchronous are checked first. Then flickering case is checked. Then cases which return DecodingMode::Asynchronous are checked last. Two layout tests are changed to ensure the image is displayed on a fresh layer when the image has the decoding="async". * LayoutTests/fast/images/decode-decoding-change-image-src-expected.html: Added. * LayoutTests/fast/images/decode-decoding-change-image-src.html: Added. * Source/WebCore/rendering/RenderBoxModelObject.cpp: (WebCore::RenderBoxModelObject::decodingModeForImageDraw const): Canonical link: https://commits.webkit.org/264433@main
WebKit · May 23, 2023 · 36adb36 · 36adb36
1 parent a7ab5e9
commit 36adb36
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 14 deletions.
diff --git a/LayoutTests/fast/images/decode-decoding-change-image-src-expected.html b/LayoutTests/fast/images/decode-decoding-change-image-src-expected.html
@@ -0,0 +1,4 @@
+<body>
+    <p>This test ensures if an image with decoding="async" is decoded after the first paint, it will not flicker.</p>
+    <img src="resources/green-400x400.png">
+</body>
diff --git a/LayoutTests/fast/images/decode-decoding-change-image-src.html b/LayoutTests/fast/images/decode-decoding-change-image-src.html
@@ -0,0 +1,22 @@
+<body>
+    <p>This test ensures if an image with decoding="async" is decoded after the first paint, it will not flicker.</p>
+    <img id="image" decoding="async" src="">
+    <script>
+        if (window.internals && window.testRunner) {
+            internals.clearMemoryCache();
+            testRunner.waitUntilDone();
+        }
+
+        function setImageSrc() {
+            var image = document.getElementById("image");
+            image.onload = (() => {
+                // Force layout and display so the image frame starts decoding.
+                document.body.offsetHeight;
+                if (window.testRunner)
+                    testRunner.notifyDone();
+            });
+            image.src = "resources/green-400x400.png";
+        }
+        requestAnimationFrame(setImageSrc);
+    </script>
+</body>
diff --git a/LayoutTests/fast/images/decoding-attribute-async-small-image.html b/LayoutTests/fast/images/decoding-attribute-async-small-image.html
@@ -5,6 +5,12 @@
         function loadImage(image, src) {
             return new Promise((resolve) => {
                 image.onload = (() => {
+                    // Move the image to a new separate layer.
+                    var box = document.createElement("div");
+                    box.style.willChange = "transform";
+                    box.appendChild(image);
+                    document.body.appendChild(box);
+
                     if (window.internals && window.testRunner) {
                         // Force layout and display so the image gets drawn.
                         document.body.offsetHeight;

diff --git a/LayoutTests/fast/images/decoding-attribute-dynamic-async-small-image.html b/LayoutTests/fast/images/decoding-attribute-dynamic-async-small-image.html
@@ -5,6 +5,12 @@
         function loadImage(image, src) {
             return new Promise((resolve) => {
                 image.onload = (() => {
+                    // Move the image to a new separate layer.
+                    var box = document.createElement("div");
+                    box.style.willChange = "transform";
+                    box.appendChild(image);
+                    document.body.appendChild(box);
+
                     if (window.internals && window.testRunner) {
                         // Force layout and display so the image gets drawn.
                         document.body.offsetHeight;

diff --git a/Source/WebCore/rendering/RenderBoxModelObject.cpp b/Source/WebCore/rendering/RenderBoxModelObject.cpp
@@ -311,34 +311,47 @@ DecodingMode RenderBoxModelObject::decodingModeForImageDraw(const Image& image,
     }
 
     // Large image case.
+    // Some document types force synchronous decoding.
 #if PLATFORM(IOS_FAMILY)
     if (IOSApplication::isIBooksStorytime())
         return DecodingMode::Synchronous;
 #endif
+    if (document().isImageDocument())
+        return DecodingMode::Synchronous;
+
+    // A PaintBehavior may force synchronous decoding.
     if (paintInfo.paintBehavior.contains(PaintBehavior::Snapshotting))
         return DecodingMode::Synchronous;
     if (paintInfo.paintBehavior.contains(PaintBehavior::ForceSynchronousImageDecode))
         return DecodingMode::Synchronous;
+
+    // <img decoding="sync"> forces synchronous decoding.
     if (is<HTMLImageElement>(element())) {
-        auto decodingMode = downcast<HTMLImageElement>(*element()).decodingMode();
-        if (decodingMode == DecodingMode::Asynchronous)
-            return DecodingMode::Asynchronous;
-        if (decodingMode == DecodingMode::Synchronous)
+        if (downcast<HTMLImageElement>(*element()).decodingMode() == DecodingMode::Synchronous)
             return DecodingMode::Synchronous;
     }
+
+    // Layout tests may force asynchronous decoding.
     if (bitmapImage.isLargeImageAsyncDecodingEnabledForTesting())
         return DecodingMode::Asynchronous;
-    if (document().isImageDocument())
-        return DecodingMode::Synchronous;
-    if (!settings().largeImageAsyncDecodingEnabled())
-        return DecodingMode::Synchronous;
-    if (!bitmapImage.canUseAsyncDecodingForLargeImages())
-        return DecodingMode::Synchronous;
-    if (paintInfo.paintBehavior.contains(PaintBehavior::DefaultAsynchronousImageDecode))
-        return DecodingMode::Asynchronous;
-    // FIXME: isVisibleInViewport() is not cheap. Find a way to make this condition faster.
-    if (!isVisibleInViewport())
+
+    // Not first paint, so we have to avoid flickering anyway.
+    if (!paintInfo.paintBehavior.contains(PaintBehavior::DefaultAsynchronousImageDecode)) {
+        // FIXME: isVisibleInViewport() is not cheap. Find a way to make this condition faster.
+        if (isVisibleInViewport())
+            return DecodingMode::Synchronous;
+    }
+
+    // <img decoding="async"> forces asynchronous decoding.
+    if (is<HTMLImageElement>(element())) {
+        if (downcast<HTMLImageElement>(*element()).decodingMode() == DecodingMode::Asynchronous)
+            return DecodingMode::Asynchronous;
+    }
+
+    // Resepect the web preferences key: largeImageAsyncDecodingEnabled.
+    if (settings().largeImageAsyncDecodingEnabled() && bitmapImage.canUseAsyncDecodingForLargeImages())
         return DecodingMode::Asynchronous;
+
     return DecodingMode::Synchronous;
 }