Audit backends and test cases for correct Y orientation. (#5440)

android/filament-android/src/main/java/com/google/android/filament/Renderer.java

@@ -531,6 +531,10 @@ public class Renderer {
      * Typically, this will happen after multiple calls to {@link #beginFrame},
      * {@link #render}, {@link #endFrame}.</p>
      * <br>
+     * <p>OpenGL only: if issuing a <code>readPixels</code> on a {@link RenderTarget} backed by a
+     * {@link Texture} that had data uploaded to it via {@link Texture#setImage}, the data returned
+     * from <code>readPixels</code> will be y-flipped with respect to the {@link Texture#setImage}
+     * call.</p>
      * <p><code>readPixels</code> is intended for debugging and testing.
      * It will impact performance significantly.</p>
      *

filament/backend/src/vulkan/VulkanDriver.cpp

@@ -1600,14 +1600,8 @@ void VulkanDriver::readPixels(Handle<HwRenderTarget> src, uint32_t x, uint32_t y
     vkMapMemory(device, stagingMemory, 0, VK_WHOLE_SIZE, 0, (void**) &srcPixels);
     srcPixels += subResourceLayout.offset;
 
-    // NOTE: the reasons for this are unclear, but issuing ReadPixels on a VkImage that has been
-    // extracted from a swap chain does not need a Y flip, but explicitly created VkImages do. (The
-    // former can be tested with "Export Screenshots" in gltf_viewer, the latter can be tested with
-    // test_ReadPixels.cpp). We've seen this behavior with both SwiftShader and MoltenVK.
-    const bool flipY = !srcTarget->isSwapChain();
-
     if (!DataReshaper::reshapeImage(&pbd, getComponentType(srcFormat), srcPixels,
-            subResourceLayout.rowPitch, width, height, swizzle, flipY)) {
+            subResourceLayout.rowPitch, width, height, swizzle, false)) {
         utils::slog.e << "Unsupported PixelDataFormat or PixelDataType" << utils::io::endl;
     }
 

filament/backend/test/TrianglePrimitive.h

@@ -26,6 +26,14 @@ namespace test {
 /**
  * A wrapper class that manages the necessary resources for a simple triangle renderable used for
  * test cases.
+ *
+ * The default vertices orient the triangle as such:
+ * o
+ * . .
+ * .   .
+ * .     .
+ * .       .
+ * o . . . . o
  */
 class TrianglePrimitive {
 public:

filament/backend/test/test_Blit.cpp

@@ -41,6 +41,10 @@ layout(location = 0) in vec4 mesh_position;
 uniform Params { highp vec4 color; highp vec4 scale; } params;
 void main() {
     gl_Position = vec4((mesh_position.xy + 0.5) * params.scale.xy, params.scale.z, 1.0);
+#if defined(TARGET_VULKAN_ENVIRONMENT)
+    // In Vulkan, clip space is Y-down. In OpenGL and Metal, clip space is Y-up.
+    gl_Position.y = -gl_Position.y;
+#endif
 })";
 
 static const char* const triangleFs = R"(#version 450 core
@@ -106,7 +110,7 @@ static uint32_t toUintColor(float4 color) {
 }
 
 static void createBitmap(DriverApi& dapi, Handle<HwTexture> texture, int baseWidth, int baseHeight,
-        int level, int layer, float3 color) {
+        int level, int layer, float3 color, bool flipY) {
     auto cb = [](void* buffer, size_t size, void* user) { free(buffer); };
     const int width = baseWidth >> level;
     const int height = baseHeight >> level;
@@ -116,17 +120,35 @@ static void createBitmap(DriverApi& dapi, Handle<HwTexture> texture, int baseWid
 
     const float3 foreground = color;
     const float3 background = float3(1, 1, 0);
-    const float radius = 0.25f;
 
-    // Draw a circle on a yellow background.
+    // Draw a triangle on a yellow background.
+    //
+    // The triangle is oriented as such:
+    // low addresses
+    // |      .
+    // |     ...
+    // |    .....
+    // v   ........
+    // high addresses
+    //
+    // If flipY is specified (e.g., for OpenGL) we flip the image:
+    // high addresses
+    // ^      .
+    // |     ...
+    // |    .....
+    // |   ........
+    // low addresses
+    // This is because OpenGL automatically flips image data when uploading into the texture.
     uint32_t* texels = (uint32_t*) buffer0;
     for (int row = 0; row < height; row++) {
         for (int col = 0; col < width; col++) {
             float2 uv = { (col - width / 2.0f) / width, (row - height / 2.0f) / height };
-            const float d = distance(uv, float2(0));
-            const float t = d < radius ? 1.0 : 0.0;
+            const float d = abs(uv.x);
+            const float triangleWidth = uv.y >= -.3 && uv.y <= .3 ? (.4f / .6f * uv.y + .2f) : 0;
+            const float t = d < triangleWidth ? 1.0 : 0.0;
             const float3 color = mix(foreground, background, t);
-            texels[row * width + col] = toUintColor(float4(color, 1.0f));
+            int rowFlipped = flipY ? (height - 1) - row : row;
+            texels[rowFlipped * width + col] = toUintColor(float4(color, 1.0f));
         }
     }
 
@@ -135,8 +157,8 @@ static void createBitmap(DriverApi& dapi, Handle<HwTexture> texture, int baseWid
 }
 
 static void createBitmap(DriverApi& dapi, Handle<HwTexture> texture, int baseWidth, int baseHeight,
-        int level, float3 color) {
-    createBitmap(dapi, texture, baseWidth, baseHeight, level, 0u, color);
+        int level, float3 color, bool flipY) {
+    createBitmap(dapi, texture, baseWidth, baseHeight, level, 0u, color, flipY);
 }
 
 static void createFaces(DriverApi& dapi, Handle<HwTexture> texture, int baseWidth, int baseHeight,
@@ -246,8 +268,9 @@ TEST_F(BackendTest, ColorMagnify) {
     Handle<HwTexture> srcTexture = api.createTexture(
         SamplerType::SAMPLER_2D, kNumLevels, kSrcTexFormat, 1, kSrcTexWidth, kSrcTexHeight, 1,
         TextureUsage::SAMPLEABLE | TextureUsage::UPLOADABLE | TextureUsage::COLOR_ATTACHMENT);
-    createBitmap(api, srcTexture, kSrcTexWidth, kSrcTexHeight, 0, float3(0.5, 0, 0));
-    createBitmap(api, srcTexture, kSrcTexWidth, kSrcTexHeight, 1, float3(0, 0, 0.5));
+    const bool flipY = sBackend == Backend::OPENGL;
+    createBitmap(api, srcTexture, kSrcTexWidth, kSrcTexHeight, 0, float3(0.5, 0, 0), flipY);
+    createBitmap(api, srcTexture, kSrcTexWidth, kSrcTexHeight, 1, float3(0, 0, 0.5), flipY);
 
     // Create a destination texture.
     Handle<HwTexture> dstTexture = api.createTexture(
@@ -288,7 +311,7 @@ TEST_F(BackendTest, ColorMagnify) {
     getDriver().purge();
 
     // Check if the image matches perfectly to our golden run.
-    const uint32_t expected = 0xb830a36a;
+    const uint32_t expected = 0x410bdd31;
     printf("Computed hash is 0x%8.8x, Expected 0x%8.8x\n", params.pixelHashResult, expected);
     EXPECT_TRUE(params.pixelHashResult == expected);
 
@@ -320,8 +343,9 @@ TEST_F(BackendTest, ColorMinify) {
     Handle<HwTexture> srcTexture = api.createTexture(
         SamplerType::SAMPLER_2D, kNumLevels, kSrcTexFormat, 1, kSrcTexWidth, kSrcTexHeight, 1,
         TextureUsage::SAMPLEABLE | TextureUsage::UPLOADABLE | TextureUsage::COLOR_ATTACHMENT);
-    createBitmap(api, srcTexture, kSrcTexWidth, kSrcTexHeight, 0, float3(0.5, 0, 0));
-    createBitmap(api, srcTexture, kSrcTexWidth, kSrcTexHeight, 1, float3(0, 0, 0.5));
+    const bool flipY = sBackend == Backend::OPENGL;
+    createBitmap(api, srcTexture, kSrcTexWidth, kSrcTexHeight, 0, float3(0.5, 0, 0), flipY);
+    createBitmap(api, srcTexture, kSrcTexWidth, kSrcTexHeight, 1, float3(0, 0, 0.5), flipY);
 
     // Create a destination texture.
     Handle<HwTexture> dstTexture = api.createTexture(
@@ -362,7 +386,7 @@ TEST_F(BackendTest, ColorMinify) {
     getDriver().purge();
 
     // Check if the image matches perfectly to our golden run.
-    const uint32_t expected = 0xe2353ca6;
+    const uint32_t expected = 0x7739bef5;
     printf("Computed hash is 0x%8.8x, Expected 0x%8.8x\n", params.pixelHashResult, expected);
     EXPECT_TRUE(params.pixelHashResult == expected);
 
@@ -788,7 +812,8 @@ TEST_F(BackendTest, Blit2DTextureArray) {
     Handle<HwTexture> srcTexture = api.createTexture(
             SamplerType::SAMPLER_2D_ARRAY, kNumLevels, kSrcTexFormat, 1, kSrcTexWidth, kSrcTexHeight, kSrcTexDepth,
             TextureUsage::SAMPLEABLE | TextureUsage::UPLOADABLE | TextureUsage::COLOR_ATTACHMENT);
-    createBitmap(api, srcTexture, kSrcTexWidth, kSrcTexHeight, 0, kSrcTexLayer, float3(0.5, 0, 0));
+    const bool flipY = sBackend == Backend::OPENGL;
+    createBitmap(api, srcTexture, kSrcTexWidth, kSrcTexHeight, 0, kSrcTexLayer, float3(0.5, 0, 0), flipY);
 
     // Create a destination texture.
     Handle<HwTexture> dstTexture = api.createTexture(
@@ -826,7 +851,7 @@ TEST_F(BackendTest, Blit2DTextureArray) {
     getDriver().purge();
 
     // Check if the image matches perfectly to our golden run.
-    const uint32_t expected = 0xe734bc44;
+    const uint32_t expected = 0x8de7d55b;
     printf("Computed hash is 0x%8.8x, Expected 0x%8.8x\n", params.pixelHashResult, expected);
     EXPECT_TRUE(params.pixelHashResult == expected);
 

filament/backend/test/test_BufferUpdates.cpp

@@ -33,6 +33,10 @@ layout(location = 0) out uvec4 indices;
 
 void main() {
     gl_Position = vec4(mesh_position.xy, 0.0, 1.0);
+#if defined(TARGET_VULKAN_ENVIRONMENT)
+    // In Vulkan, clip space is Y-down. In OpenGL and Metal, clip space is Y-up.
+    gl_Position.y = -gl_Position.y;
+#endif
 }
 )");
 
@@ -152,16 +156,6 @@ struct MaterialParams {
     float blue;
 };
 
-static void uploadUniforms(DriverApi& dapi, Handle<HwBufferObject> ubh, MaterialParams params) {
-    MaterialParams* tmp = new MaterialParams(params);
-    auto cb = [](void* buffer, size_t size, void* user) {
-        MaterialParams* sp = (MaterialParams*) buffer;
-        delete sp;
-    };
-    BufferDescriptor bd(tmp, sizeof(MaterialParams), cb);
-    dapi.updateBufferObject(ubh, std::move(bd), 0);
-}
-
 TEST_F(BackendTest, BufferObjectUpdateWithOffset) {
     // Create a platform-specific SwapChain and make it current.
     auto swapChain = createSwapChain();

filament/backend/test/test_FeedbackLoops.cpp

@@ -115,6 +115,9 @@ static void dumpScreenshot(DriverApi& dapi, Handle<HwRenderTarget> rt) {
     dapi.readPixels(rt, 0, 0, kTexWidth, kTexHeight, std::move(pb));
 }
 
+// TODO: This test needs work to get Metal and OpenGL to agree on results.
+// The problems are caused by both uploading and rendering into the same texture, since the OpenGL
+// backend's readPixels does not work correctly with textures that have image data uploaded.
 TEST_F(BackendTest, FeedbackLoops) {
     auto& api = getDriverApi();
 

filament/backend/test/test_LoadImage.cpp

@@ -40,7 +40,7 @@ layout(location = 0) in vec4 mesh_position;
 void main() {
     gl_Position = vec4(mesh_position.xy, 0.0, 1.0);
 #if defined(TARGET_VULKAN_ENVIRONMENT)
-    //gl_Position.y = 1.0f - gl_Position.y;
+    // In Vulkan, clip space is Y-down. In OpenGL and Metal, clip space is Y-up.
     gl_Position.y *= -1.0f;
 #endif
 }

filament/backend/test/test_MRT.cpp

@@ -33,6 +33,10 @@ layout(location = 0) out uvec4 indices;
 
 void main() {
     gl_Position = vec4(mesh_position.xy, 0.0, 1.0);
+#if defined(TARGET_VULKAN_ENVIRONMENT)
+    // In Vulkan, clip space is Y-down. In OpenGL and Metal, clip space is Y-up.
+    gl_Position.y = -gl_Position.y;
+#endif
 }
 )");
 

filament/backend/test/test_ReadPixels.cpp

@@ -45,6 +45,10 @@ layout(location = 0) in vec4 mesh_position;
 
 void main() {
     gl_Position = vec4(mesh_position.xy, 0.0, 1.0);
+#if defined(TARGET_VULKAN_ENVIRONMENT)
+    // In Vulkan, clip space is Y-down. In OpenGL and Metal, clip space is Y-up.
+    gl_Position.y = -gl_Position.y;
+#endif
 }
 )");
 
@@ -123,6 +127,9 @@ TEST_F(ReadPixelsTest, ReadPixels) {
         // The offset and stride set on the pixel buffer.
         size_t left = 0, top = 0, alignment = 1;
 
+        // If true, use the default RT to render into the SwapChain as opposed to a texture.
+        bool useDefaultRT = false;
+
         size_t getPixelBufferStride() const {
             return bufferDimension;
         }
@@ -219,7 +226,13 @@ TEST_F(ReadPixelsTest, ReadPixels) {
     t7.textureFormat = TextureFormat::RG16F;
     t7.hash = 3726805703;
 
-    TestCase testCases[] = { t, t2, t3, t4, t5, t6, t7 };
+    // Check that readPixels works when rendering into the SwapChain.
+    // This requires that the test runner's native window size is 512x512.
+    TestCase t8;
+    t8.testName = "readPixels_swapchain";
+    t8.useDefaultRT = true;
+
+    TestCase testCases[] = { t, t2, t3, t4, t5, t6, t7, t8 };
 
     // Create programs.
     Handle<HwProgram> programFloat, programUint;
@@ -234,11 +247,18 @@ TEST_F(ReadPixelsTest, ReadPixels) {
         programUint = getDriverApi().createProgram(std::move(p));
     }
 
+
     for (const auto& t : testCases)
     {
         // Create a platform-specific SwapChain and make it current.
-        auto swapChain = getDriverApi().createSwapChainHeadless(t.getRenderTargetSize(),
-                t.getRenderTargetSize(), 0);
+        Handle<HwSwapChain> swapChain;
+        if (t.useDefaultRT) {
+            swapChain = createSwapChain();
+        } else {
+            swapChain = getDriverApi().createSwapChainHeadless(t.getRenderTargetSize(),
+                    t.getRenderTargetSize(), 0);
+        }
+
         getDriverApi().makeCurrent(swapChain, swapChain);
 
         // Create a Texture and RenderTarget to render into.
@@ -247,12 +267,19 @@ TEST_F(ReadPixelsTest, ReadPixels) {
                 t.mipLevels, t.textureFormat, 1, renderTargetBaseSize, renderTargetBaseSize, 1,
                 usage);
 
-        // The width and height must match the width and height of the respective mip
-        // level (at least for OpenGL).
-        Handle<HwRenderTarget> renderTarget = getDriverApi().createRenderTarget(
-                TargetBufferFlags::COLOR, t.getRenderTargetSize(),
-                t.getRenderTargetSize(), t.samples, {{ texture, uint8_t(t.mipLevel) }}, {},
-                {});
+        Handle<HwRenderTarget> renderTarget;
+        if (t.useDefaultRT) {
+            // The width and height must match the width and height of the respective mip
+            // level (at least for OpenGL).
+            renderTarget = getDriverApi().createDefaultRenderTarget();
+        } else {
+            // The width and height must match the width and height of the respective mip
+            // level (at least for OpenGL).
+            renderTarget = getDriverApi().createRenderTarget(
+                    TargetBufferFlags::COLOR, t.getRenderTargetSize(),
+                    t.getRenderTargetSize(), t.samples, {{ texture, uint8_t(t.mipLevel) }}, {},
+                    {});
+        }
 
         TrianglePrimitive triangle(getDriverApi());
 

filament/include/filament/Renderer.h

@@ -444,6 +444,10 @@ public:
      *
      * It is also possible to use a Fence to wait for the read-back.
      *
+     * OpenGL only: if issuing a readPixels on a RenderTarget backed by a Texture that had data
+     * uploaded to it via setImage, the data returned from readPixels will be y-flipped with respect
+     * to the setImage call.
+     *
      * @remark
      * readPixels() is intended for debugging and testing. It will impact performance significantly.
      *