Fixes and improvements for point selection in O3DVisualizer (fixes #6725

) (#6733)

- Make points removable by correctly forwarding the key modifier to the already implemented function
- Fix bounding box calculation for picked point size selection (manual bbox extend calculation was wrong)
- Fix crash occurring when trying to make selection and non-pickable geometries where in scene
- Fix copy&paste error in help text for MACOS

cpp/open3d/visualization/gui/PickPointsInteractor.cpp

@@ -155,8 +155,6 @@ void PickPointsInteractor::SetPickableGeometry(
     // TriangleMesh so that occluded points are not selected.
     points_.clear();
     for (auto &pg : geometry) {
-        lookup_->Add(pg.name, points_.size());
-
         auto cloud = dynamic_cast<const geometry::PointCloud *>(pg.geometry);
         auto tcloud =
                 dynamic_cast<const t::geometry::PointCloud *>(pg.tgeometry);
@@ -166,6 +164,12 @@ void PickPointsInteractor::SetPickableGeometry(
         auto lineset = dynamic_cast<const geometry::LineSet *>(pg.geometry);
         auto tlineset =
                 dynamic_cast<const t::geometry::LineSet *>(pg.tgeometry);
+
+        // only add geometry with pickable points
+        if (cloud || tcloud || mesh || tmesh || lineset || tlineset) {
+            lookup_->Add(pg.name, points_.size());
+        }
+
         if (cloud) {
             points_.insert(points_.end(), cloud->points_.begin(),
                            cloud->points_.end());
@@ -268,35 +272,38 @@ void PickPointsInteractor::SetOnStartedPolygonPicking(
 }
 
 void PickPointsInteractor::Mouse(const MouseEvent &e) {
-    if (e.type == MouseEvent::BUTTON_UP) {
-        if (e.modifiers & int(KeyModifier::ALT)) {
-            if (pending_.empty() || pending_.back().keymods == 0) {
-                pending_.push({{gui::Point(e.x, e.y)}, int(KeyModifier::ALT)});
-                if (on_ui_changed_) {
-                    on_ui_changed_({});
-                }
-            } else {
-                pending_.back().polygon.push_back(gui::Point(e.x, e.y));
-                if (on_started_poly_pick_) {
-                    on_started_poly_pick_();
-                }
-                if (on_ui_changed_) {
-                    std::vector<Eigen::Vector2i> lines;
-                    auto &polygon = pending_.back().polygon;
-                    for (size_t i = 1; i < polygon.size(); ++i) {
-                        auto &p0 = polygon[i - 1];
-                        auto &p1 = polygon[i];
-                        lines.push_back({p0.x, p0.y});
-                        lines.push_back({p1.x, p1.y});
-                    }
-                    lines.push_back({polygon.back().x, polygon.back().y});
-                    lines.push_back({polygon[0].x, polygon[0].y});
-                    on_ui_changed_(lines);
-                }
+    if (e.type != MouseEvent::BUTTON_UP) return;
+
+    bool polygon_picking_requested = e.modifiers & int(KeyModifier::ALT);
+    if (!polygon_picking_requested) {
+        // standard point picking
+        pending_.push({{gui::Point(e.x, e.y)}, e.modifiers});
+        DoPick();
+    } else {
+        // special polygon picking mode
+        if (pending_.empty() || pending_.back().keymods == 0) {
+            pending_.push({{gui::Point(e.x, e.y)}, e.modifiers});
+            if (on_ui_changed_) {
+                on_ui_changed_({});
             }
         } else {
-            pending_.push({{gui::Point(e.x, e.y)}, 0});
-            DoPick();
+            pending_.back().polygon.push_back(gui::Point(e.x, e.y));
+            if (on_started_poly_pick_) {
+                on_started_poly_pick_();
+            }
+            if (on_ui_changed_) {
+                std::vector<Eigen::Vector2i> lines;
+                auto &polygon = pending_.back().polygon;
+                for (size_t i = 1; i < polygon.size(); ++i) {
+                    auto &p0 = polygon[i - 1];
+                    auto &p1 = polygon[i];
+                    lines.push_back({p0.x, p0.y});
+                    lines.push_back({p1.x, p1.y});
+                }
+                lines.push_back({polygon.back().x, polygon.back().y});
+                lines.push_back({polygon[0].x, polygon[0].y});
+                on_ui_changed_(lines);
+            }
         }
     }
 }

cpp/open3d/visualization/visualizer/O3DVisualizer.cpp

@@ -384,7 +384,9 @@ struct O3DVisualizer::Impl {
                                std::vector<std::pair<size_t, Eigen::Vector3d>>>
                                &indices,
                        int keymods) {
-                    if ((keymods & int(KeyModifier::SHIFT)) ||
+                    bool unselect_mode_requested =
+                            keymods & int(KeyModifier::SHIFT);
+                    if (unselect_mode_requested ||
                         polygon_selection_unselects_) {
                         selections_->UnselectIndices(indices);
                     } else {
@@ -488,11 +490,13 @@ struct O3DVisualizer::Impl {
         });
 
 #if __APPLE__
-        const char *selection_help =
-                "Cmd-click to select a point\nCmd-ctrl-click to polygon select";
+        const char *selection_help = R"(Cmd-click to select a point
+Cmd-shift-click to deselect a point
+Cmd-alt-click to polygon select)";
 #else
-        const char *selection_help =
-                "Ctrl-click to select a point\nCmd-alt-click to polygon select";
+        const char *selection_help = R"(Ctrl-click to select a point
+Ctrl-shift-click to deselect a point
+Ctrl-alt-click to polygon select)";
 #endif  // __APPLE__
         h = new Horiz();
         h->AddStretch();
@@ -1545,12 +1549,10 @@ struct O3DVisualizer::Impl {
             OverrideMaterial(o.name, o.material, ui_state_.scene_shader);
         }
 
-        auto bbox = scene_->GetScene()->GetBoundingBox();
-        auto xdim = bbox.max_bound_.x() - bbox.min_bound_.x();
-        auto ydim = bbox.max_bound_.y() - bbox.min_bound_.z();
-        auto zdim = bbox.max_bound_.z() - bbox.min_bound_.y();
+        auto bbox_extend = scene_->GetScene()->GetBoundingBox().GetExtent();
         auto psize = double(std::max(5, px)) * 0.000666 *
-                     std::max(xdim, std::max(ydim, zdim));
+                     std::max(bbox_extend.x(),
+                              std::max(bbox_extend.y(), bbox_extend.z()));
         selections_->SetPointSize(psize);
 
         scene_->SetPickablePointSize(px);