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

UDIM resolution broken for symlinks to non-UDIM paths #1329

Closed
marsupial opened this issue Sep 17, 2020 · 4 comments
Closed

UDIM resolution broken for symlinks to non-UDIM paths #1329

marsupial opened this issue Sep 17, 2020 · 4 comments

Comments

@marsupial
Copy link
Contributor

marsupial commented Sep 17, 2020

Description of Issue

New[ish] code to share UDIM textures when they are symlinks breaks down if they are symlinks to non-UDIM textures.
_ResolveAssetAttribute in pxr/usdImaging/materialParamUtils.cpp attempts to deduce the following:

/symlink/a_<UDIM>.exr => /realpath/realfile_1001.exr
/symlink/b_<UDIM>.exr => /realpath/realfile_1001.exr

This seems to allow sharing a common key for /symlink/a_.exr and /symlink/b_.exr, but will break down when the symlinks point to a file that is not a UDIM tile (i.e. /realpath/realfile_some_other_convention.exr)

It occurs to me that this may actually be fundamentally flawed and even the solution in #1330 may be insufficient, given that there could be two UDIM paths that resolve to the same first-tile, but diverge on subsequent tiles:

/symlink/a_1001.exr => /realpath/realfile_1001.exr
/symlink/a_1002.exr => /realpath/another_1002.exr

/symlink/b_1001.exr => /realpath/realfile_1001.exr
/symlink/b_1002.exr => /realpath/totallydifferent_1002.exr

I think any resolution/transformation via symlinks in this case would make /realpath/realfile_1001.exr the key, for both a_<UDIM>.exr and b_<UDIM>.exr which would give incorrect results.

@spitzak
Copy link

spitzak commented Sep 17, 2020

Changing the paths also makes error messages hard for users to figure out.
It would be better if the code that actually read the image did the readlink calls and determines if the image is one it has already loaded and if so reuse it.

@marsupial
Copy link
Contributor Author

I'm leaning towards @spitzak's conclusion too...all of this magic seems better suited at the ArResolver level or in the render itself.

Should #1330 be closed or changes to just remove the symlink resolution?

@jtran56
Copy link

jtran56 commented Sep 17, 2020

Filed as internal issue #USD-6351

@sunyab
Copy link
Contributor

sunyab commented Jun 7, 2021

I spent a little bit of time looking at this and I agree with you both. I've made a change internally that just removes the symlink processing, so the UDIM pattern matching is done on the authored paths and not the paths of the symlinked files. This should go out in the next push. Thanks!

marktucker added a commit to sideeffects/USD that referenced this issue Feb 26, 2022
… resolution was removed, so

was any testing for whether or not the "Resolve" call succeeded. This change
restores the looping to look for an existing tile file rather than always
returning the attempted resolution of the first tile file.
marktucker added a commit to sideeffects/USD that referenced this issue Feb 26, 2022
… resolution was removed, so

was any testing for whether or not the "Resolve" call succeeded. This change
restores the looping to look for an existing tile file rather than always
returning the attempted resolution of the first tile file.
marktucker added a commit to sideeffects/USD that referenced this issue Feb 26, 2022
… resolution was removed, so

was any testing for whether or not the "Resolve" call succeeded. This change
restores the looping to look for an existing tile file rather than always
returning the attempted resolution of the first tile file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants