<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[202616] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/202616">202616</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2016-06-29 00:23:51 -0700 (Wed, 29 Jun 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>REGRESSION(<a href="http://trac.webkit.org/projects/webkit/changeset/198782">r198782</a>, <a href="http://trac.webkit.org/projects/webkit/changeset/201043">r201043</a>): [image-decoders] Flickering with some animated gif
https://bugs.webkit.org/show_bug.cgi?id=159089
Reviewed by Antonio Gomes.
There's some flickering when loading big enough animated gifs running the animation in a loop. The first time it
loads everything is fine, but after the first loop iteration there are several flickering effects, once every
time the animation finishes and some others happening in the middle of the animation loop. The flickering
happens because we fail to render some of the frames, and it has two diferent causes:
- In <a href="http://trac.webkit.org/projects/webkit/changeset/198782">r198782</a>, ImageDecoder::createFrameImageAtIndex(), was modified to check first if the image is empty to
return early, and then try to get the frame image from the decoder. This is the aone causing the flickering
always on the first frame after one iteration. It happens because ImageDecoder::size() is always empty at that
point. The first time doesn't happen because the gif is loaded and BitmapImage calls isSizeAvailable() from
BitmapImage::dataChanged(). The isSizeAvailable call makes the gif decoder calculate the size. But for the next
iterations, frames are cached and BitmapImage already has the decoded data so isSizeAvailable is not called
again. When createFrameImageAtIndex() is called again for the first frame, size is empty until the gif decoder
creates the reader again, which happens when frameBufferAtIndex() is called, so after that the size is
available. So, we could call isSizeAvailable before checking the size, or simply do the check after the
frameBufferAtIndex() call as we used to do.
- In <a href="http://trac.webkit.org/projects/webkit/changeset/201043">r201043</a> BitmapImage::destroyDecodedDataIfNecessary() was fixed to use the actual bytes used by the frame
in order to decide whether to destroy decoded data or not. This actually revealed a bug, it didn't happen before
because we were never destroying frames before. The bug is in the gif decoder that doesn't correctly handle the
case of destroying only some of the frames from the buffer cache. The gif decoder is designed to always process the
frames in order, the reader keeps an index of the currently processed frame, so when some frames are read from the
cache, and then we ask the decoder for a not cached frame, the currently processed frame is not in sync with the
actual frame we are asking for, and we end do not processing any frame at all.
* platform/image-decoders/ImageDecoder.cpp:
(WebCore::ImageDecoder::createFrameImageAtIndex): Check the size after calling frameBufferAtIndex().
* platform/image-decoders/gif/GIFImageDecoder.cpp:
(WebCore::GIFImageDecoder::clearFrameBufferCache): Delete the reader when clearing the cache since it's out of sync.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformimagedecodersImageDecodercpp">trunk/Source/WebCore/platform/image-decoders/ImageDecoder.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformimagedecodersgifGIFImageDecodercpp">trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (202615 => 202616)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-06-29 06:33:57 UTC (rev 202615)
+++ trunk/Source/WebCore/ChangeLog 2016-06-29 07:23:51 UTC (rev 202616)
</span><span class="lines">@@ -1,3 +1,39 @@
</span><ins>+2016-06-29 Carlos Garcia Campos <cgarcia@igalia.com>
+
+ REGRESSION(r198782, r201043): [image-decoders] Flickering with some animated gif
+ https://bugs.webkit.org/show_bug.cgi?id=159089
+
+ Reviewed by Antonio Gomes.
+
+ There's some flickering when loading big enough animated gifs running the animation in a loop. The first time it
+ loads everything is fine, but after the first loop iteration there are several flickering effects, once every
+ time the animation finishes and some others happening in the middle of the animation loop. The flickering
+ happens because we fail to render some of the frames, and it has two diferent causes:
+
+ - In r198782, ImageDecoder::createFrameImageAtIndex(), was modified to check first if the image is empty to
+ return early, and then try to get the frame image from the decoder. This is the aone causing the flickering
+ always on the first frame after one iteration. It happens because ImageDecoder::size() is always empty at that
+ point. The first time doesn't happen because the gif is loaded and BitmapImage calls isSizeAvailable() from
+ BitmapImage::dataChanged(). The isSizeAvailable call makes the gif decoder calculate the size. But for the next
+ iterations, frames are cached and BitmapImage already has the decoded data so isSizeAvailable is not called
+ again. When createFrameImageAtIndex() is called again for the first frame, size is empty until the gif decoder
+ creates the reader again, which happens when frameBufferAtIndex() is called, so after that the size is
+ available. So, we could call isSizeAvailable before checking the size, or simply do the check after the
+ frameBufferAtIndex() call as we used to do.
+
+ - In r201043 BitmapImage::destroyDecodedDataIfNecessary() was fixed to use the actual bytes used by the frame
+ in order to decide whether to destroy decoded data or not. This actually revealed a bug, it didn't happen before
+ because we were never destroying frames before. The bug is in the gif decoder that doesn't correctly handle the
+ case of destroying only some of the frames from the buffer cache. The gif decoder is designed to always process the
+ frames in order, the reader keeps an index of the currently processed frame, so when some frames are read from the
+ cache, and then we ask the decoder for a not cached frame, the currently processed frame is not in sync with the
+ actual frame we are asking for, and we end do not processing any frame at all.
+
+ * platform/image-decoders/ImageDecoder.cpp:
+ (WebCore::ImageDecoder::createFrameImageAtIndex): Check the size after calling frameBufferAtIndex().
+ * platform/image-decoders/gif/GIFImageDecoder.cpp:
+ (WebCore::GIFImageDecoder::clearFrameBufferCache): Delete the reader when clearing the cache since it's out of sync.
+
</ins><span class="cx"> 2016-06-28 Carlos Garcia Campos <cgarcia@igalia.com>
</span><span class="cx">
</span><span class="cx"> [GStreamer] Adaptive streaming issues
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformimagedecodersImageDecodercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/image-decoders/ImageDecoder.cpp (202615 => 202616)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/image-decoders/ImageDecoder.cpp 2016-06-29 06:33:57 UTC (rev 202615)
+++ trunk/Source/WebCore/platform/image-decoders/ImageDecoder.cpp 2016-06-29 07:23:51 UTC (rev 202616)
</span><span class="lines">@@ -312,13 +312,11 @@
</span><span class="cx">
</span><span class="cx"> NativeImagePtr ImageDecoder::createFrameImageAtIndex(size_t index, SubsamplingLevel)
</span><span class="cx"> {
</span><del>- // Zero-height images can cause problems for some ports. If we have an
- // empty image dimension, just bail.
- if (size().isEmpty())
- return nullptr;
-
</del><span class="cx"> ImageFrame* buffer = frameBufferAtIndex(index);
</span><del>- if (!buffer || buffer->status() == ImageFrame::FrameEmpty)
</del><ins>+ // Zero-height images can cause problems for some ports. If we have an empty image dimension, just bail.
+ // It's important to check the size after calling frameBufferAtIndex() to ensure the decoder has updated the size.
+ // See https://bugs.webkit.org/show_bug.cgi?id=159089.
+ if (!buffer || buffer->status() == ImageFrame::FrameEmpty || size().isEmpty())
</ins><span class="cx"> return nullptr;
</span><span class="cx">
</span><span class="cx"> // Return the buffer contents as a native image. For some ports, the data
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformimagedecodersgifGIFImageDecodercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp (202615 => 202616)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp 2016-06-29 06:33:57 UTC (rev 202615)
+++ trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp 2016-06-29 07:23:51 UTC (rev 202616)
</span><span class="lines">@@ -176,6 +176,10 @@
</span><span class="cx"> if (j->status() != ImageFrame::FrameEmpty)
</span><span class="cx"> j->clearPixelData();
</span><span class="cx"> }
</span><ins>+
+ // When some frames are cleared, the reader is out of sync, since the caller might ask for any frame not
+ // necessarily in the order expected by the reader. See https://bugs.webkit.org/show_bug.cgi?id=159089.
+ m_reader = nullptr;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool GIFImageDecoder::haveDecodedRow(unsigned frameIndex, const Vector<unsigned char>& rowBuffer, size_t width, size_t rowNumber, unsigned repeatCount, bool writeTransparentPixels)
</span></span></pre>
</div>
</div>
</body>
</html>