From 0d9ae43378d6d3304e0d8d35083343f7047d2c69 Mon Sep 17 00:00:00 2001 From: Eric Mehl Date: Wed, 17 Jul 2024 17:42:18 -0400 Subject: [PATCH 1/5] IECoreUSD : Use `/` for resolved paths. On Windows, the result of USD's `GetResolvedPath()` method returns the resolved path using `\` between directories. This can cause problems when doing string substition on a resolved path since the `\` will be interpreted as the start of an escape character. Using `/` also conforms to Cortex's standard of always using `/` for paths because they work on all operating systems. --- Changes | 1 + .../IECoreUSD/include/IECoreUSD/DataAlgo.h | 2 ++ contrib/IECoreUSD/src/IECoreUSD/DataAlgo.cpp | 31 +++++++++++++++++-- .../IECoreUSD/src/IECoreUSD/VolumeAlgo.cpp | 3 +- .../IECoreUSD/test/IECoreUSD/USDSceneTest.py | 25 +++++++++++++++ 5 files changed, 59 insertions(+), 3 deletions(-) diff --git a/Changes b/Changes index 9605a88505..0820367d85 100644 --- a/Changes +++ b/Changes @@ -9,6 +9,7 @@ Fixes - Fixed loading of skinned facevarying normals. - `lightLink` and `shadowLink` collections on UsdLuxLightAPI are no longer treated as sets. - Version.h : Fixed `*Version()` functions to return the runtime version of the library, not the version that client code was compiled with. Use the `CORTEX_*_VERSION` macros for compile time checks. +- IECoreUSD : Asset and volume paths now use `/` in the resolved path on all operating systems. This behavior can be disabled by setting the `IECOREUSD_FORCE_ASSET_PATH_FORWARD_SLASH` environment variable to a value of `0`. 10.5.9.2 (relative to 10.5.9.1) ======== diff --git a/contrib/IECoreUSD/include/IECoreUSD/DataAlgo.h b/contrib/IECoreUSD/include/IECoreUSD/DataAlgo.h index 9f8d172188..4918f8c511 100644 --- a/contrib/IECoreUSD/include/IECoreUSD/DataAlgo.h +++ b/contrib/IECoreUSD/include/IECoreUSD/DataAlgo.h @@ -79,6 +79,8 @@ IECOREUSD_API IECore::DataPtr fromUSD( const pxr::VtValue &value, const pxr::Sdf /// as above. IECOREUSD_API IECore::DataPtr fromUSD( const pxr::UsdAttribute &attribute, pxr::UsdTimeCode time=pxr::UsdTimeCode::Default(), bool arrayAccepted = true ); +IECOREUSD_API std::string fromUSD( const pxr::SdfAssetPath &assetPath ); + /// From Cortex to USD /// ================== diff --git a/contrib/IECoreUSD/src/IECoreUSD/DataAlgo.cpp b/contrib/IECoreUSD/src/IECoreUSD/DataAlgo.cpp index 6b4eca217e..a4f4b562ae 100644 --- a/contrib/IECoreUSD/src/IECoreUSD/DataAlgo.cpp +++ b/contrib/IECoreUSD/src/IECoreUSD/DataAlgo.cpp @@ -49,6 +49,10 @@ IECORE_POP_DEFAULT_VISIBILITY #include "boost/unordered_map.hpp" +#ifdef _MSC_VER +#include +#endif + using namespace std; using namespace pxr; using namespace IECore; @@ -92,6 +96,19 @@ GeometricData::Interpretation interpretation( TfToken role ) return GeometricData::None; } +#ifdef _MSC_VER + +static const bool g_forceAssetPathForwardSlash = []() -> bool { + const char *c = getenv( "IECOREUSD_FORCE_ASSET_PATH_FORWARD_SLASH" ); + if( !c ) + { + return true; + } + return strcmp( c, "0" ); +}(); + +#endif + } // namespace @@ -142,9 +159,10 @@ IECore::DataPtr dataFromArray( const pxr::VtValue &value, GeometricData::Interpr IECore::DataPtr dataFromSdfAssetPath( const SdfAssetPath &assetPath, const pxr::UsdAttribute *attribute = nullptr ) { - if( assetPath.GetResolvedPath().size() || !assetPath.GetAssetPath().size() || !attribute ) + const std::string p = DataAlgo::fromUSD( assetPath ); + if( p.size() || !assetPath.GetAssetPath().size() || !attribute ) { - return new StringData( assetPath.GetResolvedPath() ); + return new StringData( p ); } // Path resolution failed, for a couple of possible reasons : @@ -332,6 +350,15 @@ IECore::DataPtr IECoreUSD::DataAlgo::fromUSD( const pxr::UsdAttribute &attribute } } +std::string IECoreUSD::DataAlgo::fromUSD( const pxr::SdfAssetPath &assetPath ) +{ +#ifdef _MSC_VER + return g_forceAssetPathForwardSlash ? std::filesystem::path( assetPath.GetResolvedPath() ).generic_string() : assetPath.GetResolvedPath(); +#else + return assetPath.GetResolvedPath(); +#endif +} + ////////////////////////////////////////////////////////////////////////// // Implementation of `toUSD()` ////////////////////////////////////////////////////////////////////////// diff --git a/contrib/IECoreUSD/src/IECoreUSD/VolumeAlgo.cpp b/contrib/IECoreUSD/src/IECoreUSD/VolumeAlgo.cpp index 4a58cc6bc7..178f300fe8 100644 --- a/contrib/IECoreUSD/src/IECoreUSD/VolumeAlgo.cpp +++ b/contrib/IECoreUSD/src/IECoreUSD/VolumeAlgo.cpp @@ -32,6 +32,7 @@ // ////////////////////////////////////////////////////////////////////////// +#include "IECoreUSD/DataAlgo.h" #include "IECoreUSD/ObjectAlgo.h" #include "IECoreVDB/VDBObject.h" @@ -98,7 +99,7 @@ IECore::ObjectPtr readVolume( pxr::UsdVolVolume &volume, pxr::UsdTimeCode time, SdfAssetPath fieldAssetPath; fieldAsset.GetFilePathAttr().Get( &fieldAssetPath, time ); - const std::string fieldFileName = fieldAssetPath.GetResolvedPath(); + const std::string fieldFileName = DataAlgo::fromUSD( fieldAssetPath ); if( fileName.empty() ) { diff --git a/contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py b/contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py index a66f11b6f8..e0b1e33c8e 100644 --- a/contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py +++ b/contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py @@ -32,6 +32,8 @@ # ########################################################################## +import IECore + import importlib import os import math @@ -4371,5 +4373,28 @@ def testWriteToOpenScene( self ) : with self.assertRaisesRegex( RuntimeError, f"USDScene : Failed to open USD stage : '{fileName}'" ) : IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Write ) + def testAsetPathSlashes ( self ) : + + root = IECoreScene.SceneInterface.create( + os.path.join( os.path.dirname( __file__ ), "data", "assetPathAttribute.usda" ), + IECore.IndexedIO.OpenMode.Read + ) + xform = root.child( "xform" ) + + self.assertEqual( xform.attributeNames(), [ "render:testAsset" ] ) + self.assertNotIn( "\\", xform.readAttribute( "render:testAsset", 0 ).value ) + + @unittest.skipIf( not haveVDB, "No IECoreVDB" ) + def testUsdVolVolumeSlashes( self ) : + + import IECoreVDB + + fileName = os.path.dirname( __file__ ) + "/data/volume.usda" + root = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Read ) + child = root.child( "volume" ) + + vdbObject = child.readObject( 0 ) + self.assertNotIn( "\\", vdbObject.fileName() ) + if __name__ == "__main__": unittest.main() From 041cfa70f1a50ddc72a01fc4e8628b531940c4d9 Mon Sep 17 00:00:00 2001 From: Eric Mehl Date: Mon, 16 Sep 2024 15:59:43 -0400 Subject: [PATCH 2/5] fixup! IECoreUSD : Use `/` for resolved paths. --- contrib/IECoreUSD/src/IECoreUSD/DataAlgo.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/contrib/IECoreUSD/src/IECoreUSD/DataAlgo.cpp b/contrib/IECoreUSD/src/IECoreUSD/DataAlgo.cpp index a4f4b562ae..8b4fa5e7d7 100644 --- a/contrib/IECoreUSD/src/IECoreUSD/DataAlgo.cpp +++ b/contrib/IECoreUSD/src/IECoreUSD/DataAlgo.cpp @@ -186,9 +186,16 @@ IECore::DataPtr dataFromSdfAssetPath( const SdfAssetPath &assetPath, const pxr:: spec->GetLayer()->GetNumTimeSamplesForPath( spec->GetPath() ) ) { +#ifdef _MSC_VER + const std::string result = SdfComputeAssetPathRelativeToLayer( spec->GetLayer(), assetPath.GetAssetPath() ); + return new StringData( + g_forceAssetPathForwardSlash ? std::filesystem::path( result ).generic_string() : result + ); +#else return new StringData( SdfComputeAssetPathRelativeToLayer( spec->GetLayer(), assetPath.GetAssetPath() ) ); +#endif } } From 4ddab6d68c408d09f0f20505a43b6aeee1ae5790 Mon Sep 17 00:00:00 2001 From: Eric Mehl Date: Mon, 16 Sep 2024 17:24:32 -0400 Subject: [PATCH 3/5] fixup! IECoreUSD : Use `/` for resolved paths. --- contrib/IECoreUSD/include/IECoreUSD/DataAlgo.h | 2 -- contrib/IECoreUSD/src/IECoreUSD/DataAlgo.cpp | 17 +++++++---------- contrib/IECoreUSD/src/IECoreUSD/VolumeAlgo.cpp | 6 +++--- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/contrib/IECoreUSD/include/IECoreUSD/DataAlgo.h b/contrib/IECoreUSD/include/IECoreUSD/DataAlgo.h index 4918f8c511..9f8d172188 100644 --- a/contrib/IECoreUSD/include/IECoreUSD/DataAlgo.h +++ b/contrib/IECoreUSD/include/IECoreUSD/DataAlgo.h @@ -79,8 +79,6 @@ IECOREUSD_API IECore::DataPtr fromUSD( const pxr::VtValue &value, const pxr::Sdf /// as above. IECOREUSD_API IECore::DataPtr fromUSD( const pxr::UsdAttribute &attribute, pxr::UsdTimeCode time=pxr::UsdTimeCode::Default(), bool arrayAccepted = true ); -IECOREUSD_API std::string fromUSD( const pxr::SdfAssetPath &assetPath ); - /// From Cortex to USD /// ================== diff --git a/contrib/IECoreUSD/src/IECoreUSD/DataAlgo.cpp b/contrib/IECoreUSD/src/IECoreUSD/DataAlgo.cpp index 8b4fa5e7d7..436d534ae8 100644 --- a/contrib/IECoreUSD/src/IECoreUSD/DataAlgo.cpp +++ b/contrib/IECoreUSD/src/IECoreUSD/DataAlgo.cpp @@ -159,7 +159,13 @@ IECore::DataPtr dataFromArray( const pxr::VtValue &value, GeometricData::Interpr IECore::DataPtr dataFromSdfAssetPath( const SdfAssetPath &assetPath, const pxr::UsdAttribute *attribute = nullptr ) { - const std::string p = DataAlgo::fromUSD( assetPath ); + +#ifdef _MSC_VER + const std::string p = g_forceAssetPathForwardSlash ? std::filesystem::path( assetPath.GetResolvedPath() ).generic_string() : assetPath.GetResolvedPath(); +#else + const std::string p = assetPath.GetResolvedPath(); +#endif + if( p.size() || !assetPath.GetAssetPath().size() || !attribute ) { return new StringData( p ); @@ -357,15 +363,6 @@ IECore::DataPtr IECoreUSD::DataAlgo::fromUSD( const pxr::UsdAttribute &attribute } } -std::string IECoreUSD::DataAlgo::fromUSD( const pxr::SdfAssetPath &assetPath ) -{ -#ifdef _MSC_VER - return g_forceAssetPathForwardSlash ? std::filesystem::path( assetPath.GetResolvedPath() ).generic_string() : assetPath.GetResolvedPath(); -#else - return assetPath.GetResolvedPath(); -#endif -} - ////////////////////////////////////////////////////////////////////////// // Implementation of `toUSD()` ////////////////////////////////////////////////////////////////////////// diff --git a/contrib/IECoreUSD/src/IECoreUSD/VolumeAlgo.cpp b/contrib/IECoreUSD/src/IECoreUSD/VolumeAlgo.cpp index 178f300fe8..bf1d424d7e 100644 --- a/contrib/IECoreUSD/src/IECoreUSD/VolumeAlgo.cpp +++ b/contrib/IECoreUSD/src/IECoreUSD/VolumeAlgo.cpp @@ -38,6 +38,7 @@ #include "IECoreVDB/VDBObject.h" #include "IECore/MessageHandler.h" +#include "IECore/SimpleTypedData.h" IECORE_PUSH_DEFAULT_VISIBILITY #include "pxr/usd/usdVol/volume.h" @@ -97,9 +98,8 @@ IECore::ObjectPtr readVolume( pxr::UsdVolVolume &volume, pxr::UsdTimeCode time, continue; } - SdfAssetPath fieldAssetPath; - fieldAsset.GetFilePathAttr().Get( &fieldAssetPath, time ); - const std::string fieldFileName = DataAlgo::fromUSD( fieldAssetPath ); + ConstDataPtr fieldFileNameData = DataAlgo::fromUSD( fieldAsset.GetFilePathAttr(), time ); + const std::string fieldFileName = runTimeCast( fieldFileNameData )->readable(); if( fileName.empty() ) { From 2a2ff78571b5f105ef1740ff9fa230e4f5468346 Mon Sep 17 00:00:00 2001 From: Eric Mehl Date: Mon, 16 Sep 2024 17:24:42 -0400 Subject: [PATCH 4/5] fixup! IECoreUSD : Use `/` for resolved paths. --- contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py b/contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py index e0b1e33c8e..9e944f9ab2 100644 --- a/contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py +++ b/contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py @@ -4373,7 +4373,7 @@ def testWriteToOpenScene( self ) : with self.assertRaisesRegex( RuntimeError, f"USDScene : Failed to open USD stage : '{fileName}'" ) : IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Write ) - def testAsetPathSlashes ( self ) : + def testAssetPathSlashes ( self ) : root = IECoreScene.SceneInterface.create( os.path.join( os.path.dirname( __file__ ), "data", "assetPathAttribute.usda" ), @@ -4383,6 +4383,7 @@ def testAsetPathSlashes ( self ) : self.assertEqual( xform.attributeNames(), [ "render:testAsset" ] ) self.assertNotIn( "\\", xform.readAttribute( "render:testAsset", 0 ).value ) + self.assertTrue( pathlib.Path( xform.readAttribute( "render:testAsset", 0 ).value ).is_file() ) @unittest.skipIf( not haveVDB, "No IECoreVDB" ) def testUsdVolVolumeSlashes( self ) : @@ -4395,6 +4396,7 @@ def testUsdVolVolumeSlashes( self ) : vdbObject = child.readObject( 0 ) self.assertNotIn( "\\", vdbObject.fileName() ) + self.assertTrue( pathlib.Path( vdbObject.fileName() ).is_file() ) if __name__ == "__main__": unittest.main() From ffe45071a4b37022e22bd37ca7601c8815aeee0a Mon Sep 17 00:00:00 2001 From: Eric Mehl Date: Tue, 17 Sep 2024 09:45:48 -0400 Subject: [PATCH 5/5] fixup! IECoreUSD : Use `/` for resolved paths. --- contrib/IECoreUSD/src/IECoreUSD/VolumeAlgo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/IECoreUSD/src/IECoreUSD/VolumeAlgo.cpp b/contrib/IECoreUSD/src/IECoreUSD/VolumeAlgo.cpp index bf1d424d7e..4984065721 100644 --- a/contrib/IECoreUSD/src/IECoreUSD/VolumeAlgo.cpp +++ b/contrib/IECoreUSD/src/IECoreUSD/VolumeAlgo.cpp @@ -99,7 +99,7 @@ IECore::ObjectPtr readVolume( pxr::UsdVolVolume &volume, pxr::UsdTimeCode time, } ConstDataPtr fieldFileNameData = DataAlgo::fromUSD( fieldAsset.GetFilePathAttr(), time ); - const std::string fieldFileName = runTimeCast( fieldFileNameData )->readable(); + const std::string fieldFileName = static_cast( fieldFileNameData.get() )->readable(); if( fileName.empty() ) {