Fix the triggering in PickPointsInteractor when there are linesets pr…

[EwingKang]

Adds handling of LineSet that is similar to Mesh and PointCloud in SetPickableGeometry()

Type

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

This change fixes #6498

Checklist:

• I have run python util/check_style.py --apply to apply Open3D code style
  to my code. (On U22.04, clang_format 10 is not available, the default install script didn't worked, I eyeballed the format instead)
• 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

GUI no-longer crashes when there's lineset object in the scene when switching to Actions -> show settings -> Mouse control (drop down) -> Selection (tab).

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

[godwincharan]

I could help review this PR

[EwingKang]

Seems like Codacy caught an error (thanks!)
I fixed it. But not sure if I should just rebase (squash) the to make the commit tree cleaner.
Also, automated style check reported an error, but it only says "Style error found in the following files:... Error: Process completed with exit code 1.". I'm also not sure where to check the error report.

[godwincharan]

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

---

cpp/open3d/visualization/gui/PickPointsInteractor.cpp line 166 at r1 (raw file):

        auto tmesh =
                dynamic_cast<const t::geometry::TriangleMesh *>(pg.tgeometry);
        auto lineset = dynamic_cast<const geometry::LineSet *>(pg.geometry);

Can we implement polymorphism here rather than dynamically casting and checking for nullptr?

[EwingKang]

Hi @godwincharan ,
I think it would be possible. However the structure PickableGeometry defined in SceneWidget.h#L131 is not a templated class by itself. It actually stores two raw pointers to objects:

        const geometry::Geometry3D* geometry = nullptr;
        const t::geometry::Geometry* tgeometry = nullptr;

So one would have to check which one is the valid pointer essentially.
Speaking about polymorphism, my first idea of the solution is to make PickableGeometry templated by itself, and implement specializations for different objects.

Another way to implement this is to add some sort of base function that returns interest points in their parent class.
For the newer open3d::t::geometry::XXXXX namespace objects, There are 5 chlidren in DrawableGeometry. I think it is doable to add a virtual function that returns points required by the picking function.
For the old open3d::geometry::XXXXX namespace objects, there are many chlidren that all inherits open3d::geometry. I don't think it's a good idea to enforce the base function.

The third way is to make pickable objects inherit a new base class. This would require some careful design directly from the owner/maintainers IMHO.

My original intent is to quickly fix this crashing issue for me, so I simply follow what has been done. But I would love to implement a more complete solution (at least for the newer open3d::t:: namespace as described in solution 2). What does everyone think about this issue? I would be happy to hear your suggestions.

[ssheorey]

Thanks @EwingKang for the fix! Tested can pick points from line sets.

PS: Here's the last part of the output of make apply-style for me:

...
Please correct license header *manually* in the following files (see util/check_style.py for the standard header):
/home/ssheorey/Documents/Open3D/Code/Open3D/examples/cpp/TImage.cpp
make[3]: *** [CMakeFiles/apply-style.dir/build.make:71: CMakeFiles/apply-style] Error 1
make[2]: *** [CMakeFiles/Makefile2:2053: CMakeFiles/apply-style.dir/all] Error 2
make[1]: *** [CMakeFiles/Makefile2:2060: CMakeFiles/apply-style.dir/rule] Error 2
make: *** [Makefile:709: apply-style] Error 2

I was able to fix it by updating the license header text. Is this the issue you ran into?

[ssheorey]

@godwincharan agree with your assessment that polymorphism may help to simplify this code. However, the refactoring will take some careful planning and effort as @EwingKang has noted. Please reach out to me if you would like to help.

[EwingKang]

@ssheorey Thanks for merging the code.
I'm actively using Open3D in my project so I'm definitely going to have some feedbacks. It is probably within the reach for me to contribute the polymorphism part, but I think I'll first take some time to get familiar with the code base and design philosophy.

About the style checker, I just re-ran the checker with python3 util/check_style.py --apply and it did worked. But I've done a lot to my system so practically everything is changed. I'll try it again in a few weeks when I'm going to get a new PC, to see which step is missing.
Cheers.

[EwingKang]

About the style checker, I just re-ran the checker with python3 util/check_style.py --apply and it did worked. But I've done a lot to my system so practically everything is changed. I'll try it again in a few weeks when I'm going to get a new PC, to see which step is missing. Cheers.

So I've test it again with a fresh Ubuntu 22.04 install. It turns out that the when installing the style checker dependencies following the instructions, the install defaults to ~/.local/bin. It also gave warning like this:

...
  WARNING: The scripts clang-format, clang-format-diff.py and git-clang-format are installed in '/home/user/.local/bin' which is not on PATH.
  Consider adding this directory to PATH or, if you prefer to suppress this warning, use --no-warn-script-location.
  Attempting uninstall: nbformat
    Found existing installation: nbformat 5.9.2
    Uninstalling nbformat-5.9.2:
      Successfully uninstalled nbformat-5.9.2
  WARNING: The script jupyter-trust is installed in '/home/user/.local/bin' which is not on PATH.
  Consider adding this directory to PATH or, if you prefer to suppress this warning, use --no-warn-script-location.
Successfully installed clang-format-10.0.1.1 nbformat-5.7.0
...

I just installed it to the system with sudo and the clang-format style checker python util/check_style.py can be executed correctly

sudo pip3 install clang-format==10.0.1.1