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

[Impeller] fix obvious memory leak. #55036

Merged
merged 5 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions shell/gpu/gpu_surface_gl_impeller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ std::unique_ptr<SurfaceFrame> GPUSurfaceGLImpeller::AcquireFrame(

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

#if EXPERIMENTAL_CANVAS
auto skia_cull_rect = SkIRect::MakeWH(cull_rect.width, cull_rect.height);
Expand All @@ -133,17 +132,16 @@ std::unique_ptr<SurfaceFrame> GPUSurfaceGLImpeller::AcquireFrame(
display_list->Dispatch(impeller_dispatcher, skia_cull_rect);
impeller_dispatcher.FinishRecording();
aiks_context->GetContentContext().GetLazyGlyphAtlas()->ResetTextFrames();
if (reset_host_buffer) {
aiks_context->GetContentContext().GetTransientsBuffer().Reset();
}
aiks_context->GetContentContext().GetTransientsBuffer().Reset();
return true;
#else
impeller::DlDispatcher impeller_dispatcher(dl_cull_rect);
display_list->Dispatch(impeller_dispatcher,
SkIRect::MakeWH(cull_rect.width, cull_rect.height));
auto picture = impeller_dispatcher.EndRecordingAsPicture();

return aiks_context->Render(picture, render_target, reset_host_buffer);
return aiks_context->Render(picture, render_target,
/*reset_host_buffer=*/false);

#endif // EXPERIMENTAL_CANVAS
};
Expand Down
7 changes: 2 additions & 5 deletions shell/gpu/gpu_surface_metal_impeller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@

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

impeller::RenderTarget render_target = surface->GetTargetRenderPassDescriptor();
surface->SetFrameBoundary(surface_frame.submit_info().frame_boundary);
Expand All @@ -183,9 +182,7 @@
display_list->Dispatch(impeller_dispatcher, sk_cull_rect);
impeller_dispatcher.FinishRecording();
aiks_context->GetContentContext().GetLazyGlyphAtlas()->ResetTextFrames();
if (reset_host_buffer) {
aiks_context->GetContentContext().GetTransientsBuffer().Reset();
}
aiks_context->GetContentContext().GetTransientsBuffer().Reset();

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

if (!surface->PreparePresent()) {
return false;
Expand Down
7 changes: 4 additions & 3 deletions shell/gpu/gpu_surface_metal_impeller_unittests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ GPUCAMetalLayerHandle GetCAMetalLayer(const SkISize& frame_info) const override
ASSERT_TRUE(frame->Submit());
}

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

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

ASSERT_TRUE(frame->Submit());
EXPECT_EQ(host_buffer.GetStateForTest().current_frame, 0u);
EXPECT_EQ(host_buffer.GetStateForTest().current_frame, 1u);

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

ASSERT_TRUE(frame->Submit());
EXPECT_EQ(host_buffer.GetStateForTest().current_frame, 1u);
EXPECT_EQ(host_buffer.GetStateForTest().current_frame, 2u);
}

#ifdef IMPELLER_DEBUG
Expand Down
9 changes: 3 additions & 6 deletions shell/gpu/gpu_surface_vulkan_impeller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ std::unique_ptr<SurfaceFrame> GPUSurfaceVulkanImpeller::AcquireFrame(
return false;
}

const bool reset_host_buffer = surface_frame.submit_info().frame_boundary;
#if EXPERIMENTAL_CANVAS
impeller::TextFrameDispatcher collector(aiks_context->GetContentContext(),
impeller::Matrix());
Expand All @@ -94,19 +93,17 @@ std::unique_ptr<SurfaceFrame> GPUSurfaceVulkanImpeller::AcquireFrame(
display_list->Dispatch(impeller_dispatcher,
SkIRect::MakeWH(cull_rect.width, cull_rect.height));
impeller_dispatcher.FinishRecording();
aiks_context->GetContentContext().GetTransientsBuffer().Reset();
aiks_context->GetContentContext().GetLazyGlyphAtlas()->ResetTextFrames();
if (reset_host_buffer) {
aiks_context->GetContentContext().GetTransientsBuffer().Reset();
}
aiks_context->GetContentContext().GetTransientsBuffer().Reset();
return true;
#else
impeller::Rect dl_cull_rect = impeller::Rect::MakeSize(cull_rect);
impeller::DlDispatcher impeller_dispatcher(dl_cull_rect);
display_list->Dispatch(impeller_dispatcher,
SkIRect::MakeWH(cull_rect.width, cull_rect.height));
auto picture = impeller_dispatcher.EndRecordingAsPicture();
return aiks_context->Render(picture, render_target, reset_host_buffer);
return aiks_context->Render(picture, render_target,
/*reset_host_buffer=*/false);
#endif
};

Expand Down