isl-org / Open3D

Open3D: A Modern Library for 3D Data Processing
http://www.open3d.org
Other
11.28k stars 2.28k forks source link

read_triangle_mesh enable_post_processing=false just reduces post processing but does not turn off. #6137

Open hernot opened 1 year ago

hernot commented 1 year ago

Checklist

Describe the issue

When storing a triangle mesh in .obj which contains unused vertices and reading back than all unused vertices are removed by read_triangle_mesh even if enable_post_processing is set to false.

This is not acceptable when there exists additional data not storeable in .obj format which is attached to individual vertices including those not touched by any surface triangle. The enforced magic removal of vertices breaks any link between vertices and data which is only based on the weak index based relationship. Data can be electrical field parameters defined per vertex, can be information about type of vertex or whether vertex needs further processing etc.

Steps to reproduce the bug

import open3d as o3d

mesh = o3d.geometry.TriangleMesh(
  o3d.utility.Vector3dVector(
    [[0,0,0],[0,0,1],[0,1,0],[0,1,1],[1,0,0],[1,0,1],[1,1,0],[1,1,1]]
  ),
  o3d.utility.Vector3iVector(
     [[0,1,2],[1,2,5],[2,5,6]]
  )
)
# the next two would not be necessary, just there for being closer to realistic scenario
mesh.compute_vertex_normals()
mesh.paint_uinform_color([1,0,0])
o3d.io.write_triangle_mesh('test.obj',write_vertex_normals=True,write_vertex_colors=True)
mesh2 = o3d.io.read_triangle_mesh('test.obj',enable_post_processing=false)
assert len(mesh.vertices) == len(mesh2.vertices)

Error message

AssertionError indicating that unused vertices in mesh have been magically removed. mesh2 will only contain 5 vertices compared to mehs which has 8 therof 3 unused.

Expected behavior

That 'test.obj' is loaded as is without any post processing. not even any removal of unused or identical vertices when enable_post_processing=false is used.

Or at least an read_triangle_mesh_ex function which allows direct access to all assimp aiProcess_* flags affecting read_triangle_mesh by which caller can control which postprocessing is applied if it is not desired to extend existing interface by additional none value for enable_post_processing. which even disables aiProcess_JoinIdenticalVertices. In exchange id expect a remove_duplicate_points on o3d.geometry.PointCloud which uses an L2 metric to figure based upon a definable epsilon whether two points are identical or not. Would be a better solution compared to hard-coded magic vertex dropping on read.

Open3D, Python and System information

- Operating system: Ubuntu 20.04 
- Python version: Python 3.8 (not really relevant as originating from core c/cpp files)
- Open3D version: output from python: `print(open3d.__version__)` 0.16 (not relevant either as originating form core  c/ccp file)
- System architecture: x86 
- Is this a remote workstation?: no
- How did you install Open3D?: pip 
- Compiler version (if built from source): N/R

Additional information

The whole is caused by the unsigned int post_process_flags = kPostProcessFlags_compulsory; hardcoded on line 162 of file cpp/open3d/io/file_format/FileASSIMP.cpp. The kPostProcessFlags_compulaory mapps on lines 39,40 to const unsigned int kPostProcessFlags_compulsory = aiProcess_JoinIdenticalVertices; which requests libassimp to join identical vertices and makes it drop unused vertices.

hernot commented 1 year ago

I tried now to directly assess assimp load and turn off postprocessing bad idea. The bug is using assimp at all for loading object files and others. It duplicates on reading the triangles the vertices it finds referenced by triangles. Destroying any information in which order they were stored. And than it requires wierd postprocessing to get things back in state as it believes it should be. Instead of loading the vertices as are and keeping indexing of triangles as are. I do guess they will not change. So my suggestion is reconsider whether use of assimp was such a wise idea. As it causes issues like #2614 or this one without any chance to fix unless assimp maintainers would reflect upon the just opened issue https://github.com/assimp/assimp/issues/5091

EDIT For now i resorted to trimesh package for loading *.obj files. My solution currently (no textured mesh at hand, this instant) is;

import trimesh

def read_triangle_mesh(avatar_name,enable_post_processing=False):
        scene_patch = trimesh.load(avatar_name,process=enable_post_processing)
        mesh = o3d.geometry.TriangleMesh(
            o3d.utility.Vector3dVector(scene_patch.vertices),
            o3d.utility.Vector3iVector(scene_patch.faces)
        )
        if scene_patch.vertex_normals.size:
            mesh.vertex_normals = o3d.utility.Vector3dVector(scene_patch.vertex_normals.copy())
        if scene_patch.visual.defined:
            if scene_patch.visual.kind == 'vertex':
                mesh.vertex_colors = o3d.utility.Vector3dVector(scene_patch.visual.vertex_colors[:,:3]/255) # no alpha channel support
        # TODO add textures and textures + vertex colors when textured surface at hand
        # TODO think about adding face normals if defined
        return mesh
hernot commented 1 year ago

Just to summarize. When there is a triangle which references later vertices earlier out of their natural order than libassip based loader will change the order of the vertices, breaking any data linked to the vertices using vertex index only. This affects all fileformats for which reading the data is dispatched to libassimp by [o,TriangleMesh.cpp](https://github.com/isl-org/Open3D/blob/master/cpp/open3d/io/TriangleMeshIO.cpp)

 file_extension_to_trianglemesh_read_function{
                {"ply", ReadTriangleMeshFromPLY},
                {"stl", ReadTriangleMeshUsingASSIMP},
                {"obj", ReadTriangleMeshUsingASSIMP},
                {"off", ReadTriangleMeshFromOFF},
                {"gltf", ReadTriangleMeshUsingASSIMP},
                {"glb", ReadTriangleMeshUsingASSIMP},
                {"fbx", ReadTriangleMeshUsingASSIMP},
        };

In other words be prepared to sooner or later also receive reports similar to this one (#6137) #6139 and #2614 for gltf, glb (possibly affecting #5851) and fbx file formats.

hernot commented 1 year ago

Here is the extended version also capable of reading of textures. Having meshes with both vertex_colors and textures is not forseen by trimesh yet.

def read_triangle_mesh(avatar_name,enable_post_processing=False):
    # EDIT: next 4 lines replace to maintain order even in case of degenerate and non referenced
    # scene_patch = trimesh.load(avatar_name,process=enable_post_processing)
    if enable_post_processing:
        scene_patch = trimesh.load(avatar_name,process=True)
    else:
        scene_patch = trimesh.load(avatar_name,process=False,maintain_order=True) 
    mesh = o3d.geometry.TriangleMesh(
        o3d.utility.Vector3dVector(scene_patch.vertices),
        o3d.utility.Vector3iVector(scene_patch.faces)
    ) 
    if scene_patch.vertex_normals.size:
        mesh.vertex_normals = o3d.utility.Vector3dVector(scene_patch.vertex_normals.copy())
    if scene_patch.visual.defined:
        # either texture or vertex colors if no uvs present.
        if scene_patch.visual.kind == 'vertex':
            mesh.vertex_colors = o3d.utility.Vector3dVector(scene_patch.visual.vertex_colors[:,:3]/255) # no alpha channel support
        elif scene_patch.visual.kind == 'texture':
            uv = scene_patch.visual.uv
            if uv.shape[0] == scene_patch.vertices.shape[0]:
                mesh.triangle_uvs = o3d.utility.Vector2dVector(uv[scene_patch.faces.flatten()])
            elif uv.shape[0] != scene_patch.faces.shape[0] * 3:
                assert False
            else:
                mesh.triangle_uvs = o3d.utility.Vector2dVector(uv)
                if scene_patch.visual.material is not None and scene_patch.visual.material.image is not None:
                    if scene_patch.visual.material.image.mode == 'RGB':
                        mesh.textures = [o3d.geometry.Image(np.asarray(scene_patch.visual.material.image))]
                    else:
                        assert False
        else:
            assert False
    return mesh
hernot commented 10 months ago

Alternatively the following workaround mitigates the vertex reordering issues when writing the TriangleMesh instead at load time.

import open3d as o3d

true_colors = np.array(mesh.vertex_colors,copy=True)
vertex_reindexer = np.arange(len(mesh.vertices))
mesh.vertex_colors = o3d.utility.Vector3dVector((vertex_reindexer / vertex_reindexer[-1]).reshape(-1,1)[:,[0,0,0]])
o3d.io.write_triangle_mesh(
    '/tmp/fix_vertex_order.obj',mesh,
    write_ascii=False,write_vertex_normals = True,write_vertex_colors = True
)
hardened_mesh = o3d.io.read_triangle_mesh('/tmp/fix_vertex_order.obj',enable_post_processing = False)
remaining_reordered_vertices = (np.asarray(hardened_mesh.vertex_colors)[:,0] * vertex_reindexer[-1] + 0.5).astype(int)
hardened_mesh.vertex_colors = o3d.utility.Vector3dVector(true_colors[remaining_reordered_vertices])

# reorder all additional data linked to the vertices using the remaining_reordered_vertices
# store the hardened_mesh using the final filename