From 0b9f9288caf3fedf0eb651de2c1465789633a06f Mon Sep 17 00:00:00 2001 From: Robert Ancell Date: Wed, 14 May 2025 20:31:16 +1200 Subject: [PATCH] Fix Linux Impeller support broken by incorrect deletion of stencil buffer. (#168668) The buffer was leaking and deleted in 9d8e5e0b2f0dbff358286a1f83c83e65077d0b7a. This fixes the location where the buffer is deleted. --- .../shell/platform/linux/fl_framebuffer.cc | 14 ++++++---- .../platform/linux/fl_framebuffer_test.cc | 14 ++++++++++ .../platform/linux/testing/mock_epoxy.cc | 28 +++++++++++++++++-- .../shell/platform/linux/testing/mock_epoxy.h | 10 +++++++ 4 files changed, 58 insertions(+), 8 deletions(-) diff --git a/engine/src/flutter/shell/platform/linux/fl_framebuffer.cc b/engine/src/flutter/shell/platform/linux/fl_framebuffer.cc index 9a1f850cdff..f6374fa7e77 100644 --- a/engine/src/flutter/shell/platform/linux/fl_framebuffer.cc +++ b/engine/src/flutter/shell/platform/linux/fl_framebuffer.cc @@ -20,6 +20,9 @@ struct _FlFramebuffer { // Texture backing framebuffer. GLuint texture_id; + + // Stencil buffer associated with this framebuffer. + GLuint depth_stencil; }; G_DEFINE_TYPE(FlFramebuffer, fl_framebuffer, G_TYPE_OBJECT) @@ -29,6 +32,7 @@ static void fl_framebuffer_dispose(GObject* object) { glDeleteFramebuffers(1, &self->framebuffer_id); glDeleteTextures(1, &self->texture_id); + glDeleteRenderbuffers(1, &self->depth_stencil); G_OBJECT_CLASS(fl_framebuffer_parent_class)->dispose(object); } @@ -63,19 +67,17 @@ FlFramebuffer* fl_framebuffer_new(GLint format, size_t width, size_t height) { glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, provider->texture_id, 0); - GLuint depth_stencil; - glGenRenderbuffers(1, &depth_stencil); - glBindRenderbuffer(GL_RENDERBUFFER, depth_stencil); + glGenRenderbuffers(1, &provider->depth_stencil); + glBindRenderbuffer(GL_RENDERBUFFER, provider->depth_stencil); glRenderbufferStorage(GL_RENDERBUFFER, // target GL_DEPTH24_STENCIL8, // internal format width, // width height // height ); glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, - GL_RENDERBUFFER, depth_stencil); + GL_RENDERBUFFER, provider->depth_stencil); glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_STENCIL_ATTACHMENT, - GL_RENDERBUFFER, depth_stencil); - glDeleteRenderbuffers(1, &depth_stencil); + GL_RENDERBUFFER, provider->depth_stencil); return provider; } diff --git a/engine/src/flutter/shell/platform/linux/fl_framebuffer_test.cc b/engine/src/flutter/shell/platform/linux/fl_framebuffer_test.cc index 44f2e78dc7e..f18f6570fe7 100644 --- a/engine/src/flutter/shell/platform/linux/fl_framebuffer_test.cc +++ b/engine/src/flutter/shell/platform/linux/fl_framebuffer_test.cc @@ -24,3 +24,17 @@ TEST(FlFramebufferTest, HasDepthStencil) { &stencil_type); EXPECT_NE(stencil_type, GL_NONE); } + +TEST(FlFramebufferTest, ResourcesRemoved) { + ::testing::NiceMock epoxy; + + EXPECT_CALL(epoxy, glGenFramebuffers); + EXPECT_CALL(epoxy, glGenTextures); + EXPECT_CALL(epoxy, glGenRenderbuffers); + FlFramebuffer* framebuffer = fl_framebuffer_new(GL_RGB, 100, 100); + + EXPECT_CALL(epoxy, glDeleteFramebuffers); + EXPECT_CALL(epoxy, glDeleteTextures); + EXPECT_CALL(epoxy, glDeleteRenderbuffers); + g_object_unref(framebuffer); +} diff --git a/engine/src/flutter/shell/platform/linux/testing/mock_epoxy.cc b/engine/src/flutter/shell/platform/linux/testing/mock_epoxy.cc index d453b983fd0..ff138d5c5c9 100644 --- a/engine/src/flutter/shell/platform/linux/testing/mock_epoxy.cc +++ b/engine/src/flutter/shell/platform/linux/testing/mock_epoxy.cc @@ -401,11 +401,25 @@ GLuint _glCreateShader(GLenum shaderType) { return 0; } -void _glDeleteFramebuffers(GLsizei n, const GLuint* framebuffers) {} +void _glDeleteFramebuffers(GLsizei n, const GLuint* framebuffers) { + if (mock) { + mock->glDeleteFramebuffers(n, framebuffers); + } +} + +void _glDeleteRenderbuffers(GLsizei n, const GLuint* renderbuffers) { + if (mock) { + mock->glDeleteRenderbuffers(n, renderbuffers); + } +} void _glDeleteShader(GLuint shader) {} -void _glDeleteTextures(GLsizei n, const GLuint* textures) {} +void _glDeleteTextures(GLsizei n, const GLuint* textures) { + if (mock) { + mock->glDeleteTextures(n, textures); + } +} static void _glFramebufferRenderbuffer(GLenum target, GLenum attachment, @@ -424,18 +438,27 @@ static void _glGenTextures(GLsizei n, GLuint* textures) { for (GLsizei i = 0; i < n; i++) { textures[i] = 0; } + if (mock) { + mock->glGenTextures(n, textures); + } } static void _glGenFramebuffers(GLsizei n, GLuint* framebuffers) { for (GLsizei i = 0; i < n; i++) { framebuffers[i] = 0; } + if (mock) { + mock->glGenFramebuffers(n, framebuffers); + } } static void _glGenRenderbuffers(GLsizei n, GLuint* renderbuffers) { for (GLsizei i = 0; i < n; i++) { renderbuffers[i] = 0; } + if (mock) { + mock->glGenRenderbuffers(n, renderbuffers); + } } static void _glGetFramebufferAttachmentParameteriv(GLenum target, @@ -659,6 +682,7 @@ static void library_init() { epoxy_glCreateProgram = _glCreateProgram; epoxy_glCreateShader = _glCreateShader; epoxy_glDeleteFramebuffers = _glDeleteFramebuffers; + epoxy_glDeleteRenderbuffers = _glDeleteRenderbuffers; epoxy_glDeleteShader = _glDeleteShader; epoxy_glDeleteTextures = _glDeleteTextures; epoxy_glFramebufferRenderbuffer = _glFramebufferRenderbuffer; diff --git a/engine/src/flutter/shell/platform/linux/testing/mock_epoxy.h b/engine/src/flutter/shell/platform/linux/testing/mock_epoxy.h index 80bc05bb36d..94f0d084c0a 100644 --- a/engine/src/flutter/shell/platform/linux/testing/mock_epoxy.h +++ b/engine/src/flutter/shell/platform/linux/testing/mock_epoxy.h @@ -34,6 +34,16 @@ class MockEpoxy { GLint dstY1, GLbitfield mask, GLenum filter)); + MOCK_METHOD(void, + glDeleteFramebuffers, + (GLsizei n, const GLuint* framebuffers)); + MOCK_METHOD(void, + glDeleteRenderbuffers, + (GLsizei n, const GLuint* renderbuffers)); + MOCK_METHOD(void, glDeleteTextures, (GLsizei n, const GLuint* textures)); + MOCK_METHOD(void, glGenFramebuffers, (GLsizei n, GLuint* framebuffers)); + MOCK_METHOD(void, glGenRenderbuffers, (GLsizei n, GLuint* renderbuffers)); + MOCK_METHOD(void, glGenTextures, (GLsizei n, GLuint* textures)); MOCK_METHOD(const GLubyte*, glGetString, (GLenum pname)); };