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

Add IO for NPZ files for Tensor-based TriangleMeshes #6019

Merged
merged 8 commits into from
Mar 24, 2023

Conversation

errissa
Copy link
Collaborator

@errissa errissa commented Mar 21, 2023

The following script tests the basic functionality:

import open3d as o3d

# Load OBJ file
d = o3d.data.MonkeyModel()
mesh = o3d.t.io.read_triangle_mesh(d.path)
print(mesh)
o3d.visualization.draw(mesh)

# Save mesh as NPZ and reload it
o3d.t.io.write_triangle_mesh('monkey.npz', mesh)
mesh_npz = o3d.t.io.read_triangle_mesh('monkey.npz')
print(mesh_npz)

# Verify it looks correct
o3d.visualization.draw(mesh_npz)

This change is Reviewable

@errissa errissa requested a review from benjaminum March 21, 2023 15:24
@update-docs
Copy link

update-docs bot commented Mar 21, 2023

Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes.

@errissa
Copy link
Collaborator Author

errissa commented Mar 22, 2023

Texture maps are now supported. Try this updated script:

import open3d as o3d

# Load OBJ file
d = o3d.data.MonkeyModel()
mesh = o3d.t.io.read_triangle_mesh(d.path)
mesh.material.set_default_properties()
mesh.material.texture_maps['albedo'] = o3d.t.io.read_image(d.path_map['albedo'])

print(mesh)
o3d.visualization.draw(mesh)

# Save mesh as NPZ and reload it
o3d.t.io.write_triangle_mesh('monkey.npz', mesh)
mesh_npz = o3d.t.io.read_triangle_mesh('monkey.npz')
print(mesh_npz)

# Verify it looks correct
o3d.visualization.draw(mesh_npz)

Copy link
Contributor

@benjaminum benjaminum left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @errissa)


cpp/open3d/t/io/TriangleMeshIO.cpp line 243 at r2 (raw file):

    }
    if (mesh.HasTriangleAttr("texture_uvs")) {
        mesh_attributes["uvmap"] = mesh.GetTriangleAttr("texture_uvs");

Maybe we want to change this one.
I think the options we have are
triangle_texture_uvs (+valid identifier -multiple underscores)
triangle.texture_uvs (+like accessing an o3d attribute -can be saved to npz but not a valid identifier)
triangle__texture_uvs (+valid identifier -even more _)

More options after TriangleMesh->Mesh
face{_,.,__}texture_uvs

What do you think? @errissa

Copy link
Contributor

@benjaminum benjaminum left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @errissa)


cpp/open3d/t/io/TriangleMeshIO.cpp line 243 at r2 (raw file):

Previously, benjaminum (Benjamin Ummenhofer) wrote…

Maybe we want to change this one.
I think the options we have are
triangle_texture_uvs (+valid identifier -multiple underscores)
triangle.texture_uvs (+like accessing an o3d attribute -can be saved to npz but not a valid identifier)
triangle__texture_uvs (+valid identifier -even more _)

More options after TriangleMesh->Mesh
face{_,.,__}texture_uvs

What do you think? @errissa

The mentioned options apply to all attributes

@errissa errissa marked this pull request as ready for review March 23, 2023 02:17
@errissa
Copy link
Collaborator Author

errissa commented Mar 23, 2023

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @errissa)

cpp/open3d/t/io/TriangleMeshIO.cpp line 243 at r2 (raw file):

    }
    if (mesh.HasTriangleAttr("texture_uvs")) {
        mesh_attributes["uvmap"] = mesh.GetTriangleAttr("texture_uvs");

Maybe we want to change this one. I think the options we have are triangle_texture_uvs (+valid identifier -multiple underscores) triangle.texture_uvs (+like accessing an o3d attribute -can be saved to npz but not a valid identifier) triangle__texture_uvs (+valid identifier -even more _)

More options after TriangleMesh->Mesh face{_,.,__}texture_uvs

What do you think? @errissa

I thought we might want to change this one. It doesn't fit any other patterns we're using. I think the simplest is triangle_texture_uvs which is consistent with the way we name everything else. I have made the change.

Copy link
Contributor

@benjaminum benjaminum left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion

@errissa errissa merged commit 4d63591 into master Mar 24, 2023
@errissa errissa deleted the errissa/tmesh-npz-io branch March 24, 2023 15:24
dbs4261 pushed a commit to dbs4261/Open3D that referenced this pull request Apr 13, 2023
* Add IO to/from NPZ files

* Add texture maps and generic attriubtes to IO

* Style fixes

* Read/write material name and fix read with metallic texture

* Add unit test for TriangleMesh NPZ IO

* Apply style fixes

* Rename uvmap to triangle_texture_uvs

* Style fixes
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

Successfully merging this pull request may close these issues.

2 participants