From 20a4d68338ca000e3ee0c5c71a01552eab1061e1 Mon Sep 17 00:00:00 2001 From: John Reck Date: Tue, 11 Jul 2023 17:11:51 -0400 Subject: [PATCH] Destroy prefetched layers when the context is destroyed Also fix a few places preventing the tests from running in SkiaVulkan, although that is still not enabled in general yet Fixes: 283315634 Test: CanvasContext_buildLayerDoesntLeak unit test Change-Id: I4d8a50db447e47afd8e8a267df332a173c88e888 --- libs/hwui/renderthread/CacheManager.h | 3 +- libs/hwui/renderthread/CanvasContext.cpp | 6 ++++ libs/hwui/renderthread/CanvasContext.h | 4 ++- libs/hwui/tests/common/TestUtils.h | 29 ++++++++++++--- libs/hwui/tests/unit/CanvasContextTests.cpp | 36 +++++++++++++++++++ libs/hwui/tests/unit/RenderNodeTests.cpp | 3 +- libs/hwui/tests/unit/SkiaDisplayListTests.cpp | 6 ++-- .../tests/unit/WebViewFunctorManagerTests.cpp | 22 +++++++++--- 8 files changed, 92 insertions(+), 17 deletions(-) diff --git a/libs/hwui/renderthread/CacheManager.h b/libs/hwui/renderthread/CacheManager.h index 5e43ac209696c..bcfa4f359d837 100644 --- a/libs/hwui/renderthread/CacheManager.h +++ b/libs/hwui/renderthread/CacheManager.h @@ -64,12 +64,13 @@ public: void unregisterCanvasContext(CanvasContext* context); void onContextStopped(CanvasContext* context); + bool areAllContextsStopped(); + private: friend class RenderThread; explicit CacheManager(RenderThread& thread); void setupCacheLimits(); - bool areAllContextsStopped(); void checkUiHidden(); void scheduleDestroyContext(); void cancelDestroyContext(); diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp index a5518eb9f8541..2ef7802c0a3c0 100644 --- a/libs/hwui/renderthread/CanvasContext.cpp +++ b/libs/hwui/renderthread/CanvasContext.cpp @@ -125,6 +125,7 @@ CanvasContext::CanvasContext(RenderThread& thread, bool translucent, RenderNode* , mRenderPipeline(std::move(renderPipeline)) , mHintSessionWrapper(uiThreadId, renderThreadId) { mRenderThread.cacheManager().registerCanvasContext(this); + mRenderThread.renderState().registerContextCallback(this); rootRenderNode->makeRoot(); mRenderNodes.emplace_back(rootRenderNode); mProfiler.setDensity(DeviceInfo::getDensity()); @@ -137,6 +138,7 @@ CanvasContext::~CanvasContext() { } mRenderNodes.clear(); mRenderThread.cacheManager().unregisterCanvasContext(this); + mRenderThread.renderState().removeContextCallback(this); } void CanvasContext::addRenderNode(RenderNode* node, bool placeFront) { @@ -963,6 +965,10 @@ void CanvasContext::destroyHardwareResources() { } } +void CanvasContext::onContextDestroyed() { + destroyHardwareResources(); +} + DeferredLayerUpdater* CanvasContext::createTextureLayer() { return mRenderPipeline->createTextureLayer(); } diff --git a/libs/hwui/renderthread/CanvasContext.h b/libs/hwui/renderthread/CanvasContext.h index 32ac5af94c146..3f02674d3b531 100644 --- a/libs/hwui/renderthread/CanvasContext.h +++ b/libs/hwui/renderthread/CanvasContext.h @@ -43,6 +43,7 @@ #include "Lighting.h" #include "ReliableSurface.h" #include "RenderNode.h" +#include "renderstate/RenderState.h" #include "renderthread/RenderTask.h" #include "renderthread/RenderThread.h" #include "utils/RingBuffer.h" @@ -64,7 +65,7 @@ class Frame; // This per-renderer class manages the bridge between the global EGL context // and the render surface. // TODO: Rename to Renderer or some other per-window, top-level manager -class CanvasContext : public IFrameCallback { +class CanvasContext : public IFrameCallback, public IGpuContextCallback { public: static CanvasContext* create(RenderThread& thread, bool translucent, RenderNode* rootRenderNode, IContextFactory* contextFactory, pid_t uiThreadId, @@ -154,6 +155,7 @@ public: void markLayerInUse(RenderNode* node); void destroyHardwareResources(); + void onContextDestroyed() override; DeferredLayerUpdater* createTextureLayer(); diff --git a/libs/hwui/tests/common/TestUtils.h b/libs/hwui/tests/common/TestUtils.h index 81ecfe59d3bc8..7ef82a7ff6e50 100644 --- a/libs/hwui/tests/common/TestUtils.h +++ b/libs/hwui/tests/common/TestUtils.h @@ -307,13 +307,21 @@ public: int destroyed = 0; int removeOverlays = 0; int glesDraw = 0; + int vkInitialize = 0; + int vkDraw = 0; + int vkPostDraw = 0; }; static void expectOnRenderThread(const std::string_view& function = "unknown") { EXPECT_EQ(gettid(), TestUtils::getRenderThreadTid()) << "Called on wrong thread: " << function; } - static WebViewFunctorCallbacks createMockFunctor(RenderMode mode) { + static int createMockFunctor() { + const auto renderMode = WebViewFunctor_queryPlatformRenderMode(); + return WebViewFunctor_create(nullptr, createMockFunctorCallbacks(renderMode), renderMode); + } + + static WebViewFunctorCallbacks createMockFunctorCallbacks(RenderMode mode) { auto callbacks = WebViewFunctorCallbacks{ .onSync = [](int functor, void* client_data, const WebViewSyncData& data) { @@ -345,9 +353,22 @@ public: sMockFunctorCounts[functor].glesDraw++; }; break; - default: - ADD_FAILURE(); - return WebViewFunctorCallbacks{}; + case RenderMode::Vulkan: + callbacks.vk.initialize = [](int functor, void* data, + const VkFunctorInitParams& params) { + expectOnRenderThread("initialize"); + sMockFunctorCounts[functor].vkInitialize++; + }; + callbacks.vk.draw = [](int functor, void* data, const VkFunctorDrawParams& params, + const WebViewOverlayData& overlayParams) { + expectOnRenderThread("draw"); + sMockFunctorCounts[functor].vkDraw++; + }; + callbacks.vk.postDraw = [](int functor, void* data) { + expectOnRenderThread("postDraw"); + sMockFunctorCounts[functor].vkPostDraw++; + }; + break; } return callbacks; } diff --git a/libs/hwui/tests/unit/CanvasContextTests.cpp b/libs/hwui/tests/unit/CanvasContextTests.cpp index 9e376e32f8ea5..47a41057cac90 100644 --- a/libs/hwui/tests/unit/CanvasContextTests.cpp +++ b/libs/hwui/tests/unit/CanvasContextTests.cpp @@ -19,6 +19,7 @@ #include "AnimationContext.h" #include "IContextFactory.h" #include "renderthread/CanvasContext.h" +#include "renderthread/VulkanManager.h" #include "tests/common/TestUtils.h" using namespace android; @@ -42,3 +43,38 @@ RENDERTHREAD_TEST(CanvasContext, create) { canvasContext->destroy(); } + +RENDERTHREAD_TEST(CanvasContext, buildLayerDoesntLeak) { + auto node = TestUtils::createNode(0, 0, 200, 400, [](RenderProperties& props, Canvas& canvas) { + canvas.drawColor(0xFFFF0000, SkBlendMode::kSrc); + }); + ASSERT_TRUE(node->isValid()); + EXPECT_EQ(LayerType::None, node->stagingProperties().effectiveLayerType()); + node->mutateStagingProperties().mutateLayerProperties().setType(LayerType::RenderLayer); + + auto& cacheManager = renderThread.cacheManager(); + EXPECT_TRUE(cacheManager.areAllContextsStopped()); + ContextFactory contextFactory; + std::unique_ptr canvasContext( + CanvasContext::create(renderThread, false, node.get(), &contextFactory, 0, 0)); + canvasContext->buildLayer(node.get()); + EXPECT_TRUE(node->hasLayer()); + if (Properties::getRenderPipelineType() == RenderPipelineType::SkiaVulkan) { + auto instance = VulkanManager::peekInstance(); + if (instance) { + EXPECT_TRUE(instance->hasVkContext()); + } else { + ADD_FAILURE() << "VulkanManager wasn't initialized to buildLayer?"; + } + } + renderThread.destroyRenderingContext(); + EXPECT_FALSE(node->hasLayer()) << "Node still has a layer after rendering context destroyed"; + + if (Properties::getRenderPipelineType() == RenderPipelineType::SkiaVulkan) { + auto instance = VulkanManager::peekInstance(); + if (instance) { + ADD_FAILURE() << "VulkanManager still exists"; + EXPECT_FALSE(instance->hasVkContext()); + } + } +} diff --git a/libs/hwui/tests/unit/RenderNodeTests.cpp b/libs/hwui/tests/unit/RenderNodeTests.cpp index 80796f4a41116..8273524167f96 100644 --- a/libs/hwui/tests/unit/RenderNodeTests.cpp +++ b/libs/hwui/tests/unit/RenderNodeTests.cpp @@ -231,8 +231,7 @@ TEST(RenderNode, multiTreeValidity) { } TEST(RenderNode, releasedCallback) { - int functor = WebViewFunctor_create( - nullptr, TestUtils::createMockFunctor(RenderMode::OpenGL_ES), RenderMode::OpenGL_ES); + int functor = TestUtils::createMockFunctor(); auto node = TestUtils::createNode(0, 0, 200, 400, [&](RenderProperties& props, Canvas& canvas) { canvas.drawWebViewFunctor(functor); diff --git a/libs/hwui/tests/unit/SkiaDisplayListTests.cpp b/libs/hwui/tests/unit/SkiaDisplayListTests.cpp index f825d7c5d9cc6..1a1ce1e9cf668 100644 --- a/libs/hwui/tests/unit/SkiaDisplayListTests.cpp +++ b/libs/hwui/tests/unit/SkiaDisplayListTests.cpp @@ -48,8 +48,7 @@ TEST(SkiaDisplayList, reset) { SkCanvas dummyCanvas; RenderNodeDrawable drawable(nullptr, &dummyCanvas); skiaDL->mChildNodes.emplace_back(nullptr, &dummyCanvas); - int functor1 = WebViewFunctor_create( - nullptr, TestUtils::createMockFunctor(RenderMode::OpenGL_ES), RenderMode::OpenGL_ES); + int functor1 = TestUtils::createMockFunctor(); GLFunctorDrawable functorDrawable{functor1, &dummyCanvas}; WebViewFunctor_release(functor1); skiaDL->mChildFunctors.push_back(&functorDrawable); @@ -101,8 +100,7 @@ TEST(SkiaDisplayList, syncContexts) { SkCanvas dummyCanvas; - int functor1 = WebViewFunctor_create( - nullptr, TestUtils::createMockFunctor(RenderMode::OpenGL_ES), RenderMode::OpenGL_ES); + int functor1 = TestUtils::createMockFunctor(); auto& counts = TestUtils::countsForFunctor(functor1); skiaDL.mChildFunctors.push_back( skiaDL.allocateDrawable(functor1, &dummyCanvas)); diff --git a/libs/hwui/tests/unit/WebViewFunctorManagerTests.cpp b/libs/hwui/tests/unit/WebViewFunctorManagerTests.cpp index e1fb8b7069ff5..5e8f13d261c7c 100644 --- a/libs/hwui/tests/unit/WebViewFunctorManagerTests.cpp +++ b/libs/hwui/tests/unit/WebViewFunctorManagerTests.cpp @@ -26,9 +26,15 @@ using namespace android; using namespace android::uirenderer; +#define ASSUME_GLES() \ + if (WebViewFunctor_queryPlatformRenderMode() != RenderMode::OpenGL_ES) \ + GTEST_SKIP() << "Not in GLES, skipping test" + TEST(WebViewFunctor, createDestroyGLES) { + ASSUME_GLES(); int functor = WebViewFunctor_create( - nullptr, TestUtils::createMockFunctor(RenderMode::OpenGL_ES), RenderMode::OpenGL_ES); + nullptr, TestUtils::createMockFunctorCallbacks(RenderMode::OpenGL_ES), + RenderMode::OpenGL_ES); ASSERT_NE(-1, functor); WebViewFunctor_release(functor); TestUtils::runOnRenderThreadUnmanaged([](renderthread::RenderThread&) { @@ -41,8 +47,10 @@ TEST(WebViewFunctor, createDestroyGLES) { } TEST(WebViewFunctor, createSyncHandleGLES) { + ASSUME_GLES(); int functor = WebViewFunctor_create( - nullptr, TestUtils::createMockFunctor(RenderMode::OpenGL_ES), RenderMode::OpenGL_ES); + nullptr, TestUtils::createMockFunctorCallbacks(RenderMode::OpenGL_ES), + RenderMode::OpenGL_ES); ASSERT_NE(-1, functor); auto handle = WebViewFunctorManager::instance().handleFor(functor); ASSERT_TRUE(handle); @@ -82,8 +90,10 @@ TEST(WebViewFunctor, createSyncHandleGLES) { } TEST(WebViewFunctor, createSyncDrawGLES) { + ASSUME_GLES(); int functor = WebViewFunctor_create( - nullptr, TestUtils::createMockFunctor(RenderMode::OpenGL_ES), RenderMode::OpenGL_ES); + nullptr, TestUtils::createMockFunctorCallbacks(RenderMode::OpenGL_ES), + RenderMode::OpenGL_ES); ASSERT_NE(-1, functor); auto handle = WebViewFunctorManager::instance().handleFor(functor); ASSERT_TRUE(handle); @@ -108,9 +118,11 @@ TEST(WebViewFunctor, createSyncDrawGLES) { EXPECT_EQ(1, counts.destroyed); } -TEST(WebViewFunctor, contextDestroyed) { +TEST(WebViewFunctor, contextDestroyedGLES) { + ASSUME_GLES(); int functor = WebViewFunctor_create( - nullptr, TestUtils::createMockFunctor(RenderMode::OpenGL_ES), RenderMode::OpenGL_ES); + nullptr, TestUtils::createMockFunctorCallbacks(RenderMode::OpenGL_ES), + RenderMode::OpenGL_ES); ASSERT_NE(-1, functor); auto handle = WebViewFunctorManager::instance().handleFor(functor); ASSERT_TRUE(handle);