Fix NPatch memory leak

Make NPatchInfo hold unique_ptr of NPatchData.
and delete copy assign operator so we can guard
untracked unique_ptr<NPatchData> memory movement.

Change-Id: I7d003cc08a51b60f91d0d8fdb33ce74b5d5cd562
Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>

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 (file)
--- 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<int> 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<int>(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<int>(i, i + 1, i + 2, i + 3), TEST_LOCATION);
+    TestBorderImage(1, gImage_34_RGBA, Rect<int>(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 (file)
--- 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<int>& 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<int>& 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 (file)
--- a/dali-toolkit/internal/visuals/npatch-loader.h
+++ b/dali-toolkit/internal/visuals/npatch-loader.h
@@ -20,6 +20,7 @@
 // EXTERNAL INCLUDES
 #include <dali/devel-api/adaptor-framework/pixel-buffer.h>
 #include <dali/public-api/rendering/texture-set.h>
+#include <memory> // for std::unique_ptr
 #include <string>
 
 // 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<NPatchData> mData;
+    std::int16_t                mReferenceCount; ///< The number of N-patch visuals that use this data.
   };
 
   /**