Skip to site navigation (Press enter)

[webkit-changes] [210951] trunk

commit-queue Thu, 19 Jan 2017 19:56:37 -0800

Title: [210951] trunk

Revision: 210951
Author: [email protected]
Date: 2017-01-19 19:56:20 -0800 (Thu, 19 Jan 2017)

Log Message

REGRESSION(r206156): Animated images are repeated one extra iteration than the value which is saved in the image file
https://bugs.webkit.org/show_bug.cgi?id=167174


Patch by Said Abou-Hallawa <[email protected]> on 2017-01-19
Reviewed by Simon Fraser.

Source/WebCore:

Before r206156, BitmapImage::repetitionCount() used to return zero for
the case loopCount = 1, -1 for loopCount = Infinity and loopCount for
all other cases. Having repetitionCount() return zero for loopCount = 1
makes the condition if (++m_repetitionsComplete > repetitionCount())
break the animation loop after one iteration. But it was wrong for all
loopCount > 1. It was causing an extra iteration to be played for the
animated image. After r206156, BitmapImage::repetitionCount() returns
loopCount for all cases loopCount != Infinity. Keeping the same condition
causes the extra iteration to be played even for loopCount = 1.

Test: fast/images/animated-image-loop-count.html

* platform/graphics/BitmapImage.cpp:
(WebCore::BitmapImage::internalStartAnimation):

LayoutTests:

* fast/images/animated-image-loop-count-expected.html: Added.
* fast/images/animated-image-loop-count.html: Added.
* fast/images/resources/animated-red-green-blue-repeat-1.gif: Added.
* fast/images/resources/animated-red-green-blue-repeat-2.gif: Added.
Ensure the animated image stops animating after loopCount iterations.

Modified Paths

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

Added Paths

trunk/LayoutTests/fast/images/animated-image-loop-count-expected.html
trunk/LayoutTests/fast/images/animated-image-loop-count.html
trunk/LayoutTests/fast/images/resources/animated-red-green-blue-repeat-1.gif
trunk/LayoutTests/fast/images/resources/animated-red-green-blue-repeat-2.gif

Diff

Modified: trunk/LayoutTests/ChangeLog (210950 => 210951)


--- trunk/LayoutTests/ChangeLog	2017-01-20 03:49:37 UTC (rev 210950)
+++ trunk/LayoutTests/ChangeLog	2017-01-20 03:56:20 UTC (rev 210951)
@@ -1,3 +1,16 @@
+2017-01-19  Said Abou-Hallawa  <[email protected]>
+
+        REGRESSION(r206156): Animated images are repeated one extra iteration than the value which is saved in the image file
+        https://bugs.webkit.org/show_bug.cgi?id=167174
+
+        Reviewed by Simon Fraser.
+
+        * fast/images/animated-image-loop-count-expected.html: Added.
+        * fast/images/animated-image-loop-count.html: Added.
+        * fast/images/resources/animated-red-green-blue-repeat-1.gif: Added.
+        * fast/images/resources/animated-red-green-blue-repeat-2.gif: Added.
+        Ensure the animated image stops animating after loopCount iterations.
+
 2017-01-19  Zalan Bujtas  <[email protected]>
 
         Simple line layout: Extend coverage for justified content.

Added: trunk/LayoutTests/fast/images/animated-image-loop-count-expected.html (0 => 210951)


--- trunk/LayoutTests/fast/images/animated-image-loop-count-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/images/animated-image-loop-count-expected.html	2017-01-20 03:56:20 UTC (rev 210951)
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<style>
+    .box {
+        width: 100px;
+        height: 100px;
+        display: inline-block;
+    } 
+</style>    
+<body>
+    <div>
+        <p>Frames of a 3-frame animated image with loopCount = 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 = 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>
+</body>
+</html>

Added: trunk/LayoutTests/fast/images/animated-image-loop-count.html (0 => 210951)


--- trunk/LayoutTests/fast/images/animated-image-loop-count.html	                        (rev 0)
+++ trunk/LayoutTests/fast/images/animated-image-loop-count.html	2017-01-20 03:56:20 UTC (rev 210951)
@@ -0,0 +1,80 @@
+<!DOCTYPE html>
+<html>
+<style>
+    canvas {
+        width: 100px;
+        height: 100px;
+    }
+</style>    
+<body>
+    <div>
+        <p>Frames of a 3-frame animated image with loopCount = 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 = 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>
+    <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 = ""
+            });
+        }
+
+        (function() {
+            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 }
+            ];
+
+            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>

Added: trunk/LayoutTests/fast/images/resources/animated-red-green-blue-repeat-1.gif (0 => 210951)


--- trunk/LayoutTests/fast/images/resources/animated-red-green-blue-repeat-1.gif	                        (rev 0)
+++ trunk/LayoutTests/fast/images/resources/animated-red-green-blue-repeat-1.gif	2017-01-20 03:56:20 UTC (rev 210951)
@@ -0,0 +1,10 @@
+GIF89add\xF0\xFF!\xF9!\xFFImageMagick+gamma=0.45455,dds\x84\x8F\xA9\xCB\xED\xA3\x9C\xB4ڋ\xB3޼\xFB\x86\xE2H\x96扦\xEAʶ\xEE\xC7\xF2L\xD7\xF6\x8D\xE7\xFA\xCE\xF7\xFE
+\x87Ģ\xF1\x88L*\x97̦\xF3	\x8DJ\xA7Ԫ\xF5\x8A\xCDj\xB7ܮ\xF7\x8B\xC7\xE4\xB2\xF9\x8CN\xAB\xD7\xEC\xB6\xFB+\x8F\xCB\xE7\xF4\xBA\xFD\x8E\xCF\xEB\xF7\xFC\xBE\xFF(8HXhx\x88\x98\xA8\xB8\xC8\xD8\xE8\xF8iX!\xF9!\xFFImageMagick+gamma=0.45455,dd\x80\x80s\x84\x8F\xA9\xCB\xED\xA3\x9C\xB4ڋ\xB3޼\xFB\x86\xE2H\x96扦\xEAʶ\xEE\xC7\xF2L\xD7\xF6\x8D\xE7\xFA\xCE\xF7\xFE
+\x87Ģ\xF1\x88L*\x97̦\xF3	\x8DJ\xA7Ԫ\xF5\x8A\xCDj\xB7ܮ\xF7\x8B\xC7\xE4\xB2\xF9\x8CN\xAB\xD7\xEC\xB6\xFB+\x8F\xCB\xE7\xF4\xBA\xFD\x8E\xCF\xEB\xF7\xFC\xBE\xFF(8HXhx\x88\x98\xA8\xB8\xC8\xD8\xE8\xF8iX!\xF9!\xFFImageMagick+gamma=0.45455,dd\x80\xFFs\x84\x8F\xA9\xCB\xED\xA3\x9C\xB4ڋ\xB3޼\xFB\x86\xE2H\x96扦\xEAʶ\xEE\xC7\xF2L\xD7\xF6\x8D\xE7\xFA\xCE\xF7\xFE
+\x87Ģ\xF1\x88L*\x97̦\xF3	\x8DJ\xA7Ԫ\xF5\x8A\xCDj\xB7ܮ\xF7\x8B\xC7\xE4\xB2\xF9\x8CN\xAB\xD7\xEC\xB6\xFB+\x8F\xCB\xE7\xF4\xBA\xFD\x8E\xCF\xEB\xF7\xFC\xBE\xFF(8HXhx\x88\x98\xA8\xB8\xC8\xD8\xE8\xF8iX;
\ No newline at end of file

Added: trunk/LayoutTests/fast/images/resources/animated-red-green-blue-repeat-2.gif (0 => 210951)


--- trunk/LayoutTests/fast/images/resources/animated-red-green-blue-repeat-2.gif	                        (rev 0)
+++ trunk/LayoutTests/fast/images/resources/animated-red-green-blue-repeat-2.gif	2017-01-20 03:56:20 UTC (rev 210951)
@@ -0,0 +1,10 @@
+GIF89add\xF0\xFF!\xF9!\xFFNETSCAPE2.0!\xFFImageMagick+gamma=0.45455,dds\x84\x8F\xA9\xCB\xED\xA3\x9C\xB4ڋ\xB3޼\xFB\x86\xE2H\x96扦\xEAʶ\xEE\xC7\xF2L\xD7\xF6\x8D\xE7\xFA\xCE\xF7\xFE
+\x87Ģ\xF1\x88L*\x97̦\xF3	\x8DJ\xA7Ԫ\xF5\x8A\xCDj\xB7ܮ\xF7\x8B\xC7\xE4\xB2\xF9\x8CN\xAB\xD7\xEC\xB6\xFB+\x8F\xCB\xE7\xF4\xBA\xFD\x8E\xCF\xEB\xF7\xFC\xBE\xFF(8HXhx\x88\x98\xA8\xB8\xC8\xD8\xE8\xF8iX!\xF9!\xFFImageMagick+gamma=0.45455,dd\x80\x80s\x84\x8F\xA9\xCB\xED\xA3\x9C\xB4ڋ\xB3޼\xFB\x86\xE2H\x96扦\xEAʶ\xEE\xC7\xF2L\xD7\xF6\x8D\xE7\xFA\xCE\xF7\xFE
+\x87Ģ\xF1\x88L*\x97̦\xF3	\x8DJ\xA7Ԫ\xF5\x8A\xCDj\xB7ܮ\xF7\x8B\xC7\xE4\xB2\xF9\x8CN\xAB\xD7\xEC\xB6\xFB+\x8F\xCB\xE7\xF4\xBA\xFD\x8E\xCF\xEB\xF7\xFC\xBE\xFF(8HXhx\x88\x98\xA8\xB8\xC8\xD8\xE8\xF8iX!\xF9!\xFFImageMagick+gamma=0.45455,dd\x80\xFFs\x84\x8F\xA9\xCB\xED\xA3\x9C\xB4ڋ\xB3޼\xFB\x86\xE2H\x96扦\xEAʶ\xEE\xC7\xF2L\xD7\xF6\x8D\xE7\xFA\xCE\xF7\xFE
+\x87Ģ\xF1\x88L*\x97̦\xF3	\x8DJ\xA7Ԫ\xF5\x8A\xCDj\xB7ܮ\xF7\x8B\xC7\xE4\xB2\xF9\x8CN\xAB\xD7\xEC\xB6\xFB+\x8F\xCB\xE7\xF4\xBA\xFD\x8E\xCF\xEB\xF7\xFC\xBE\xFF(8HXhx\x88\x98\xA8\xB8\xC8\xD8\xE8\xF8iX;
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (210950 => 210951)


--- trunk/Source/WebCore/ChangeLog	2017-01-20 03:49:37 UTC (rev 210950)
+++ trunk/Source/WebCore/ChangeLog	2017-01-20 03:56:20 UTC (rev 210951)
@@ -1,3 +1,25 @@
+2017-01-19  Said Abou-Hallawa  <[email protected]>
+
+        REGRESSION(r206156): Animated images are repeated one extra iteration than the value which is saved in the image file
+        https://bugs.webkit.org/show_bug.cgi?id=167174
+
+        Reviewed by Simon Fraser.
+
+        Before r206156, BitmapImage::repetitionCount() used to return zero for
+        the case loopCount = 1, -1 for loopCount = Infinity and loopCount for
+        all other cases. Having repetitionCount() return zero for loopCount = 1
+        makes the condition if (++m_repetitionsComplete > repetitionCount())
+        break the animation loop after one iteration. But it was wrong for all
+        loopCount > 1. It was causing an extra iteration to be played for the
+        animated image. After r206156, BitmapImage::repetitionCount() returns
+        loopCount for all cases loopCount != Infinity. Keeping the same condition
+        causes the extra iteration to be played even for loopCount = 1.
+
+        Test: fast/images/animated-image-loop-count.html
+
+        * platform/graphics/BitmapImage.cpp:
+        (WebCore::BitmapImage::internalStartAnimation):
+
 2017-01-19  Chris Dumez  <[email protected]>
 
         iterable<> should be enabled on WK1

Modified: trunk/Source/WebCore/platform/graphics/BitmapImage.cpp (210950 => 210951)


--- trunk/Source/WebCore/platform/graphics/BitmapImage.cpp	2017-01-20 03:49:37 UTC (rev 210950)
+++ trunk/Source/WebCore/platform/graphics/BitmapImage.cpp	2017-01-20 03:56:20 UTC (rev 210951)
@@ -251,7 +251,7 @@
         ++m_repetitionsComplete;
 
         // Check for the end of animation.
-        if (repetitionCount() != RepetitionCountInfinite && m_repetitionsComplete > repetitionCount()) {
+        if (repetitionCount() != RepetitionCountInfinite && m_repetitionsComplete >= repetitionCount()) {
             m_animationFinished = true;
             destroyDecodedDataIfNecessary(false);
             return StartAnimationResult::CannotStart;

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes