EMSUSD-280 import relative textures #3259

pierrebai-adsk · 2023-08-03T17:05:47Z

Refactor out the texture resolution code into a common reusable function:

Add a file system function to make a path absolute but relative to the project folder.
Add code to make the texture path relative to the project.
Use the common texture import code for materialX.
Split the texture handling into multiple helper functions to make each one clearer.
Warn if a USDZ archive is imported without the required corresponding flag to extract textures.
Properly detect absolute or relative path in automatic mode.

Add import command flag:

Add the "importRelativeTextures" flag to the mayaUSDImport command.
Added a "none" relative textures mode for backward compatibility that leave all file names alone.
Made it the default for the command.
Add handling of the flag.
Add a "importRelativeTextures" option to the import job arguments class.
Add handling and validation of the option.
Wrap the option for Python.
Update the import command documentation.
Use the job args option in the import code.

Add some unit tests.

Refactor out the texture resolution code into a common reusable function: - Add a file system function to make a path absolute but relative to the project folder. - Add code to make the texture path relative to the project. - Use the common texture import code for materialX. - Split the texture handling into multiple helper functions to make each one clearer. - Warn if a USDZ archive is imported without the required corresponding flag to extract textures. - Properly detect absolute or relative path in automatic mode. Add import command flag: - Add the "importRelativeTextures" flag to the mayaUSDImport command. - Added a "none" relative textures mode for backward compatibility that leave all file names alone. - Made it the default for the command. - Add handling of the flag. - Add a "importRelativeTextures" option to the import job arguments class. - Add handling and validation of the option. - Wrap the option for Python. - Update the import command documentation. - Use the job args option in the import code. Add some unit tests.

pierrebai-adsk · 2023-08-03T17:59:28Z

lib/usd/translators/shading/shadingAsset.cpp

+    return val.UncheckedGet<SdfAssetPath>();
+}
+
+static bool handleMissingResolvedPath(SdfAssetPath* resolvedAssetPath)


This all code taken from usdUVTextureReader,cpp and refactored into multiple functions. You would need to diff with that file by hand to see what changed.

lib/mayaUsd/utils/utilFileSystem.cpp

lib/usd/translators/shading/shadingAsset.cpp

JGamache-autodesk · 2023-08-04T15:26:23Z

lib/usd/translators/shading/mtlxImageReader.cpp

-            }
-        }
+    if (!ResolveTextureAssetPath(prim, shaderSchema, depFn, _GetArgs().GetJobArguments())) {
+        return false;


That error is not fatal. Please continue loading the other USD attributes.

IDK what is best here and the other place that uses that function. I'll do what you suggest, we can revisit the best practice in the future. Note that in the original usdUVTextureReader, many error were considered fatal, so I'm just keeping the existing behavior.

lib/usd/translators/shading/shadingAsset.cpp

JGamache-autodesk · 2023-08-04T15:32:19Z

lib/usd/translators/shading/usdUVTextureReader.cpp

-            }
-        }
+    if (!ResolveTextureAssetPath(prim, shaderSchema, depFn, _GetArgs().GetJobArguments())) {
+        return false;


Non fatal, please continue with the import.

The previous code had fatal error, this merely keep the old behavior.

JGamache-autodesk · 2023-08-04T15:34:38Z

test/lib/usd/translators/testUsdImportRelativeTextures.py

+        """
+        Tests that the textures file paths mode is automatically detected.
+        """
+        self.runImportRelativeTexturesTest('automatic')


Test an error case where paths can not be resolved?

There are no new failure mode, the only existing failures are failing to extract from a USDZ archive, which I guess can only happen when the archive is corrupted or the disk is full. This makes it hard to test.

- Use rfind instead of find. - Make handleMissingResolvedPath() always succeeed. - Document that there are no current failure mode for relative path resolution.

samuelliu-adsk · 2023-08-04T16:51:49Z

lib/mayaUsd/utils/utilFileSystem.cpp

    if (pos != 0)
        return {};

-    auto relativePathAndSuccess = makePathRelativeTo(fileName, projectPath.asChar());
+    auto relativePathAndSuccess = makePathRelativeTo(fileName, projectPath);


Can we use the makeProjectRelatedPath method? Seems like makeProjectRelatedPath handles all the checkings and returns the related path

No because this function wants to make a true relative path, but makeProjectRelativePath makes an absolute path, but relative to the absolute path of the project.

samuelliu-adsk · 2023-08-04T17:32:54Z

LGTM

- Add a drop-down to select the relative textures mode in the import and edit-as-Maya dialogs. - Make sure the mode is remembered.

seando-adsk · 2023-08-07T21:06:34Z

lib/mayaUsd/utils/utilFileSystem.cpp

+std::string UsdMayaUtilFileSystem::getProjectPath()
+{
+    MString projectPath = MGlobal::executeCommandStringResult("workspace -q -rd");
+    return projectPath.asChar();
+}


This function already exists as UsdMayaUtil::GetCurrentMayaWorkspacePath()

seando-adsk

Pierre added a new helper function that is identical (except return value std::string vs MString) to an existing helper. For code maintenance I don't think we should have two helper functions that do the same thing.

@samuelliu-adsk Since PierreB is on vacation can you checkout his branch and make the fix?

…urrentMayaWorkspacePath

seando-adsk

Thanks for the changes, but needs a couple of extra fixes.

seando-adsk · 2023-08-08T17:13:30Z

lib/mayaUsd/utils/utilFileSystem.cpp

    // Note: don't use isEmpty() because it is not available in Maya 2022 and earlier.
-    if (projectPath.length() == 0)
+    const MString projectPath = UsdMayaUtil::GetCurrentMayaWorkspacePath();
+    if (projectPath.isEmpty())


The comment right above says you cannot use MString::isEmpty().

Oh my bad. I missed that comment..

seando-adsk · 2023-08-08T17:15:17Z

lib/mayaUsd/utils/utilFileSystem.cpp

+    const MString projectPath = UsdMayaUtil::GetCurrentMayaWorkspacePath();
+    if (projectPath.isEmpty())


Again, no isEmpty here. Since you need it as a char for both cases below I would just convert it to std::string here.

Suggested change

const MString projectPath = UsdMayaUtil::GetCurrentMayaWorkspacePath();

if (projectPath.isEmpty())

const std::string projectPath(UsdMayaUtil::GetCurrentMayaWorkspacePath().asChar());

if (projectPath.empty())

…n 2022

seando-adsk · 2023-08-08T19:40:22Z

lib/mayaUsd/utils/utilFileSystem.cpp

@@ -180,8 +180,8 @@ std::string UsdMayaUtilFileSystem::getPathRelativeToProject(const std::string& f
        return {};

    // Note: don't use isEmpty() because it is not available in Maya 2022 and earlier.


Would have been nice if you removed this comment as its not relevant anymore since we are using std::string instead of MString.

Remove extra "<b/>" tag on annotation.

…xtures-ui EMSUSD-280 import relative texture UI

pierrebai-adsk added adsk Related to Autodesk plugin import-export Related to Import and/or Export labels Aug 3, 2023

pierrebai-adsk assigned ecp-maya-devops-adsk Aug 3, 2023

pierrebai-adsk requested a review from AramAzhari-adsk August 3, 2023 17:58

pierrebai-adsk commented Aug 3, 2023

View reviewed changes

pierrebai-adsk requested review from samuelliu-adsk and JGamache-autodesk and removed request for AramAzhari-adsk August 4, 2023 12:21

JGamache-autodesk requested changes Aug 4, 2023

View reviewed changes

pierrebai-adsk removed the request for review from samuelliu-adsk August 4, 2023 15:51

EMSUSD-280 code imrpvoements

0ea58bf

- Use rfind instead of find. - Make handleMissingResolvedPath() always succeeed. - Document that there are no current failure mode for relative path resolution.

pierrebai-adsk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Aug 4, 2023

JGamache-autodesk previously approved these changes Aug 4, 2023

View reviewed changes

samuelliu-adsk reviewed Aug 4, 2023

View reviewed changes

pierrebai-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Aug 4, 2023

samuelliu-adsk previously approved these changes Aug 4, 2023

View reviewed changes

EMSUSD-280 import relative texture UI

bb8a664

- Add a drop-down to select the relative textures mode in the import and edit-as-Maya dialogs. - Make sure the mode is remembered.

seando-adsk reviewed Aug 7, 2023

View reviewed changes

seando-adsk requested changes Aug 7, 2023

View reviewed changes

Remove getProjectPath method and replace it with existing helper GetC…

c7f1144

…urrentMayaWorkspacePath

samuelliu-adsk dismissed stale reviews from JGamache-autodesk and themself via c7f1144 August 8, 2023 17:04

samuelliu-adsk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Aug 8, 2023

seando-adsk requested changes Aug 8, 2023

View reviewed changes

seando-adsk removed the ready-for-merge Development process is finished, PR is ready for merge label Aug 8, 2023

Use empty() instead of isEmpty() to support maya versions earlier tha…

90150d3

…n 2022

seando-adsk reviewed Aug 8, 2023

View reviewed changes

Comment clean up

985674a

samuelliu-adsk requested a review from seando-adsk August 8, 2023 20:03

samuelliu-adsk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Aug 8, 2023

seando-adsk previously approved these changes Aug 8, 2023

View reviewed changes

samuelliu-adsk and others added 2 commits August 8, 2023 16:50

Fix the annotation to keep it consistent with label

2d0fd50

Update mayaUSDRegisterStrings.mel

7527584

Remove extra "<b/>" tag on annotation.

seando-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Aug 9, 2023

Merge pull request #3260 from Autodesk/bailp/EMSUSD-280/import-rel-te…

1046343

…xtures-ui EMSUSD-280 import relative texture UI

seando-adsk dismissed their stale review via 1046343 August 9, 2023 15:33

samuelliu-adsk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Aug 9, 2023

seando-adsk approved these changes Aug 9, 2023

View reviewed changes

seando-adsk removed the ready-for-merge Development process is finished, PR is ready for merge label Aug 9, 2023

samuelliu-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Aug 9, 2023

seando-adsk merged commit 89c5e0b into dev Aug 9, 2023
12 checks passed

seando-adsk deleted the bailp/EMSUSD-280/import-relative-textures branch August 9, 2023 17:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EMSUSD-280 import relative textures #3259

EMSUSD-280 import relative textures #3259

pierrebai-adsk commented Aug 3, 2023

pierrebai-adsk Aug 3, 2023

JGamache-autodesk Aug 4, 2023

pierrebai-adsk Aug 4, 2023

JGamache-autodesk Aug 4, 2023

pierrebai-adsk Aug 4, 2023

JGamache-autodesk Aug 4, 2023

pierrebai-adsk Aug 4, 2023

samuelliu-adsk Aug 4, 2023

pierrebai-adsk Aug 4, 2023

samuelliu-adsk Aug 4, 2023

samuelliu-adsk commented Aug 4, 2023

seando-adsk Aug 7, 2023

seando-adsk left a comment

seando-adsk left a comment

seando-adsk Aug 8, 2023

samuelliu-adsk Aug 8, 2023

seando-adsk Aug 8, 2023

seando-adsk Aug 8, 2023

		const MString projectPath = UsdMayaUtil::GetCurrentMayaWorkspacePath();
		if (projectPath.isEmpty())

		@@ -180,8 +180,8 @@ std::string UsdMayaUtilFileSystem::getPathRelativeToProject(const std::string& f
		return {};

		// Note: don't use isEmpty() because it is not available in Maya 2022 and earlier.

EMSUSD-280 import relative textures #3259

EMSUSD-280 import relative textures #3259

Conversation

pierrebai-adsk commented Aug 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samuelliu-adsk commented Aug 4, 2023

Choose a reason for hiding this comment

seando-adsk left a comment

Choose a reason for hiding this comment

seando-adsk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment