[Webkit-unassigned] [Bug 159089] REGRESSION(r198782, r201043): [image-decoders] Flickering with some animated gif

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=159089

--- Comment #6 from Said Abou-Hallawa <sabouhallawa at apple.com> ---
(In reply to comment #4)
> (In reply to comment #3)
> > Comment on attachment 281971 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=281971&action=review
> > 
> > > 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?
> 
> 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.
> 
> > 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(); }
> 
> 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.

I don't think your reasoning about decoding the first frame twice is correct. The parser of the ImageDecoder is state machine. It moves along the encoded data and changes its state when finishing decoding the data for the current state. All the decoded data, either image metadata or images frames are cached. So querying the size from the first frame should not be repeated when querying the frame itself.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160629/25ae299e/attachment-0001.html>