[webkit-reviews] review denied: [Bug 88424] Optimization in image decoding : [Attachment 176155] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has denied Viatcheslav Ostapenko
<ostap73 at gmail.com>'s request for review:
Bug 88424: Optimization in image decoding
https://bugs.webkit.org/show_bug.cgi?id=88424

Attachment 176155: Patch
https://bugs.webkit.org/attachment.cgi?id=176155&action=review

------- Additional Comments from Brent Fulgham <bfulgham at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=176155&action=review


As I mentioned previously, I'd like to land the GIF part first, and the JPEG
part separately.  Could you please revise this patch to just be the GIF part
(and the inlining), and save the JPEG for a second step?

Also, I think we can avoid exporting more symbols by just using the getAddr
call (rather than making width() and height() public).	Let's do this, unless
you have a strong reason for wanting to expose them.

r- for these reasons.

> Source/WebCore/platform/image-decoders/ImageDecoder.h:-184
> -    private:

Can we please keep width() and height() private (see below comment).

> Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp:233
> +    ImageFrame::PixelData* currentAddress = buffer.data() + yBegin *
buffer.width() + xBegin;

Note: As Noel Gordon points out, we can avoid exposing the width and height
accessors entirely by using the getAddr method:

ImageFrame::PixelData* address = buffer.getAddr(0, y);