Fix up screenshot saving for trace tests.

src/tests/BUILD.gn

@@ -238,10 +238,7 @@ template("angle_perftests_common") {
       "perf_tests/third_party/perf/perf_test.cc",
       "perf_tests/third_party/perf/perf_test.h",
     ]
-    deps = [
-      "$angle_jsoncpp_dir:jsoncpp",
-      "$angle_root/util:angle_png_utils",
-    ]
+    deps = [ "$angle_jsoncpp_dir:jsoncpp" ]
     public_deps = [ "${invoker.test_utils}" ]
     public_configs += [ "${angle_root}:libANGLE_config" ]
   }
@@ -328,6 +325,7 @@ if (is_win || is_linux || is_android || is_mac || is_fuchsia) {
       deps = [
         ":angle_perftests_shared",
         "$angle_root:angle_compression",
+        "$angle_root/util:angle_png_utils",
       ]
       suppressed_configs +=
           [ "$angle_root:constructor_and_destructor_warnings" ]

src/tests/perf_tests/ANGLEPerfTest.cpp

@@ -16,7 +16,6 @@
 #include "common/utilities.h"
 #include "third_party/perf/perf_test.h"
 #include "third_party/trace_event/trace_event.h"
-#include "util/png_utils.h"
 #include "util/shader_utils.h"
 #include "util/test_utils.h"
 
@@ -565,6 +564,16 @@ void ANGLERenderTest::SetUp()
         FAIL() << "Please initialize 'iterationsPerStep'.";
         // FAIL returns.
     }
+
+    // Capture a screenshot if enabled.
+    if (gScreenShotDir != nullptr)
+    {
+        std::stringstream screenshotNameStr;
+        screenshotNameStr << gScreenShotDir << GetPathSeparator() << "angle" << mBackend << "_"
+                          << mStory << ".png";
+        std::string screenshotName = screenshotNameStr.str();
+        saveScreenshot(screenshotName);
+    }
 }
 
 void ANGLERenderTest::TearDown()
@@ -655,9 +664,6 @@ void ANGLERenderTest::step()
     {
         drawBenchmark();
 
-        // Saves a screenshot. The test will also exit early if we're taking screenshots.
-        saveScreenshotIfEnabled();
-
         // Swap is needed so that the GPU driver will occasionally flush its
         // internal command queue to the GPU. This is enabled for null back-end
         // devices because some back-ends (e.g. Vulkan) also accumulate internal
@@ -758,33 +764,6 @@ std::vector<TraceEvent> &ANGLERenderTest::getTraceEventBuffer()
     return mTraceEventBuffer;
 }
 
-void ANGLERenderTest::saveScreenshotIfEnabled()
-{
-    if (gScreenShotDir == nullptr)
-    {
-        return;
-    }
-
-    std::stringstream screenshotNameStr;
-    screenshotNameStr << gScreenShotDir << GetPathSeparator() << "angle" << mBackend << "_"
-                      << mStory << ".png";
-    std::string screenshotName = screenshotNameStr.str();
-
-    // RGBA 4-byte data.
-    std::vector<uint8_t> pixelData(mTestParams.windowWidth * mTestParams.windowHeight * 4);
-
-    glBindFramebuffer(GL_FRAMEBUFFER, 0);
-    glReadPixels(0, 0, mTestParams.windowWidth, mTestParams.windowHeight, GL_RGBA, GL_UNSIGNED_BYTE,
-                 pixelData.data());
-
-    angle::SavePNG(screenshotName.c_str(), "ANGLE Screenshot", mTestParams.windowWidth,
-                   mTestParams.windowHeight, pixelData);
-
-    // Early exit.
-    abortTest();
-    mSkipTest = true;
-}
-
 namespace angle
 {
 double GetHostTimeSeconds()

src/tests/perf_tests/ANGLEPerfTest.h

@@ -85,6 +85,9 @@ class ANGLEPerfTest : public testing::Test, angle::NonCopyable
     unsigned int getNumStepsPerformed() const { return mNumStepsPerformed; }
     void doRunLoop(double maxRunTime);
 
+    // Overriden in trace perf tests.
+    virtual void saveScreenshot(const std::string &screenshotName) {}
+
     std::string mName;
     std::string mBackend;
     std::string mStory;
@@ -154,8 +157,6 @@ class ANGLERenderTest : public ANGLEPerfTest
 
     bool mIsTimestampQueryAvailable;
 
-    void saveScreenshotIfEnabled();
-
   private:
     void SetUp() override;
     void TearDown() override;

src/tests/perf_tests/TracePerfTest.cpp

@@ -14,6 +14,7 @@
 #include "tests/perf_tests/DrawCallPerfParams.h"
 #include "util/egl_loader_autogen.h"
 #include "util/frame_capture_utils.h"
+#include "util/png_utils.h"
 
 #include "restricted_traces/restricted_traces_autogen.h"
 
@@ -90,6 +91,7 @@ class TracePerfTest : public ANGLERenderTest, public ::testing::WithParamInterfa
     };
 
     void sampleTime();
+    void saveScreenshot(const std::string &screenshotName) override;
 
     // For tracking RenderPass/FBO change timing.
     QueryInfo mCurrentQuery = {};
@@ -292,6 +294,45 @@ void TracePerfTest::onFramebufferChange(GLenum target, GLuint framebuffer)
     mCurrentQuery.framebuffer = framebuffer;
 }
 
+void TracePerfTest::saveScreenshot(const std::string &screenshotName)
+{
+    // Render a single frame.
+    RestrictedTraceID testID   = GetParam().testID;
+    const TraceInfo &traceInfo = kTraceInfos[testID];
+    ReplayFrame(testID, traceInfo.startFrame);
+
+    // RGBA 4-byte data.
+    uint32_t pixelCount = mTestParams.windowWidth * mTestParams.windowHeight;
+    std::vector<uint8_t> pixelData(pixelCount * 4);
+
+    glBindFramebuffer(GL_FRAMEBUFFER, 0);
+    glReadPixels(0, 0, mTestParams.windowWidth, mTestParams.windowHeight, GL_RGBA, GL_UNSIGNED_BYTE,
+                 pixelData.data());
+
+    // Convert to RGB and flip y.
+    std::vector<uint8_t> rgbData(pixelCount * 3);
+    for (EGLint y = 0; y < mTestParams.windowHeight; ++y)
+    {
+        for (EGLint x = 0; x < mTestParams.windowWidth; ++x)
+        {
+            EGLint srcPixel = x + y * mTestParams.windowWidth;
+            EGLint dstPixel = x + (mTestParams.windowHeight - y - 1) * mTestParams.windowWidth;
+            memcpy(&rgbData[dstPixel * 3], &pixelData[srcPixel * 4], 3);
+        }
+    }
+
+    angle::SavePNGRGB(screenshotName.c_str(), "ANGLE Screenshot", mTestParams.windowWidth,
+                      mTestParams.windowHeight, rgbData);
+
+    // Finish the frame loop.
+    for (uint32_t nextFrame = traceInfo.startFrame + 1; nextFrame < traceInfo.endFrame; ++nextFrame)
+    {
+        ReplayFrame(testID, nextFrame);
+    }
+    getGLWindow()->swap();
+    glFinish();
+}
+
 ANGLE_MAYBE_UNUSED void FramebufferChangeCallback(void *userData, GLenum target, GLuint framebuffer)
 {
     reinterpret_cast<TracePerfTest *>(userData)->onFramebufferChange(target, framebuffer);

util/png_utils.cpp

@@ -39,11 +39,11 @@ class ScopedFILE
 };
 }  // namespace
 
-bool SavePNG(const char *fileName,
-             const char *title,
-             uint32_t width,
-             uint32_t height,
-             const std::vector<uint8_t> &data)
+bool SavePNGRGB(const char *fileName,
+                const char *title,
+                uint32_t width,
+                uint32_t height,
+                const std::vector<uint8_t> &data)
 {
     ScopedFILE fp(fopen(fileName, "wb"));
     if (!fp.get())
@@ -78,7 +78,7 @@ bool SavePNG(const char *fileName,
     png_init_io(writeStruct, fp.get());
 
     // Write header (8 bit colour depth)
-    png_set_IHDR(writeStruct, infoStruct, width, height, 8, PNG_COLOR_TYPE_RGBA, PNG_INTERLACE_NONE,
+    png_set_IHDR(writeStruct, infoStruct, width, height, 8, PNG_COLOR_TYPE_RGB, PNG_INTERLACE_NONE,
                  PNG_COMPRESSION_TYPE_BASE, PNG_FILTER_TYPE_BASE);
 
     // Set title
@@ -98,8 +98,8 @@ bool SavePNG(const char *fileName,
 
     png_write_info(writeStruct, infoStruct);
 
-    // RGBA 4-byte stride.
-    const uint32_t rowStride = width * 4;
+    // RGB 3-byte stride.
+    const uint32_t rowStride = width * 3;
     for (uint32_t row = 0; row < height; ++row)
     {
         uint32_t rowOffset = row * rowStride;

util/png_utils.h

@@ -14,11 +14,11 @@
 
 namespace angle
 {
-bool SavePNG(const char *fileName,
-             const char *title,
-             uint32_t width,
-             uint32_t height,
-             const std::vector<uint8_t> &data);
+bool SavePNGRGB(const char *fileName,
+                const char *title,
+                uint32_t width,
+                uint32_t height,
+                const std::vector<uint8_t> &rgbData);
 }  // namespace angle
 
 #endif  // UTIL_PNG_UTILS_H_