[webkit-reviews] review denied: [Bug 23687] ImageBuffer::toDataURL not implemented for Skia : [Attachment 27259] Fix for 23687

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Feb 2 15:30:10 PST 2009


Eric Seidel <eric at webkit.org> has denied Scott Violet <sky at google.com>'s
request for review:
Bug 23687: ImageBuffer::toDataURL not implemented for Skia
https://bugs.webkit.org/show_bug.cgi?id=23687

Attachment 27259: Fix for 23687
https://bugs.webkit.org/attachment.cgi?id=27259&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
 46 static void convertBetweenBGRAandRGBA(const unsigned char* input, int
pixelWidth,
Having read the rest of your code, I understand what pixelWidth means, but I
think numberOfPixels would be clearer in the local context of that function.

Still has WTF::, which can be removed per darin's comment.

PngWriteStructDestroyer should be PNGWriteStructDestroyer to match
PNGImageEncoder

likewise PngEncoderState should be PNGEncoderState

You could actually use an OwnArrayPtr here:
 162	 unsigned char* row = new unsigned char[imageSize.width() *
outputColorComponents];

    OwnArrayPtr<unsigned char> rowPixels(new unsigned char[width * height]);

I'm not sure it's super-useful here, but it does save you the delete[] at the
end (which is more useful when there is lots of code in between the
initialization and the delete).

Thanks for all your corrections.  I'm ready to land this for you after this
last round of nits.


More information about the webkit-reviews mailing list