<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - REGRESSION(r198782, r201043): [image-decoders] Flickering with some animated gif"
href="https://bugs.webkit.org/show_bug.cgi?id=159089#c4">Comment # 4</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - REGRESSION(r198782, r201043): [image-decoders] Flickering with some animated gif"
href="https://bugs.webkit.org/show_bug.cgi?id=159089">bug 159089</a>
from <span class="vcard"><a class="email" href="mailto:cgarcia@igalia.com" title="Carlos Garcia Campos <cgarcia@igalia.com>"> <span class="fn">Carlos Garcia Campos</span></a>
</span></b>
<pre>(In reply to <a href="show_bug.cgi?id=159089#c3">comment #3</a>)
<span class="quote">> Comment on <span class=""><a href="attachment.cgi?id=281971&action=diff" name="attach_281971" title="Patch">attachment 281971</a> <a href="attachment.cgi?id=281971&action=edit" title="Patch">[details]</a></span>
> Patch
>
> View in context:
> <a href="https://bugs.webkit.org/attachment.cgi?id=281971&action=review">https://bugs.webkit.org/attachment.cgi?id=281971&action=review</a>
>
> > Source/WebCore/platform/image-decoders/ImageDecoder.cpp:319
> > + if (!buffer || buffer->status() == ImageFrame::FrameEmpty || size().isEmpty())
>
> I don't like this kind of dependency and implicit hidden calculation. How do
> I know or remember that to get the size of image in the ImageDecoder I have
> to call frameBufferAtIndex() first?</span >
Yes, I agree, that's why I added the comment there. But note that it's not required to call frameBufferAtIndex specifically, it just happens that frameBufferAtIndex ensures the size is retrieved from the image data. In this case, we just take advantage that we are calling this method to avoid any other check.
<span class="quote">> Can't we fix this by changing the base
> class function ImageDecoder::setSize() to be like this:
>
> virtual IntSize size() { return frameBufferAtIndex(0) ? m_size : IntSize(); }</span >
I thought about that, although not using frameBufferAtIndex, but isSizeAvailable() instead, that decodes enough data to get the size. However, I thought there was a reason why we have isSizeAvailable() and size() public, instead of just making size() do the isSizeAvailable() implicitly. In this particular case, we would end up decoding twice, first to get the size and then to get the buffer. We could avoid that by checking the size after frameBufferAtIndex (because the isSizeAvailable is cached), but we would be in the same situation.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>