Guard textureId during CheckForWaitingTexture

We can remove & assign textures during NotifyObserver.
In this case, we might have some timing issue of
same-texture-id.

This patch increase for each textureId's reference
so we can assume that this textureId is valid during
CheckForWaitingTexture API running.

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

diff --git a/automated-tests/src/dali-toolkit-internal/utc-Dali-TextureManager.cpp b/automated-tests/src/dali-toolkit-internal/utc-Dali-TextureManager.cpp
index c80d1b5..f12b096 100644 (file)
--- a/automated-tests/src/dali-toolkit-internal/utc-Dali-TextureManager.cpp
+++ b/automated-tests/src/dali-toolkit-internal/utc-Dali-TextureManager.cpp
@@ -56,6 +56,8 @@ namespace
 {
 const char* TEST_IMAGE_FILE_NAME   = TEST_RESOURCE_DIR "/gallery-small-1.jpg";
 const char* TEST_IMAGE_2_FILE_NAME = TEST_RESOURCE_DIR "/icon-delete.png";
+const char* TEST_IMAGE_3_FILE_NAME = TEST_RESOURCE_DIR "/icon-edit.png";
+const char* TEST_IMAGE_4_FILE_NAME = TEST_RESOURCE_DIR "/application-icon-20.png";
 const char* TEST_MASK_FILE_NAME    = TEST_RESOURCE_DIR "/mask.png";
 
 class TestObserver : public Dali::Toolkit::TextureUploadObserver
@@ -135,6 +137,55 @@ protected:
   TextureManager* mTextureManagerPtr; // Keep the pointer of texture manager.
 };
 
+class TestObserverWithCustomFunction : public TestObserver
+{
+public:
+  TestObserverWithCustomFunction()
+  : TestObserver(),
+    mSignals{},
+    mData{nullptr},
+    mKeepSignal{false}
+  {
+  }
+
+  virtual void LoadComplete(bool loadSuccess, TextureInformation textureInformation) override
+  {
+    if(textureInformation.returnType == TextureUploadObserver::ReturnType::TEXTURE)
+    {
+      mCompleteType = CompleteType::UPLOAD_COMPLETE;
+    }
+    else
+    {
+      mCompleteType = CompleteType::LOAD_COMPLETE;
+    }
+    mLoaded         = loadSuccess;
+    mObserverCalled = true;
+    mTextureSet     = textureInformation.textureSet;
+
+    // Execute signals.
+    for(size_t i = 0; i < mSignals.size(); i++)
+    {
+      mSignals[i](mData);
+    }
+
+    // Clear signals.
+    if(!mKeepSignal)
+    {
+      mSignals.clear();
+    }
+  }
+
+  void ConnectFunction(std::function<void(void*)> signal)
+  {
+    mSignals.push_back(signal);
+  }
+
+public:
+  std::vector<std::function<void(void*)>> mSignals;
+  void*                                   mData;
+  bool                                    mKeepSignal;
+};
+
 } // namespace
 
 int UtcTextureManagerRequestLoad(void)
@@ -1376,3 +1427,252 @@ int UtcTextureManagerMaskCacheTest(void)
 
   END_TEST;
 }
+
+int UtcTextureManagerRemoveDuringGPUMasking(void)
+{
+  ToolkitTestApplication application;
+  tet_infoline("UtcTextureManagerRemoveDuringGPUMasking");
+  tet_infoline("Request 3 different GPU masking image.");
+  tet_infoline("Control to mask image load last. and then, check execute result.");
+
+  TextureManager textureManager; // Create new texture manager
+
+  TestObserverWithCustomFunction observer1;
+  TestObserverWithCustomFunction observer2;
+  TestObserver                   observer3;
+  TestObserver                   observer4;
+
+  std::string filename1(TEST_IMAGE_FILE_NAME);
+  std::string filename2(TEST_IMAGE_2_FILE_NAME);
+  std::string filename3(TEST_IMAGE_3_FILE_NAME);
+  std::string filename4(TEST_IMAGE_4_FILE_NAME);
+
+  auto textureId1(TextureManager::INVALID_TEXTURE_ID);
+  auto textureId2(TextureManager::INVALID_TEXTURE_ID);
+  auto textureId3(TextureManager::INVALID_TEXTURE_ID);
+  auto textureId4(TextureManager::INVALID_TEXTURE_ID);
+
+  std::string                        maskname(TEST_MASK_FILE_NAME);
+  TextureManager::MaskingDataPointer maskInfo[3] = {nullptr, nullptr, nullptr};
+  for(int i = 0; i < 3; i++)
+  {
+    maskInfo[i].reset(new TextureManager::MaskingData());
+    maskInfo[i]->mAlphaMaskUrl       = maskname;
+    maskInfo[i]->mAlphaMaskId        = TextureManager::INVALID_TEXTURE_ID;
+    maskInfo[i]->mCropToMask         = true;
+    maskInfo[i]->mPreappliedMasking  = false; // To make GPU masking
+    maskInfo[i]->mContentScaleFactor = 1.0f;
+  }
+  Vector4                       atlasRect(0.f, 0.f, 1.f, 1.f);
+  Dali::ImageDimensions         atlasRectSize(0, 0);
+  bool                          synchronousLoading(false);
+  bool                          atlasingStatus(false);
+  bool                          loadingStatus(false);
+  auto                          preMultiply         = TextureManager::MultiplyOnLoad::LOAD_WITHOUT_MULTIPLY;
+  ImageAtlasManagerPtr          atlasManager        = nullptr;
+  Toolkit::AtlasUploadObserver* atlasUploadObserver = nullptr;
+
+  // Request image 1, 2, 3 with GPU masking
+  textureManager.LoadTexture(
+    filename1,
+    ImageDimensions(),
+    FittingMode::SCALE_TO_FILL,
+    SamplingMode::BOX_THEN_LINEAR,
+    maskInfo[0],
+    synchronousLoading,
+    textureId1,
+    atlasRect,
+    atlasRectSize,
+    atlasingStatus,
+    loadingStatus,
+    &observer1,
+    atlasUploadObserver,
+    atlasManager,
+    true,
+    TextureManager::ReloadPolicy::CACHED,
+    preMultiply);
+
+  textureManager.LoadTexture(
+    filename2,
+    ImageDimensions(),
+    FittingMode::SCALE_TO_FILL,
+    SamplingMode::BOX_THEN_LINEAR,
+    maskInfo[1],
+    synchronousLoading,
+    textureId2,
+    atlasRect,
+    atlasRectSize,
+    atlasingStatus,
+    loadingStatus,
+    &observer2,
+    atlasUploadObserver,
+    atlasManager,
+    true,
+    TextureManager::ReloadPolicy::CACHED,
+    preMultiply);
+
+  textureManager.LoadTexture(
+    filename3,
+    ImageDimensions(),
+    FittingMode::SCALE_TO_FILL,
+    SamplingMode::BOX_THEN_LINEAR,
+    maskInfo[2],
+    synchronousLoading,
+    textureId3,
+    atlasRect,
+    atlasRectSize,
+    atlasingStatus,
+    loadingStatus,
+    &observer3,
+    atlasUploadObserver,
+    atlasManager,
+    true,
+    TextureManager::ReloadPolicy::CACHED,
+    preMultiply);
+
+  DALI_TEST_EQUALS(observer1.mLoaded, false, TEST_LOCATION);
+  DALI_TEST_EQUALS(observer1.mObserverCalled, false, TEST_LOCATION);
+  DALI_TEST_EQUALS(observer2.mLoaded, false, TEST_LOCATION);
+  DALI_TEST_EQUALS(observer2.mObserverCalled, false, TEST_LOCATION);
+  DALI_TEST_EQUALS(observer3.mLoaded, false, TEST_LOCATION);
+  DALI_TEST_EQUALS(observer3.mObserverCalled, false, TEST_LOCATION);
+  DALI_TEST_EQUALS(observer4.mLoaded, false, TEST_LOCATION);
+  DALI_TEST_EQUALS(observer4.mObserverCalled, false, TEST_LOCATION);
+
+  // Check we use cached mask image
+  DALI_TEST_CHECK(maskInfo[0]->mAlphaMaskId != TextureManager::INVALID_TEXTURE_ID);
+  DALI_TEST_EQUALS(maskInfo[0]->mAlphaMaskId, maskInfo[1]->mAlphaMaskId, TEST_LOCATION);
+  DALI_TEST_EQUALS(maskInfo[0]->mAlphaMaskId, maskInfo[2]->mAlphaMaskId, TEST_LOCATION);
+
+  // Connect observer1 custom function
+  struct CustomData1
+  {
+    TextureManager*           textureManagerPtr{nullptr};
+    TextureManager::TextureId removeTextureId{TextureManager::INVALID_TEXTURE_ID};
+    TestObserver*             removeTextureObserver{nullptr};
+  };
+  CustomData1 data1;
+  data1.textureManagerPtr     = &textureManager;
+  data1.removeTextureId       = textureId3;
+  data1.removeTextureObserver = &observer3;
+
+  observer1.mData = &data1;
+  observer1.ConnectFunction(
+    [](void* data) {
+      DALI_TEST_CHECK(data);
+      CustomData1 data1 = *(CustomData1*)data;
+
+      DALI_TEST_CHECK(data1.textureManagerPtr);
+      DALI_TEST_CHECK(data1.removeTextureId != TextureManager::INVALID_TEXTURE_ID);
+      DALI_TEST_CHECK(data1.removeTextureObserver);
+
+      // Remove textureId3
+      data1.textureManagerPtr->Remove(data1.removeTextureId, data1.removeTextureObserver);
+    });
+
+  // Connect observer2 custom function
+  struct CustomData2
+  {
+    TextureManager*            textureManagerPtr{nullptr};
+    std::string                addTextureUrl{};
+    TextureManager::TextureId* addTextureIdPtr{nullptr};
+    TestObserver*              addTextureObserver{nullptr};
+  };
+  CustomData2 data2;
+  data2.textureManagerPtr  = &textureManager;
+  data2.addTextureUrl      = filename4;
+  data2.addTextureIdPtr    = &textureId4;
+  data2.addTextureObserver = &observer4;
+
+  observer2.mData = &data2;
+  observer2.ConnectFunction(
+    [](void* data) {
+      DALI_TEST_CHECK(data);
+      CustomData2 data2 = *(CustomData2*)data;
+
+      DALI_TEST_CHECK(data2.textureManagerPtr);
+      DALI_TEST_CHECK(!data2.addTextureUrl.empty());
+      DALI_TEST_CHECK(data2.addTextureIdPtr);
+      DALI_TEST_CHECK(data2.addTextureObserver);
+
+      auto preMultiply = TextureManager::MultiplyOnLoad::LOAD_WITHOUT_MULTIPLY;
+
+      // Load textureId4
+      (*data2.addTextureIdPtr) = data2.textureManagerPtr->RequestLoad(
+        data2.addTextureUrl,
+        ImageDimensions(),
+        FittingMode::SCALE_TO_FILL,
+        SamplingMode::BOX_THEN_LINEAR,
+        TextureManager::UseAtlas::NO_ATLAS,
+        data2.addTextureObserver,
+        true,
+        TextureManager::ReloadPolicy::CACHED,
+        preMultiply);
+    });
+
+  application.SendNotification();
+  application.Render();
+
+  tet_printf("Id info - mask : {%d}, 1 : {%d}, 2 : {%d}, 3 : {%d}, 4 : {%d}\n", static_cast<int>(maskInfo[0]->mAlphaMaskId), static_cast<int>(textureId1), static_cast<int>(textureId2), static_cast<int>(textureId3), static_cast<int>(textureId4));
+
+  // CAPTION : HARD-CODING.
+  {
+    // Complete async load 1, 2, 3.
+    std::vector<Devel::PixelBuffer> pixelBuffers;
+
+    pixelBuffers.clear();
+    pixelBuffers.push_back(Devel::PixelBuffer::New(1, 1, Pixel::Format::RGB888));
+    textureManager.AsyncLoadComplete(textureId1, pixelBuffers);
+    pixelBuffers.clear();
+    pixelBuffers.push_back(Devel::PixelBuffer::New(1, 1, Pixel::Format::RGB888));
+    textureManager.AsyncLoadComplete(textureId2, pixelBuffers);
+    pixelBuffers.clear();
+    pixelBuffers.push_back(Devel::PixelBuffer::New(1, 1, Pixel::Format::RGB888));
+    textureManager.AsyncLoadComplete(textureId3, pixelBuffers);
+
+    DALI_TEST_EQUALS(observer1.mLoaded, false, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer1.mObserverCalled, false, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer2.mLoaded, false, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer2.mObserverCalled, false, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer3.mLoaded, false, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer3.mObserverCalled, false, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer4.mLoaded, false, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer4.mObserverCalled, false, TEST_LOCATION);
+
+    // Complete mask load.
+    pixelBuffers.clear();
+    pixelBuffers.push_back(Devel::PixelBuffer::New(1, 1, Pixel::Format::L8));
+    textureManager.AsyncLoadComplete(maskInfo[0]->mAlphaMaskId, pixelBuffers);
+
+    tet_printf("Id info after observer notify - mask : {%d}, 1 : {%d}, 2 : {%d}, 3 : {%d}, 4 : {%d}\n", static_cast<int>(maskInfo[0]->mAlphaMaskId), static_cast<int>(textureId1), static_cast<int>(textureId2), static_cast<int>(textureId3), static_cast<int>(textureId4));
+
+    // Check observer 1 and 2 called, but 3 and 4 not called.
+    DALI_TEST_EQUALS(observer1.mLoaded, true, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer1.mObserverCalled, true, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer2.mLoaded, true, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer2.mObserverCalled, true, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer3.mLoaded, false, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer3.mObserverCalled, false, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer4.mLoaded, false, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer4.mObserverCalled, false, TEST_LOCATION);
+
+    // Check textureId4
+    DALI_TEST_CHECK(textureId4 != TextureManager::INVALID_TEXTURE_ID);
+
+    // Complete 4.
+    pixelBuffers.clear();
+    pixelBuffers.push_back(Devel::PixelBuffer::New(1, 1, Pixel::Format::RGB888));
+    textureManager.AsyncLoadComplete(textureId4, pixelBuffers);
+
+    DALI_TEST_EQUALS(observer1.mLoaded, true, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer1.mObserverCalled, true, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer2.mLoaded, true, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer2.mObserverCalled, true, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer3.mLoaded, false, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer3.mObserverCalled, false, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer4.mLoaded, true, TEST_LOCATION);
+    DALI_TEST_EQUALS(observer4.mObserverCalled, true, TEST_LOCATION);
+  }
+
+  END_TEST;
+}
\ No newline at end of file

diff --git a/dali-toolkit/internal/texture-manager/texture-manager-impl.cpp b/dali-toolkit/internal/texture-manager/texture-manager-impl.cpp
index ab237a3..33405be 100644 (file)
--- a/dali-toolkit/internal/texture-manager/texture-manager-impl.cpp
+++ b/dali-toolkit/internal/texture-manager/texture-manager-impl.cpp
@@ -1149,6 +1149,12 @@ void TextureManager::CheckForWaitingTexture(TextureManager::TextureInfo& maskTex
           pixelBuffers.push_back(textureInfo.pixelBuffer);
           UploadTextures(pixelBuffers, textureInfo);
 
+          // Increase reference counts for notify required textureId.
+          // Now we can assume that we don't remove & re-assign this textureId
+          // during NotifyObserver signal emit.
+          maskTextureInfo.referenceCount++;
+          textureInfo.referenceCount++;
+
           notifyRequiredTextureIds.push_back(textureInfo.textureId);
         }
       }
@@ -1160,6 +1166,12 @@ void TextureManager::CheckForWaitingTexture(TextureManager::TextureInfo& maskTex
         pixelBuffers.push_back(textureInfo.pixelBuffer);
         UploadTextures(pixelBuffers, textureInfo);
 
+        // Increase reference counts for notify required textureId.
+        // Now we can assume that we don't remove & re-assign this textureId
+        // during NotifyObserver signal emit.
+        maskTextureInfo.referenceCount++;
+        textureInfo.referenceCount++;
+
         notifyRequiredTextureIds.push_back(textureInfo.textureId);
       }
     }
@@ -1175,6 +1187,12 @@ void TextureManager::CheckForWaitingTexture(TextureManager::TextureInfo& maskTex
       NotifyObservers(textureInfo, true);
     }
   }
+
+  // Decrease reference count
+  for(const auto textureId : notifyRequiredTextureIds)
+  {
+    Remove(textureId, nullptr);
+  }
 }
 
 void TextureManager::ApplyMask(TextureManager::TextureInfo& textureInfo, const TextureManager::TextureId& maskTextureId)