Diff
Modified: trunk/Tools/ChangeLog (104350 => 104351)
--- trunk/Tools/ChangeLog 2012-01-06 23:28:47 UTC (rev 104350)
+++ trunk/Tools/ChangeLog 2012-01-06 23:34:53 UTC (rev 104351)
@@ -1,3 +1,32 @@
+2012-01-06 Simon Fraser <simon.fra...@apple.com>
+
+ Pixel results from DumpRenderTree and WebKitTestRunner don't match because of colorspace issues
+ https://bugs.webkit.org/show_bug.cgi?id=75662
+
+ Reviewed by Dan Bernstein.
+
+ The pixel results generated by DumpRenderTree and WebKitTestRunner differed because
+ of color profile issues. Fix by keeping everything in device RGB and ensuring that the
+ test window uses the main display's color space, so that the pixel values in the bitmap
+ that gets checksummed are identical to the CSS colors.
+
+ Removed the code that switches the display profile in DRT, since that is no longer required.
+
+ * DumpRenderTree/PixelDumpSupport.h: Remove unused function declarations.
+ * DumpRenderTree/mac/DumpRenderTree.mm:
+ (crashHandler): Remove code that switches display profiles.
+ (prepareConsistentTestingEnvironment): Ditto.
+ (dumpRenderTree): Ditto.
+ * DumpRenderTree/mac/PixelDumpSupportMac.mm: Ditto.
+ (createBitmapContext): Use DeviceRGB for the bitmap colorspace.
+ (createBitmapContextFromWebView): Add comment about the colorspace handling.
+ * WebKitTestRunner/cg/TestInvocationCG.cpp:
+ (WTR::createCGContextFromImage): Use a RetainPtr, add comment.
+ (WTR::computeMD5HashStringForContext): Simplify the #ifdefs around the braces.
+ * WebKitTestRunner/mac/PlatformWebViewMac.mm:
+ (WTR::PlatformWebView::PlatformWebView): Set the window's colorspace to that of the main display.
+ (WTR::PlatformWebView::windowSnapshotImage): Add comment about colorspaces.
+
2012-01-06 David Kilzer <ddkil...@apple.com>
run-api-tests: specify individual suites and tests on the command-line
Modified: trunk/Tools/DumpRenderTree/PixelDumpSupport.h (104350 => 104351)
--- trunk/Tools/DumpRenderTree/PixelDumpSupport.h 2012-01-06 23:28:47 UTC (rev 104350)
+++ trunk/Tools/DumpRenderTree/PixelDumpSupport.h 2012-01-06 23:34:53 UTC (rev 104351)
@@ -42,14 +42,4 @@
void dumpWebViewAsPixelsAndCompareWithExpected(const std::string& expectedHash);
void printPNG(const unsigned char* data, const size_t dataLength, const char* checksum);
-#if PLATFORM(MAC)
-
-// Can be used as a signal handler
-void restoreMainDisplayColorProfile(int ignored);
-
-// May change your color space, requiring a call to restoreMainDisplayColorProfile
-void setupMainDisplayColorProfile();
-
-#endif
-
#endif // PixelDumpSupport_h
Modified: trunk/Tools/DumpRenderTree/mac/DumpRenderTree.mm (104350 => 104351)
--- trunk/Tools/DumpRenderTree/mac/DumpRenderTree.mm 2012-01-06 23:28:47 UTC (rev 104350)
+++ trunk/Tools/DumpRenderTree/mac/DumpRenderTree.mm 2012-01-06 23:34:53 UTC (rev 104351)
@@ -699,7 +699,6 @@
char *signalName = strsignal(sig);
write(STDERR_FILENO, signalName, strlen(signalName));
write(STDERR_FILENO, "\n", 1);
- restoreMainDisplayColorProfile(0);
exit(128 + sig);
}
@@ -810,8 +809,6 @@
adjustFonts();
registerMockScrollbars();
- if (dumpPixels)
- setupMainDisplayColorProfile();
allocateGlobalControllers();
makeLargeMallocFailSilently();
@@ -879,9 +876,6 @@
CFRelease(disallowedURLs);
disallowedURLs = 0;
}
-
- if (dumpPixels)
- restoreMainDisplayColorProfile(0);
}
int main(int argc, const char *argv[])
Modified: trunk/Tools/DumpRenderTree/mac/PixelDumpSupportMac.mm (104350 => 104351)
--- trunk/Tools/DumpRenderTree/mac/PixelDumpSupportMac.mm 2012-01-06 23:28:47 UTC (rev 104350)
+++ trunk/Tools/DumpRenderTree/mac/PixelDumpSupportMac.mm 2012-01-06 23:34:53 UTC (rev 104351)
@@ -44,63 +44,6 @@
#import <WebKit/WebKit.h>
#import <WebKit/WebViewPrivate.h>
-
-// To ensure pixel tests consistency, we need to always render in the same colorspace.
-// Unfortunately, because of AppKit / WebKit constraints, we can't render directly in the colorspace of our choice.
-// This implies we have to temporarily change the profile of the main display to the colorspace we want to render into.
-// We also need to make sure the CGBitmapContext we return is in that same colorspace.
-
-#define PROFILE_PATH "/System/Library/ColorSync/Profiles/Generic RGB Profile.icc" // FIXME: This cannot be more than CS_MAX_PATH (256 characters)
-
-static CMProfileLocation sInitialProfileLocation; // The locType field is initialized to 0 which is the same as cmNoProfileBase
-
-void restoreMainDisplayColorProfile(int ignored)
-{
- // This is used as a signal handler, and thus the calls into ColorSync are unsafe
- // But we might as well try to restore the user's color profile, we're going down anyway...
- if (sInitialProfileLocation.locType != cmNoProfileBase) {
- const CMDeviceScope scope = { kCFPreferencesCurrentUser, kCFPreferencesCurrentHost };
- int error = CMSetDeviceProfile(cmDisplayDeviceClass, (CMDeviceID)kCGDirectMainDisplay, &scope, cmDefaultProfileID, &sInitialProfileLocation);
- if (error)
- fprintf(stderr, "Failed to restore initial color profile for main display! Open System Preferences > Displays > Color and manually re-select the profile. (Error: %i)\n", error);
- sInitialProfileLocation.locType = cmNoProfileBase;
- }
-}
-
-void setupMainDisplayColorProfile()
-{
- const CMDeviceScope scope = { kCFPreferencesCurrentUser, kCFPreferencesCurrentHost };
- int error;
-
- CMProfileRef profile = ""
- error = CMGetProfileByAVID((CMDisplayIDType)kCGDirectMainDisplay, &profile);
- if (!error) {
- UInt32 size = sizeof(CMProfileLocation);
- error = NCMGetProfileLocation(profile, &sInitialProfileLocation, &size);
- CMCloseProfile(profile);
- }
- if (error) {
- fprintf(stderr, "Failed to retrieve current color profile for main display, thus it won't be changed. Many pixel tests may fail as a result. (Error: %i)\n", error);
- sInitialProfileLocation.locType = cmNoProfileBase;
- return;
- }
-
- CMProfileLocation location;
- location.locType = cmPathBasedProfile;
- strcpy(location.u.pathLoc.path, PROFILE_PATH);
- error = CMSetDeviceProfile(cmDisplayDeviceClass, (CMDeviceID)kCGDirectMainDisplay, &scope, cmDefaultProfileID, &location);
- if (error) {
- fprintf(stderr, "Failed to set color profile for main display! Many pixel tests may fail as a result. (Error: %i)\n", error);
- sInitialProfileLocation.locType = cmNoProfileBase;
- return;
- }
-
- // Other signals are handled in installSignalHandlers() which also calls restoreMainDisplayColorProfile()
- signal(SIGINT, restoreMainDisplayColorProfile);
- signal(SIGHUP, restoreMainDisplayColorProfile);
- signal(SIGTERM, restoreMainDisplayColorProfile);
-}
-
static PassRefPtr<BitmapContext> createBitmapContext(size_t pixelsWide, size_t pixelsHigh, size_t& rowBytes, void*& buffer)
{
rowBytes = (4 * pixelsWide + 63) & ~63; // Use a multiple of 64 bytes to improve CG performance
@@ -109,19 +52,9 @@
if (!buffer)
return 0;
- static CGColorSpaceRef colorSpace = 0;
- if (!colorSpace) {
- CMProfileLocation location;
- location.locType = cmPathBasedProfile;
- strcpy(location.u.pathLoc.path, PROFILE_PATH);
- CMProfileRef profile;
- if (CMOpenProfile(&profile, &location) == noErr) {
- colorSpace = CGColorSpaceCreateWithPlatformColorSpace(profile);
- CMCloseProfile(profile);
- }
- }
-
- CGContextRef context = CGBitmapContextCreate(buffer, pixelsWide, pixelsHigh, 8, rowBytes, colorSpace, kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Host); // Use ARGB8 on PPC or BGRA8 on X86 to improve CG performance
+ // Creating this bitmap in the device color space prevents any color conversion when the image of the web view is drawn into it.
+ RetainPtr<CGColorSpaceRef> colorSpace(AdoptCF, CGColorSpaceCreateDeviceRGB());
+ CGContextRef context = CGBitmapContextCreate(buffer, pixelsWide, pixelsHigh, 8, rowBytes, colorSpace.get(), kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Host); // Use ARGB8 on PPC or BGRA8 on X86 to improve CG performance
if (!context) {
free(buffer);
return 0;
@@ -207,7 +140,7 @@
[view display];
// Ask the window server to provide us a composited version of the *real* window content including surfaces (i.e. OpenGL content)
- // Note that the returned image might differ very slightly from the window backing because of dithering artifacts in the window server compositor
+ // Note that the returned image might differ very slightly from the window backing because of dithering artifacts in the window server compositor.
CGImageRef image = CGWindowListCreateImage(CGRectNull, kCGWindowListOptionIncludingWindow, [[view window] windowNumber], kCGWindowImageBoundsIgnoreFraming | kCGWindowImageShouldBeOpaque);
CGContextDrawImage(context, CGRectMake(0, 0, CGImageGetWidth(image), CGImageGetHeight(image)), image);
CGImageRelease(image);
Modified: trunk/Tools/WebKitTestRunner/cg/TestInvocationCG.cpp (104350 => 104351)
--- trunk/Tools/WebKitTestRunner/cg/TestInvocationCG.cpp 2012-01-06 23:28:47 UTC (rev 104350)
+++ trunk/Tools/WebKitTestRunner/cg/TestInvocationCG.cpp 2012-01-06 23:34:53 UTC (rev 104351)
@@ -59,9 +59,9 @@
size_t rowBytes = (4 * pixelsWide + 63) & ~63;
void* buffer = calloc(pixelsHigh, rowBytes);
- CGColorSpaceRef colorSpace = CGColorSpaceCreateDeviceRGB();
- CGContextRef context = CGBitmapContextCreate(buffer, pixelsWide, pixelsHigh, 8, rowBytes, colorSpace, kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Host);
- CGColorSpaceRelease(colorSpace);
+ // Creating this bitmap in the device color space should prevent any color conversion when the image of the web view is drawn into it.
+ RetainPtr<CGColorSpaceRef> colorSpace(AdoptCF, CGColorSpaceCreateDeviceRGB());
+ CGContextRef context = CGBitmapContextCreate(buffer, pixelsWide, pixelsHigh, 8, rowBytes, colorSpace.get(), kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Host);
if (flip == FlipGraphicsContext) {
CGContextSaveGState(context);
@@ -95,15 +95,14 @@
md5.addBytes(buffer);
bitmapData += bytesPerRow;
}
- } else {
+ } else
#endif
+ {
for (unsigned row = 0; row < pixelsHigh; row++) {
md5.addBytes(bitmapData, 4 * pixelsWide);
bitmapData += bytesPerRow;
}
-#if PLATFORM(MAC)
}
-#endif
Vector<uint8_t, 16> hash;
md5.checksum(hash);
Modified: trunk/Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm (104350 => 104351)
--- trunk/Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm 2012-01-06 23:28:47 UTC (rev 104350)
+++ trunk/Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm 2012-01-06 23:34:53 UTC (rev 104351)
@@ -56,7 +56,7 @@
NSRect windowRect = NSOffsetRect(rect, -10000, [(NSScreen *)[[NSScreen screens] objectAtIndex:0] frame].size.height - rect.size.height + 10000);
m_window = [[WebKitTestRunnerWindow alloc] initWithContentRect:windowRect styleMask:NSBorderlessWindowMask backing:NSBackingStoreBuffered defer:YES];
m_window.platformWebView = this;
- [m_window setColorSpace:[NSColorSpace genericRGBColorSpace]];
+ [m_window setColorSpace:[[NSScreen mainScreen] colorSpace]];
[[m_window contentView] addSubview:m_view];
[m_window orderBack:nil];
[m_window setReleasedWhenClosed:NO];
@@ -132,6 +132,9 @@
{
[m_view display];
RetainPtr<CGImageRef> windowSnapshotImage(AdoptCF, CGWindowListCreateImage(CGRectNull, kCGWindowListOptionIncludingWindow, [m_window windowNumber], kCGWindowImageBoundsIgnoreFraming | kCGWindowImageShouldBeOpaque));
+
+ // windowSnapshotImage will be in the display's color space, but WKImageCreateFromCGImage() will draw
+ // this image into a GenericRGB bitmap context, so the returned image is GenericRGB.
return adoptWK(WKImageCreateFromCGImage(windowSnapshotImage.get(), 0));
}
