[webkit-reviews] review granted: [Bug 16133] Implement pixel test support on Windows : [Attachment 17518] [3/5] Implement pixel dumping in Windows DRT

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Nov 25 14:57:38 PST 2007


Sam Weinig <sam at webkit.org> has granted Adam Roben <aroben at apple.com>'s request
for review:
Bug 16133: Implement pixel test support on Windows
http://bugs.webkit.org/show_bug.cgi?id=16133

Attachment 17518: [3/5] 	Implement pixel dumping in Windows DRT
http://bugs.webkit.org/attachment.cgi?id=17518&action=edit

------- Additional Comments from Sam Weinig <sam at webkit.org>
Style wise, this should be the first #include (we should really add a
config.h).
+#include "PixelDumpSupportCG.h"

You should include <wtf/StringExtras.h> instead of doing this #define.
+#define snprintf _snprintf


Please use the WTF ASSERT
+    assert(bitsPerPixel == 32); // ImageDiff assumes 32 bit RGBA, we must as
well.

+    assert(bytesPerRow >= (pixelsWide * bytesPerPixel));

In PixelDumpSupportCG.h it doesn't look like you need to #include
<wtf/Platform.h>.  Also, are you sure it's a good idea to return a RetainPtr? 
That feels weird to me.

None of these are show-stoppers.  r=me


More information about the webkit-reviews mailing list