Diff
Modified: trunk/Tools/ChangeLog (286284 => 286285)
--- trunk/Tools/ChangeLog 2021-11-30 05:24:37 UTC (rev 286284)
+++ trunk/Tools/ChangeLog 2021-11-30 07:28:29 UTC (rev 286285)
@@ -1,3 +1,44 @@
+2021-11-29 Simon Fraser <[email protected]>
+
+ Store image resolution in layout test snapshots, and have ImageDiff read it
+ https://bugs.webkit.org/show_bug.cgi?id=233609
+
+ Reviewed by Darin Adler.
+
+ On the way to fixing bug 232525, we need to communicate to ImageDiff the resolution of
+ layout test snapshots so that it can account for the resolution when computing pixel
+ tolerance values, which should be reported in CSS pixels.
+
+ So have DumpRenderTree and WebKitTestRunner, on Cocoa platforms, add image resolution
+ metadata, where a 2x snapshot (i.e. a hidpi test) reports a horizontal and vertical
+ resolution of 144dpi.
+
+ Change ImageDiff to read these values and store them in PlatformImage; a future patch will
+ make use of the data.
+
+ * DumpRenderTree/cg/PixelDumpSupportCG.cpp:
+ (printPNG):
+ (dumpBitmap):
+ * DumpRenderTree/cg/PixelDumpSupportCG.h:
+ (BitmapContext::scaleFactor const):
+ (BitmapContext::setScaleFactor):
+ * DumpRenderTree/ios/PixelDumpSupportIOS.mm:
+ (createBitmapContextFromWebView):
+ * DumpRenderTree/mac/PixelDumpSupportMac.mm:
+ (createBitmapContextFromWebView):
+ * ImageDiff/ImageDiff.cpp:
+ (main):
+ * ImageDiff/PlatformImage.h:
+ (ImageDiff::PlatformImage::scaleFactor const):
+ * ImageDiff/cg/PlatformImageCG.cpp:
+ (ImageDiff::createImageFromDataProvider):
+ (ImageDiff::PlatformImage::createFromFile):
+ (ImageDiff::PlatformImage::createFromStdin):
+ (ImageDiff::PlatformImage::PlatformImage):
+ * WebKitTestRunner/cg/TestInvocationCG.cpp:
+ (WTR::dumpBitmap):
+ (WTR::TestInvocation::dumpPixelsAndCompareWithExpected):
+
2021-11-29 Jonathan Bedard <[email protected]>
[webkitscmpy] Guard git-webkit with __name__
Modified: trunk/Tools/DumpRenderTree/cg/PixelDumpSupportCG.cpp (286284 => 286285)
--- trunk/Tools/DumpRenderTree/cg/PixelDumpSupportCG.cpp 2021-11-30 05:24:37 UTC (rev 286284)
+++ trunk/Tools/DumpRenderTree/cg/PixelDumpSupportCG.cpp 2021-11-30 07:28:29 UTC (rev 286285)
@@ -33,7 +33,7 @@
#include "DumpRenderTree.h"
#include "PixelDumpSupport.h"
-#include <ImageIO/CGImageDestination.h>
+#include <ImageIO/ImageIO.h>
#include <algorithm>
#include <ctype.h>
#include <wtf/Assertions.h>
@@ -57,11 +57,20 @@
static const CFStringRef kUTTypePNG = CFSTR("public.png");
#endif
-static void printPNG(CGImageRef image, const char* checksum)
+static void printPNG(CGImageRef image, const char* checksum, double scaleFactor)
{
- RetainPtr<CFMutableDataRef> imageData = adoptCF(CFDataCreateMutable(0, 0));
- RetainPtr<CGImageDestinationRef> imageDest = adoptCF(CGImageDestinationCreateWithData(imageData.get(), kUTTypePNG, 1, 0));
- CGImageDestinationAddImage(imageDest.get(), image, 0);
+ auto imageData = adoptCF(CFDataCreateMutable(0, 0));
+ auto imageDest = adoptCF(CGImageDestinationCreateWithData(imageData.get(), kUTTypePNG, 1, 0));
+
+ auto propertiesDictionary = adoptCF(CFDictionaryCreateMutable(kCFAllocatorDefault, 1, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));
+ double resolutionWidth = 72.0 * scaleFactor;
+ double resolutionHeight = 72.0 * scaleFactor;
+ auto resolutionWidthNumber = adoptCF(CFNumberCreate(kCFAllocatorDefault, kCFNumberDoubleType, &resolutionWidth));
+ auto resolutionHeightNumber = adoptCF(CFNumberCreate(kCFAllocatorDefault, kCFNumberDoubleType, &resolutionHeight));
+ CFDictionarySetValue(propertiesDictionary.get(), kCGImagePropertyDPIWidth, resolutionWidthNumber.get());
+ CFDictionarySetValue(propertiesDictionary.get(), kCGImagePropertyDPIHeight, resolutionHeightNumber.get());
+
+ CGImageDestinationAddImage(imageDest.get(), image, propertiesDictionary.get());
CGImageDestinationFinalize(imageDest.get());
const UInt8* data = ""
@@ -110,7 +119,7 @@
void dumpBitmap(BitmapContext* context, const char* checksum)
{
RetainPtr<CGImageRef> image = adoptCF(CGBitmapContextCreateImage(context->cgContext()));
- printPNG(image.get(), checksum);
+ printPNG(image.get(), checksum, context->scaleFactor());
}
#if PLATFORM(COCOA)
Modified: trunk/Tools/DumpRenderTree/cg/PixelDumpSupportCG.h (286284 => 286285)
--- trunk/Tools/DumpRenderTree/cg/PixelDumpSupportCG.h 2021-11-30 05:24:37 UTC (rev 286284)
+++ trunk/Tools/DumpRenderTree/cg/PixelDumpSupportCG.h 2021-11-30 07:28:29 UTC (rev 286285)
@@ -63,6 +63,9 @@
}
CGContextRef cgContext() const { return m_context.get(); }
+
+ double scaleFactor() const { return m_scaleFactor; }
+ void setScaleFactor(double scaleFactor) { m_scaleFactor = scaleFactor; }
private:
@@ -74,7 +77,7 @@
PlatformBitmapBuffer m_buffer;
RetainPtr<CGContextRef> m_context;
-
+ double m_scaleFactor { 1.0 };
};
#if PLATFORM(COCOA)
Modified: trunk/Tools/DumpRenderTree/ios/PixelDumpSupportIOS.mm (286284 => 286285)
--- trunk/Tools/DumpRenderTree/ios/PixelDumpSupportIOS.mm 2021-11-30 05:24:37 UTC (rev 286284)
+++ trunk/Tools/DumpRenderTree/ios/PixelDumpSupportIOS.mm 2021-11-30 07:28:29 UTC (rev 286285)
@@ -81,6 +81,8 @@
if (!bitmapContext)
return nullptr;
+ bitmapContext->setScaleFactor(deviceScaleFactor);
+
CGContextDrawImage(bitmapContext->cgContext(), CGRectMake(0, 0, bufferWidth, bufferHeight), cgImage.get());
return bitmapContext;
}
Modified: trunk/Tools/DumpRenderTree/mac/PixelDumpSupportMac.mm (286284 => 286285)
--- trunk/Tools/DumpRenderTree/mac/PixelDumpSupportMac.mm 2021-11-30 05:24:37 UTC (rev 286284)
+++ trunk/Tools/DumpRenderTree/mac/PixelDumpSupportMac.mm 2021-11-30 07:28:29 UTC (rev 286285)
@@ -103,6 +103,9 @@
auto bitmapContext = createBitmapContext(pixelsWide, pixelsHigh, rowBytes, buffer);
if (!bitmapContext)
return nullptr;
+
+ bitmapContext->setScaleFactor(deviceScaleFactor);
+
CGContextRef context = bitmapContext->cgContext();
// The final scaling gets doubled on the screen capture surface when we use the hidpi backingScaleFactor value for CTM.
// This is a workaround to push the scaling back.
Modified: trunk/Tools/ImageDiff/ImageDiff.cpp (286284 => 286285)
--- trunk/Tools/ImageDiff/ImageDiff.cpp 2021-11-30 05:24:37 UTC (rev 286284)
+++ trunk/Tools/ImageDiff/ImageDiff.cpp 2021-11-30 07:28:29 UTC (rev 286285)
@@ -151,7 +151,7 @@
}
if (verbose)
- fprintf(stderr, "Comparing files actual: %s and baseline: %s\n", file1Path, file2Path);
+ fprintf(stderr, "Comparing files actual: %s (resolution %.1f) and baseline: %s (resolution %.1f)\n", file1Path, actualImage->scaleFactor(), file2Path, baselineImage->scaleFactor());
return processImages(std::move(actualImage), std::move(baselineImage), exact, printDifference);
}
@@ -199,6 +199,9 @@
}
if (actualImage && baselineImage) {
+ if (verbose)
+ fprintf(stderr, "Actual image resolution: %.1f, baseline image resolution: %.1f\n", actualImage->scaleFactor(), baselineImage->scaleFactor());
+
auto result = processImages(std::exchange(actualImage, { }), std::exchange(baselineImage, { }), exact, printDifference);
if (result != EXIT_SUCCESS)
return result;
Modified: trunk/Tools/ImageDiff/PlatformImage.h (286284 => 286285)
--- trunk/Tools/ImageDiff/PlatformImage.h 2021-11-30 05:24:37 UTC (rev 286284)
+++ trunk/Tools/ImageDiff/PlatformImage.h 2021-11-30 07:28:29 UTC (rev 286285)
@@ -43,7 +43,7 @@
#if defined(USE_CAIRO) && USE_CAIRO
PlatformImage(cairo_surface_t*);
#else
- PlatformImage(CGImageRef);
+ PlatformImage(CGImageRef, double scaleFactor = 1);
#endif
~PlatformImage();
@@ -51,6 +51,7 @@
size_t height() const;
size_t rowBytes() const;
bool hasAlpha() const;
+ double scaleFactor() const { return m_scaleFactor; }
unsigned char* pixels() const;
bool isCompatible(const PlatformImage&) const;
@@ -73,6 +74,7 @@
CGImageRef m_image;
mutable void* m_buffer { nullptr };
#endif
+ double m_scaleFactor { 1 }; // Essentially resolution, but not in DPI.
};
} // namespace ImageDiff
Modified: trunk/Tools/ImageDiff/cg/PlatformImageCG.cpp (286284 => 286285)
--- trunk/Tools/ImageDiff/cg/PlatformImageCG.cpp 2021-11-30 05:24:37 UTC (rev 286284)
+++ trunk/Tools/ImageDiff/cg/PlatformImageCG.cpp 2021-11-30 07:28:29 UTC (rev 286285)
@@ -28,7 +28,7 @@
#include <CoreGraphics/CGBitmapContext.h>
#include <CoreGraphics/CGImage.h>
-#include <ImageIO/CGImageDestination.h>
+#include <ImageIO/ImageIO.h>
#include <algorithm>
#include <stdio.h>
@@ -71,6 +71,34 @@
namespace ImageDiff {
+static std::unique_ptr<PlatformImage> createImageFromDataProvider(CGDataProviderRef dataProvider)
+{
+ double scaleFactor = 1;
+
+ auto imageSource = CGImageSourceCreateWithDataProvider(dataProvider, 0);
+ if (auto properties = CGImageSourceCopyPropertiesAtIndex(imageSource, 0, nullptr)) {
+ auto resolutionXProperty = (CFNumberRef)CFDictionaryGetValue(properties, kCGImagePropertyDPIWidth);
+ auto resolutionYProperty = (CFNumberRef)CFDictionaryGetValue(properties, kCGImagePropertyDPIHeight);
+ if (resolutionXProperty && resolutionYProperty) {
+ double resolutionX, resolutionY;
+ if (CFNumberGetValue(resolutionXProperty, kCFNumberDoubleType, &resolutionX) && CFNumberGetValue(resolutionYProperty, kCFNumberDoubleType, &resolutionY)) {
+ if (resolutionX != resolutionY)
+ fprintf(stderr, "Image has different horizontal and vertical resolutions, which is not supported");
+
+ scaleFactor = resolutionX / 72.0;
+ }
+ }
+ CFRelease(properties);
+ }
+
+ auto image = CGImageSourceCreateImageAtIndex(imageSource, 0, nullptr);
+ CFRelease(imageSource);
+ if (!image)
+ return nullptr;
+
+ return std::make_unique<PlatformImage>(image, scaleFactor);
+}
+
std::unique_ptr<PlatformImage> PlatformImage::createFromFile(const char* filePath)
{
auto dataProvider = CGDataProviderCreateWithFilename(filePath);
@@ -77,12 +105,9 @@
if (!dataProvider)
return nullptr;
- auto image = CGImageCreateWithPNGDataProvider(dataProvider, 0, false, kCGRenderingIntentDefault);
+ auto result = createImageFromDataProvider(dataProvider);
CGDataProviderRelease(dataProvider);
- if (!image)
- return nullptr;
-
- return std::make_unique<PlatformImage>(image);
+ return result;
}
std::unique_ptr<PlatformImage> PlatformImage::createFromStdin(size_t imageSize)
@@ -99,10 +124,10 @@
}
CGDataProviderRef dataProvider = CGDataProviderCreateWithCFData(data);
CFRelease(data);
- CGImageRef image = CGImageCreateWithPNGDataProvider(dataProvider, 0, false, kCGRenderingIntentDefault);
- CFRelease(dataProvider);
- return std::make_unique<PlatformImage>(image);
+ auto result = createImageFromDataProvider(dataProvider);
+ CGDataProviderRelease(dataProvider);
+ return result;
}
std::unique_ptr<PlatformImage> PlatformImage::createFromDiffData(void* data, size_t width, size_t height)
@@ -117,8 +142,9 @@
return std::make_unique<PlatformImage>(image);
}
-PlatformImage::PlatformImage(CGImageRef image)
+PlatformImage::PlatformImage(CGImageRef image, double scaleFactor)
: m_image(image)
+ , m_scaleFactor(scaleFactor)
{
}
Modified: trunk/Tools/WebKitTestRunner/cg/TestInvocationCG.cpp (286284 => 286285)
--- trunk/Tools/WebKitTestRunner/cg/TestInvocationCG.cpp 2021-11-30 05:24:37 UTC (rev 286284)
+++ trunk/Tools/WebKitTestRunner/cg/TestInvocationCG.cpp 2021-11-30 07:28:29 UTC (rev 286285)
@@ -29,7 +29,7 @@
#include "PixelDumpSupport.h"
#include "PlatformWebView.h"
#include "TestController.h"
-#include <ImageIO/CGImageDestination.h>
+#include <ImageIO/ImageIO.h>
#include <WebKit/WKImageCG.h>
#include <wtf/RetainPtr.h>
#include <wtf/SHA1.h>
@@ -107,12 +107,21 @@
snprintf(hashString, 33, "%s%02x", hashString, hash[i]);
}
-static void dumpBitmap(CGContextRef bitmapContext, const char* checksum)
+static void dumpBitmap(CGContextRef bitmapContext, const char* checksum, WKSize imageSize, WKSize windowSize)
{
auto image = adoptCF(CGBitmapContextCreateImage(bitmapContext));
auto imageData = adoptCF(CFDataCreateMutable(0, 0));
auto imageDest = adoptCF(CGImageDestinationCreateWithData(imageData.get(), kUTTypePNG, 1, 0));
- CGImageDestinationAddImage(imageDest.get(), image.get(), 0);
+
+ auto propertiesDictionary = adoptCF(CFDictionaryCreateMutable(kCFAllocatorDefault, 1, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));
+ double resolutionWidth = 72.0 * imageSize.width / windowSize.width;
+ double resolutionHeight = 72.0 * imageSize.height / windowSize.height;
+ auto resolutionWidthNumber = adoptCF(CFNumberCreate(kCFAllocatorDefault, kCFNumberDoubleType, &resolutionWidth));
+ auto resolutionHeightNumber = adoptCF(CFNumberCreate(kCFAllocatorDefault, kCFNumberDoubleType, &resolutionHeight));
+ CFDictionarySetValue(propertiesDictionary.get(), kCGImagePropertyDPIWidth, resolutionWidthNumber.get());
+ CFDictionarySetValue(propertiesDictionary.get(), kCGImagePropertyDPIHeight, resolutionHeightNumber.get());
+
+ CGImageDestinationAddImage(imageDest.get(), image.get(), propertiesDictionary.get());
CGImageDestinationFinalize(imageDest.get());
const unsigned char* data = ""
@@ -181,10 +190,12 @@
if (repaintRects)
paintRepaintRectOverlay(context.get(), imageSize, repaintRects);
+ auto windowSize = TestController::singleton().mainWebView()->windowFrame().size;
+
char actualHash[33];
computeSHA1HashStringForContext(context.get(), actualHash);
if (!compareActualHashToExpectedAndDumpResults(actualHash))
- dumpBitmap(context.get(), actualHash);
+ dumpBitmap(context.get(), actualHash, imageSize, windowSize);
}
} // namespace WTR
