From 8f0353c77a46306f6e630db69e8abe777b448d45 Mon Sep 17 00:00:00 2001 From: "Eunki, Hong" Date: Thu, 29 Sep 2022 21:33:32 +0900 Subject: [PATCH] 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 --- .../utc-Dali-TextureManager.cpp | 300 +++++++++++++++++++++ .../texture-manager/texture-manager-impl.cpp | 18 ++ 2 files changed, 318 insertions(+) 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 --- 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 signal) + { + mSignals.push_back(signal); + } + +public: + std::vector> 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(maskInfo[0]->mAlphaMaskId), static_cast(textureId1), static_cast(textureId2), static_cast(textureId3), static_cast(textureId4)); + + // CAPTION : HARD-CODING. + { + // Complete async load 1, 2, 3. + std::vector 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(maskInfo[0]->mAlphaMaskId), static_cast(textureId1), static_cast(textureId2), static_cast(textureId3), static_cast(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 --- 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) -- 2.7.4