An animated PNG plays the frames one time more than the image loopCount
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 22 Mar 2020 05:02:55 +0000 (05:02 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 22 Mar 2020 05:02:55 +0000 (05:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=205640

Patch by Said Abou-Hallawa <sabouhallawa@apple.com> on 2020-03-21
Reviewed by Darin Adler.

Source/WebCore:

Make the repetitionCount calculation for GIFs different from it for other
image formats.

Tests: fast/images/animated-gif-loop-count.html
       fast/images/animated-png-loop-count.html

* platform/graphics/cg/ImageDecoderCG.cpp:
(WebCore::ImageDecoderCG::repetitionCount const):
* platform/graphics/cg/UTIRegistry.cpp:
(WebCore::isGIFImageType):
* platform/graphics/cg/UTIRegistry.h:

LayoutTests:

Refactor the js code to a separate js file. Add two layout tests: one for
animated GIFs and the other for animated PNGs.

* fast/images/animated-gif-loop-count-expected.html: Added.
* fast/images/animated-gif-loop-count.html: Added.
* fast/images/animated-image-loop-count-expected.html: Removed.
* fast/images/animated-image-loop-count.html: Removed.
* fast/images/animated-png-loop-count-expected.html: Added.
* fast/images/animated-png-loop-count.html: Added.
* fast/images/resources/animated-image-loop-count.js: Added.
* fast/images/resources/animated-red-green-blue-repeat-1.png: Added.
* fast/images/resources/animated-red-green-blue-repeat-2.png: Added.
* fast/images/resources/animated-red-green-blue-repeat-infinite.png: Added.
* platform/ios/TestExpectations:
* platform/mac-wk1/TestExpectations:
* platform/win/TestExpectations:

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

17 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/images/animated-gif-loop-count-expected.html [moved from LayoutTests/fast/images/animated-image-loop-count-expected.html with 100% similarity]
LayoutTests/fast/images/animated-gif-loop-count.html [new file with mode: 0644]
LayoutTests/fast/images/animated-image-loop-count.html [deleted file]
LayoutTests/fast/images/animated-png-loop-count-expected.html [new file with mode: 0644]
LayoutTests/fast/images/animated-png-loop-count.html [new file with mode: 0644]
LayoutTests/fast/images/resources/animated-image-loop-count.js [new file with mode: 0644]
LayoutTests/fast/images/resources/animated-red-green-blue-repeat-1.png [new file with mode: 0644]
LayoutTests/fast/images/resources/animated-red-green-blue-repeat-2.png [new file with mode: 0644]
LayoutTests/fast/images/resources/animated-red-green-blue-repeat-infinite.png [new file with mode: 0644]
LayoutTests/platform/ios/TestExpectations
LayoutTests/platform/mac-wk1/TestExpectations
LayoutTests/platform/win/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp
Source/WebCore/platform/graphics/cg/UTIRegistry.cpp
Source/WebCore/platform/graphics/cg/UTIRegistry.h

index 3c648c8..7d4033b 100644 (file)
@@ -1,3 +1,27 @@
+2020-03-21  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        An animated PNG plays the frames one time more than the image loopCount
+        https://bugs.webkit.org/show_bug.cgi?id=205640
+
+        Reviewed by Darin Adler.
+
+        Refactor the js code to a separate js file. Add two layout tests: one for
+        animated GIFs and the other for animated PNGs.
+
+        * fast/images/animated-gif-loop-count-expected.html: Added.
+        * fast/images/animated-gif-loop-count.html: Added.
+        * fast/images/animated-image-loop-count-expected.html: Removed.
+        * fast/images/animated-image-loop-count.html: Removed.
+        * fast/images/animated-png-loop-count-expected.html: Added.
+        * fast/images/animated-png-loop-count.html: Added.
+        * fast/images/resources/animated-image-loop-count.js: Added.
+        * fast/images/resources/animated-red-green-blue-repeat-1.png: Added.
+        * fast/images/resources/animated-red-green-blue-repeat-2.png: Added.
+        * fast/images/resources/animated-red-green-blue-repeat-infinite.png: Added.
+        * platform/ios/TestExpectations:
+        * platform/mac-wk1/TestExpectations:
+        * platform/win/TestExpectations:
+
 2020-03-21  Jack Lee  <shihchieh_lee@apple.com>
 
         Nullptr crash in RenderObject::RenderObjectBitfields::isBox when current renderer is the RenderView
diff --git a/LayoutTests/fast/images/animated-gif-loop-count.html b/LayoutTests/fast/images/animated-gif-loop-count.html
new file mode 100644 (file)
index 0000000..fef400c
--- /dev/null
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<html>
+<style>
+    canvas {
+        width: 100px;
+        height: 100px;
+    }
+</style>
+<script src="resources/animated-image-loop-count.js"></script>
+<body>
+    <div>
+        <p>Frames of a 3-frame animated image with missing loopCount, (repetitionCount = 1):</p>
+        <canvas id="canvas-1"></canvas>
+        <canvas id="canvas-2"></canvas>
+        <canvas id="canvas-3"></canvas>
+        <canvas id="canvas-4"></canvas>
+    </div>
+    <div>
+        <p>Frames of a 3-frame animated image with loopCount = 1, (repetitionCount = 2):</p>
+        <canvas id="canvas-a"></canvas>
+        <canvas id="canvas-b"></canvas>
+        <canvas id="canvas-c"></canvas>
+        <canvas id="canvas-d"></canvas>
+        <canvas id="canvas-e"></canvas>
+        <canvas id="canvas-f"></canvas>
+        <canvas id="canvas-g"></canvas>
+    </div>
+    <div>
+        <p>Frames of a 3-frame animated image with loopCount = 0, (repetitionCount = infinite):</p>
+        <canvas id="canvas-A"></canvas>
+        <canvas id="canvas-B"></canvas>
+        <canvas id="canvas-C"></canvas>
+        <canvas id="canvas-D"></canvas>
+        <canvas id="canvas-E"></canvas>
+        <canvas id="canvas-F"></canvas>
+        <canvas id="canvas-G"></canvas>
+    </div>
+    <script>
+        (function() {
+            var images = [
+                { src: "resources/animated-red-green-blue-repeat-1.gif", canvasId: '1', frameCount: 4 },
+                { src: "resources/animated-red-green-blue-repeat-2.gif", canvasId: 'a', frameCount: 7 },
+                { src: "resources/animated-red-green-blue-repeat-infinite.gif", canvasId: 'A', frameCount: 7 }
+            ];
+            runTest(images);
+        })();
+    </script>
+</body>
+</html>
diff --git a/LayoutTests/fast/images/animated-image-loop-count.html b/LayoutTests/fast/images/animated-image-loop-count.html
deleted file mode 100644 (file)
index bd3dfe9..0000000
+++ /dev/null
@@ -1,96 +0,0 @@
-<!DOCTYPE html>
-<html>
-<style>
-    canvas {
-        width: 100px;
-        height: 100px;
-    }
-</style>    
-<body>
-    <div>
-        <p>Frames of a 3-frame animated image with missing loopCount, (repetitionCount = 1):</p>
-        <canvas id="canvas-1"></canvas>
-        <canvas id="canvas-2"></canvas>
-        <canvas id="canvas-3"></canvas>
-        <canvas id="canvas-4"></canvas>
-    </div>
-    <div>
-        <p>Frames of a 3-frame animated image with loopCount = 1, (repetitionCount = 2):</p>
-        <canvas id="canvas-a"></canvas>
-        <canvas id="canvas-b"></canvas>
-        <canvas id="canvas-c"></canvas>
-        <canvas id="canvas-d"></canvas>
-        <canvas id="canvas-e"></canvas>
-        <canvas id="canvas-f"></canvas>
-        <canvas id="canvas-g"></canvas>
-    </div>
-    <div>
-        <p>Frames of a 3-frame animated image with loopCount = 0, (repetitionCount = infinite):</p>
-        <canvas id="canvas-A"></canvas>
-        <canvas id="canvas-B"></canvas>
-        <canvas id="canvas-C"></canvas>
-        <canvas id="canvas-D"></canvas>
-        <canvas id="canvas-E"></canvas>
-        <canvas id="canvas-F"></canvas>
-        <canvas id="canvas-G"></canvas>
-    </div>
-    <script>
-        function drawFrame(image, canvasId) {
-            return new Promise((resolve) => {
-                let canvas = document.getElementById("canvas-" + canvasId);
-                let context = canvas.getContext("2d");
-                context.drawImage(image, 0, 0, canvas.width, canvas.height);
-                setTimeout(() => {
-                    resolve(String.fromCharCode(canvasId.charCodeAt() + 1));
-                }, 30);
-            });
-        }
-
-        function drawImage(image, canvasId, frameCount) {
-            let promise = drawFrame(image, canvasId);
-            for (let frame = 1; frame < frameCount; ++frame) {
-                promise = promise.then((canvasId) => {
-                    return drawFrame(image, canvasId);
-                });
-            }
-            return promise;
-        }
-
-        function loadImage(src, canvasId, frameCount) {
-            return new Promise((resolve) => {
-                let image = new Image;
-                image.onload = (() => {
-                    drawImage(image, canvasId, frameCount).then(resolve);
-                });
-                image.src = src;
-            });
-        }
-
-        (function() {
-            if (window.internals) {
-                internals.clearMemoryCache();
-                internals.settings.setAnimatedImageDebugCanvasDrawingEnabled(true);
-            }
-
-            if (window.testRunner)
-                testRunner.waitUntilDone();
-
-            var images = [
-                { src: "resources/animated-red-green-blue-repeat-1.gif", canvasId: '1', frameCount: 4 },
-                { src: "resources/animated-red-green-blue-repeat-2.gif", canvasId: 'a', frameCount: 7 },
-                { src: "resources/animated-red-green-blue-repeat-infinite.gif", canvasId: 'A', frameCount: 7 }
-            ];
-
-            var promises = [];
-
-            for (let image of images)
-                promises.push(loadImage(image.src, image.canvasId, image.frameCount));
-            
-            Promise.all(promises).then(() => {
-                if (window.testRunner)
-                    testRunner.notifyDone();
-            });
-        })();
-    </script>
-</body>
-</html>
diff --git a/LayoutTests/fast/images/animated-png-loop-count-expected.html b/LayoutTests/fast/images/animated-png-loop-count-expected.html
new file mode 100644 (file)
index 0000000..f0c8f7a
--- /dev/null
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<html>
+<style>
+    .box {
+        width: 100px;
+        height: 100px;
+        display: inline-block;
+    } 
+</style>    
+<body>
+    <div>
+        <p>Frames of a 3-frame animated image with missing loopCount, (repetitionCount = 1):</p>
+        <div class="box" style="background-color: red;"></div>
+        <div class="box" style="background-color: green;"></div>
+        <div class="box" style="background-color: blue;"></div>
+        <div class="box" style="background-color: blue;"></div>
+    </div>
+    <div>
+        <p>Frames of a 3-frame animated image with loopCount = 1, (repetitionCount = 2):</p>
+        <div class="box" style="background-color: red;"></div>
+        <div class="box" style="background-color: green;"></div>
+        <div class="box" style="background-color: blue;"></div>
+        <div class="box" style="background-color: red;"></div>
+        <div class="box" style="background-color: green;"></div>
+        <div class="box" style="background-color: blue;"></div>
+        <div class="box" style="background-color: blue;"></div>
+    </div>
+    <div>
+        <p>Frames of a 3-frame animated image with loopCount = 0, (repetitionCount = infinite):</p>
+        <div class="box" style="background-color: red;"></div>
+        <div class="box" style="background-color: green;"></div>
+        <div class="box" style="background-color: blue;"></div>
+        <div class="box" style="background-color: red;"></div>
+        <div class="box" style="background-color: green;"></div>
+        <div class="box" style="background-color: blue;"></div>
+        <div class="box" style="background-color: red;"></div>
+    </div>
+</body>
+</html>
diff --git a/LayoutTests/fast/images/animated-png-loop-count.html b/LayoutTests/fast/images/animated-png-loop-count.html
new file mode 100644 (file)
index 0000000..64f8208
--- /dev/null
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<html>
+<style>
+    canvas {
+        width: 100px;
+        height: 100px;
+    }
+</style>
+<script src="resources/animated-image-loop-count.js"></script>
+<body>
+    <div>
+        <p>Frames of a 3-frame animated image with missing loopCount, (repetitionCount = 1):</p>
+        <canvas id="canvas-1"></canvas>
+        <canvas id="canvas-2"></canvas>
+        <canvas id="canvas-3"></canvas>
+        <canvas id="canvas-4"></canvas>
+    </div>
+    <div>
+        <p>Frames of a 3-frame animated image with loopCount = 1, (repetitionCount = 2):</p>
+        <canvas id="canvas-a"></canvas>
+        <canvas id="canvas-b"></canvas>
+        <canvas id="canvas-c"></canvas>
+        <canvas id="canvas-d"></canvas>
+        <canvas id="canvas-e"></canvas>
+        <canvas id="canvas-f"></canvas>
+        <canvas id="canvas-g"></canvas>
+    </div>
+    <div>
+        <p>Frames of a 3-frame animated image with loopCount = 0, (repetitionCount = infinite):</p>
+        <canvas id="canvas-A"></canvas>
+        <canvas id="canvas-B"></canvas>
+        <canvas id="canvas-C"></canvas>
+        <canvas id="canvas-D"></canvas>
+        <canvas id="canvas-E"></canvas>
+        <canvas id="canvas-F"></canvas>
+        <canvas id="canvas-G"></canvas>
+    </div>
+    <script>
+        (function() {
+            var images = [
+                { src: "resources/animated-red-green-blue-repeat-1.png", canvasId: '1', frameCount: 4 },
+                { src: "resources/animated-red-green-blue-repeat-2.png", canvasId: 'a', frameCount: 7 },
+                { src: "resources/animated-red-green-blue-repeat-infinite.png", canvasId: 'A', frameCount: 7 }
+            ];
+            runTest(images);
+        })();
+    </script>
+</body>
+</html>
diff --git a/LayoutTests/fast/images/resources/animated-image-loop-count.js b/LayoutTests/fast/images/resources/animated-image-loop-count.js
new file mode 100644 (file)
index 0000000..b5ce439
--- /dev/null
@@ -0,0 +1,50 @@
+function drawFrame(image, canvasId) {
+    return new Promise((resolve) => {
+        let canvas = document.getElementById("canvas-" + canvasId);
+        let context = canvas.getContext("2d");
+        context.drawImage(image, 0, 0, canvas.width, canvas.height);
+        setTimeout(() => {
+            resolve(String.fromCharCode(canvasId.charCodeAt() + 1));
+        }, 30);
+    });
+}
+
+function drawImage(image, canvasId, frameCount) {
+    let promise = drawFrame(image, canvasId);
+    for (let frame = 1; frame < frameCount; ++frame) {
+        promise = promise.then((canvasId) => {
+            return drawFrame(image, canvasId);
+        });
+    }
+    return promise;
+}
+
+function loadImage(src, canvasId, frameCount) {
+    return new Promise((resolve) => {
+        let image = new Image;
+        image.onload = (() => {
+            drawImage(image, canvasId, frameCount).then(resolve);
+        });
+        image.src = src;
+    });
+}
+
+function runTest(images) {
+    if (window.internals) {
+        internals.clearMemoryCache();
+        internals.settings.setAnimatedImageDebugCanvasDrawingEnabled(true);
+    }
+
+    if (window.testRunner)
+        testRunner.waitUntilDone();
+
+    var promises = [];
+
+    for (let image of images)
+        promises.push(loadImage(image.src, image.canvasId, image.frameCount));
+            
+    Promise.all(promises).then(() => {
+        if (window.testRunner)
+            testRunner.notifyDone();
+    });
+}
diff --git a/LayoutTests/fast/images/resources/animated-red-green-blue-repeat-1.png b/LayoutTests/fast/images/resources/animated-red-green-blue-repeat-1.png
new file mode 100644 (file)
index 0000000..030a2a0
Binary files /dev/null and b/LayoutTests/fast/images/resources/animated-red-green-blue-repeat-1.png differ
diff --git a/LayoutTests/fast/images/resources/animated-red-green-blue-repeat-2.png b/LayoutTests/fast/images/resources/animated-red-green-blue-repeat-2.png
new file mode 100644 (file)
index 0000000..bbb337d
Binary files /dev/null and b/LayoutTests/fast/images/resources/animated-red-green-blue-repeat-2.png differ
diff --git a/LayoutTests/fast/images/resources/animated-red-green-blue-repeat-infinite.png b/LayoutTests/fast/images/resources/animated-red-green-blue-repeat-infinite.png
new file mode 100644 (file)
index 0000000..c3f7de2
Binary files /dev/null and b/LayoutTests/fast/images/resources/animated-red-green-blue-repeat-infinite.png differ
index b81ab85..a33bb5f 100644 (file)
@@ -2804,7 +2804,8 @@ canvas/philip/tests/2d.pattern.animated.gif.html [ Failure ]
 fast/images/slower-animation-than-decoding-image.html [ Failure ]
 fast/images/slower-decoding-than-animation-image.html [ Failure ]
 fast/images/reset-image-animation.html [ Failure ]
-fast/images/animated-image-loop-count.html [ ImageOnlyFailure ]
+fast/images/animated-gif-loop-count.html [ ImageOnlyFailure ]
+fast/images/animated-png-loop-count.html [ ImageOnlyFailure ]
 fast/images/animated-image-different-dest-size.html [ ImageOnlyFailure ]
 
 # <rdar://problem/32659595> iOS 11: LayoutTest fast/text/international/system-language/arabic-glyph-cache-fill-combine.html is failing
index dc59120..51b5cf6 100644 (file)
@@ -783,7 +783,8 @@ webkit.org/b/190849 http/tests/security/XFrameOptions/x-frame-options-multiple-h
 
 webkit.org/b/186406 [ Mojave+ ] compositing/iframes/display-none-subframe.html [ Pass Failure ]
 
-webkit.org/b/190383 fast/images/animated-image-loop-count.html [ Pass ImageOnlyFailure ]
+webkit.org/b/190383 fast/images/animated-gif-loop-count.html [ Pass ImageOnlyFailure ]
+webkit.org/b/190383 fast/images/animated-png-loop-count.html [ Pass ImageOnlyFailure ]
 webkit.org/b/190383 fast/images/animated-image-different-dest-size.html [ Pass ImageOnlyFailure ]
 
 webkit.org/b/191639 imported/blink/compositing/squashing/squashing-into-ancestor-painted-layer.html [ Pass ImageOnlyFailure ]
index d248c2d..959430c 100644 (file)
@@ -412,7 +412,7 @@ http/wpt/css/css-highlight-api/ [ Skip ]
 # Pre-HMTL5 parser quirks only apply to the mac port for now.
 fast/parser/pre-html5-parser-quirks.html [ Skip ]
 
-# Requires WebP support.
+# Requires APNG and WebP support.
 fast/canvas/canvas-toDataURL-webp.html [ Skip ]
 fast/images/webp-image-decoding.html [ Skip ]
 fast/images/webp-color-profile-lossless.html [ Skip ]
@@ -421,6 +421,7 @@ fast/images/webp-color-profile-lossy.html [ Skip ]
 http/tests/images/webp-partial-load.html [ Skip ]
 http/tests/images/webp-progressive-load.html [ Skip ]
 fast/images/animated-webp-expected.html [ Skip ]
+fast/images/animated-png-loop-count.html [ Skip ]
 
 # TODO The following tests requires the DRT's dumpUserGestureInFrameLoadCallbacks
 # method. But that method is not implemented in win port since win port can't
index 6080b5d..660cac6 100644 (file)
@@ -1,3 +1,22 @@
+2020-03-21  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        An animated PNG plays the frames one time more than the image loopCount
+        https://bugs.webkit.org/show_bug.cgi?id=205640
+
+        Reviewed by Darin Adler.
+
+        Make the repetitionCount calculation for GIFs different from it for other
+        image formats.
+
+        Tests: fast/images/animated-gif-loop-count.html
+               fast/images/animated-png-loop-count.html
+
+        * platform/graphics/cg/ImageDecoderCG.cpp:
+        (WebCore::ImageDecoderCG::repetitionCount const):
+        * platform/graphics/cg/UTIRegistry.cpp:
+        (WebCore::isGIFImageType):
+        * platform/graphics/cg/UTIRegistry.h:
+
 2020-03-21  Jack Lee  <shihchieh_lee@apple.com>
 
         Nullptr crash in RenderObject::RenderObjectBitfields::isBox when current renderer is the RenderView
index 1a4dd0c..39eb562 100644 (file)
@@ -291,9 +291,15 @@ RepetitionCount ImageDecoderCG::repetitionCount() const
     CFNumberGetValue(num, kCFNumberIntType, &loopCount);
 
     // A property with value 0 means loop forever.
-    // For loopCount > 0, the specs is not clear about it. But it looks the meaning
+    if (!loopCount)
+        return RepetitionCountInfinite;
+
+    if (!isGIFImageType(uti()))
+        return loopCount;
+
+    // For GIF and loopCount > 0, the specs is not clear about it. But it looks the meaning
     // is: play once + loop loopCount which is equivalent to play loopCount + 1.
-    return loopCount ? loopCount + 1 : RepetitionCountInfinite;
+    return loopCount + 1;
 }
 
 Optional<IntPoint> ImageDecoderCG::hotSpot() const
index 9f9c5f4..5a360cc 100644 (file)
@@ -102,6 +102,11 @@ bool isSupportedImageType(const String& imageType)
     return defaultSupportedImageTypes().contains(imageType) || additionalSupportedImageTypes().contains(imageType);
 }
 
+bool isGIFImageType(StringView imageType)
+{
+    return imageType == "com.compuserve.gif";
+}
+
 }
 
 #endif
index 2c3ad32..5b8fe21 100644 (file)
@@ -35,5 +35,6 @@ HashSet<String>& additionalSupportedImageTypes();
 WEBCORE_EXPORT void setAdditionalSupportedImageTypes(const Vector<String>&);
 WEBCORE_EXPORT void setAdditionalSupportedImageTypesForTesting(const String&);
 bool isSupportedImageType(const String&);
+bool isGIFImageType(StringView);
 
 }