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

added texture mode on load_obj #1886

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ashimdahal
Copy link
Contributor

ShapeNetCore throws error texture UV coordinates are outside the range [0,1], this fixes it by clamping it with the clamp texture mode.

ShapeNetCore throws error texture UV coordinates are outside the range [0,1], this fixes it by clamping it with the clamp texture mode.
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 6, 2024
@bottler
Copy link
Contributor

bottler commented Oct 6, 2024

I think you need to pass texture_wrap not texture_mode, so you might as well (1) call the new thing texture_wrap and (2) fix the docstring in https://github.com/facebookresearch/pytorch3d/blob/main/pytorch3d/io/obj_io.py#L151-L153 to say texture_wrap instead of texture_mode to avoid this confusion (you can do that in this PR).

And did you actually run the code and see success?

@ashimdahal
Copy link
Contributor Author

Thanks, I fixed the issues you mentioned, it should run fine now.
best,

@bottler
Copy link
Contributor

bottler commented Oct 6, 2024

"it should run fine now" - please actually check. Does it fix a problem that you have been experiencing - i.e. can you load a model now which you couldn't before? Perhaps you can paste an image of what you now get?

In particular, clamping may not actually do the right thing for any shapenet models.

@bottler
Copy link
Contributor

bottler commented Oct 6, 2024

And while adding a new option is not dangerous, changing the default is. I think you really want a much more restricted change here.

@ashimdahal
Copy link
Contributor Author

Thanks, sorry about not providing snippets and detailed report on the commit.

The following error was showing up persistently alongside the regular warnings while I was loading the items on the shapenetcore dataset.

screenshot-20241006-214931Z-selected

Clamping the textures fixed the issue of the texture coordinates in shapenet being outside the range of [0,1].

best,

@bottler
Copy link
Contributor

bottler commented Oct 20, 2024

But we need to know that this is the right fix, and that will take more investigation. I think coordinates might need to wrap around, which might mean we want to use the new multi-map option for TexturesUV.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants