[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