From: Eunki, Hong Date: Fri, 23 Sep 2022 07:14:54 +0000 (+0900) Subject: Minor refactor about load/remove queue X-Git-Tag: dali_2.1.42~1 X-Git-Url: http://review.tizen.org/git/?p=platform%2Fcore%2Fuifw%2Fdali-toolkit.git;a=commitdiff_plain;h=ca3e0ebcf6a67c3f6f04b74ce334fde849a7f9a4 Minor refactor about load/remove queue 1. Fix crash issue when we try to load mask image during ResourceReady 2. Remove useless duplicated codes for mRemoveQueue. 3. Make some API works even observer is nullptr (for mask case) 4. Notify observers timing changed when mask image postload Change-Id: I73f4816c628d3808d7355e17d358226af2985db8 Signed-off-by: Eunki, Hong --- diff --git a/automated-tests/src/dali-toolkit/utc-Dali-ImageView.cpp b/automated-tests/src/dali-toolkit/utc-Dali-ImageView.cpp index 1559ae7..34aa8a3 100644 --- a/automated-tests/src/dali-toolkit/utc-Dali-ImageView.cpp +++ b/automated-tests/src/dali-toolkit/utc-Dali-ImageView.cpp @@ -3474,7 +3474,7 @@ void OnResourceReadySignal06(Control control) // We hope this request result is return later than gImageView2. Property::Map map1; - map1[Toolkit::ImageVisual::Property::URL] = TEST_IMAGE_1; + map1[Toolkit::ImageVisual::Property::URL] = TEST_IMAGE_1; map1[Toolkit::ImageVisual::Property::ALPHA_MASK_URL] = TEST_BROKEN_IMAGE_DEFAULT; gImageView3 = ImageView::New(); @@ -3482,9 +3482,9 @@ void OnResourceReadySignal06(Control control) gImageView3.ResourceReadySignal().Connect(&OnResourceReadySignal06); Property::Map map2; - map2[Toolkit::ImageVisual::Property::URL] = TEST_IMAGE_2; + map2[Toolkit::ImageVisual::Property::URL] = TEST_IMAGE_2; map2[Toolkit::ImageVisual::Property::ALPHA_MASK_URL] = TEST_BROKEN_IMAGE_S; - gImageView4 = ImageView::New(); + gImageView4 = ImageView::New(); gImageView4.SetProperty(Toolkit::ImageView::Property::IMAGE, map2); gImageView4.ResourceReadySignal().Connect(&OnResourceReadySignal06); @@ -3529,6 +3529,28 @@ void OnResourceReadySignal06(Control control) } } +void OnResourceReadySignal07(Control control) +{ + gResourceReadySignalCounter++; + // Load masked image + tet_printf("rc %d %d\n", gResourceReadySignalCounter, static_cast(gImageView2)); + + if(!gImageView2) + { + auto scene = gImageView1.GetParent(); + + Property::Map map1; + map1[Toolkit::ImageVisual::Property::URL] = TEST_IMAGE_1; + map1[Toolkit::ImageVisual::Property::ALPHA_MASK_URL] = TEST_BROKEN_IMAGE_DEFAULT; + + gImageView2 = ImageView::New(); + gImageView2.SetProperty(Toolkit::ImageView::Property::IMAGE, map1); + gImageView2.ResourceReadySignal().Connect(&OnResourceReadySignal07); + + scene.Add(gImageView2); + } +} + } // namespace int UtcDaliImageViewSetImageOnResourceReadySignal01(void) @@ -3966,7 +3988,7 @@ int UtcDaliImageViewSetImageOnResourceReadySignal06(void) gResourceReadySignal06ComesOrder = 0; Property::Map map; - map[Toolkit::ImageVisual::Property::URL] = "invalid.jpg"; + map[Toolkit::ImageVisual::Property::URL] = "invalid.jpg"; map[Toolkit::ImageVisual::Property::ALPHA_MASK_URL] = "invalid.png"; gImageView1 = ImageView::New(); // request invalid image, to make loading failed fast. @@ -4015,6 +4037,57 @@ int UtcDaliImageViewSetImageOnResourceReadySignal06(void) END_TEST; } +int UtcDaliImageViewSetImageOnResourceReadySignal07(void) +{ + tet_infoline("Test texturemanager's remove image & mask queue works well within signal handler 02."); + + ToolkitTestApplication application; + + gResourceReadySignalCounter = 0; + + Property::Map map; + map[Toolkit::ImageVisual::Property::URL] = TEST_IMAGE_1; + + // Clear image view for clear test + + if(gImageView1) + { + gImageView1.Reset(); + } + if(gImageView2) + { + gImageView2.Reset(); + } + + gImageView1 = ImageView::New(); + gImageView1.SetProperty(Toolkit::ImageView::Property::IMAGE, map); + gImageView1.ResourceReadySignal().Connect(&OnResourceReadySignal07); + application.GetScene().Add(gImageView1); + + application.SendNotification(); + application.Render(); + + // Load gImageView1 + + DALI_TEST_EQUALS(Test::WaitForEventThreadTrigger(1), true, TEST_LOCATION); + DALI_TEST_EQUALS(gResourceReadySignalCounter, 1, TEST_LOCATION); + + tet_infoline("load image1 done"); + + // Load gImageView2 and mask + + DALI_TEST_EQUALS(Test::WaitForEventThreadTrigger(2), true, TEST_LOCATION); + DALI_TEST_EQUALS(gResourceReadySignalCounter, 1, TEST_LOCATION); + + // gImageView2 mask apply done + + DALI_TEST_EQUALS(Test::WaitForEventThreadTrigger(1), true, TEST_LOCATION); + DALI_TEST_EQUALS(gResourceReadySignalCounter, 2, TEST_LOCATION); + + tet_infoline("load image2 done"); + END_TEST; +} + int UtcDaliImageViewUseSameUrlWithAnimatedImageVisual(void) { tet_infoline("Test multiple views with same image in animated image visual"); diff --git a/dali-toolkit/internal/texture-manager/texture-manager-impl.cpp b/dali-toolkit/internal/texture-manager/texture-manager-impl.cpp index f761781..ab237a3 100644 --- a/dali-toolkit/internal/texture-manager/texture-manager-impl.cpp +++ b/dali-toolkit/internal/texture-manager/texture-manager-impl.cpp @@ -711,29 +711,39 @@ void TextureManager::Remove(const TextureManager::TextureId& textureId, TextureU { TextureManager::TextureId maskTextureId = INVALID_TEXTURE_ID; TextureInfo& textureInfo(mTextureCacheManager[textureCacheIndex]); - if(textureInfo.maskTextureId != INVALID_TEXTURE_ID) + // We only need to consider maskTextureId when texture's loadState is not CANCELLED. Because it is already deleted. + if(textureInfo.loadState != LoadState::CANCELLED) { - maskTextureId = textureInfo.maskTextureId; + if(textureInfo.maskTextureId != INVALID_TEXTURE_ID) + { + maskTextureId = textureInfo.maskTextureId; + } } // the case that LoadingQueue is working. if(mLoadingQueueTextureId != INVALID_TEXTURE_ID) { // If textureId is not same, this observer need to delete when ProcessRemoveQueue() is called. - TextureUploadObserver* queueObserver = nullptr; - if(mLoadingQueueTextureId != textureId) + // If textureId is same, we should not call RemoveTextureObserver. + // Because ObserverDestroyed signal already disconnected in NotifyObservers + TextureUploadObserver* queueObserver = observer; + if(mLoadingQueueTextureId == textureId) { - queueObserver = observer; + queueObserver = nullptr; } - // Remove textureId after NotifyObserver finished - if(maskTextureId != INVALID_TEXTURE_ID) + // Remove element from the mLoadQueue + for(auto&& element : mLoadQueue) { - if(textureInfo.loadState != LoadState::CANCELLED) + if(element.mTextureId == textureId && element.mObserver == observer) { - mRemoveQueue.PushBack(QueueElement(maskTextureId, nullptr)); + // Do not erase the item. We will clear it later in ProcessLoadQueue(). + element.mTextureId = INVALID_TEXTURE_ID; + element.mObserver = nullptr; + break; } } + mRemoveQueue.PushBack(QueueElement(textureId, queueObserver)); } else @@ -741,10 +751,7 @@ void TextureManager::Remove(const TextureManager::TextureId& textureId, TextureU // Remove its observer RemoveTextureObserver(textureInfo, observer); - // Keep loadState due to the textureInfo validate problem. - auto textureLoadState = textureInfo.loadState; - - // Remove textureId in CacheManager + // Remove textureId in CacheManager. Now, textureInfo is invalidate. mTextureCacheManager.RemoveCache(textureInfo); // Remove maskTextureId in CacheManager @@ -754,30 +761,11 @@ void TextureManager::Remove(const TextureManager::TextureId& textureId, TextureU if(maskCacheIndex != INVALID_CACHE_INDEX) { TextureInfo& maskTextureInfo(mTextureCacheManager[maskCacheIndex]); - - // Only Remove maskTexture when texture's loadState is not CANCELLED. because it is already deleted. - if(textureLoadState != LoadState::CANCELLED) - { - mTextureCacheManager.RemoveCache(maskTextureInfo); - } + mTextureCacheManager.RemoveCache(maskTextureInfo); } } } } - - if(observer) - { - // Remove element from the LoadQueue - for(auto&& element : mLoadQueue) - { - if(element.mObserver == observer) - { - // Do not erase the item. We will clear it later in ProcessLoadQueue(). - element.mObserver = nullptr; - break; - } - } - } } } @@ -888,7 +876,10 @@ void TextureManager::QueueLoadTexture(const TextureManager::TextureInfo& texture const auto& textureId = textureInfo.textureId; mLoadQueue.PushBack(QueueElement(textureId, observer)); - observer->DestructionSignal().Connect(this, &TextureManager::ObserverDestroyed); + if(observer) + { + observer->DestructionSignal().Connect(this, &TextureManager::ObserverDestroyed); + } } void TextureManager::LoadTexture(TextureManager::TextureInfo& textureInfo, TextureUploadObserver* observer) @@ -918,7 +909,7 @@ void TextureManager::ProcessLoadQueue() { for(auto&& element : mLoadQueue) { - if(!element.mObserver) + if(element.mTextureId == INVALID_TEXTURE_ID) { continue; } @@ -929,7 +920,10 @@ void TextureManager::ProcessLoadQueue() TextureInfo& textureInfo(mTextureCacheManager[cacheIndex]); if((textureInfo.loadState == LoadState::UPLOADED) || (textureInfo.loadState == LoadState::LOAD_FINISHED && textureInfo.storageType == StorageType::RETURN_PIXEL_BUFFER)) { - EmitLoadComplete(element.mObserver, textureInfo, true); + if(element.mObserver) + { + EmitLoadComplete(element.mObserver, textureInfo, true); + } } else if(textureInfo.loadState == LoadState::LOADING) { @@ -948,15 +942,11 @@ void TextureManager::ProcessLoadQueue() void TextureManager::ProcessRemoveQueue() { - TextureCacheIndex textureCacheIndex = INVALID_CACHE_INDEX; for(auto&& element : mRemoveQueue) { - textureCacheIndex = mTextureCacheManager.GetCacheIndexFromId(element.mTextureId); - if(textureCacheIndex != INVALID_CACHE_INDEX) + if(element.mTextureId != INVALID_TEXTURE_ID) { - TextureInfo& textureInfo(mTextureCacheManager[textureCacheIndex]); - RemoveTextureObserver(textureInfo, element.mObserver); - mTextureCacheManager.RemoveCache(textureInfo); + Remove(element.mTextureId, element.mObserver); } } mRemoveQueue.Clear(); @@ -1127,10 +1117,13 @@ void TextureManager::CheckForWaitingTexture(TextureManager::TextureInfo& maskTex UploadTextures(pixelBuffers, maskTextureInfo); } - // Search the cache, checking if any texture has this texture id as a - // maskTextureId: + // Search the cache, checking if any texture has this texture id as a maskTextureId const std::size_t size = mTextureCacheManager.size(); + // Keep notify observer required textureIds. + // Note : NotifyObservers can change mTextureCacheManager cache struct. We should check id's validation before notify. + std::vector notifyRequiredTextureIds; + // TODO : Refactorize here to not iterate whole cached image. for(TextureCacheIndex cacheIndex = TextureCacheIndex(TextureManagerType::TEXTURE_CACHE_INDEX_TYPE_LOCAL, 0u); cacheIndex.GetIndex() < size; ++cacheIndex.detailValue.index) { @@ -1156,8 +1149,7 @@ void TextureManager::CheckForWaitingTexture(TextureManager::TextureInfo& maskTex pixelBuffers.push_back(textureInfo.pixelBuffer); UploadTextures(pixelBuffers, textureInfo); - // notify mask texture set. - NotifyObservers(textureInfo, true); + notifyRequiredTextureIds.push_back(textureInfo.textureId); } } else @@ -1167,10 +1159,22 @@ void TextureManager::CheckForWaitingTexture(TextureManager::TextureInfo& maskTex std::vector pixelBuffers; pixelBuffers.push_back(textureInfo.pixelBuffer); UploadTextures(pixelBuffers, textureInfo); - NotifyObservers(textureInfo, true); + + notifyRequiredTextureIds.push_back(textureInfo.textureId); } } } + + // Notify textures are masked + for(const auto textureId : notifyRequiredTextureIds) + { + TextureCacheIndex textureCacheIndex = mTextureCacheManager.GetCacheIndexFromId(textureId); + if(textureCacheIndex != INVALID_CACHE_INDEX) + { + TextureInfo& textureInfo(mTextureCacheManager[textureCacheIndex]); + NotifyObservers(textureInfo, true); + } + } } void TextureManager::ApplyMask(TextureManager::TextureInfo& textureInfo, const TextureManager::TextureId& maskTextureId) @@ -1317,7 +1321,8 @@ void TextureManager::ObserverDestroyed(TextureUploadObserver* observer) { if(element.mObserver == observer) { - element.mObserver = nullptr; + element.mTextureId = INVALID_TEXTURE_ID; + element.mObserver = nullptr; } } }