diff --git a/engine/src/flutter/shell/platform/windows/compositor_opengl.cc b/engine/src/flutter/shell/platform/windows/compositor_opengl.cc index 2e2de8a90a80e..33f87711668e2 100644 --- a/engine/src/flutter/shell/platform/windows/compositor_opengl.cc +++ b/engine/src/flutter/shell/platform/windows/compositor_opengl.cc @@ -16,8 +16,9 @@ constexpr uint32_t kWindowFrameBufferId = 0; // The metadata for an OpenGL framebuffer backing store. struct FramebufferBackingStore { - uint32_t framebuffer_id; - uint32_t texture_id; + uint32_t framebuffer_id = 0; + uint32_t texture_id = 0; + uint32_t depth_stencil_id = 0; }; typedef const impeller::GLProc BlitFramebufferProc; @@ -66,28 +67,31 @@ bool CompositorOpenGL::CreateBackingStore( gl_->BindTexture(GL_TEXTURE_2D, 0); if (enable_impeller_) { - // Impeller requries that its onscreen surface is Multisampled and already - // has depth/stencil attached in order for anti-aliasing to work. - gl_->FramebufferTexture2DMultisampleEXT(GL_FRAMEBUFFER, - GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, - store->texture_id, 0, 4); - - // Set up depth/stencil attachment for impeller renderer. - GLuint depth_stencil; - gl_->GenRenderbuffers(1, &depth_stencil); - gl_->BindRenderbuffer(GL_RENDERBUFFER, depth_stencil); - gl_->RenderbufferStorageMultisampleEXT( - GL_RENDERBUFFER, // target - 4, // samples - GL_DEPTH24_STENCIL8, // internal format - config.size.width, // width - config.size.height // height - ); + if (supports_implicit_msaa_) { + // MSAA color attachment + gl_->FramebufferTexture2DMultisampleEXT( + GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, + store->texture_id, 0, 4); + } else { + gl_->FramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, + GL_TEXTURE_2D, store->texture_id, 0); + } + + // Impeller always requires depth/stencil attachment. + gl_->GenRenderbuffers(1, &store->depth_stencil_id); + gl_->BindRenderbuffer(GL_RENDERBUFFER, store->depth_stencil_id); + if (supports_implicit_msaa_) { + gl_->RenderbufferStorageMultisampleEXT( + GL_RENDERBUFFER, 4, GL_DEPTH24_STENCIL8, config.size.width, + config.size.height); + } else { + gl_->RenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH24_STENCIL8, + config.size.width, config.size.height); + } gl_->FramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, - GL_RENDERBUFFER, depth_stencil); + GL_RENDERBUFFER, store->depth_stencil_id); gl_->FramebufferRenderbuffer(GL_FRAMEBUFFER, GL_STENCIL_ATTACHMENT, - GL_RENDERBUFFER, depth_stencil); - + GL_RENDERBUFFER, store->depth_stencil_id); } else { gl_->FramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, store->texture_id, 0); @@ -116,6 +120,10 @@ bool CompositorOpenGL::CollectBackingStore(const FlutterBackingStore* store) { gl_->DeleteFramebuffers(1, &user_data->framebuffer_id); gl_->DeleteTextures(1, &user_data->texture_id); + if (user_data->depth_stencil_id != 0) { + gl_->DeleteRenderbuffers(1, &user_data->depth_stencil_id); + } + delete user_data; return true; } @@ -227,6 +235,9 @@ bool CompositorOpenGL::Initialize() { return false; } + supports_implicit_msaa_ = + gl_->GetCapabilities()->SupportsImplicitResolvingMSAA(); + is_initialized_ = true; return true; } diff --git a/engine/src/flutter/shell/platform/windows/compositor_opengl.h b/engine/src/flutter/shell/platform/windows/compositor_opengl.h index 18451c467604d..eaf702c2065b4 100644 --- a/engine/src/flutter/shell/platform/windows/compositor_opengl.h +++ b/engine/src/flutter/shell/platform/windows/compositor_opengl.h @@ -63,6 +63,9 @@ class CompositorOpenGL : public Compositor { // Whether the Impeller rendering backend is enabled. bool enable_impeller_ = false; + // Whether the OpenGL context supports implicit MSAA. + bool supports_implicit_msaa_ = false; + // Initialize the compositor. This must run on the raster thread. bool Initialize(); diff --git a/engine/src/flutter/shell/platform/windows/compositor_opengl_unittests.cc b/engine/src/flutter/shell/platform/windows/compositor_opengl_unittests.cc index c4e10012afa14..97fb7bd934b48 100644 --- a/engine/src/flutter/shell/platform/windows/compositor_opengl_unittests.cc +++ b/engine/src/flutter/shell/platform/windows/compositor_opengl_unittests.cc @@ -57,6 +57,23 @@ GLenum MockGetError() { return GL_NO_ERROR; } +void MockGetIntegervWithMSAA(GLenum name, int* value) { + if (name == GL_NUM_EXTENSIONS) { + *value = 2; + } else { + *value = 0; + } +} + +const unsigned char* MockGetStringiWithMSAA(GLenum name, int index) { + static constexpr const char* extensions[] = { + "GL_ANGLE_framebuffer_blit", "GL_EXT_multisampled_render_to_texture"}; + if (name == GL_EXTENSIONS && index < 2) { + return reinterpret_cast(extensions[index]); + } + return reinterpret_cast(""); +} + void DoNothing() {} const impeller::ProcTableGLES::Resolver kMockResolver = [](const char* name) { @@ -75,6 +92,18 @@ const impeller::ProcTableGLES::Resolver kMockResolver = [](const char* name) { } }; +const impeller::ProcTableGLES::Resolver kMockResolverWithMSAA = + [](const char* name) { + std::string_view function_name{name}; + + if (function_name == "glGetStringi") { + return reinterpret_cast(&MockGetStringiWithMSAA); + } else if (function_name == "glGetIntegerv") { + return reinterpret_cast(&MockGetIntegervWithMSAA); + } + return kMockResolver(name); + }; + class CompositorOpenGLTest : public WindowsTest { public: CompositorOpenGLTest() = default; @@ -151,17 +180,77 @@ TEST_F(CompositorOpenGLTest, CreateBackingStore) { ASSERT_TRUE(compositor.CollectBackingStore(&backing_store)); } -TEST_F(CompositorOpenGLTest, CreateBackingStoreImpeller) { +TEST_F(CompositorOpenGLTest, CreateBackingStoreImpellerNoMSAA) { UseHeadlessEngine(); + static int framebuffer_texture2d_calls = 0; + static int framebuffer_texture2d_multisample_calls = 0; + framebuffer_texture2d_calls = 0; + framebuffer_texture2d_multisample_calls = 0; + + const impeller::ProcTableGLES::Resolver resolver = + [](const char* name) -> void* { + std::string_view function_name{name}; + if (function_name == "glFramebufferTexture2D") { + return reinterpret_cast( + +[](GLenum, GLenum, GLenum, GLuint, GLint) { + framebuffer_texture2d_calls++; + }); + } else if (function_name == "glFramebufferTexture2DMultisampleEXT") { + return reinterpret_cast( + +[](GLenum, GLenum, GLenum, GLuint, GLint, GLsizei) { + framebuffer_texture2d_multisample_calls++; + }); + } + return kMockResolver(name); + }; + auto compositor = - CompositorOpenGL{engine(), kMockResolver, /*enable_impeller=*/true}; + CompositorOpenGL{engine(), resolver, /*enable_impeller=*/true}; + FlutterBackingStoreConfig config = {}; + FlutterBackingStore backing_store = {}; + + EXPECT_CALL(*render_context(), MakeCurrent).WillOnce(Return(true)); + ASSERT_TRUE(compositor.CreateBackingStore(config, &backing_store)); + EXPECT_EQ(framebuffer_texture2d_calls, 1); + EXPECT_EQ(framebuffer_texture2d_multisample_calls, 0); + ASSERT_TRUE(compositor.CollectBackingStore(&backing_store)); +} +TEST_F(CompositorOpenGLTest, CreateBackingStoreImpellerMSAA) { + UseHeadlessEngine(); + + static int framebuffer_texture2d_calls = 0; + static int framebuffer_texture2d_multisample_calls = 0; + framebuffer_texture2d_calls = 0; + framebuffer_texture2d_multisample_calls = 0; + + const impeller::ProcTableGLES::Resolver resolver = + [](const char* name) -> void* { + std::string_view function_name{name}; + if (function_name == "glFramebufferTexture2D") { + return reinterpret_cast( + +[](GLenum, GLenum, GLenum, GLuint, GLint) { + framebuffer_texture2d_calls++; + }); + } else if (function_name == "glFramebufferTexture2DMultisampleEXT") { + return reinterpret_cast( + +[](GLenum, GLenum, GLenum, GLuint, GLint, GLsizei) { + framebuffer_texture2d_multisample_calls++; + }); + } + return kMockResolverWithMSAA(name); + }; + + auto compositor = + CompositorOpenGL{engine(), resolver, /*enable_impeller=*/true}; FlutterBackingStoreConfig config = {}; FlutterBackingStore backing_store = {}; EXPECT_CALL(*render_context(), MakeCurrent).WillOnce(Return(true)); ASSERT_TRUE(compositor.CreateBackingStore(config, &backing_store)); + EXPECT_EQ(framebuffer_texture2d_calls, 0); + EXPECT_EQ(framebuffer_texture2d_multisample_calls, 1); ASSERT_TRUE(compositor.CollectBackingStore(&backing_store)); }