Add format field to EGL surface backing store (#54499)

Trying to use the `GL_BGRA8_EXT` format for the EGL surface backing store on *desktop* OpenGL platforms would fail at: https://github.com/flutter/engine/blob/bde314b856850155d0678632b62c23c84f49afab/shell/platform/embedder/embedder.cc#L869

This seems to be a known issue and both the Linux and Windows embedders have some logic to pick a different format depending on the OpenGL context information:

https://github.com/flutter/engine/blob/088dcfa6cf934088413c5d8af754de1ef7b0d1ca/shell/platform/windows/compositor_opengl.cc#L23-L34

https://github.com/flutter/engine/blob/088dcfa6cf934088413c5d8af754de1ef7b0d1ca/shell/platform/linux/fl_framebuffer.cc#L81-L104

This pull-request gets rid of the hard-coded `GL_BGRA8_EXT` format and makes it configurable by adding a `format` field to the `FlutterOpenGLSurface` struct.

_Disclaimer_: This has only been tested on desktop Linux (Wayland) using the `GR_GL_RGBA8` format which seemed to work as expected.

This change is related to the recently introduced EGL surface backing store: flutter/flutter#58363

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

Author: vially

shell/platform/embedder/embedder.cc

@@ -868,7 +868,7 @@ static sk_sp<SkSurface> MakeSkSurfaceFromBackingStore(
     const FlutterOpenGLSurface* surface) {
 #ifdef SHELL_ENABLE_GL
   GrGLFramebufferInfo framebuffer_info = {};
-  framebuffer_info.fFormat = GL_BGRA8_EXT;
+  framebuffer_info.fFormat = SAFE_ACCESS(surface, format, GL_BGRA8_EXT);
   framebuffer_info.fFBOID = 0;
 
   auto backend_render_target =
@@ -881,11 +881,17 @@ static sk_sp<SkSurface> MakeSkSurfaceFromBackingStore(
 
   SkSurfaceProps surface_properties(0, kUnknown_SkPixelGeometry);
 
+  std::optional<SkColorType> color_type =
+      FlutterFormatToSkColorType(surface->format);
+  if (!color_type) {
+    return nullptr;
+  }
+
   auto sk_surface = SkSurfaces::WrapBackendRenderTarget(
       context,                      //  context
       backend_render_target,        // backend render target
       kBottomLeft_GrSurfaceOrigin,  // surface origin
-      kN32_SkColorType,             // color type
+      color_type.value(),           // color type
       SkColorSpace::MakeSRGB(),     // color space
       &surface_properties,          // surface properties
       static_cast<SkSurfaces::RenderTargetReleaseProc>(

shell/platform/embedder/embedder.h

@@ -458,6 +458,13 @@ typedef struct {
   ///
   /// @attention required. (non-null)
   VoidCallback destruction_callback;
+
+  /// The surface format.
+  ///
+  /// Allowed values:
+  ///   - GL_RGBA8
+  ///   - GL_BGRA8_EXT
+  uint32_t format;
 } FlutterOpenGLSurface;
 
 typedef bool (*BoolCallback)(void* /* user data */);

shell/platform/embedder/tests/embedder_assertions.h

@@ -70,7 +70,7 @@ inline bool operator==(const FlutterOpenGLFramebuffer& a,
 inline bool operator==(const FlutterOpenGLSurface& a,
                        const FlutterOpenGLSurface& b) {
   return a.make_current_callback == b.make_current_callback &&
-         a.user_data == b.user_data &&
+         a.user_data == b.user_data && a.format == b.format &&
          a.destruction_callback == b.destruction_callback;
 }
 
@@ -309,7 +309,8 @@ inline std::ostream& operator<<(std::ostream& out,
                                 const FlutterOpenGLSurface& item) {
   return out << "(FlutterOpenGLSurface) Make Current Callback: "
              << reinterpret_cast<void*>(item.make_current_callback)
-             << " User Data: " << item.user_data << " Destruction Callback: "
+             << " User Data: " << item.user_data << "Format: " << item.format
+             << " Destruction Callback: "
              << reinterpret_cast<void*>(item.destruction_callback);
 }
 

shell/platform/embedder/tests/embedder_gl_unittests.cc

@@ -4527,6 +4527,65 @@ TEST_F(EmbedderTest, CreateInvalidBackingstoreOpenGLFramebuffer) {
   latch.Wait();
 }
 
+TEST_F(EmbedderTest, CreateInvalidBackingstoreOpenGLSurface) {
+  auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext);
+  EmbedderConfigBuilder builder(context);
+  builder.SetOpenGLRendererConfig(SkISize::Make(800, 600));
+  builder.SetCompositor();
+  builder.SetRenderTargetType(
+      EmbedderTestBackingStoreProducer::RenderTargetType::kOpenGLSurface);
+  builder.SetDartEntrypoint("invalid_backingstore");
+
+  fml::AutoResetWaitableEvent latch;
+
+  builder.GetCompositor().create_backing_store_callback =
+      [](const FlutterBackingStoreConfig* config,  //
+         FlutterBackingStore* backing_store_out,   //
+         void* user_data                           //
+      ) {
+        backing_store_out->type = kFlutterBackingStoreTypeOpenGL;
+        backing_store_out->user_data = user_data;
+        backing_store_out->open_gl.type = kFlutterOpenGLTargetTypeSurface;
+        backing_store_out->open_gl.surface.user_data = user_data;
+        // Deliberately set this to an invalid format
+        backing_store_out->open_gl.surface.format = 0;
+        backing_store_out->open_gl.surface.make_current_callback = [](void*,
+                                                                      bool*) {
+          ADD_FAILURE() << "make_current_callback method should not be called";
+          return true;
+        };
+        backing_store_out->open_gl.surface.clear_current_callback = [](void*,
+                                                                       bool*) {
+          ADD_FAILURE() << "clear_current_callback method should not be called";
+          return true;
+        };
+        backing_store_out->open_gl.surface.destruction_callback =
+            [](void* user_data) {
+              FAIL() << "destruction_callback method should not be called";
+            };
+
+        return true;
+      };
+
+  context.AddNativeCallback(
+      "SignalNativeTest",
+      CREATE_NATIVE_ENTRY(
+          [&latch](Dart_NativeArguments args) { latch.Signal(); }));
+
+  auto engine = builder.LaunchEngine();
+
+  // Send a window metrics events so frames may be scheduled.
+  FlutterWindowMetricsEvent event = {};
+  event.struct_size = sizeof(event);
+  event.width = 800;
+  event.height = 600;
+  event.pixel_ratio = 1.0;
+  ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event),
+            kSuccess);
+  ASSERT_TRUE(engine.is_valid());
+  latch.Wait();
+}
+
 TEST_F(EmbedderTest, ExternalTextureGLRefreshedTooOften) {
   TestGLSurface surface(SkISize::Make(100, 100));
   auto context = surface.GetGrContext();

shell/platform/embedder/tests/embedder_test_backingstore_producer.cc

@@ -258,6 +258,7 @@ bool EmbedderTestBackingStoreProducer::CreateSurface(
   backing_store_out->open_gl.surface.clear_current_callback = clear_current;
   backing_store_out->open_gl.surface.destruction_callback =
       destruction_callback;
+  backing_store_out->open_gl.surface.format = 0x93A1 /* GL_BGRA8_EXT */;
 
   return true;
 #else