Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 7ad9e5b

Browse files
author
Jonah Williams
authored
[Impeller] fix obvious memory leak. (#55036)
Fixes flutter/flutter#154549 Each overlay surface gets its own content context and each cc gets its own transient buffer. So not reseting the overlay surfaces causes a memory leak. The overlay surfaces would just continue to allocate into the same buffer which would allocate endlessly
1 parent 813ad2d commit 7ad9e5b

File tree

4 files changed

+12
-19
lines changed

4 files changed

+12
-19
lines changed

shell/gpu/gpu_surface_gl_impeller.cc

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,6 @@ std::unique_ptr<SurfaceFrame> GPUSurfaceGLImpeller::AcquireFrame(
117117

118118
auto cull_rect = render_target.GetRenderTargetSize();
119119
impeller::Rect dl_cull_rect = impeller::Rect::MakeSize(cull_rect);
120-
const bool reset_host_buffer = surface_frame.submit_info().frame_boundary;
121120

122121
#if EXPERIMENTAL_CANVAS
123122
auto skia_cull_rect = SkIRect::MakeWH(cull_rect.width, cull_rect.height);
@@ -133,17 +132,16 @@ std::unique_ptr<SurfaceFrame> GPUSurfaceGLImpeller::AcquireFrame(
133132
display_list->Dispatch(impeller_dispatcher, skia_cull_rect);
134133
impeller_dispatcher.FinishRecording();
135134
aiks_context->GetContentContext().GetLazyGlyphAtlas()->ResetTextFrames();
136-
if (reset_host_buffer) {
137-
aiks_context->GetContentContext().GetTransientsBuffer().Reset();
138-
}
135+
aiks_context->GetContentContext().GetTransientsBuffer().Reset();
139136
return true;
140137
#else
141138
impeller::DlDispatcher impeller_dispatcher(dl_cull_rect);
142139
display_list->Dispatch(impeller_dispatcher,
143140
SkIRect::MakeWH(cull_rect.width, cull_rect.height));
144141
auto picture = impeller_dispatcher.EndRecordingAsPicture();
145142

146-
return aiks_context->Render(picture, render_target, reset_host_buffer);
143+
return aiks_context->Render(picture, render_target,
144+
/*reset_host_buffer=*/false);
147145

148146
#endif // EXPERIMENTAL_CANVAS
149147
};

shell/gpu/gpu_surface_metal_impeller.mm

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,6 @@
166166

167167
impeller::IRect cull_rect = surface->coverage();
168168
SkIRect sk_cull_rect = SkIRect::MakeWH(cull_rect.GetWidth(), cull_rect.GetHeight());
169-
const bool reset_host_buffer = surface_frame.submit_info().frame_boundary;
170169

171170
impeller::RenderTarget render_target = surface->GetTargetRenderPassDescriptor();
172171
surface->SetFrameBoundary(surface_frame.submit_info().frame_boundary);
@@ -183,9 +182,7 @@
183182
display_list->Dispatch(impeller_dispatcher, sk_cull_rect);
184183
impeller_dispatcher.FinishRecording();
185184
aiks_context->GetContentContext().GetLazyGlyphAtlas()->ResetTextFrames();
186-
if (reset_host_buffer) {
187-
aiks_context->GetContentContext().GetTransientsBuffer().Reset();
188-
}
185+
aiks_context->GetContentContext().GetTransientsBuffer().Reset();
189186

190187
if (!surface->PreparePresent()) {
191188
return false;
@@ -196,7 +193,7 @@
196193
impeller::DlDispatcher impeller_dispatcher(cull_rect);
197194
display_list->Dispatch(impeller_dispatcher, sk_cull_rect);
198195
auto picture = impeller_dispatcher.EndRecordingAsPicture();
199-
auto result = aiks_context->Render(picture, render_target, reset_host_buffer);
196+
auto result = aiks_context->Render(picture, render_target, /*reset_host_buffer=*/true);
200197

201198
if (!surface->PreparePresent()) {
202199
return false;

shell/gpu/gpu_surface_metal_impeller_unittests.mm

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ GPUCAMetalLayerHandle GetCAMetalLayer(const SkISize& frame_info) const override
9797
ASSERT_TRUE(frame->Submit());
9898
}
9999

100-
TEST(GPUSurfaceMetalImpeller, ResetHostBufferBasedOnFrameBoundary) {
100+
// Because each overlay surface gets its own HostBuffer, we always need to reset.
101+
TEST(GPUSurfaceMetalImpeller, DoesNotResetHostBufferBasedOnFrameBoundary) {
101102
auto delegate = std::make_shared<TestGPUSurfaceMetalDelegate>();
102103
delegate->SetDevice();
103104

@@ -115,13 +116,13 @@ GPUCAMetalLayerHandle GetCAMetalLayer(const SkISize& frame_info) const override
115116
frame->set_submit_info({.frame_boundary = false});
116117

117118
ASSERT_TRUE(frame->Submit());
118-
EXPECT_EQ(host_buffer.GetStateForTest().current_frame, 0u);
119+
EXPECT_EQ(host_buffer.GetStateForTest().current_frame, 1u);
119120

120121
frame = surface->AcquireFrame(SkISize::Make(100, 100));
121122
frame->set_submit_info({.frame_boundary = true});
122123

123124
ASSERT_TRUE(frame->Submit());
124-
EXPECT_EQ(host_buffer.GetStateForTest().current_frame, 1u);
125+
EXPECT_EQ(host_buffer.GetStateForTest().current_frame, 2u);
125126
}
126127

127128
#ifdef IMPELLER_DEBUG

shell/gpu/gpu_surface_vulkan_impeller.cc

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ std::unique_ptr<SurfaceFrame> GPUSurfaceVulkanImpeller::AcquireFrame(
8080
return false;
8181
}
8282

83-
const bool reset_host_buffer = surface_frame.submit_info().frame_boundary;
8483
#if EXPERIMENTAL_CANVAS
8584
impeller::TextFrameDispatcher collector(aiks_context->GetContentContext(),
8685
impeller::Matrix());
@@ -94,19 +93,17 @@ std::unique_ptr<SurfaceFrame> GPUSurfaceVulkanImpeller::AcquireFrame(
9493
display_list->Dispatch(impeller_dispatcher,
9594
SkIRect::MakeWH(cull_rect.width, cull_rect.height));
9695
impeller_dispatcher.FinishRecording();
97-
aiks_context->GetContentContext().GetTransientsBuffer().Reset();
9896
aiks_context->GetContentContext().GetLazyGlyphAtlas()->ResetTextFrames();
99-
if (reset_host_buffer) {
100-
aiks_context->GetContentContext().GetTransientsBuffer().Reset();
101-
}
97+
aiks_context->GetContentContext().GetTransientsBuffer().Reset();
10298
return true;
10399
#else
104100
impeller::Rect dl_cull_rect = impeller::Rect::MakeSize(cull_rect);
105101
impeller::DlDispatcher impeller_dispatcher(dl_cull_rect);
106102
display_list->Dispatch(impeller_dispatcher,
107103
SkIRect::MakeWH(cull_rect.width, cull_rect.height));
108104
auto picture = impeller_dispatcher.EndRecordingAsPicture();
109-
return aiks_context->Render(picture, render_target, reset_host_buffer);
105+
return aiks_context->Render(picture, render_target,
106+
/*reset_host_buffer=*/false);
110107
#endif
111108
};
112109

0 commit comments

Comments
 (0)