From 6dffeb3bd296d689c93fa5034722d8ecae2c8d21 Mon Sep 17 00:00:00 2001 From: Roggie Zhang Date: Wed, 20 Nov 2024 22:25:41 +0800 Subject: [PATCH 1/5] Improve Alembic plugin to read assets with ArResolver to support other sources except local files --- pxr/usd/plugin/usdAbc/alembicReader.cpp | 32 +++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/pxr/usd/plugin/usdAbc/alembicReader.cpp b/pxr/usd/plugin/usdAbc/alembicReader.cpp index 7f3d28ce3c..d06c195df3 100644 --- a/pxr/usd/plugin/usdAbc/alembicReader.cpp +++ b/pxr/usd/plugin/usdAbc/alembicReader.cpp @@ -7,12 +7,16 @@ /// \file alembicReader.cpp #include "pxr/pxr.h" +#include "pxr/usd/ar/asset.h" +#include "pxr/usd/ar/resolvedPath.h" +#include "pxr/usd/ar/resolver.h" #include "pxr/usd/plugin/usdAbc/alembicReader.h" #include "pxr/usd/plugin/usdAbc/alembicUtil.h" #include "pxr/usd/usdGeom/hermiteCurves.h" #include "pxr/usd/usdGeom/tokens.h" #include "pxr/usd/usdGeom/xformable.h" #include "pxr/usd/sdf/schema.h" +#include "pxr/base/arch/fileSystem.h" #include "pxr/base/work/threadLimits.h" #include "pxr/base/trace/trace.h" #include "pxr/base/tf/envSetting.h" @@ -801,6 +805,8 @@ class _ReaderContext { const ISampleSelector& selector, const UsdAbc_AlembicDataAny& value) const; + std::string _OpenAndGetMappedFilePath(const std::string& filePath); + // Custom auto-lock that safely ignores a NULL pointer. class _Lock { public: @@ -872,6 +878,28 @@ _ReaderContext::_ReaderContext() : // Do nothing } +std::string +_ReaderContext::_OpenAndGetMappedFilePath(const std::string& filePath) +{ + // Open asset with Ar to support URLs other than local paths. + std::shared_ptr asset = + ArGetResolver().OpenAsset(ArResolvedPath(filePath)); + if (asset) + { + FILE* fileHandle = asset->GetFileUnsafe().first; + if (fileHandle) + { + // If file handle is presented, use mapped file path instead of original one. + const std::string mappedFilePath = ArchGetFileName(fileHandle); + + return mappedFilePath; + } + } + + // Otherwise, fallback to original asset path. + return filePath; +} + bool _ReaderContext::Open(const std::string& filePath, std::string* errorLog, const SdfFileFormat::FileFormatArguments& args) @@ -883,11 +911,11 @@ _ReaderContext::Open(const std::string& filePath, std::string* errorLog, auto abcLayers = args.find("abcLayers"); if (abcLayers != args.end()) { for (auto&& l : TfStringSplit(abcLayers->second, ",")) { - layeredABC.emplace_back(std::move(l)); + layeredABC.emplace_back(_OpenAndGetMappedFilePath(l)); } } } - layeredABC.emplace_back(filePath); + layeredABC.emplace_back(_OpenAndGetMappedFilePath(filePath)); #if PXR_HDF5_SUPPORT_ENABLED && !H5_HAVE_THREADSAFE // HDF5 may not be thread-safe. From f4a8aa2196df82cfc1d934ca88a878eee4a94cf7 Mon Sep 17 00:00:00 2001 From: Roggie Zhang Date: Tue, 17 Dec 2024 16:57:02 +0800 Subject: [PATCH 2/5] Returns original filePath if it exists locally --- pxr/usd/plugin/usdAbc/alembicReader.cpp | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/pxr/usd/plugin/usdAbc/alembicReader.cpp b/pxr/usd/plugin/usdAbc/alembicReader.cpp index d06c195df3..0c48edd5fd 100644 --- a/pxr/usd/plugin/usdAbc/alembicReader.cpp +++ b/pxr/usd/plugin/usdAbc/alembicReader.cpp @@ -23,6 +23,7 @@ #include "pxr/base/tf/staticData.h" #include "pxr/base/tf/staticTokens.h" #include "pxr/base/tf/ostreamMethods.h" +#include "pxr/base/tf/fileUtils.h" #include #include #include @@ -881,18 +882,22 @@ _ReaderContext::_ReaderContext() : std::string _ReaderContext::_OpenAndGetMappedFilePath(const std::string& filePath) { - // Open asset with Ar to support URLs other than local paths. - std::shared_ptr asset = - ArGetResolver().OpenAsset(ArResolvedPath(filePath)); - if (asset) + // Return as it is if it's a local path and not a symlink. + if (!TfIsFile(filePath)) { - FILE* fileHandle = asset->GetFileUnsafe().first; - if (fileHandle) + // Open asset with Ar to support URLs other than local paths. + std::shared_ptr asset = + ArGetResolver().OpenAsset(ArResolvedPath(filePath)); + if (asset) { - // If file handle is presented, use mapped file path instead of original one. - const std::string mappedFilePath = ArchGetFileName(fileHandle); + FILE* fileHandle = asset->GetFileUnsafe().first; + if (fileHandle) + { + // If file handle is presented, use mapped file path instead of original one. + const std::string mappedFilePath = ArchGetFileName(fileHandle); - return mappedFilePath; + return mappedFilePath; + } } } From d5d484eb8b52546fe569b18bfb201d86ce9ef0f2 Mon Sep 17 00:00:00 2001 From: Roggie Zhang Date: Wed, 18 Dec 2024 13:08:41 +0800 Subject: [PATCH 3/5] Hold opened assets within the lifetime of _ReaderContext --- pxr/usd/plugin/usdAbc/alembicReader.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pxr/usd/plugin/usdAbc/alembicReader.cpp b/pxr/usd/plugin/usdAbc/alembicReader.cpp index 0c48edd5fd..c1d5131c64 100644 --- a/pxr/usd/plugin/usdAbc/alembicReader.cpp +++ b/pxr/usd/plugin/usdAbc/alembicReader.cpp @@ -860,6 +860,8 @@ class _ReaderContext { _PrimMap _prims; Prim* _pseudoRoot; UsdAbc_TimeSamples _allTimeSamples; + + std::vector> _assetHolders; }; static @@ -895,6 +897,7 @@ _ReaderContext::_OpenAndGetMappedFilePath(const std::string& filePath) { // If file handle is presented, use mapped file path instead of original one. const std::string mappedFilePath = ArchGetFileName(fileHandle); + _assetHolders.emplace_back(std::move(asset)); return mappedFilePath; } @@ -1540,6 +1543,7 @@ _ReaderContext::_Clear() _allTimeSamples.clear(); _instanceSources.clear(); _instances.clear(); + _assetHolders.clear(); } const _ReaderContext::Prim* From 2b26e6c41f227a8241bcaacda1e7890ebd478232 Mon Sep 17 00:00:00 2001 From: Roggie Zhang Date: Wed, 8 Jan 2025 11:25:43 +0800 Subject: [PATCH 4/5] Resolve symlinks and more checks before using mapped file path --- pxr/usd/plugin/usdAbc/alembicReader.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pxr/usd/plugin/usdAbc/alembicReader.cpp b/pxr/usd/plugin/usdAbc/alembicReader.cpp index c1d5131c64..00fd5ea30d 100644 --- a/pxr/usd/plugin/usdAbc/alembicReader.cpp +++ b/pxr/usd/plugin/usdAbc/alembicReader.cpp @@ -861,6 +861,8 @@ class _ReaderContext { Prim* _pseudoRoot; UsdAbc_TimeSamples _allTimeSamples; + // Asset holders to keep a reference alive so that resolver doesn't + // attempt to cleanup asset until the asset is no longer in use. std::vector> _assetHolders; }; @@ -884,8 +886,8 @@ _ReaderContext::_ReaderContext() : std::string _ReaderContext::_OpenAndGetMappedFilePath(const std::string& filePath) { - // Return as it is if it's a local path and not a symlink. - if (!TfIsFile(filePath)) + // Return as it is if it's a local path or symlink. + if (!TfIsFile(filePath, true)) { // Open asset with Ar to support URLs other than local paths. std::shared_ptr asset = @@ -893,7 +895,7 @@ _ReaderContext::_OpenAndGetMappedFilePath(const std::string& filePath) if (asset) { FILE* fileHandle = asset->GetFileUnsafe().first; - if (fileHandle) + if (fileHandle && ftell(fileHandle) == 0) { // If file handle is presented, use mapped file path instead of original one. const std::string mappedFilePath = ArchGetFileName(fileHandle); From 46e0e3339ce6850630b22a7bad9667bec176acf1 Mon Sep 17 00:00:00 2001 From: Roggie Zhang Date: Thu, 9 Jan 2025 10:46:03 +0800 Subject: [PATCH 5/5] Check fileOffset --- pxr/usd/plugin/usdAbc/alembicReader.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pxr/usd/plugin/usdAbc/alembicReader.cpp b/pxr/usd/plugin/usdAbc/alembicReader.cpp index 00fd5ea30d..d198d4343b 100644 --- a/pxr/usd/plugin/usdAbc/alembicReader.cpp +++ b/pxr/usd/plugin/usdAbc/alembicReader.cpp @@ -894,8 +894,11 @@ _ReaderContext::_OpenAndGetMappedFilePath(const std::string& filePath) ArGetResolver().OpenAsset(ArResolvedPath(filePath)); if (asset) { - FILE* fileHandle = asset->GetFileUnsafe().first; - if (fileHandle && ftell(fileHandle) == 0) + FILE* fileHandle; size_t fileOffset; + std::tie(fileHandle, fileOffset) = asset->GetFileUnsafe(); + // Check file offset also to ensure that the .abc file + // we're looking at isn't embedded in a .usdz or other package + if (fileHandle && fileOffset == 0) { // If file handle is presented, use mapped file path instead of original one. const std::string mappedFilePath = ArchGetFileName(fileHandle);