[Webkit-unassigned] [Bug 50905] [chromium] Simplify the PNG encoder.

[bugzilla-daemon at webkit.org]

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


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #76832|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #5 from David Levin <levin at chromium.org>  2010-12-17 10:57:29 PST ---
(From update of attachment 76832)
View in context: https://bugs.webkit.org/attachment.cgi?id=76832&action=review

Looks good. It could possibly be checked in as is but I have a concern about passing 0 to png_create_info_struct.

A few things to consider or respond to and then I'd be happy to r+ this.

> WebCore/platform/image-encoders/skia/PNGImageEncoder.cpp:69
> +    png_struct* png = png_create_write_struct(PNG_LIBPNG_VER_STRING, 0, 0, 0);

Why not just do 

if (!png)
  return false;

as before?

This way NULL will never get passed to png_create_info_struct and it simplifies the call to png_destroy_write_struct (by getting rid of the check for png).

> WebCore/platform/image-encoders/skia/PNGImageEncoder.cpp:82
> +    row.resize(imageSize.width() * bitmap.bytesPerPixel());

Why use bitmap.bytesPerPixel() when preMultipliedBGRAtoRGBA will always use 4 bytes per pixel (4 unsigned char)?

Also why not move Vector<unsigned char> row; to here? (You could even pass this size to the constructor.)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.