[webkit-reviews] review granted: [Bug 178080] [GTK][WPE] Fix review comments on WEBPImageDecoder : [Attachment 324005] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Oct 19 12:08:23 PDT 2017


Said Abou-Hallawa <sabouhallawa at apple.com> has granted Miguel Gomez
<magomez at igalia.com>'s request for review:
Bug 178080: [GTK][WPE] Fix review comments on WEBPImageDecoder
https://bugs.webkit.org/show_bug.cgi?id=178080

Attachment 324005: Patch

https://bugs.webkit.org/attachment.cgi?id=324005&action=review




--- Comment #6 from Said Abou-Hallawa <sabouhallawa at apple.com> ---
Comment on attachment 324005
  --> https://bugs.webkit.org/attachment.cgi?id=324005
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=324005&action=review

> Source/WebCore/platform/image-decoders/webp/WEBPImageDecoder.cpp:38
> +    return WebPDemuxGetFrame(demuxer, index + 1, webpFrame);

Maybe it is worthy adding a comment that this wrapper was added because
WebPDemuxGetFrame is 1-based function.

> Source/WebCore/platform/image-decoders/webp/WEBPImageDecoder.cpp:123
>	   }

I think this loop is a little bit confusing. The problem is you have two
different things you want to check in the same loop. I would suggest splitting
the complete test from the alpha and the disposal method test:

size_t firstIncompleteFrame = frameIndex;
for (; firstIncompleteFrame; --firstIncompleteFrame) {
    if (m_frameBufferCache[firstIncompleteFrame - 1].isComplete())
	break;
}

for (size_t firstIndependentFrame = frameIndex; firstIndependentFrame >
firstIncompleteFrame ; --firstIndependentFrame) {
    WebPIterator webpFrame;
    if (!webpFrameAtIndex(demuxer, firstIndependentFrame, &webpFrame))
	continue;

    IntRect frameRect(webpFrame.x_offset, webpFrame.y_offset, webpFrame.width,
webpFrame.height);
    if (!frameRect.contains({ { }, size() }))
	continue;

    if (!webpFrame.has_alpha)
	return firstIndependentFrame;

    if (firstIndependentFrame < frameIndex &&
m_frameBufferCache[firstIndependentFrame].disposalMethod() ==
ImageFrame::DisposalMethod::RestoreToBackground)
	return firstIndependentFrame + 1;  
}

return firstIncompleteFrame;


More information about the webkit-reviews mailing list