Diff
Modified: trunk/Tools/ChangeLog (215175 => 215176)
--- trunk/Tools/ChangeLog 2017-04-10 10:01:46 UTC (rev 215175)
+++ trunk/Tools/ChangeLog 2017-04-10 10:16:26 UTC (rev 215176)
@@ -1,3 +1,37 @@
+2017-04-10 Carlos Garcia Campos <cgar...@igalia.com>
+
+ WTR: Avoid conversion from platform image to WKImage and then to platform image again when dumping pixel results
+ https://bugs.webkit.org/show_bug.cgi?id=170653
+
+ Reviewed by Tim Horton.
+
+ When dumping pixels from a web view snapshot, we create a platform image that is then converted to a WKImage,
+ which is a ShareableBitmap, so the image is rendered into a graphics context. Then we immediately extract the
+ platform image back from the WKImage to dump the pixels. We could avoid that conversion by taking the web
+ snapshot from TestInvocation::dumpPixelsAndCompareWithExpected().
+
+ * WebKitTestRunner/PlatformWebView.h: Add PlatformImage typedef and use it as return value of windowSnapshotImage().
+ * WebKitTestRunner/TestInvocation.cpp:
+ (WTR::TestInvocation::dumpResults): Pass the WKImage to dumpPixelsAndCompareWithExpected() only when pixel
+ results were created in the web process.
+ * WebKitTestRunner/TestInvocation.h: Make WKImage a default paramater of dumpPixelsAndCompareWithExpected().
+ * WebKitTestRunner/cairo/TestInvocationCairo.cpp:
+ (WTR::TestInvocation::dumpPixelsAndCompareWithExpected): Create the cairo surface from the given WKimage in case
+ of web contents snapshot, and use PlatformWebView::windowSnapshotImage() in case of web view snapshot.
+ * WebKitTestRunner/cg/TestInvocationCG.cpp:
+ (WTR::createCGContextFromCGImage): Changed to receive a CGImageRef and renamed.
+ (WTR::createCGContextFromImage): Get the CGImageRef from the WKImage and call createCGContextFromCGImage().
+ (WTR::paintRepaintRectOverlay): It receives now the image size instead of the WKImage.
+ (WTR::TestInvocation::dumpPixelsAndCompareWithExpected): Create the CGContextRef from the WKImage in case of web
+ contents snpashot, and use PlatformWebView::windowSnapshotImage() in case of web view snapshot.
+ * WebKitTestRunner/gtk/PlatformWebViewGtk.cpp:
+ (WTR::PlatformWebView::windowSnapshotImage): Return the cairo surface instead of creating a WKImage. Also use
+ RGB24 format to match what mac does (kCGWindowImageShouldBeOpaque).
+ * WebKitTestRunner/ios/PlatformWebViewIOS.mm:
+ (WTR::PlatformWebView::windowSnapshotImage): Return the CGImageRef instead of creating a WKImage.
+ * WebKitTestRunner/mac/PlatformWebViewMac.mm:
+ (WTR::PlatformWebView::windowSnapshotImage): Ditto.
+
2017-04-10 Chris Dumez <cdu...@apple.com>
Drop Timer::startOneShot() overload taking a double
Modified: trunk/Tools/WebKitTestRunner/PlatformWebView.h (215175 => 215176)
--- trunk/Tools/WebKitTestRunner/PlatformWebView.h 2017-04-10 10:01:46 UTC (rev 215175)
+++ trunk/Tools/WebKitTestRunner/PlatformWebView.h 2017-04-10 10:16:26 UTC (rev 215176)
@@ -27,15 +27,16 @@
#define PlatformWebView_h
#include "TestOptions.h"
-#include <WebKit/WKRetainPtr.h>
#if PLATFORM(COCOA) && !defined(BUILDING_GTK__)
#include <WebKit/WKFoundation.h>
+#include <wtf/RetainPtr.h>
OBJC_CLASS NSView;
OBJC_CLASS UIView;
OBJC_CLASS TestRunnerWKWebView;
OBJC_CLASS WKWebViewConfiguration;
OBJC_CLASS WebKitTestRunnerWindow;
+typedef struct CGImage *CGImageRef;
#if WK_API_ENABLED
typedef TestRunnerWKWebView *PlatformWKView;
@@ -43,10 +44,12 @@
typedef NSView *PlatformWKView;
#endif
typedef WebKitTestRunnerWindow *PlatformWindow;
+typedef RetainPtr<CGImageRef> PlatformImage;
#elif defined(BUILDING_GTK__)
typedef struct _GtkWidget GtkWidget;
typedef WKViewRef PlatformWKView;
typedef GtkWidget* PlatformWindow;
+typedef cairo_surface_t *PlatformImage;
#elif PLATFORM(EFL)
typedef Evas_Object* PlatformWKView;
typedef Ecore_Evas* PlatformWindow;
@@ -92,7 +95,7 @@
bool viewSupportsOptions(const TestOptions&) const;
- WKRetainPtr<WKImageRef> windowSnapshotImage();
+ PlatformImage windowSnapshotImage();
const TestOptions& options() const { return m_options; }
void changeWindowScaleIfNeeded(float newScale);
Modified: trunk/Tools/WebKitTestRunner/TestInvocation.cpp (215175 => 215176)
--- trunk/Tools/WebKitTestRunner/TestInvocation.cpp 2017-04-10 10:01:46 UTC (rev 215175)
+++ trunk/Tools/WebKitTestRunner/TestInvocation.cpp 2017-04-10 10:16:26 UTC (rev 215176)
@@ -267,7 +267,7 @@
if (m_dumpPixels) {
if (m_pixelResult)
- dumpPixelsAndCompareWithExpected(m_pixelResult.get(), m_repaintRects.get(), TestInvocation::SnapshotResultType::WebContents);
+ dumpPixelsAndCompareWithExpected(SnapshotResultType::WebContents, m_repaintRects.get(), m_pixelResult.get());
else if (m_pixelResultIsPending) {
m_gotRepaint = false;
WKPageForceRepaint(TestController::singleton().mainWebView()->page(), this, TestInvocation::forceRepaintDoneCallback);
@@ -278,8 +278,7 @@
return;
}
- WKRetainPtr<WKImageRef> windowSnapshot = TestController::singleton().mainWebView()->windowSnapshotImage();
- dumpPixelsAndCompareWithExpected(windowSnapshot.get(), m_repaintRects.get(), TestInvocation::SnapshotResultType::WebView);
+ dumpPixelsAndCompareWithExpected(SnapshotResultType::WebView, m_repaintRects.get());
}
}
Modified: trunk/Tools/WebKitTestRunner/TestInvocation.h (215175 => 215176)
--- trunk/Tools/WebKitTestRunner/TestInvocation.h 2017-04-10 10:01:46 UTC (rev 215175)
+++ trunk/Tools/WebKitTestRunner/TestInvocation.h 2017-04-10 10:16:26 UTC (rev 215176)
@@ -75,7 +75,7 @@
void dumpResults();
static void dump(const char* textToStdout, const char* textToStderr = 0, bool seenError = false);
enum class SnapshotResultType { WebView, WebContents };
- void dumpPixelsAndCompareWithExpected(WKImageRef, WKArrayRef repaintRects, SnapshotResultType);
+ void dumpPixelsAndCompareWithExpected(SnapshotResultType, WKArrayRef repaintRects, WKImageRef = nullptr);
void dumpAudio(WKDataRef);
bool compareActualHashToExpectedAndDumpResults(const char[33]);
Modified: trunk/Tools/WebKitTestRunner/cairo/TestInvocationCairo.cpp (215175 => 215176)
--- trunk/Tools/WebKitTestRunner/cairo/TestInvocationCairo.cpp 2017-04-10 10:01:46 UTC (rev 215175)
+++ trunk/Tools/WebKitTestRunner/cairo/TestInvocationCairo.cpp 2017-04-10 10:16:26 UTC (rev 215176)
@@ -106,9 +106,17 @@
cairo_destroy(context);
}
-void TestInvocation::dumpPixelsAndCompareWithExpected(WKImageRef image, WKArrayRef repaintRects, SnapshotResultType)
+void TestInvocation::dumpPixelsAndCompareWithExpected(SnapshotResultType snapshotType, WKArrayRef repaintRects, WKImageRef image)
{
- cairo_surface_t* surface = WKImageCreateCairoSurface(image);
+ cairo_surface_t* surface = nullptr;
+ switch (snapshotType) {
+ case SnapshotResultType::WebContents:
+ surface = WKImageCreateCairoSurface(image);
+ break;
+ case SnapshotResultType::WebView:
+ surface = TestController::singleton().mainWebView()->windowSnapshotImage();
+ break;
+ }
if (repaintRects)
paintRepaintRectOverlay(surface, repaintRects);
Modified: trunk/Tools/WebKitTestRunner/cg/TestInvocationCG.cpp (215175 => 215176)
--- trunk/Tools/WebKitTestRunner/cg/TestInvocationCG.cpp 2017-04-10 10:01:46 UTC (rev 215175)
+++ trunk/Tools/WebKitTestRunner/cg/TestInvocationCG.cpp 2017-04-10 10:16:26 UTC (rev 215176)
@@ -46,12 +46,10 @@
namespace WTR {
-static CGContextRef createCGContextFromImage(WKImageRef wkImage)
+static CGContextRef createCGContextFromCGImage(CGImageRef image)
{
- RetainPtr<CGImageRef> image = adoptCF(WKImageCreateCGImage(wkImage));
-
- size_t pixelsWide = CGImageGetWidth(image.get());
- size_t pixelsHigh = CGImageGetHeight(image.get());
+ size_t pixelsWide = CGImageGetWidth(image);
+ size_t pixelsHigh = CGImageGetHeight(image);
size_t rowBytes = (4 * pixelsWide + 63) & ~63;
// Creating this bitmap in the device color space should prevent any color conversion when the image of the web view is drawn into it.
@@ -60,10 +58,16 @@
if (!context)
return nullptr;
- CGContextDrawImage(context, CGRectMake(0, 0, pixelsWide, pixelsHigh), image.get());
+ CGContextDrawImage(context, CGRectMake(0, 0, pixelsWide, pixelsHigh), image);
return context;
}
+static CGContextRef createCGContextFromImage(WKImageRef wkImage)
+{
+ RetainPtr<CGImageRef> image = adoptCF(WKImageCreateCGImage(wkImage));
+ return createCGContextFromCGImage(image.get());
+}
+
void computeMD5HashStringForContext(CGContextRef bitmapContext, char hashString[33])
{
if (!bitmapContext) {
@@ -118,10 +122,8 @@
printPNG(data, dataLength, checksum);
}
-static void paintRepaintRectOverlay(CGContextRef context, WKImageRef image, WKArrayRef repaintRects)
+static void paintRepaintRectOverlay(CGContextRef context, WKSize imageSize, WKArrayRef repaintRects)
{
- WKSize imageSize = WKImageGetSize(image);
-
CGContextSaveGState(context);
// Using a transparency layer is easier than futzing with clipping.
@@ -146,14 +148,31 @@
CGContextRestoreGState(context);
}
-void TestInvocation::dumpPixelsAndCompareWithExpected(WKImageRef image, WKArrayRef repaintRects, SnapshotResultType snapshotType)
+void TestInvocation::dumpPixelsAndCompareWithExpected(SnapshotResultType snapshotType, WKArrayRef repaintRects, WKImageRef wkImage)
{
- if (!image) {
- WTFLogAlways("dumpPixelsAndCompareWithExpected: image is null\n");
- return;
+ RetainPtr<CGContextRef> context;
+ WKSize imageSize;
+
+ switch (snapshotType) {
+ case SnapshotResultType::WebContents:
+ if (!wkImage) {
+ WTFLogAlways("dumpPixelsAndCompareWithExpected: image is null\n");
+ return;
+ }
+ context = adoptCF(createCGContextFromImage(wkImage));
+ imageSize = WKImageGetSize(wkImage);
+ break;
+ case SnapshotResultType::WebView:
+ auto image = TestController::singleton().mainWebView()->windowSnapshotImage();
+ if (!image) {
+ WTFLogAlways("dumpPixelsAndCompareWithExpected: image is null\n");
+ return;
+ }
+ context = adoptCF(createCGContextFromCGImage(image.get()));
+ imageSize = WKSizeMake(CGImageGetWidth(image.get()), CGImageGetHeight(image.get()));
+ break;
}
- RetainPtr<CGContextRef> context = adoptCF(createCGContextFromImage(image));
if (!context) {
WTFLogAlways("dumpPixelsAndCompareWithExpected: context is null\n");
return;
@@ -161,7 +180,7 @@
// A non-null repaintRects array means we're doing a repaint test.
if (repaintRects)
- paintRepaintRectOverlay(context.get(), image, repaintRects);
+ paintRepaintRectOverlay(context.get(), imageSize, repaintRects);
char actualHash[33];
computeMD5HashStringForContext(context.get(), actualHash);
Modified: trunk/Tools/WebKitTestRunner/gtk/PlatformWebViewGtk.cpp (215175 => 215176)
--- trunk/Tools/WebKitTestRunner/gtk/PlatformWebViewGtk.cpp 2017-04-10 10:01:46 UTC (rev 215175)
+++ trunk/Tools/WebKitTestRunner/gtk/PlatformWebViewGtk.cpp 2017-04-10 10:16:26 UTC (rev 215176)
@@ -132,7 +132,7 @@
{
}
-WKRetainPtr<WKImageRef> PlatformWebView::windowSnapshotImage()
+cairo_surface_t* PlatformWebView::windowSnapshotImage()
{
int width = gtk_widget_get_allocated_width(GTK_WIDGET(m_view));
int height = gtk_widget_get_allocated_height(GTK_WIDGET(m_view));
@@ -140,16 +140,13 @@
while (gtk_events_pending())
gtk_main_iteration();
- cairo_surface_t* imageSurface = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, width, height);
+ cairo_surface_t* imageSurface = cairo_image_surface_create(CAIRO_FORMAT_RGB24, width, height);
cairo_t* context = cairo_create(imageSurface);
gtk_widget_draw(GTK_WIDGET(m_view), context);
cairo_destroy(context);
- WKRetainPtr<WKImageRef> wkImage = adoptWK(WKImageCreateFromCairoSurface(imageSurface, 0 /* options */));
-
- cairo_surface_destroy(imageSurface);
- return wkImage;
+ return imageSurface;
}
void PlatformWebView::didInitializeClients()
Modified: trunk/Tools/WebKitTestRunner/ios/PlatformWebViewIOS.mm (215175 => 215176)
--- trunk/Tools/WebKitTestRunner/ios/PlatformWebViewIOS.mm 2017-04-10 10:01:46 UTC (rev 215175)
+++ trunk/Tools/WebKitTestRunner/ios/PlatformWebViewIOS.mm 2017-04-10 10:16:26 UTC (rev 215176)
@@ -250,8 +250,15 @@
// Retina only surface.
}
-WKRetainPtr<WKImageRef> PlatformWebView::windowSnapshotImage()
+#if !USE(IOSURFACE)
+static void releaseDataProviderData(void* info, const void*, size_t)
{
+ CARenderServerDestroyBuffer(static_cast<CARenderServerBufferRef>(info));
+}
+#endif
+
+RetainPtr<CGImageRef> PlatformWebView::windowSnapshotImage()
+{
BEGIN_BLOCK_OBJC_EXCEPTIONS;
#if USE(IOSURFACE)
return nullptr;
@@ -279,14 +286,11 @@
size_t rowBytes = CARenderServerGetBufferRowBytes(buffer);
static CGColorSpaceRef sRGBSpace = CGColorSpaceCreateWithName(kCGColorSpaceSRGB);
- RetainPtr<CGDataProviderRef> provider = adoptCF(CGDataProviderCreateWithData(0, data, CARenderServerGetBufferDataSize(buffer), nullptr));
+ RetainPtr<CGDataProviderRef> provider = adoptCF(CGDataProviderCreateWithData(0, data, CARenderServerGetBufferDataSize(buffer), releaseDataProviderData));
RetainPtr<CGImageRef> cgImage = adoptCF(CGImageCreate(bufferWidth, bufferHeight, 8, 32, rowBytes, sRGBSpace, kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Host, provider.get(), 0, false, kCGRenderingIntentDefault));
- WKRetainPtr<WKImageRef> result = adoptWK(WKImageCreateFromCGImage(cgImage.get(), 0));
- CARenderServerDestroyBuffer(buffer);
-
- return result;
+ return cgImage;
#endif
END_BLOCK_OBJC_EXCEPTIONS;
}
Modified: trunk/Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm (215175 => 215176)
--- trunk/Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm 2017-04-10 10:01:46 UTC (rev 215175)
+++ trunk/Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm 2017-04-10 10:16:26 UTC (rev 215176)
@@ -271,7 +271,7 @@
[m_window makeFirstResponder:platformView()];
}
-WKRetainPtr<WKImageRef> PlatformWebView::windowSnapshotImage()
+RetainPtr<CGImageRef> PlatformWebView::windowSnapshotImage()
{
[platformView() display];
CGWindowImageOption options = kCGWindowImageBoundsIgnoreFraming | kCGWindowImageShouldBeOpaque;
@@ -279,10 +279,7 @@
if ([m_window backingScaleFactor] == 1)
options |= kCGWindowImageNominalResolution;
- RetainPtr<CGImageRef> windowSnapshotImage = adoptCF(CGWindowListCreateImage(CGRectNull, kCGWindowListOptionIncludingWindow, [m_window windowNumber], options));
-
- // windowSnapshotImage will be in GenericRGB, as we've set the main display's color space to GenericRGB.
- return adoptWK(WKImageCreateFromCGImage(windowSnapshotImage.get(), 0));
+ return adoptCF(CGWindowListCreateImage(CGRectNull, kCGWindowListOptionIncludingWindow, [m_window windowNumber], options));
}
bool PlatformWebView::viewSupportsOptions(const TestOptions& options) const
