Open3D icon indicating copy to clipboard operation
Open3D copied to clipboard

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

Open cdbharath opened this issue 11 months ago • 3 comments

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

  • [x] Bug fix (non-breaking change which fixes an issue): Fixes # #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:

  • [x] 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.
    • [x] I have added or updated C++ and / or Python unit tests OR included test results (e.g. screenshots or numbers) here.
  • [x] I will follow up and update the code if CI fails.
  • [x] 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 Alt TextAlt TextAlt Text

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 Alt TextAlt TextAlt Text

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 Alt TextAlt TextAlt Text

cdbharath avatar Mar 27 '24 22:03 cdbharath

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

update-docs[bot] avatar Mar 27 '24 22:03 update-docs[bot]

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.

benjaminum avatar Jun 11 '24 13:06 benjaminum

@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.

cdbharath avatar Jul 10 '24 02:07 cdbharath