Purge resources on rasterizer teardown #33890
Conversation
| @@ -76,7 +77,7 @@ class MockExternalViewEmbedder : public ExternalViewEmbedder { | |||
| } // namespace | |||
|
|
|||
| TEST(RasterizerTest, create) { | |||
| MockDelegate delegate; | |||
| NiceMock<MockDelegate> delegate; | |||
There was a problem hiding this comment.
These changes are not essential to this patch, but eliminate a lot of noise when running these test targets related to unexpected mock function calls.
|
@goderbauer FYI - not sure what impact this might have on multi-window |
|
(looking into test failures...) |
|
The ShellTest vulkan related tests are crashing because they can't find libvulkan.so - it's not part of this build and isn't available on CI AFAICT. They pass before this change because no calls are made that actually touch the rendering backend, but now tearing down the rasterizer may trigger such calls. |
|
I'm not sure about the FEMU failures. The test that's timing out is ShellTest.ReportTimingsIsCalled. I do see in the log that @akbiggs do you have any ideas about how to debug this further? I do see something in the test file about tests with similar names being skipped (but those tests don't exist anymore). The test is just timing out and getting killed. One other possibility is that the new call in rasterizer teardown is somehow deadlocking... |
shell/common/rasterizer.cc
Outdated
| if (auto* context = surface_->GetContext()) { | ||
| context->purgeUnlockedResources(/*scratchResourcesOnly=*/false); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| surface_.reset(); |
There was a problem hiding this comment.
nit: can move the surface_.reset() into the if (surface_) block
dnfield
added
the
waiting for tree to go green
|
This pull request is not suitable for automatic merging in its current state.
|
fluttergithubbot
removed
the
waiting for tree to go green
|
FEMU failures seem to be related to trying to use swiftshader on FEMU. Using system libvulkan seems to be making more progress... here's hoping. |
| @@ -8,6 +8,16 @@ | |||
| #include "flutter/shell/common/context_options.h" | |||
| #include "flutter/vulkan/vulkan_utilities.h" | |||
|
|
|||
| #if OS_FUCHSIA | |||
| #define VULKAN_SO_PATH "libvulkan.so" | |||
dnfield
added
the
waiting for tree to go green
This should normally happen when the GrDirectContext is collected, but for various reasons that may not happen or it may take longer to achieve. Roughly speaking, this can happen in an Android app that:
Adds a test that causes the Skia resource cache to become occupied, and then asserts that it has purgeable bytes before teardown and no purgeable bytes after teardown.
Fixes flutter/flutter#105453 - the context may still be retained, but the app will no longer suffer from retaining purgeable cache memory.