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

Commit 741d6e3

Browse files
authored
[Impeller] Release a texture during Playground teardown (#45832)
Closes flutter/flutter#134678. Partial work towards flutter/flutter#133708, flutter/flutter#133506. This gets us closer to being able to run `aiks_unittests` with validations enabled, removing a VMA allocation error: ## Before ```txt Note: Google Test filter = Play/AiksTest.ReleasesTextureOnTeardown/Vulkan [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from Play/AiksTest [ RUN ] Play/AiksTest.ReleasesTextureOnTeardown/Vulkan [INFO:capabilities_vk.cc(50)] Vulkan validations are enabled. [FATAL:third_party/vulkan_memory_allocator/include/vk_mem_alloc.h(6179)] Check failed: (false && "Unfreed dedicated allocations found!"). Vulkan Memory Allocator Failure! ``` ## After ```txt Note: Google Test filter = Play/AiksTest.ReleasesTextureOnTeardown/Vulkan [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from Play/AiksTest [ RUN ] Play/AiksTest.ReleasesTextureOnTeardown/Vulkan [INFO:capabilities_vk.cc(50)] Vulkan validations are enabled. [ OK ] Play/AiksTest.ReleasesTextureOnTeardown/Vulkan (1192 ms) [----------] 1 test from Play/AiksTest (1192 ms total) [----------] Global test environment tear-down [==========] 1 test from 1 test suite ran. (1192 ms total) [ PASSED ] 1 test. ``` --- Added a test that will catch regressions.
1 parent 40f3a75 commit 741d6e3

File tree

5 files changed

+55
-0
lines changed

5 files changed

+55
-0
lines changed

impeller/aiks/aiks_playground.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ void AiksPlayground::SetTypographerContext(
2323
typographer_context_ = std::move(typographer_context);
2424
}
2525

26+
void AiksPlayground::TearDown() {
27+
inspector_.HackResetDueToTextureLeaks();
28+
PlaygroundTest::TearDown();
29+
}
30+
2631
bool AiksPlayground::OpenPlaygroundHere(Picture picture) {
2732
return OpenPlaygroundHere([&picture](AiksContext& renderer) -> Picture {
2833
return std::move(picture);

impeller/aiks/aiks_playground.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ class AiksPlayground : public PlaygroundTest {
2222

2323
~AiksPlayground();
2424

25+
void TearDown() override;
26+
2527
void SetTypographerContext(
2628
std::shared_ptr<TypographerContext> typographer_context);
2729

impeller/aiks/aiks_playground_inspector.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ const std::optional<Picture>& AiksInspector::RenderInspector(
6363
return last_picture_;
6464
}
6565

66+
void AiksInspector::HackResetDueToTextureLeaks() {
67+
last_picture_.reset();
68+
}
69+
6670
static const auto kPropertiesProcTable = CaptureProcTable{
6771
.boolean =
6872
[](CaptureBooleanProperty& p) {

impeller/aiks/aiks_playground_inspector.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,17 @@ class AiksInspector {
2121
AiksContext& aiks_context,
2222
const std::function<std::optional<Picture>()>& picture_callback);
2323

24+
// Resets (releases) the underlying |Picture| object.
25+
//
26+
// Underlying issue: <https://github.com/flutter/flutter/issues/134678>.
27+
//
28+
// The tear-down code is not running in the right order; we still have a
29+
// reference to the |Picture| object when the |Context| is being destroyed,
30+
// which causes the |Texture| objects to leak.
31+
//
32+
// TODO(matanlurey): https://github.com/flutter/flutter/issues/134748.
33+
void HackResetDueToTextureLeaks();
34+
2435
private:
2536
void RenderCapture(CaptureContext& capture_context);
2637
void RenderCaptureElement(CaptureElement& element);

impeller/aiks/aiks_unittests.cc

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3409,5 +3409,38 @@ TEST_P(AiksTest, CaptureInactivatedByDefault) {
34093409
ASSERT_FALSE(GetContext()->capture.IsActive());
34103410
}
34113411

3412+
// Regression test for https://github.com/flutter/flutter/issues/134678.
3413+
TEST_P(AiksTest, ReleasesTextureOnTeardown) {
3414+
auto context = GetContext();
3415+
std::weak_ptr<Texture> weak_texture;
3416+
3417+
{
3418+
auto texture = CreateTextureForFixture("table_mountain_nx.png");
3419+
3420+
Canvas canvas;
3421+
canvas.Scale(GetContentScale());
3422+
canvas.Translate({100.0f, 100.0f, 0});
3423+
3424+
Paint paint;
3425+
paint.color_source = ColorSource::MakeImage(
3426+
texture, Entity::TileMode::kClamp, Entity::TileMode::kClamp, {}, {});
3427+
canvas.DrawRect({0, 0, 600, 600}, paint);
3428+
3429+
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
3430+
}
3431+
3432+
// See https://github.com/flutter/flutter/issues/134751.
3433+
//
3434+
// If the fence waiter was working this may not be released by the end of the
3435+
// scope above. Adding a manual shutdown so that future changes to the fence
3436+
// waiter will not flake this test.
3437+
context->Shutdown();
3438+
3439+
// The texture should be released by now.
3440+
ASSERT_TRUE(weak_texture.expired()) << "When the texture is no longer in use "
3441+
"by the backend, it should be "
3442+
"released.";
3443+
}
3444+
34123445
} // namespace testing
34133446
} // namespace impeller

0 commit comments

Comments
 (0)