[Mesh] TriangleMesh's "+=" operator appends UVs regardless of the presence of existing features

[cdbharath]

Allows appending of UV coordinates, textures and material IDs with += operator overload in TriangleMesh class irrespective of the presence of these attributes in the existing mesh (mesh on LHS to += operator)

Type

• Bug fix (non-breaking change which fixes an issue): Fixes # TriangleMesh's operator "+=" gets rid of UV coordinates when no texture is attached to the mesh. #4794
• New feature (non-breaking change which adds functionality). Resolves #
• Breaking change (fix or feature that would cause existing functionality to not work as expected) Resolves #

Motivation and Context

The += operator overload of TriangleMesh class does not append UV coordinates, textures and material IDs when the existing mesh (mesh on LHS to += operator) already contains any of these attributes. There is no reason to limit += for UVs to TriangleMesh with the attributes already assigned as it currently happens here.

Checklist:

• I have run python util/check_style.py --apply to apply Open3D code style
  to my code.
• This PR changes Open3D behavior or adds new functionality.
  • Both C++ (Doxygen) and Python (Sphinx / Google style) documentation is
    updated accordingly.
  • I have added or updated C++ and / or Python unit tests OR included test
    results (e.g. screenshots or numbers) here.
• I will follow up and update the code if CI fails.
• For fork PRs, I have selected Allow edits from maintainers.

Description

The current change doesn't check if the presence of UV coordinates, textures and material IDs in the existing mesh and appends only based on their availability in the incoming mesh.

I am attaching a screenshot that shows the addition of UV coordinates based on += operator. The test code is as follows

    auto texture_image = io::CreateImageFromFile("/home/bharath/Downloads/uv1.png");

    auto box = geometry::TriangleMesh::CreateBox(1.0, 1.0, 1.0, true, true);    
    box->textures_.push_back(*texture_image);
    box->triangle_material_ids_.resize(box->triangles_.size(), 0);

    auto sphere = geometry::TriangleMesh::CreateSphere(1.0, 6, true);   
    sphere->textures_.push_back(*texture_image);
    sphere->triangle_material_ids_.resize(sphere->triangles_.size(), 0);

    auto scene = std::make_shared<geometry::TriangleMesh>();
        
    // Append new meshes using +=
    *scene += *box;
    *scene += *sphere;
            
    open3d::visualization::DrawGeometries({scene}, "Scene");

Case 1: Both box and sphere has UVs, textures and material IDs
Note: First 2 images are inputs and the 3rd image is the output

Case 2: Box has UVs, textures and material IDs. But sphere has only UVs
Note: First 2 images are inputs and the 3rd image is the output

Case 3: Sphere has UVs, textures and material IDs. But box has only textures and material IDs
Note: First 2 images are inputs and the 3rd image is the output

[update-docs]

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

[benjaminum]

@cdbharath Thanks for your PR, please have a look at the comment

[benjaminum]

If this does not have any UV coordinates then triangle_uvs_.size() will be different from the total number of triangles in the resulting mesh. This would break with the assumption that triangle_uvs_.size() == 3*number of triangles

[benjaminum]

Hi @cdbharath , we think mixing meshes with and without texture uvs can be problematic. I added a special case for empty meshes that allows your workflow with the += operator.

[ssheorey]

We need to keep this portion - clear() properties if only one mesh has them (unless the other mesh is empty), else we will get meshes with only half the triangles with material ids, etc. (e.g. if this has material_ids, but mesh doesn't, output should not have material ids.) Half populated properties will cause seg faults.

Also, textures and triangle_material_ids go together, so we should have a single if() for them. We want n_textures == max(triangle_material_ids_)+1 and either both or none should be present always.

@cdbharath this PR may break other mesh functions if there is a bug -- please do the following tests and make sure there are no errors. Check for the case where:

• a has texture_uvs but no textures / triangle_material_ids
• b has all of texture_uvs, textures and triangle_material_ids
• c has none of the 3 (but has triangles)

With these 3 meshes, perform this check:

• a+=b and b+=a should produce the same results in a and b respectively (test mesh equality). Also for the pair a+=c, c+=a, and separately for the pair b+=c and c+=b. [Add this to C++ unit tests if not present already] Visualize the resulting mesh correctly with draw_geometries() and draw() [Just test visualization on your computer - no unit tests required]

[cdbharath]

@benjaminum @ssheorey I have addressed the posted issues. I also added an unit test for += operator along with visual examples attached to the PR description. I have requested for review again. Thank you.

[benjaminum]

lgtm