From 9167b2c3ca25f815d0ef08c2efcdf4983f49050d Mon Sep 17 00:00:00 2001 From: kevingpqi Date: Mon, 21 Oct 2024 18:54:30 +0800 Subject: [PATCH 1/3] Fix the potential dangling pointer issue caused by passing two identical paths to setPathAsync. --- src/platform/ios/PAGImageView.mm | 26 +++++++++++++----------- src/platform/ios/PAGView.mm | 35 ++++++++++++++++---------------- 2 files changed, 32 insertions(+), 29 deletions(-) diff --git a/src/platform/ios/PAGImageView.mm b/src/platform/ios/PAGImageView.mm index 4740d439ca..826d09a31a 100644 --- a/src/platform/ios/PAGImageView.mm +++ b/src/platform/ios/PAGImageView.mm @@ -495,14 +495,13 @@ - (BOOL)setPath:(NSString*)newPath { - (BOOL)setPath:(NSString*)path maxFrameRate:(float)maxFrameRate { std::lock_guard autoLock(imageViewLock); - if ([filePath isEqualToString:path]) { - return YES; - } - if (filePath) { - [filePath release]; - filePath = nil; + if (![filePath isEqualToString:path]) { + if (filePath != nil) { + [filePath release]; + filePath = nil; + } + filePath = [path retain]; } - filePath = [path retain]; PAGFile* file = [PAGFile Load:path]; [self setCompositionInternal:file maxFrameRate:maxFrameRate]; return file != nil; @@ -519,14 +518,17 @@ - (void)setPathAsync:(NSString*)path completionBlock:(void (^)(PAGFile*))callbac - (void)setPathAsync:(NSString*)path maxFrameRate:(float)maxFrameRate completionBlock:(void (^)(PAGFile*))callback { - if (filePath != nil) { - [filePath release]; - filePath = nil; + if (![filePath isEqualToString:path]) { + if (filePath != nil) { + [filePath release]; + filePath = nil; + } + filePath = [path retain]; } - filePath = [path retain]; [PAGFile LoadAsync:path completionBlock:^(PAGFile* pagFile) { - [self setComposition:pagFile maxFrameRate:maxFrameRate]; + std::lock_guard autoLock(imageViewLock); + [self setCompositionInternal:pagFile maxFrameRate:maxFrameRate]; callback(pagFile); }]; } diff --git a/src/platform/ios/PAGView.mm b/src/platform/ios/PAGView.mm index 165645886a..ffa0d792c7 100644 --- a/src/platform/ios/PAGView.mm +++ b/src/platform/ios/PAGView.mm @@ -25,7 +25,6 @@ @implementation PAGView { PAGPlayer* pagPlayer; PAGSurface* pagSurface; - PAGFile* pagFile; NSString* filePath; PAGAnimator* animator; BOOL _isVisible; @@ -40,7 +39,6 @@ - (instancetype)initWithFrame:(CGRect)frame { - (void)initPAG { _isVisible = FALSE; - pagFile = nil; filePath = nil; self.contentScaleFactor = [UIScreen mainScreen].scale; self.backgroundColor = [UIColor clearColor]; @@ -65,7 +63,6 @@ - (void)dealloc { [animator release]; [pagPlayer release]; [pagSurface release]; - [pagFile release]; [filePath release]; [[NSNotificationCenter defaultCenter] removeObserver:self]; [super dealloc]; @@ -199,25 +196,29 @@ - (NSString*)getPath { } - (BOOL)setPath:(NSString*)path { - if (filePath != nil) { - [filePath release]; - filePath = nil; + if (![filePath isEqualToString:path]) { + if (filePath != nil) { + [filePath release]; + filePath = nil; + } + filePath = [path retain]; } PAGFile* file = [PAGFile Load:path]; - [self setComposition:file]; - filePath = [path retain]; + [self setCompositionInternal:file]; return file != nil; } - (void)setPathAsync:(NSString*)path completionBlock:(void (^)(PAGFile*))callback { - if (filePath != nil) { - [filePath release]; - filePath = nil; + if (![filePath isEqualToString:path]) { + if (filePath != nil) { + [filePath release]; + filePath = nil; + } + filePath = [path retain]; } - filePath = [path retain]; [PAGFile LoadAsync:path completionBlock:^(PAGFile* file) { - [self setComposition:file]; + [self setCompositionInternal:file]; callback(file); }]; } @@ -231,10 +232,10 @@ - (void)setComposition:(PAGComposition*)newComposition { [filePath release]; filePath = nil; } - if (pagFile != nil) { - [pagFile release]; - pagFile = nil; - } + [self setCompositionInternal:newComposition]; +} + +- (void)setCompositionInternal:(PAGComposition*)newComposition { [pagPlayer setComposition:newComposition]; [animator setProgress:[pagPlayer getProgress]]; if (_isVisible) { From 2fbbeeaac496cd627290b07707c064d0bc9f474b Mon Sep 17 00:00:00 2001 From: Dom Chen Date: Mon, 21 Oct 2024 19:00:18 +0800 Subject: [PATCH 2/3] Update tgfx to remove SurfaceOptions and use RenderFlags instead. (#2539) --- DEPS | 2 +- src/rendering/caches/RenderCache.cpp | 6 +++--- src/rendering/caches/RenderCache.h | 2 +- src/rendering/graphics/Canvas.cpp | 4 ++-- src/rendering/graphics/Canvas.h | 4 ++-- src/rendering/graphics/Picture.cpp | 12 ++++++------ 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/DEPS b/DEPS index 2d6d8afb5d..e2a933fce9 100644 --- a/DEPS +++ b/DEPS @@ -12,7 +12,7 @@ }, { "url": "${PAG_GROUP}/tgfx.git", - "commit": "9d6b7464dd22e3a0aa9aa22e0a789cbfed1fed73", + "commit": "57dea46bc27c4815af6fcb14d47eb5faa9cfe360", "dir": "third_party/tgfx" }, { diff --git a/src/rendering/caches/RenderCache.cpp b/src/rendering/caches/RenderCache.cpp index 7ef9d3181e..4009fea409 100644 --- a/src/rendering/caches/RenderCache.cpp +++ b/src/rendering/caches/RenderCache.cpp @@ -163,14 +163,14 @@ void RenderCache::beginFrame() { } void RenderCache::attachToContext(tgfx::Context* current, bool forDrawing) { - if (deviceID > 0 && deviceID != current->device()->uniqueID()) { + if (contextID > 0 && contextID != current->uniqueID()) { // Context 改变需要清理内部所有缓存，这里用 uniqueID // 而不用指针比较，是因为指针析构后再创建可能会地址重合。 releaseAll(); } context = current; context->setCacheLimit(MAX_GRAPHICS_MEMORY); - deviceID = context->device()->uniqueID(); + contextID = context->uniqueID(); isDrawingFrame = forDrawing; if (!isDrawingFrame) { return; @@ -199,7 +199,7 @@ void RenderCache::releaseAll() { motionBlurFilter = nullptr; delete transform3DFilter; transform3DFilter = nullptr; - deviceID = 0; + contextID = 0; } void RenderCache::detachFromContext() { diff --git a/src/rendering/caches/RenderCache.h b/src/rendering/caches/RenderCache.h index d6497428ae..e718ea987c 100644 --- a/src/rendering/caches/RenderCache.h +++ b/src/rendering/caches/RenderCache.h @@ -176,7 +176,7 @@ class RenderCache : public Performance { private: ID _uniqueID = 0; PAGStage* stage = nullptr; - uint32_t deviceID = 0; + uint32_t contextID = 0; tgfx::Context* context = nullptr; std::queue timestamps = {}; bool isDrawingFrame = false; diff --git a/src/rendering/graphics/Canvas.cpp b/src/rendering/graphics/Canvas.cpp index bec1e53dd9..c856fb6629 100644 --- a/src/rendering/graphics/Canvas.cpp +++ b/src/rendering/graphics/Canvas.cpp @@ -43,9 +43,9 @@ tgfx::Surface* Canvas::getSurface() const { return canvas->getSurface(); } -const tgfx::SurfaceOptions* Canvas::surfaceOptions() const { +uint32_t Canvas::renderFlags() const { auto surface = canvas->getSurface(); - return surface ? surface->options() : nullptr; + return surface ? surface->renderFlags() : 0; } void Canvas::save() { diff --git a/src/rendering/graphics/Canvas.h b/src/rendering/graphics/Canvas.h index 2f34f251fd..0211a729cc 100644 --- a/src/rendering/graphics/Canvas.h +++ b/src/rendering/graphics/Canvas.h @@ -19,7 +19,7 @@ #pragma once #include "tgfx/core/Canvas.h" -#include "tgfx/core/SurfaceOptions.h" +#include "tgfx/core/RenderFlags.h" namespace pag { class RenderCache; @@ -39,7 +39,7 @@ class Canvas { tgfx::Surface* getSurface() const; - const tgfx::SurfaceOptions* surfaceOptions() const; + uint32_t renderFlags() const; void save(); diff --git a/src/rendering/graphics/Picture.cpp b/src/rendering/graphics/Picture.cpp index d8ad3622bf..cbca9d541b 100644 --- a/src/rendering/graphics/Picture.cpp +++ b/src/rendering/graphics/Picture.cpp @@ -90,8 +90,8 @@ class ImageProxyPicture : public Picture { canvas->drawImage(std::move(image)); return; } - auto options = canvas->surfaceOptions(); - if (options && !options->cacheDisabled()) { + auto renderFlags = canvas->renderFlags(); + if (!(renderFlags & tgfx::RenderFlags::DisableCache)) { auto snapshot = cache->getSnapshot(this); if (snapshot) { canvas->drawImage(snapshot->getImage(), snapshot->getMatrix()); @@ -168,8 +168,8 @@ class SnapshotPicture : public Picture { } void draw(Canvas* canvas) const override { - auto options = canvas->surfaceOptions(); - if (options && options->cacheDisabled()) { + auto renderFlags = canvas->renderFlags(); + if (renderFlags & tgfx::RenderFlags::DisableCache) { graphic->draw(canvas); return; } @@ -192,9 +192,9 @@ class SnapshotPicture : public Picture { graphic->measureBounds(&bounds); auto width = static_cast(ceilf(bounds.width() * scaleFactor)); auto height = static_cast(ceilf(bounds.height() * scaleFactor)); - tgfx::SurfaceOptions options(tgfx::RenderFlags::DisableCache); + auto renderFlags = tgfx::RenderFlags::DisableCache; auto surface = - tgfx::Surface::Make(cache->getContext(), width, height, false, 1, mipmapped, &options); + tgfx::Surface::Make(cache->getContext(), width, height, false, 1, mipmapped, renderFlags); if (surface == nullptr) { return nullptr; } From 3a902e9b21d3c71b2055484fd9606896bcff1273 Mon Sep 17 00:00:00 2001 From: kevingpqi Date: Mon, 21 Oct 2024 19:58:56 +0800 Subject: [PATCH 3/3] Fix the crash issue that occurs when executing directory_iterator on the HarmonyOS platform with a non-existent cached path. --- src/rendering/caches/DiskCache.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/rendering/caches/DiskCache.cpp b/src/rendering/caches/DiskCache.cpp index eb418716c2..6409b4551a 100644 --- a/src/rendering/caches/DiskCache.cpp +++ b/src/rendering/caches/DiskCache.cpp @@ -17,6 +17,7 @@ ///////////////////////////////////////////////////////////////////////////////////////////////// #include "DiskCache.h" +#include #include "pag/pag.h" #include "platform/Platform.h" #include "rendering/utils/Directory.h" @@ -73,6 +74,9 @@ DiskCache::DiskCache() { if (!cacheDir.empty()) { configPath = Directory::JoinPath(cacheDir, "cache.cfg"); cacheFolder = Directory::JoinPath(cacheDir, "files"); + if (!std::filesystem::exists(cacheFolder)) { + Directory::CreateRecursively(cacheFolder); + } if (!readConfig()) { Directory::VisitFiles(cacheFolder, [&](const std::string& path, size_t) { remove(path.c_str()); });