Summary: | Implement pixel test support on Windows | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Roben (:aroben) <aroben> | ||||||||||||||
Component: | Tools / Tests | Assignee: | Adam Roben (:aroben) <aroben> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | ddkilzer, eric, sam | ||||||||||||||
Priority: | P2 | Keywords: | InRadar, PlatformOnly | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | PC | ||||||||||||||||
OS: | Windows XP | ||||||||||||||||
Attachments: |
|
Description
Adam Roben (:aroben)
2007-11-25 12:41:39 PST
Created attachment 17511 [details]
patch with changelog
Comment on attachment 17511 [details]
patch with changelog
I'm going to break this up into some smaller pieces.
Created attachment 17516 [details]
[1/5] Make Windows DRT stop changing LF into CRLF
Reviewed by NOBODY (OOPS!).
* DumpRenderTree/win/DumpRenderTree.cpp:
(main): Put stdout in binary mode.
* Scripts/run-webkit-tests: Remove the CRLF hack.
---
WebKitTools/ChangeLog | 10 ++++++++++
WebKitTools/DumpRenderTree/win/DumpRenderTree.cpp | 4 ++++
WebKitTools/Scripts/run-webkit-tests | 1 -
3 files changed, 14 insertions(+), 1 deletions(-)
Created attachment 17517 [details]
[2/5] Clean up Windows DRT's option parsing a little bit
Reviewed by NOBODY (OOPS!).
* DumpRenderTree/win/DumpRenderTree.cpp:
(main): Put non-option arguments into a Vector.
---
WebKitTools/ChangeLog | 9 +++++++
WebKitTools/DumpRenderTree/win/DumpRenderTree.cpp | 27 +++++++++++----------
2 files changed, 23 insertions(+), 13 deletions(-)
Created attachment 17518 [details] [3/5] Implement pixel dumping in Windows DRT Part of http://bugs.webkit.org/show_bug.cgi?id=16133 <rdar://5071708> Reviewed by NOBODY (OOPS!). * DumpRenderTree/cg/PixelDumpSupportCG.cpp: Added. (printPNG): Dumps a CGImageRef as a PNG to stdout, along with a Content-Length header. (getMD5HashStringForBitmap): (dumpWebViewAsPixelsAndCompareWithExpected): * DumpRenderTree/cg/PixelDumpSupportCG.h: Copied from WebKitTools/DumpRenderTree/mac/DumpRenderTreePasteboard.h. * DumpRenderTree/win/DumpRenderTree.cpp: (dump): Do a pixel dump if requested. (main): Parse pixel test options. * DumpRenderTree/win/DumpRenderTree.vcproj: Added new files and added the cg/ subdirectory to the include path. * DumpRenderTree/win/MD5.cpp: Added. Windows MD5 functions aren't available in a header or import library, so we have to go through this LoadLibrary/GetProcAddress dance to use them. (cryptDLL): (init): (update): (final): (MD5_Init): (MD5_Update): (MD5_Final): * DumpRenderTree/win/MD5.h: Added. * DumpRenderTree/win/PixelDumpSupport.h: Added. This file should be moved up to the top level to share it with Mac eventually. * DumpRenderTree/win/PixelDumpSupportWin.cpp: Added. (getBitmapContextFromWebView): Forces the WebView to paint using a WM_PRINTCLIENT message, and puts the result in a CGBitmapContext. --- WebKitTools/ChangeLog | 37 +++++++ .../DumpRenderTree/cg/PixelDumpSupportCG.cpp | 105 ++++++++++++++++++++ WebKitTools/DumpRenderTree/cg/PixelDumpSupportCG.h | 41 ++++++++ WebKitTools/DumpRenderTree/win/DumpRenderTree.cpp | 21 ++++ .../DumpRenderTree/win/DumpRenderTree.vcproj | 34 ++++++- WebKitTools/DumpRenderTree/win/MD5.cpp | 77 ++++++++++++++ WebKitTools/DumpRenderTree/win/MD5.h | 45 +++++++++ WebKitTools/DumpRenderTree/win/PixelDumpSupport.h | 36 +++++++ .../DumpRenderTree/win/PixelDumpSupportWin.cpp | 67 +++++++++++++ 9 files changed, 458 insertions(+), 5 deletions(-) Created attachment 17519 [details]
[4/5] Fix image diff link generation on Windows
Reviewed by NOBODY (OOPS!).
* Scripts/run-webkit-tests: Removed unnecessary and incorrect calls
to toURL.
---
WebKitTools/ChangeLog | 9 +++++++++
WebKitTools/Scripts/run-webkit-tests | 6 +++---
2 files changed, 12 insertions(+), 3 deletions(-)
Created attachment 17520 [details] [5/5] Fix Bug 16133: Implement pixel test support on Windows WebKit/win: Add ImageDiff.vcproj to WebKit.sln Reviewed by NOBODY (OOPS!). * WebKit.vcproj/WebKit.sln: WebKitTools: Port ImageDiff to CG and C++ Final part of http://bugs.webkit.org/show_bug.cgi?id=16133 <rdar://5071708> Reviewed by NOBODY (OOPS!). * DumpRenderTree/DumpRenderTree.sln: Added ImageDiff.vcproj. * DumpRenderTree/cg/ImageDiff.cpp: Added. (main): (createImageFromStdin): (compareImages): (getDifferenceBitmap): (computePercentageDifferent): * DumpRenderTree/win/ImageDiff.vcproj: Added. --- WebKit/win/ChangeLog | 8 + WebKit/win/WebKit.vcproj/WebKit.sln | 21 ++- WebKitTools/ChangeLog | 18 ++ WebKitTools/DumpRenderTree/DumpRenderTree.sln | 8 + WebKitTools/DumpRenderTree/cg/ImageDiff.cpp | 203 +++++++++++++++++++++++ WebKitTools/DumpRenderTree/win/ImageDiff.vcproj | 182 ++++++++++++++++++++ 6 files changed, 435 insertions(+), 5 deletions(-) Comment on attachment 17518 [details]
[3/5] Implement pixel dumping in Windows DRT
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
Comment on attachment 17520 [details] [5/5] Fix Bug 16133: Implement pixel test support on Windows I can't image that forwarding the main function is helpful. +/* prototypes */ +int main(int argc, const char* argv[]); Please use c++ style casts here. + percentage = (float)((int)(percentage * 100.0f)) / 100.0f; // round to 2 decimal places Please use ASSERT. + assert(!(bitmapInfo & kCGImageAlphaFirst)); + assert(!(bitmapInfo & kCGBitmapFloatComponents)); r=me (In reply to comment #8) > (From update of attachment 17518 [details] [edit]) > Also, are you sure it's a good idea to return a RetainPtr? That feels weird to me. It probably feels weird to you because we don't have a "PassRetainPtr" (the equivalent of PassRefPtr for CF types). All this means is that we'll do an extra CFRetain/CFRelease when the function returns. Since these are not performance critical functions, I think the extra retain count churn is worth the lifetime guarantees that come along with using RetainPtr as the return type. Comment on attachment 17516 [details] [1/5] Make Windows DRT stop changing LF into CRLF Landed as r28019 Comment on attachment 17517 [details] [2/5] Clean up Windows DRT's option parsing a little bit Landed as r28020 Comment on attachment 17518 [details] [3/5] Implement pixel dumping in Windows DRT Laned as r28021 Comment on attachment 17519 [details] [4/5] Fix image diff link generation on Windows Landed as r28022 Comment on attachment 17520 [details] [5/5] Fix Bug 16133: Implement pixel test support on Windows Landed as r28023 |