From d8ae2d44aff7f0dd87dbf2ddd372ae75883ee70a Mon Sep 17 00:00:00 2001 From: Robert Ancell Date: Thu, 12 Sep 2024 12:07:37 +1200 Subject: [PATCH 1/3] Match Windows logic for picking RGB/BGR textures. The existing code had a lot of additional checks that didn't seem to need to be there. BGR could be passed back to Flutter, but this was never used in creating the texture. There has been a report of a Flutter app on Linux with swapped red and blue color channels, so this seems like it is likely not working on some drivers. The original logic was introduced in f104bad4ca68bf4cd87a2ab83cb32d82fe0dea8b --- shell/platform/linux/fl_framebuffer.cc | 29 ++-------------- shell/platform/linux/fl_framebuffer.h | 13 ++----- shell/platform/linux/fl_renderer.cc | 44 ++++++++++++++++++++---- shell/platform/linux/fl_renderer_test.cc | 3 +- 4 files changed, 44 insertions(+), 45 deletions(-) diff --git a/shell/platform/linux/fl_framebuffer.cc b/shell/platform/linux/fl_framebuffer.cc index 46e9d9e7dfa59..352b496cc5443 100644 --- a/shell/platform/linux/fl_framebuffer.cc +++ b/shell/platform/linux/fl_framebuffer.cc @@ -39,7 +39,7 @@ static void fl_framebuffer_class_init(FlFramebufferClass* klass) { static void fl_framebuffer_init(FlFramebuffer* self) {} -FlFramebuffer* fl_framebuffer_new(size_t width, size_t height) { +FlFramebuffer* fl_framebuffer_new(GLint format, size_t width, size_t height) { FlFramebuffer* provider = FL_FRAMEBUFFER(g_object_new(fl_framebuffer_get_type(), nullptr)); @@ -56,7 +56,7 @@ FlFramebuffer* fl_framebuffer_new(size_t width, size_t height) { glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE); - glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, width, height, 0, GL_RGBA, + glTexImage2D(GL_TEXTURE_2D, 0, format, width, height, 0, format, GL_UNSIGNED_BYTE, NULL); glBindTexture(GL_TEXTURE_2D, 0); @@ -78,31 +78,6 @@ GLenum fl_framebuffer_get_target(FlFramebuffer* self) { return GL_TEXTURE_2D; } -GLenum fl_framebuffer_get_format(FlFramebuffer* self) { - // Flutter defines SK_R32_SHIFT=16, so SK_PMCOLOR_BYTE_ORDER should be BGRA. - // In Linux kN32_SkColorType is assumed to be kBGRA_8888_SkColorType. - // So we must choose a valid gl format to be compatible with surface format - // BGRA8. - // Following logic is copied from Skia GrGLCaps.cpp: - // https://github.com/google/skia/blob/4738ed711e03212aceec3cd502a4adb545f38e63/src/gpu/ganesh/gl/GrGLCaps.cpp#L1963-L2116 - - if (epoxy_is_desktop_gl()) { - // For OpenGL. - if (epoxy_gl_version() >= 12 || epoxy_has_gl_extension("GL_EXT_bgra")) { - return GL_RGBA8; - } - } else { - // For OpenGL ES. - if (epoxy_has_gl_extension("GL_EXT_texture_format_BGRA8888") || - (epoxy_has_gl_extension("GL_APPLE_texture_format_BGRA8888") && - epoxy_gl_version() >= 30)) { - return GL_BGRA8_EXT; - } - } - g_critical("Failed to determine valid GL format for Flutter rendering"); - return GL_RGBA8; -} - size_t fl_framebuffer_get_width(FlFramebuffer* self) { return self->width; } diff --git a/shell/platform/linux/fl_framebuffer.h b/shell/platform/linux/fl_framebuffer.h index b70593a4339c2..0df5d155c3e9b 100644 --- a/shell/platform/linux/fl_framebuffer.h +++ b/shell/platform/linux/fl_framebuffer.h @@ -21,6 +21,7 @@ G_DECLARE_FINAL_TYPE(FlFramebuffer, fl_framebuffer, FL, FRAMEBUFFER, GObject) /** * fl_framebuffer_new: + * @format: format, e.g. GL_RGB, GL_BGR * @width: width of texture. * @height: height of texture. * @@ -28,7 +29,7 @@ G_DECLARE_FINAL_TYPE(FlFramebuffer, fl_framebuffer, FL, FRAMEBUFFER, GObject) * * Returns: a new #FlFramebuffer. */ -FlFramebuffer* fl_framebuffer_new(size_t width, size_t height); +FlFramebuffer* fl_framebuffer_new(GLint format, size_t width, size_t height); /** * fl_framebuffer_get_id: @@ -60,16 +61,6 @@ GLuint fl_framebuffer_get_texture_id(FlFramebuffer* framebuffer); */ GLenum fl_framebuffer_get_target(FlFramebuffer* framebuffer); -/** - * fl_framebuffer_get_format: - * @framebuffer: an #FlFramebuffer. - * - * Gets format of texture backing the framebuffer (example GL_RGBA8). - * - * Returns: texture format. - */ -GLenum fl_framebuffer_get_format(FlFramebuffer* framebuffer); - /** * fl_framebuffer_get_width: * @framebuffer: an #FlFramebuffer. diff --git a/shell/platform/linux/fl_renderer.cc b/shell/platform/linux/fl_renderer.cc index e7ffd35531809..55eaf9dcc2677 100644 --- a/shell/platform/linux/fl_renderer.cc +++ b/shell/platform/linux/fl_renderer.cc @@ -42,6 +42,15 @@ typedef struct { // Engine we are rendering. GWeakRef engine; + // Flag to track lazy initialization. + gboolean initialized; + + // The pixel format passed to the engine. + GLint sized_format; + + // The format used to create textures. + GLint general_format; + // Views being rendered. GHashTable* views; @@ -105,6 +114,25 @@ static GLfloat pixels_to_gl_coords(GLfloat position, GLfloat pixels) { return (2.0 * position / pixels) - 1.0; } +// Perform single run OpenGL initialization. +static void initialize(FlRenderer* self) { + FlRendererPrivate* priv = reinterpret_cast( + fl_renderer_get_instance_private(self)); + + if (priv->initialized) { + return; + } + priv->initialized = TRUE; + + if (epoxy_has_gl_extension("GL_EXT_texture_format_BGRA8888")) { + priv->sized_format = GL_BGRA8_EXT; + priv->general_format = GL_BGRA_EXT; + } else { + priv->sized_format = GL_RGBA8; + priv->general_format = GL_RGBA; + } +} + static void fl_renderer_unblock_main_thread(FlRenderer* self) { FlRendererPrivate* priv = reinterpret_cast( fl_renderer_get_instance_private(self)); @@ -328,13 +356,18 @@ guint32 fl_renderer_get_fbo(FlRenderer* self) { } gboolean fl_renderer_create_backing_store( - FlRenderer* renderer, + FlRenderer* self, const FlutterBackingStoreConfig* config, FlutterBackingStore* backing_store_out) { - fl_renderer_make_current(renderer); + FlRendererPrivate* priv = reinterpret_cast( + fl_renderer_get_instance_private(self)); + + fl_renderer_make_current(self); + + initialize(self); - FlFramebuffer* framebuffer = - fl_framebuffer_new(config->size.width, config->size.height); + FlFramebuffer* framebuffer = fl_framebuffer_new( + priv->general_format, config->size.width, config->size.height); if (!framebuffer) { g_warning("Failed to create backing store"); return FALSE; @@ -345,8 +378,7 @@ gboolean fl_renderer_create_backing_store( backing_store_out->open_gl.framebuffer.user_data = framebuffer; backing_store_out->open_gl.framebuffer.name = fl_framebuffer_get_id(framebuffer); - backing_store_out->open_gl.framebuffer.target = - fl_framebuffer_get_format(framebuffer); + backing_store_out->open_gl.framebuffer.target = priv->sized_format; backing_store_out->open_gl.framebuffer.destruction_callback = [](void* p) { // Backing store destroyed in fl_renderer_collect_backing_store(), set // on FlutterCompositor.collect_backing_store_callback during engine start. diff --git a/shell/platform/linux/fl_renderer_test.cc b/shell/platform/linux/fl_renderer_test.cc index 620767eb174ed..2ab50462d56b6 100644 --- a/shell/platform/linux/fl_renderer_test.cc +++ b/shell/platform/linux/fl_renderer_test.cc @@ -52,7 +52,8 @@ TEST(FlRendererTest, RestoresGLState) { g_autoptr(FlDartProject) project = fl_dart_project_new(); g_autoptr(FlView) view = fl_view_new(project); g_autoptr(FlMockRenderer) renderer = fl_mock_renderer_new(); - g_autoptr(FlFramebuffer) framebuffer = fl_framebuffer_new(kWidth, kHeight); + g_autoptr(FlFramebuffer) framebuffer = + fl_framebuffer_new(GL_RGB, kWidth, kHeight); fl_renderer_add_view(FL_RENDERER(renderer), 0, view); fl_renderer_wait_for_frame(FL_RENDERER(renderer), kWidth, kHeight); From 46f6cf9b40a361a29f4ff2fa1b90d272621dd1c3 Mon Sep 17 00:00:00 2001 From: Robert Ancell Date: Fri, 13 Sep 2024 09:51:44 +1200 Subject: [PATCH 2/3] Fix tests --- shell/platform/linux/fl_renderer_test.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/shell/platform/linux/fl_renderer_test.cc b/shell/platform/linux/fl_renderer_test.cc index 2ab50462d56b6..b9336a141cd91 100644 --- a/shell/platform/linux/fl_renderer_test.cc +++ b/shell/platform/linux/fl_renderer_test.cc @@ -145,6 +145,8 @@ TEST(FlRendererTest, BlitFramebufferExtension) { ::testing::Return(reinterpret_cast("Intel"))); ON_CALL(epoxy, epoxy_is_desktop_gl).WillByDefault(::testing::Return(true)); EXPECT_CALL(epoxy, epoxy_gl_version).WillRepeatedly(::testing::Return(20)); + EXPECT_CALL(epoxy, epoxy_has_gl_extension(::testing::_)) + .WillRepeatedly(::testing::Return(false)); EXPECT_CALL(epoxy, epoxy_has_gl_extension( ::testing::StrEq("GL_EXT_framebuffer_blit"))) .WillRepeatedly(::testing::Return(true)); From 37ff6cffaddd078b8cf307170f389c558efa1b22 Mon Sep 17 00:00:00 2001 From: Robert Ancell Date: Mon, 16 Sep 2024 11:03:06 +1200 Subject: [PATCH 3/3] Use non-EXT format for glFramebufferTexture2D --- shell/platform/linux/fl_framebuffer.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/linux/fl_framebuffer.cc b/shell/platform/linux/fl_framebuffer.cc index 352b496cc5443..947b540301cc4 100644 --- a/shell/platform/linux/fl_framebuffer.cc +++ b/shell/platform/linux/fl_framebuffer.cc @@ -60,8 +60,8 @@ FlFramebuffer* fl_framebuffer_new(GLint format, size_t width, size_t height) { GL_UNSIGNED_BYTE, NULL); glBindTexture(GL_TEXTURE_2D, 0); - glFramebufferTexture2D(GL_FRAMEBUFFER_EXT, GL_COLOR_ATTACHMENT0_EXT, - GL_TEXTURE_2D, provider->texture_id, 0); + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, + provider->texture_id, 0); return provider; }