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

Supports multiple textures in .obj file #1517

Merged
merged 3 commits into from Feb 21, 2020
Merged

Conversation

theNded
Copy link
Contributor

@theNded theNded commented Feb 15, 2020

Enhancement of PR #1194.

This PR supports loading multiple textures for one single mesh, addressing #1484.
ScreenCapture_2020-02-16-13-39-58

Major changes:

  • Added property material_ids per face in TriangleMesh supported by tinyobjloader.
  • Supports rendering multiple textures in several passes.

Note: due to the general API design in the IO module, an .obj containing multiple shapes is still not supported.


This change is Reviewable

@update-docs
Copy link

update-docs bot commented Feb 15, 2020

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

@theNded theNded closed this Feb 15, 2020
@theNded theNded deleted the multi-tex branch February 15, 2020 01:25
@theNded theNded restored the multi-tex branch February 15, 2020 01:26
@theNded theNded deleted the multi-tex branch February 15, 2020 01:28
@theNded theNded restored the multi-tex branch February 15, 2020 01:29
@theNded theNded reopened this Feb 15, 2020
@theNded
Copy link
Contributor Author

theNded commented Feb 15, 2020

Sorry that I messed up the log... Any idea how to clean this shit?
Update: alright fixed, but the delete and restore history stays. Can make a new PR if you feel that it looks dirty...

Copy link
Contributor

@griegler griegler left a comment

Choose a reason for hiding this comment

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

Thanks. I did not check the visualization part, maybe @yxlao can have a look at that.
Is the mesh from #1484 now rendering correctly?

Reviewed 3 of 9 files at r1.
Reviewable status: 3 of 9 files reviewed, 1 unresolved discussion (waiting on @griegler, @theNded, and @yxlao)


src/Open3D/IO/FileFormat/FileOBJ.cpp, line 280 at r1 (raw file):

    if (write_triangle_uvs && mesh.HasTexture()) {
        if (mesh.textures_.size() > 1) {

Shouldn't we also support writing multiple textures. What are the blockers?

Copy link
Contributor Author

@theNded theNded 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: 1 of 9 files reviewed, 1 unresolved discussion (waiting on @griegler and @yxlao)


src/Open3D/IO/FileFormat/FileOBJ.cpp, line 280 at r1 (raw file):

Previously, griegler (Gernot) wrote…

Shouldn't we also support writing multiple textures. What are the blockers?

Done.

@germanros1987
Copy link
Contributor

Codacy Here is an overview of what got changed by this pull request:

Clones removed
==============
+ src/Open3D/Visualization/Shader/TexturePhongShader.cpp  -9
+ src/Open3D/Visualization/Shader/TextureSimpleShader.cpp  -2
         

See the complete overview on Codacy

@theNded theNded requested a review from syncle February 16, 2020 20:22
Copy link
Contributor

@syncle syncle left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 9 files at r1, 1 of 5 files at r2.
Reviewable status: 3 of 10 files reviewed, 1 unresolved discussion (waiting on @griegler, @syncle, and @yxlao)

Copy link
Contributor

@griegler griegler left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 5 files at r2.
Reviewable status: 7 of 10 files reviewed, all discussions resolved (waiting on @yxlao)

Copy link
Collaborator

@yxlao yxlao left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 9 files at r1, 5 of 5 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@yxlao yxlao merged commit 534f548 into isl-org:master Feb 21, 2020
@sadams207
Copy link

In o3d version 0.9, python bindings for using textures worked. Only one texture per mesh was accepted as the textures attribute.
In 03d version 0.10, in python, multiple textures are accepted as the textures attribute takes a list of o3d Images;
however, there is not a clear way to set the new material_ids mesh attribute to select the appropriate texture.
The attached script demonstrates the problem, i.e. crashes with signal 11: SIGSEGV due to missing material_ids.
Need to put in some other image to run the code.

`import open3d as o3d
import numpy as np

o_image = o3d.io.read_image('Selection_028.png')

points = np.array([
[0., 0, 0],
[1, 0, 0],
[0, 1, 0]
])

triangles = np.array([
[0, 1, 2]
])

uvs = np.array([
[0., 0.],
[1, 0],
[0, 1]
])

mesh = o3d.geometry.TriangleMesh()
mesh.vertices = o3d.utility.Vector3dVector(points)
mesh.triangles = o3d.utility.Vector3iVector(triangles)
mesh.triangle_uvs = o3d.utility.Vector2dVector(uvs)
mesh.textures = [o_image]

print(mesh.has_triangle_material_ids())
o3d.visualization.draw_geometries([mesh])`

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.

None yet

6 participants