From be673aef7227685c6aac16c74ce2abf2142e469d Mon Sep 17 00:00:00 2001 From: "Eunki, Hong" Date: Tue, 8 Nov 2022 17:41:16 +0900 Subject: [PATCH] Fix NPatch memory leak Make NPatchInfo hold unique_ptr of NPatchData. and delete copy assign operator so we can guard untracked unique_ptr memory movement. Change-Id: I7d003cc08a51b60f91d0d8fdb33ce74b5d5cd562 Signed-off-by: Eunki, Hong --- .../src/dali-toolkit/utc-Dali-ImageView.cpp | 144 +++++++++++++++++++++ dali-toolkit/internal/visuals/npatch-loader.cpp | 6 +- dali-toolkit/internal/visuals/npatch-loader.h | 20 ++- 3 files changed, 155 insertions(+), 15 deletions(-) diff --git a/automated-tests/src/dali-toolkit/utc-Dali-ImageView.cpp b/automated-tests/src/dali-toolkit/utc-Dali-ImageView.cpp index dd400db..5797b88 100644 --- a/automated-tests/src/dali-toolkit/utc-Dali-ImageView.cpp +++ b/automated-tests/src/dali-toolkit/utc-Dali-ImageView.cpp @@ -4236,3 +4236,147 @@ int UtcDaliImageViewUseSameUrlWithAnimatedImageVisual(void) DALI_TEST_EQUALS(Test::WaitForEventThreadTrigger(1), true, TEST_LOCATION); END_TEST; } + +int UtcDaliImageViewNpatchImageCacheTest01(void) +{ + tet_infoline("Test npatch image cached"); + + ToolkitTestApplication application; + ImageView imageView[2]; + + auto& gl = application.GetGlAbstraction(); + gl.EnableTextureCallTrace(true); + auto& textureCallStack = gl.GetTextureTrace(); + textureCallStack.Enable(true); + textureCallStack.EnableLogging(true); + + auto TestNPatch = [&](int index, const std::string& nPatchImageUrl, const char* location) { + if(imageView[index]) + { + imageView[index].Unparent(); + } + imageView[index] = ImageView::New(nPatchImageUrl); + imageView[index].SetProperty(Actor::Property::SIZE, Vector2(100.0f, 200.0f)); + application.GetScene().Add(imageView[index]); + }; + + TestNPatch(0, TEST_BROKEN_IMAGE_M, TEST_LOCATION); + DALI_TEST_EQUALS(Test::WaitForEventThreadTrigger(1), true, TEST_LOCATION); + + application.SendNotification(); + application.Render(); + + tet_printf("trace : \n%s\n", textureCallStack.GetTraceString().c_str()); + + // Check we gen only 1 textures + DALI_TEST_EQUALS(textureCallStack.CountMethod("GenTextures"), 1, TEST_LOCATION); + textureCallStack.Reset(); + + // Let we use cached textures + for(int i = 0; i < 10; i++) + { + TestNPatch(1, TEST_BROKEN_IMAGE_M, TEST_LOCATION); + TestNPatch(0, TEST_BROKEN_IMAGE_M, TEST_LOCATION); + } + + application.SendNotification(); + application.Render(); + // Check we use cached npatch data so we don't generate new texture textures + DALI_TEST_EQUALS(textureCallStack.CountMethod("GenTextures"), 0, TEST_LOCATION); + + // Clear all cached + imageView[0].Unparent(); + imageView[0].Reset(); + imageView[1].Unparent(); + imageView[1].Reset(); + + application.SendNotification(); + application.Render(); + + textureCallStack.Reset(); + // Let we use deference textures + TestNPatch(1, TEST_BROKEN_IMAGE_S, TEST_LOCATION); + DALI_TEST_EQUALS(Test::WaitForEventThreadTrigger(1), true, TEST_LOCATION); + + application.SendNotification(); + application.Render(); + // Try to load multiple times. + for(int i = 0; i < 3; i++) + { + TestNPatch(0, TEST_BROKEN_IMAGE_M, TEST_LOCATION); + DALI_TEST_EQUALS(Test::WaitForEventThreadTrigger(1), true, TEST_LOCATION); + TestNPatch(1, TEST_BROKEN_IMAGE_S, TEST_LOCATION); + DALI_TEST_EQUALS(Test::WaitForEventThreadTrigger(1), true, TEST_LOCATION); + } + application.SendNotification(); + application.Render(); + + imageView[0].Unparent(); + imageView[0].Reset(); + imageView[1].Unparent(); + imageView[1].Reset(); + + application.SendNotification(); + application.Render(); + + // Check memory leak + DALI_TEST_EQUALS(textureCallStack.CountMethod("GenTextures"), textureCallStack.CountMethod("DeleteTextures"), TEST_LOCATION); + + END_TEST; +} + +int UtcDaliImageViewNpatchImageCacheTest02(void) +{ + tet_infoline("Test npatch image cached with border"); + + ToolkitTestApplication application; + ImageView imageView[2]; + + auto& gl = application.GetGlAbstraction(); + gl.EnableTextureCallTrace(true); + auto& textureCallStack = gl.GetTextureTrace(); + textureCallStack.Enable(true); + textureCallStack.EnableLogging(true); + + auto TestBorderImage = [&](int index, const std::string& normalImageUrl, const Rect border, const char* location) { + if(imageView[index]) + { + imageView[index].Unparent(); + } + Property::Map map; + map[Toolkit::Visual::Property::TYPE] = Toolkit::Visual::N_PATCH; + map[Toolkit::ImageVisual::Property::URL] = normalImageUrl; + map[Toolkit::ImageVisual::Property::BORDER] = border; + + imageView[index] = ImageView::New(); + imageView[index].SetProperty(Toolkit::ImageView::Property::IMAGE, map); + imageView[index].SetProperty(Actor::Property::SIZE, Vector2(100.0f, 200.0f)); + application.GetScene().Add(imageView[index]); + }; + + TestBorderImage(0, gImage_34_RGBA, Rect(0, 0, 0, 0), TEST_LOCATION); + DALI_TEST_EQUALS(Test::WaitForEventThreadTrigger(1), true, TEST_LOCATION); + + application.SendNotification(); + application.Render(); + + tet_printf("trace : \n%s\n", textureCallStack.GetTraceString().c_str()); + + // Check we gen only 1 textures + DALI_TEST_EQUALS(textureCallStack.CountMethod("GenTextures"), 1, TEST_LOCATION); + textureCallStack.Reset(); + + // Let we use cached textures + for(int i = 0; i < 10; i++) + { + TestBorderImage(0, gImage_34_RGBA, Rect(i, i + 1, i + 2, i + 3), TEST_LOCATION); + TestBorderImage(1, gImage_34_RGBA, Rect(i + 1, i, i + 3, i + 2), TEST_LOCATION); + } + + application.SendNotification(); + application.Render(); + // Check we use cached npatch data so we don't generate new texture textures + DALI_TEST_EQUALS(textureCallStack.CountMethod("GenTextures"), 0, TEST_LOCATION); + + END_TEST; +} \ No newline at end of file diff --git a/dali-toolkit/internal/visuals/npatch-loader.cpp b/dali-toolkit/internal/visuals/npatch-loader.cpp index 4c6471e..5b2b713 100644 --- a/dali-toolkit/internal/visuals/npatch-loader.cpp +++ b/dali-toolkit/internal/visuals/npatch-loader.cpp @@ -120,7 +120,7 @@ bool NPatchLoader::GetNPatchData(const NPatchData::NPatchDataId id, const NPatch int32_t cacheIndex = GetCacheIndexFromId(id); if(cacheIndex != INVALID_CACHE_INDEX) { - data = mCache[cacheIndex].mData; + data = mCache[cacheIndex].mData.get(); return true; } data = nullptr; @@ -164,7 +164,7 @@ NPatchData* NPatchLoader::GetNPatchData(const VisualUrl& url, const Rect& b if(mCache[index].mData->GetBorder() == border) { mCache[index].mReferenceCount++; - return mCache[index].mData; + return mCache[index].mData.get(); } else { @@ -245,7 +245,7 @@ NPatchData* NPatchLoader::GetNPatchData(const VisualUrl& url, const Rect& b DALI_ASSERT_ALWAYS(infoPtr && "NPatchInfo creation failed!"); - return infoPtr->mData; + return infoPtr->mData.get(); } } // namespace Internal diff --git a/dali-toolkit/internal/visuals/npatch-loader.h b/dali-toolkit/internal/visuals/npatch-loader.h index 48301a6..539009b 100644 --- a/dali-toolkit/internal/visuals/npatch-loader.h +++ b/dali-toolkit/internal/visuals/npatch-loader.h @@ -20,6 +20,7 @@ // EXTERNAL INCLUDES #include #include +#include // for std::unique_ptr #include // INTERNAL INCLUDES @@ -117,32 +118,27 @@ private: } ~NPatchInfo() { - if(mData) - { - delete mData; - } } - NPatchInfo(NPatchInfo&& info) // move constructor + NPatchInfo(NPatchInfo&& info) noexcept // move constructor { mData = std::move(info.mData); mReferenceCount = info.mReferenceCount; - info.mData = nullptr; info.mReferenceCount = 0u; } - NPatchInfo& operator=(NPatchInfo&& info) // move operator + NPatchInfo& operator=(NPatchInfo&& info) noexcept // move operator { mData = std::move(info.mData); mReferenceCount = info.mReferenceCount; - info.mData = nullptr; info.mReferenceCount = 0u; return *this; } - NPatchInfo() = delete; // Do not use default constructor - NPatchInfo(const NPatchInfo& info) = delete; // Do not use copy constructor + NPatchInfo() = delete; // Do not use default constructor + NPatchInfo(const NPatchInfo& info) = delete; // Do not use copy constructor + NPatchInfo& operator=(const NPatchInfo& info) = delete; // Do not use copy assign - NPatchData* mData; - std::int16_t mReferenceCount; ///< The number of N-patch visuals that use this data. + std::unique_ptr mData; + std::int16_t mReferenceCount; ///< The number of N-patch visuals that use this data. }; /** -- 2.7.4