Bug 21124 – --pixel should be ignored for tests that use dumpAsText()

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Pierre-Olivier Latour]

If I understand things right, tests that use the dumpAsText() instead of dumping the render tree are supposed to be platform agnostic, so their results are in the same directory as the tests instead of in "platform".

Pixel test are by definition not platform agnostic (destination screen color profile, font rendering differences, etc...), so their result should always be in "platform".

If you combine these two assumptions, then having a test that uses dumpAsText() also have its rendering be pixel-tested does not make sense.
But today, if you run "WebKitTools/Scripts/run-webkit-tests --pixel LayoutTests/", it will generate images for such tests.

For instance, 3 out of the 6 tests in LayoutTests/transforms/2d are dumpAsText() tests and do not print the render tree:
- compound-2d-transforms.html
- transform-2d.html
- transform-accuracy.html
But since there are no "base" images in LayoutTests/platform/mac/transforms/2d for these tests, DRT always creates news ones in LayoutTests/transforms/2d and this pollutes the directory.

I think the DRT tool when running in --pixel-test mode should not compare pixel rendering of tests that use dumpAsText().

[Mark Rowe (bdash)]

I thought that this was already the case.  Perhaps we broke this behaviour somewhere along the way.

[Pierre-Olivier Latour]

We want to add a bunch of tests for transforms, transitions and animations, and some of them require pixel testing. Having this fixed sooner than later would help, thanks.

[Pierre-Olivier Latour]

For some reason, you can't just bypass dumpWebViewAsPixelsAndCompareWithExpected() completely in DRT or the tests hangs.

Here's a diff that seems to fix the issue (it's a one-line change, but because of (re)tabbing a bunch of lines, it makes it look quite bigger):

Index: WebKitTools/DumpRenderTree/cg/PixelDumpSupportCG.cpp
===================================================================
--- WebKitTools/DumpRenderTree/cg/PixelDumpSupportCG.cpp	(revision 36958)
+++ WebKitTools/DumpRenderTree/cg/PixelDumpSupportCG.cpp	(working copy)
@@ -106,21 +106,25 @@
     if (gLayoutTestController->dumpSelectionRect())
         drawSelectionRect(context.get(), getSelectionRect());
 #endif
+    
+    // Only process the rendered image if have a test that is not platform-agnositc i.e. doesn't use "dumpAsText()"
+    if(!gLayoutTestController->dumpAsText()) {
 
-    // Compute the actual hash to compare to the expected image's hash.
-    char actualHash[33];
-    getMD5HashStringForBitmap(context.get(), actualHash);
-    printf("\nActualHash: %s\n", actualHash);
+        // Compute the actual hash to compare to the expected image's hash.
+        char actualHash[33];
+        getMD5HashStringForBitmap(context.get(), actualHash);
+        printf("\nActualHash: %s\n", actualHash);
 
-    // FIXME: We should compare the actualHash to the expected hash here and
-    // only set dumpImage to true if they don't match, but DRT doesn't have
-    // enough information currently to find the expected checksum file.
-    bool dumpImage = true;
+        // FIXME: We should compare the actualHash to the expected hash here and
+        // only set dumpImage to true if they don't match, but DRT doesn't have
+        // enough information currently to find the expected checksum file.
+        bool dumpImage = true;
 
-    if (dumpImage) {
-        RetainPtr<CGImageRef> image(AdoptCF, CGBitmapContextCreateImage(context.get()));
-        printPNG(image.get());
+        if (dumpImage) {
+            RetainPtr<CGImageRef> image(AdoptCF, CGBitmapContextCreateImage(context.get()));
+            printPNG(image.get());
+        }
     }
-
+    
     printf("#EOF\n");
 }

[Simon Fraser (smfr)]

Created attachment 23965 [details]
Patch, changelog

Comment on attachment 23965 [details]
Patch, changelog

r=me

[Simon Fraser (smfr)]

	M	WebKitTools/DumpRenderTree/mac/DumpRenderTree.mm
	M	WebKitTools/ChangeLog
	M	WebKitTools/Scripts/run-webkit-tests
r37129 = 783a6ba5d73a582590947cb76f7859d8d42b4f9c (trunk)