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

Implement multitexture-obj-io-support #1572

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

justinhchae
Copy link

@justinhchae justinhchae commented Jun 22, 2023

Pull Request from Esri's AI Prototypes Team. Seeking to implement multitexture support for OBJ for IO to include loading obj, loading objs as meshes, subsetting objs, and saving objs.

We leverage PyTorch3D quite a bit on our applied research in mesh segmentation and often use obj datasets that depict entire cities or entire regions. They mostly all require multiple texture files and we eventually need to subset meshes for nearly all stages of the pipeline.

We've written a story about the full scope of changes at GeoAI/PyTorch3D.

Key Excerpt from Our Article:

multitexture-obj-io-support: This branch establishes multitexture support for obj meshes by modifying pytorch3d.io.obj_io. Currently, PyTorch3D does not fully support reading and writing obj meshes with multiple texture files. In such cases, only the first of many textures are read into memory. For meshes that contain varying textures and multiple files, the results of texture sampling may lead to undesirable results, i.e., vegetation textures sampled onto building faces (Figure 2a and 2b). Specifically, we created pytorch3d.io.obj_io.subset_obj and modified pytorch3d.io.obj_io.save_obj to implement these features. In addition, a new utility function is provided in the form of pytorch3d.utils.obj_utils that consolidates multiple helper functions used to support obj sub-setting and validation operations. This branch and following branches are updated to support recent changes to the API that now support I/O for face vert normals as of PyTorch release 0.7.4. Addresses multiple existing issues to include #694, #1017, and #1277.

Other Existing Issues Addressed Include: #694 , #1017 , and #1277.

esri_pytorch3d_multitexture_high_precision_support_0

@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 Jun 22, 2023
@bottler
Copy link
Contributor

bottler commented Jul 5, 2023

Wow! Impressive work! I'm afraid it may take a while to get all these things landed, because we are busy with other things.

Some minor things which would help:

  • There's some irrelevant test data files added here (everything in tests/pulsar/test_out, two glb files, ...) . I suspect they are test output. It would be good to remove.
  • Can obj_utils be moved to io please? If you want, it could be indicated as internal by calling it _obj_utils.py. The structure of packages in pytorch3d is not well documented, but utils is meant to be "utils for the user" which are not called by other parts of pytorch3d.
  • Could the _reindex_*_by_index functions all be unified into a single function?
  • There are a few comments and function names where the word "obj" is used to mean the data in an OBJ file, or most of the data which could be in an obj file (e.g. texture atlas is excluded). I think it would be clearer to replace all these instances with "obj data" or "obj_data" or similar, because when I see "obj" I think "OBJ file".

@justinhchae
Copy link
Author

Hi! Thanks for the suggestions, I will work on making the edits you described.

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