Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAYA-105710: fix USD files fail to load on mapped mounted volume. #1422

Merged
merged 2 commits into from
May 26, 2021

Conversation

HamedSabri-adsk
Copy link
Contributor

This PR fixes the issue with loading usd from mapped drivers.

Steps to reproduce the error:

    Plugin and external drive or USB Drive to your computer.
    Go to Disk Manager.
    Right Click on your connected drive.
    Select "Change Drive Letter and Path..."
    Select "Add".
    Use "Mount in the following empty NTFS folder:"
    Locate the folder you want to mount to.
    Extract the attached folder to that location.
    Open Maya and then open the *.ma file that contains usd with relative path
    Notice that the USD file doesn't load.

Note: Opening the same file if saved on a local drive or a network drive works without issues.

Description:

There is a bug in gulark's filesystem::canonical which fails to properly resolve relative path. C++17 std::filesystem::canonical doesn't have this issue and properly resolves the relative path.

ghc::filesystem::canonical

auto fullPath = ghc::filesystem::path(currentFileDir).append(relativeFilePath); 
// C:\\MOUNT_ME\\RTS\\RTS\\master_files\\..\\referenced_files\\USD\\barrelTests\\Library\\Assets\\Barrels\\Barrels.usd
auto path = ghc::filesystem::canonical(fullPath, errorCode); // error 

std::filesystem::canonical

auto fullPath = std::filesystem::path(currentFileDir).append(relativeFilePath);
//"C:\\MOUNT_ME\\RTS\\RTS\\master_files\\../referenced_files/USD/barrelTests/Library/Assets/Barrels/Barrels.usd"
auto path = std::filesystem::canonical(fullPath, errorCode); // Ok, good 

Workaround:

To work around this, I removed the need to call filesystem::canonical all together since the given path to ArGetResolver().ConfigureResolverForAsset is resolved properly.

PXR_NS::ArGetResolver().ConfigureResolverForAsset(fileString);

e.g
C:\\MOUNT_ME\\RTS\\RTS\\master_files\\..\\referenced_files\\USD\\barrelTests\\Library\\Assets\\Barrels\\Barrels.usd

Copy link

@kxl-adsk kxl-adsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great find @HamedSabri-adsk !

We are changing what resolveRelativePathWithinMayaContext is returning, i.e. used to be the canonical version of the path. This path is consumed then by a few different methods inside of proxy shape compute, including opening/finding a layer for a given path. I was wondering how it will work for layer identifiers and whether or not they will include non-canonical paths...but everything seems to be correct there.

I'm fine with the change, but two things would be good considering doing:

  1. Creating a defect in gulrak
  2. Check with @fabal if this change will work for AL as well. I remember we refactored this method out of the AL plugin so there may be additional dependencies we don't know about that require returned path to be canonical.

@HamedSabri-adsk
Copy link
Contributor Author

@kxl-adsk Great points. I will log a defect against gulark sometimes today or tomorrow.

@fabal I would be happy to hear your thoughts on this change ? Thanks

…canonical

- Revert back changes to use ghc::filesystem::canonical again.
@@ -38,7 +38,7 @@ else()
FetchContent_Declare(
${CONTENT_NAME}
GIT_REPOSITORY https://github.com/gulrak/filesystem.git
GIT_TAG 3d3c02ce35dcc68b5ebb34f21cb1fc507be9a66e
GIT_TAG 4e21ab305794f5309a1454b4ae82ab9a0f5e0d25
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching to v 1.5.6 which fixed the bug in ghc::filesystem::canonical

gulrak/filesystem#124

auto path = ghc::filesystem::path(currentFileDir).append(relativeFilePath);
std::error_code errorCode;
auto path = ghc::filesystem::canonical(
ghc::filesystem::path(currentFileDir).append(relativeFilePath), errorCode);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert my workaround now that bug is fixed.

@HamedSabri-adsk HamedSabri-adsk added the ready-for-merge Development process is finished, PR is ready for merge label May 26, 2021
@kxl-adsk kxl-adsk merged commit 662c29f into dev May 26, 2021
@kxl-adsk kxl-adsk deleted the sabrih/MAYA-105710/fix_path_mapped_drive branch May 26, 2021 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to core library ready-for-merge Development process is finished, PR is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants