Delete Camera Fix

[kmeirlaen]

Delete Camera Fix

Summary

This patch enables deleting individual camera nodes from the RmlUi scene tree, keeps camera counts current after camera removal, allows COLMAP sparse export after a camera has been deleted, and tightens scene-tree action guards so structural availability and runtime enabled state are handled separately.

Description of the observed issues

1. Load a COLMAP dataset with multiple cameras/images.
2. Delete one camera from the scene.
3. Observe that camera counts may remain at the original load count.
4. Start training and right-click a camera row.
5. Observe that context-menu actions may still offer operations that are blocked elsewhere.
6. Export the dataset as COLMAP sparse.
7. The export fails when the original COLMAP images list still contains the deleted camera's image.

After fixing, more things surfaced:

• Context-menu Delete could bypass the training-active guard that disabled inline delete and the Del key path.
• The delete enabled state only checked CAMERA rows during training, while the backend also rejects the training model node, its ancestor groups, and any subtree containing a training-enabled camera when DeleteTrainingNode is blocked.
• MCP/Python delete paths could discard the backend delete result, making a training-blocked deletion look like a success/no-op to callers.
• Rename was offered on camera rows even though cameras are not intended to be renamed from the shared scene-tree context menu; normal node rename was also over-restricted by the UI during training even though the backend has no training-state rename guard.

Root Cause

Camera deletion only removed scene graph nodes. The load-time image count stored in the import/app state was not recomputed after camera removal, scene-tree menu actions were not all checking the same runtime guards, the UI delete guard used a camera-type heuristic instead of the backend training-removal policy, training camera groups baked the initial count into the node name, and the COLMAP writer expected every source image from the original sparse model to still have a matching live scene camera.

What This Patch Fixes

• Camera rows are deletable in the scene tree and render the existing trash icon between the visibility and type icons.
• Camera deletion now separates structural availability (can_delete) from runtime enabled state (delete_enabled); the trash icon remains visible but greyed out only for nodes blocked by the backend training-removal policy, and inline click, Del, context-menu Delete, and executeAction("delete") all use the same enabled guard.
• SceneManager exposes result-returning delete helpers so GUI/MCP/Python paths can preserve the backend rejection result; existing void delete methods remain compatibility wrappers that log failures.
• MCP scene.delete_node and Python remove_node() now surface a backend delete rejection instead of reporting success or silently no-oping.
• Python stubs and binding docs now note that remove_node() may raise RuntimeError when the GUI scene manager rejects removal; the headless fallback path without a SceneManager still calls raw Scene::removeNode() and does not apply training validation.
• Rename now separates structural availability (can_rename) from runtime enabled state (rename_enabled); cameras and camera groups are not offered Rename, while normal renamable nodes remain rename-enabled during training to match the backend.
• Camera group labels use the live child count so scene-tree counts update after deletion.
• Training and validation camera group names no longer bake the initial count into the node name, preventing doubled count labels.
• Successful scene node removals that remove cameras recompute the live camera count and publish it through the existing import overlay/app-store path used by GUI, Python UI state, and MCP runtime state consumers.
• COLMAP export now skips source images whose cameras have been deleted instead of throwing, and clears observation/track links when images are dropped to avoid stale references.
• Empty Training (0) / Validation (0) camera groups are accepted for this patch; camera groups remain structurally non-deletable and are not auto-pruned.
• Multi-target delete now validates all requested nodes before removing any of them, reuses the cached training-removal impact during removal, and avoids the duplicate training-impact traversal that happened when the delete operator preflighted and then deleted each target.

Before:

After:

Test Results

• Tests: lichtfeld_tests.exe --gtest_filter=OperatorRegistryPropsTest.*Delete*:ColmapImageLayoutTest.WriteBackAppliesSceneTransformsToTextSparseModel:ColmapImageLayoutTest.WriteBackRemovesStaleOppositeFormatSparseFiles:UndoHistoryTest.*SceneGraph*:UndoHistoryTest.*Delete* --gtest_brief=1 --gtest_color=no passed, 25/25 tests.

[Copilot]

Pull request overview

This PR fixes camera-node deletion and related downstream behaviors across the GUI scene tree, backend removal policy enforcement, Python/MCP surfaces, and COLMAP export. It aligns structural affordances (what actions are available) with runtime guards (what actions are currently enabled), and ensures state derived from cameras (counts, exports) stays consistent after deletions.

Changes:

• Unifies delete/rename enablement across scene-tree inline actions, context menus, keyboard, and operator execution by separating “can_” (structural) vs “_enabled” (runtime policy).
• Adds result-returning removal helpers in SceneManager and propagates backend rejection as errors through GUI operators, MCP, and Python bindings/stubs.
• Makes COLMAP sparse export resilient to deleted cameras by dropping unmatched source images and clearing stale observation/track references.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/test_undo_history.cpp	Adds regression test verifying delete results respect training removal policy.
src/visualizer/scene/scene_manager.hpp	Adds result-returning node deletion APIs and a removal capability query.
src/visualizer/scene/scene_manager.cpp	Implements canRemoveNode, result-returning delete wrappers, and camera-count recomputation after camera removals.
src/visualizer/operator/ops/edit_ops.cpp	Ensures delete operator validates/returns errors instead of silently ignoring backend rejections.
src/visualizer/gui/rmlui/resources/scene_tree.theme.rcss	Adds theme rule for disabled trash icon color when selected.
src/visualizer/gui/rmlui/resources/scene_tree.rcss	Styles disabled trash icon states in the scene tree.
src/visualizer/gui/rmlui/elements/scene_graph_element.hpp	Splits delete/rename state into structural vs enabled booleans; tracks training-delete policy changes.
src/visualizer/gui/rmlui/elements/scene_graph_element.cpp	Implements new delete/rename enablement logic, training-delete blocked subtree detection, and live camera-group count labels.
src/visualizer/gui/async_task_manager.hpp	Adds setter to update import overlay camera/image count.
src/visualizer/gui/async_task_manager.cpp	Implements setImportNumImages() and republishes overlay state.
src/training/training_setup.cpp	Stops baking camera counts into training/validation group names.
src/python/stubs/lichtfeld/scene.pyi	Documents that remove_node() may raise RuntimeError when GUI manager rejects removal.
src/python/stubs/lichtfeld/init.pyi	Documents that remove_node() may raise RuntimeError when GUI manager rejects removal.
src/python/lfs/py_scene.cpp	Throws std::runtime_error on GUI-manager deletion rejection so Python sees RuntimeError.
src/io/formats/colmap.cpp	Drops source images whose cameras were deleted and clears stale observation/track references when needed.
src/app/mcp_operator_tools.cpp	Returns operator-provided error messages on cancellation (instead of generic cancel messaging).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.