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.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@104351 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent e0f6b2a6
2012-01-06 Simon Fraser <simon.fraser@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 <ddkilzer@apple.com>
run-api-tests: specify individual suites and tests on the command-line
......@@ -42,14 +42,4 @@ void dumpBitmap(BitmapContext*, const char* checksum);
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
......@@ -699,7 +699,6 @@ static void crashHandler(int sig)
char *signalName = strsignal(sig);
write(STDERR_FILENO, signalName, strlen(signalName));
write(STDERR_FILENO, "\n", 1);
restoreMainDisplayColorProfile(0);
exit(128 + sig);
}
......@@ -810,8 +809,6 @@ static void prepareConsistentTestingEnvironment()
adjustFonts();
registerMockScrollbars();
if (dumpPixels)
setupMainDisplayColorProfile();
allocateGlobalControllers();
makeLargeMallocFailSilently();
......@@ -879,9 +876,6 @@ void dumpRenderTree(int argc, const char *argv[])
CFRelease(disallowedURLs);
disallowedURLs = 0;
}
if (dumpPixels)
restoreMainDisplayColorProfile(0);
}
int main(int argc, const char *argv[])
......
......@@ -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 = 0;
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 @@ static PassRefPtr<BitmapContext> createBitmapContext(size_t pixelsWide, size_t p
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 @@ PassRefPtr<BitmapContext> createBitmapContextFromWebView(bool onscreen, bool inc
[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);
......
......@@ -59,9 +59,9 @@ static CGContextRef createCGContextFromImage(WKImageRef wkImage, FlipGraphicsCon
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 @@ void computeMD5HashStringForContext(CGContextRef bitmapContext, char hashString[
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);
......
......@@ -56,7 +56,7 @@ PlatformWebView::PlatformWebView(WKContextRef contextRef, WKPageGroupRef pageGro
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 @@ WKRetainPtr<WKImageRef> PlatformWebView::windowSnapshotImage()
{
[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));
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment