[Impeller] ensure that OpenGL "flipped" textures do not leak via texture readback. (#163501)

Fixes https://github.com/flutter/flutter/issues/163315
Fixes https://github.com/flutter/flutter/issues/163521
Fixes https://github.com/flutter/flutter/issues/142641

OpenGL has an inverted coordinate system (bottom left is zero) compared
to Metal/Vulkan (top left is zero). We handle this by rendering things
upside down on OpenGL. Unfortunately this can leak out of the renderer
via readback (toImage), so we need to make sure to undo the inversion.

This is not performed for the "TextureCoordinateSystem::kUploadFromHost"
state as that indicates the texture is already "right side up".

engine/src/flutter/impeller/display_list/aiks_dl_unittests.cc

@@ -19,7 +19,12 @@
 #include "flutter/display_list/dl_color.h"
 #include "flutter/display_list/dl_paint.h"
 #include "flutter/testing/testing.h"
+#include "fml/synchronization/count_down_latch.h"
 #include "imgui.h"
+#include "impeller/core/device_buffer.h"
+#include "impeller/core/device_buffer_descriptor.h"
+#include "impeller/core/formats.h"
+#include "impeller/core/texture_descriptor.h"
 #include "impeller/display_list/dl_dispatcher.h"
 #include "impeller/display_list/dl_image_impeller.h"
 #include "impeller/geometry/scalar.h"
@@ -1027,5 +1032,77 @@ TEST_P(AiksTest, DepthValuesForPolygonMode) {
   ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
 }
 
+// Verifies that an image rasterized and readback is in the correct orientation
+// by re-uploading it.
+TEST_P(AiksTest, ToImageFromImage) {
+  DisplayListBuilder builder;
+  DlPath path = DlPath::MakeArc(DlRect::MakeLTRB(0, 0, 100, 100), DlDegrees(0),
+                                DlDegrees(90),
+                                /*use_center=*/true);
+
+  builder.DrawPath(path, DlPaint().setColor(DlColor::kRed()));
+
+  AiksContext renderer(GetContext(), nullptr);
+  auto texture =
+      DisplayListToTexture(builder.Build(), ISize(100, 100), renderer);
+
+  // First, Readback the texture data into a host buffer.
+  impeller::DeviceBufferDescriptor desc;
+  desc.size = texture->GetTextureDescriptor().GetByteSizeOfBaseMipLevel();
+  desc.readback = true;
+  desc.storage_mode = StorageMode::kHostVisible;
+
+  auto device_buffer = GetContext()->GetResourceAllocator()->CreateBuffer(desc);
+  {
+    auto cmd_buffer = GetContext()->CreateCommandBuffer();
+    auto blit_pass = cmd_buffer->CreateBlitPass();
+
+    blit_pass->AddCopy(texture, device_buffer);
+    blit_pass->EncodeCommands();
+
+    auto latch = std::make_shared<fml::CountDownLatch>(1u);
+    GetContext()->GetCommandQueue()->Submit(
+        {cmd_buffer},
+        [latch](CommandBuffer::Status status) { latch->CountDown(); });
+    latch->Wait();
+  }
+
+  impeller::TextureDescriptor tex_desc = texture->GetTextureDescriptor();
+  auto reupload_texture =
+      GetContext()->GetResourceAllocator()->CreateTexture(tex_desc);
+
+  // Next, Re-upload the data into a new texture.
+  {
+    auto cmd_buffer = GetContext()->CreateCommandBuffer();
+    auto blit_pass = cmd_buffer->CreateBlitPass();
+    blit_pass->AddCopy(DeviceBuffer::AsBufferView(device_buffer),
+                       reupload_texture);
+    blit_pass->ConvertTextureToShaderRead(texture);
+    blit_pass->EncodeCommands();
+
+    auto latch = std::make_shared<fml::CountDownLatch>(1u);
+    GetContext()->GetCommandQueue()->Submit(
+        {cmd_buffer},
+        [latch](CommandBuffer::Status status) { latch->CountDown(); });
+    latch->Wait();
+  }
+
+  // Draw the results side by side. These should look the same.
+  DisplayListBuilder canvas;
+  DlPaint paint = DlPaint();
+  canvas.DrawRect(
+      DlRect::MakeLTRB(0, 0, 100, 100),
+      DlPaint().setColor(DlColor::kBlue()).setDrawStyle(DlDrawStyle::kStroke));
+  canvas.DrawImage(DlImageImpeller::Make(texture), DlPoint(0, 0),
+                   DlImageSampling::kNearestNeighbor, &paint);
+
+  canvas.DrawRect(
+      DlRect::MakeLTRB(0, 100, 100, 200),
+      DlPaint().setColor(DlColor::kRed()).setDrawStyle(DlDrawStyle::kStroke));
+  canvas.DrawImage(DlImageImpeller::Make(reupload_texture), DlPoint(0, 100),
+                   DlImageSampling::kNearestNeighbor, &paint);
+  OpenPlaygroundHere(canvas.Build());
+}
+
 }  // namespace testing
 }  // namespace impeller

engine/src/flutter/impeller/golden_tests/vulkan_screenshotter.mm

@@ -76,25 +76,6 @@ std::unique_ptr<Screenshot> ReadTexture(
   CGImagePtr image(CGBitmapContextCreateImage(context.get()), &CGImageRelease);
   FML_CHECK(image);
 
-  // TODO(142641): Perform the flip at the blit stage to avoid this slow copy.
-  if (texture->GetYCoordScale() == -1) {
-    CGContextPtr flipped_context(
-        CGBitmapContextCreate(
-            nullptr, texture->GetSize().width, texture->GetSize().height,
-            /*bitsPerComponent=*/8,
-            /*bytesPerRow=*/0, color_space.get(), bitmap_info),
-        &CGContextRelease);
-    CGContextTranslateCTM(flipped_context.get(), 0, texture->GetSize().height);
-    CGContextScaleCTM(flipped_context.get(), 1.0, -1.0);
-    CGContextDrawImage(
-        flipped_context.get(),
-        CGRectMake(0, 0, texture->GetSize().width, texture->GetSize().height),
-        image.get());
-    CGImagePtr flipped_image(CGBitmapContextCreateImage(flipped_context.get()),
-                             &CGImageRelease);
-    image.swap(flipped_image);
-  }
-
   return std::make_unique<MetalScreenshot>(image.release());
 }
 }  // namespace

engine/src/flutter/impeller/renderer/backend/gles/blit_command_gles.cc

@@ -7,6 +7,7 @@
 #include "flutter/fml/closure.h"
 #include "fml/trace_event.h"
 #include "impeller/base/validation.h"
+#include "impeller/core/formats.h"
 #include "impeller/geometry/point.h"
 #include "impeller/renderer/backend/gles/device_buffer_gles.h"
 #include "impeller/renderer/backend/gles/reactor_gles.h"
@@ -14,6 +15,29 @@
 
 namespace impeller {
 
+namespace {
+static void FlipImage(uint8_t* buffer,
+                      size_t width,
+                      size_t height,
+                      size_t stride) {
+  if (buffer == nullptr || stride == 0) {
+    return;
+  }
+
+  const auto byte_width = width * stride;
+
+  for (size_t top = 0; top < height; top++) {
+    size_t bottom = height - top - 1;
+    if (top >= bottom) {
+      break;
+    }
+    auto* top_row = buffer + byte_width * top;
+    auto* bottom_row = buffer + byte_width * bottom;
+    std::swap_ranges(top_row, top_row + byte_width, bottom_row);
+  }
+}
+}  // namespace
+
 BlitEncodeGLES::~BlitEncodeGLES() = default;
 
 static void DeleteFBO(const ProcTableGLES& gl, GLuint fbo, GLenum type) {
@@ -314,6 +338,7 @@ bool BlitCopyTextureToBufferCommandGLES::Encode(
   }
 
   const auto& gl = reactor.GetProcTable();
+  TextureCoordinateSystem coord_system = source->GetCoordinateSystem();
 
   GLuint read_fbo = GL_NONE;
   fml::ScopedCleanupClosure delete_fbos(
@@ -328,10 +353,22 @@ bool BlitCopyTextureToBufferCommandGLES::Encode(
   }
 
   DeviceBufferGLES::Cast(*destination)
-      .UpdateBufferData([&gl, this](uint8_t* data, size_t length) {
+      .UpdateBufferData([&gl, this, coord_system,
+                         rows = source->GetSize().height](uint8_t* data,
+
+                                                          size_t length) {
         gl.ReadPixels(source_region.GetX(), source_region.GetY(),
                       source_region.GetWidth(), source_region.GetHeight(),
                       GL_RGBA, GL_UNSIGNED_BYTE, data + destination_offset);
+        switch (coord_system) {
+          case TextureCoordinateSystem::kUploadFromHost:
+            break;
+          case TextureCoordinateSystem::kRenderToTexture:
+            // The texture is upside down, and must be inverted when copying
+            // byte data out.
+            FlipImage(data + destination_offset, source_region.GetWidth(),
+                      source_region.GetHeight(), 4);
+        }
       });
 
   return true;

engine/src/flutter/testing/impeller_golden_tests_output.txt

@@ -919,6 +919,9 @@ impeller_Play_AiksTest_TextForegroundShaderWithTransform_Vulkan.png
 impeller_Play_AiksTest_TextFrameSubpixelAlignment_Metal.png
 impeller_Play_AiksTest_TextFrameSubpixelAlignment_OpenGLES.png
 impeller_Play_AiksTest_TextFrameSubpixelAlignment_Vulkan.png
+impeller_Play_AiksTest_ToImageFromImage_Metal.png
+impeller_Play_AiksTest_ToImageFromImage_OpenGLES.png
+impeller_Play_AiksTest_ToImageFromImage_Vulkan.png
 impeller_Play_AiksTest_TranslucentSaveLayerDrawsCorrectly_Metal.png
 impeller_Play_AiksTest_TranslucentSaveLayerDrawsCorrectly_OpenGLES.png
 impeller_Play_AiksTest_TranslucentSaveLayerDrawsCorrectly_Vulkan.png