diff --git a/src/app/mcp_operator_tools.cpp b/src/app/mcp_operator_tools.cpp index 0f6902fde..d147ddb9c 100644 --- a/src/app/mcp_operator_tools.cpp +++ b/src/app/mcp_operator_tools.cpp @@ -485,6 +485,9 @@ namespace lfs::app { if (result.is_running_modal()) { return json{{"success", true}, {"status", "running_modal"}}; } + if (const auto error = props.get("error"); error && !error->empty()) { + return json{{"error", *error}}; + } return json{{"error", operator_cancel_message(*descriptor)}}; } diff --git a/src/io/formats/colmap.cpp b/src/io/formats/colmap.cpp index d267a7cbe..51fd3ae00 100644 --- a/src/io/formats/colmap.cpp +++ b/src/io/formats/colmap.cpp @@ -2046,7 +2046,7 @@ namespace lfs::io { }; } - void update_image_poses_from_cameras( + size_t update_image_poses_from_cameras( std::vector& images, const std::vector& cameras) { std::unordered_map camera_by_name; @@ -2058,25 +2058,33 @@ namespace lfs::io { camera_by_name.emplace(item.camera->image_name(), &item); } + const size_t source_image_count = images.size(); size_t matched = 0; - for (auto& image : images) { + std::erase_if(images, [&](ImageData& image) { + // Keep live-camera images by updating their poses; drop source images whose cameras were deleted. const auto it = camera_by_name.find(image.name); if (it == camera_by_name.end()) { - throw std::runtime_error(std::format("No current scene camera found for COLMAP image '{}'", image.name)); + return true; } auto [qvec, tvec] = transformed_camera_pose(*it->second->camera, it->second->data_world_transform); image.qvec = std::move(qvec); image.tvec = std::move(tvec); ++matched; - } + return false; + }); if (matched == 0) { throw std::runtime_error("No COLMAP image poses were matched for export"); } + if (images.size() != source_image_count) { + LOG_WARN("COLMAP export skipped {} source images with no current scene camera", + source_image_count - images.size()); + } if (matched != camera_by_name.size()) { LOG_WARN("COLMAP export matched {} source images but received {} scene cameras", matched, camera_by_name.size()); } + return source_image_count - images.size(); } uint8_t float_color_to_u8(const float value) { @@ -2179,6 +2187,13 @@ namespace lfs::io { } } + void clear_point3D_tracks(std::vector& points) { + for (auto& point : points) { + point.track.clear(); + point.track_count = 0; + } + } + double mean_observations_per_image(const std::vector& images) { if (images.empty()) { return 0.0; @@ -2823,11 +2838,13 @@ namespace lfs::io { try { ColmapSparseModelData model = read_colmap_sparse_model_for_write(source_base); - update_image_poses_from_cameras(model.images, cameras); + const size_t dropped_images = update_image_poses_from_cameras(model.images, cameras); const bool clear_observation_links = point_cloud_requires_untracked_export(model.points3D, point_cloud); model.points3D = build_points3D_for_write(model.points3D, point_cloud, point_cloud_transform); - if (clear_observation_links) { + if (clear_observation_links || dropped_images > 0) { + // Omitted images can leave point tracks referencing image ids that are no longer exported. clear_image_point3D_references(model.images); + clear_point3D_tracks(model.points3D); } std::error_code ec; diff --git a/src/python/lfs/py_scene.cpp b/src/python/lfs/py_scene.cpp index 5455d2568..02cbe0d82 100644 --- a/src/python/lfs/py_scene.cpp +++ b/src/python/lfs/py_scene.cpp @@ -19,6 +19,7 @@ #include "visualizer/training/training_state.hpp" #include #include +#include namespace lfs::python { @@ -600,7 +601,9 @@ namespace lfs::python { void PyScene::remove_node(const std::string& name, bool keep_children) { if (auto* const scene_manager = get_scene_manager()) { - scene_manager->removePLY(name, keep_children); + if (auto result = scene_manager->removePLYWithResult(name, keep_children); !result) { + throw std::runtime_error(result.error()); + } return; } scene_->removeNode(name, keep_children); @@ -1282,7 +1285,7 @@ namespace lfs::python { )doc") .def("remove_node", &PyScene::remove_node, nb::arg("name"), nb::arg("keep_children") = false, - "Remove a node by name, optionally keeping its children") + "Remove a node by name, optionally keeping its children. Raises RuntimeError if the GUI scene manager rejects removal.") .def("rename_node", &PyScene::rename_node, nb::arg("old_name"), nb::arg("new_name"), "Rename a node, returns true on success") diff --git a/src/python/stubs/lichtfeld/__init__.pyi b/src/python/stubs/lichtfeld/__init__.pyi index c60b9a38c..6462a9c02 100644 --- a/src/python/stubs/lichtfeld/__init__.pyi +++ b/src/python/stubs/lichtfeld/__init__.pyi @@ -324,7 +324,12 @@ def set_camera_training_enabled(name: str, enabled: bool) -> None: """Enable or disable a camera for training by name""" def remove_node(name: str, keep_children: bool = False) -> None: - """Remove a scene node by name""" + """Remove a scene node by name. + + Raises: + RuntimeError: If the GUI scene manager rejects removal, for example + when training protects the target node or subtree. + """ def select_node(name: str) -> None: """Select a scene node by name""" diff --git a/src/python/stubs/lichtfeld/scene.pyi b/src/python/stubs/lichtfeld/scene.pyi index 978bc5346..cb550a804 100644 --- a/src/python/stubs/lichtfeld/scene.pyi +++ b/src/python/stubs/lichtfeld/scene.pyi @@ -511,7 +511,12 @@ class Scene: """ def remove_node(self, name: str, keep_children: bool = False) -> None: - """Remove a node by name, optionally keeping its children""" + """Remove a node by name, optionally keeping its children. + + Raises: + RuntimeError: If the GUI scene manager rejects removal, for example + when training protects the target node or subtree. + """ def rename_node(self, old_name: str, new_name: str) -> bool: """Rename a node, returns true on success""" diff --git a/src/training/training_setup.cpp b/src/training/training_setup.cpp index 012d6a26f..ea9af5c1e 100644 --- a/src/training/training_setup.cpp +++ b/src/training/training_setup.cpp @@ -524,7 +524,7 @@ namespace lfs::training { const auto cameras_group_id = scene.addGroup("Cameras", dataset_id); const auto train_cameras_id = scene.addCameraGroup( - std::format("Training ({})", train_count), + "Training", cameras_group_id, train_count); @@ -536,7 +536,7 @@ namespace lfs::training { if (enable_eval && val_count > 0) { const auto val_cameras_id = scene.addCameraGroup( - std::format("Validation ({})", val_count), + "Validation", cameras_group_id, val_count); @@ -885,7 +885,7 @@ namespace lfs::training { const auto cameras_group_id = scene.addGroup("Cameras", dataset_id); const auto train_cameras_id = scene.addCameraGroup( - std::format("Training ({})", train_count), cameras_group_id, train_count); + "Training", cameras_group_id, train_count); for (size_t i = 0; i < cameras.size(); ++i) { if (!enable_eval || (i % test_every) != 0) { @@ -895,7 +895,7 @@ namespace lfs::training { if (enable_eval && val_count > 0) { const auto val_cameras_id = scene.addCameraGroup( - std::format("Validation ({})", val_count), cameras_group_id, val_count); + "Validation", cameras_group_id, val_count); for (size_t i = 0; i < cameras.size(); ++i) { if ((i % test_every) == 0) { scene.addCamera(cameras[i]->image_name(), val_cameras_id, cameras[i]); diff --git a/src/visualizer/gui/async_task_manager.cpp b/src/visualizer/gui/async_task_manager.cpp index 48ed2f6b3..f7e5b0e7d 100644 --- a/src/visualizer/gui/async_task_manager.cpp +++ b/src/visualizer/gui/async_task_manager.cpp @@ -1274,6 +1274,14 @@ namespace lfs::vis::gui { lfs::vis::app_store().import_overlay_state.set(std::move(state)); } + void AsyncTaskManager::setImportNumImages(const size_t num_images) { + { + const std::lock_guard lock(import_state_.mutex); + import_state_.num_images = num_images; + } + publishImportOverlayState(); + } + void AsyncTaskManager::publishVideoExportOverlayState() { lfs::vis::AppStore::VideoExportOverlayState state; state.active = video_export_state_.active.load(); diff --git a/src/visualizer/gui/async_task_manager.hpp b/src/visualizer/gui/async_task_manager.hpp index d79cfff7d..200914e20 100644 --- a/src/visualizer/gui/async_task_manager.hpp +++ b/src/visualizer/gui/async_task_manager.hpp @@ -100,6 +100,7 @@ namespace lfs::vis { std::lock_guard lock(import_state_.mutex); return import_state_.num_images; } + void setImportNumImages(size_t num_images); [[nodiscard]] size_t getImportNumPoints() const { std::lock_guard lock(import_state_.mutex); return import_state_.num_points; diff --git a/src/visualizer/gui/rmlui/elements/scene_graph_element.cpp b/src/visualizer/gui/rmlui/elements/scene_graph_element.cpp index cc84a71d3..f0ee103fd 100644 --- a/src/visualizer/gui/rmlui/elements/scene_graph_element.cpp +++ b/src/visualizer/gui/rmlui/elements/scene_graph_element.cpp @@ -314,6 +314,13 @@ namespace lfs::vis::gui { } [[nodiscard]] bool isDeletable(const core::NodeType type, const bool parent_is_dataset) { + return type != core::NodeType::CAMERA_GROUP && + type != core::NodeType::KEYFRAME && + type != core::NodeType::KEYFRAME_GROUP && + !parent_is_dataset; + } + + [[nodiscard]] bool isRenamable(const core::NodeType type, const bool parent_is_dataset) { return type != core::NodeType::CAMERA && type != core::NodeType::CAMERA_GROUP && type != core::NodeType::KEYFRAME && @@ -352,6 +359,42 @@ namespace lfs::vis::gui { } } + [[nodiscard]] std::unordered_set buildTrainingDeleteBlockedNodeIds( + const core::Scene& scene, + const TrainerManager* trainer) { + std::unordered_set blocked; + if (!trainer || trainer->canPerform(TrainingAction::DeleteTrainingNode)) { + return blocked; + } + + const auto mark_with_ancestors = [&](core::NodeId id) { + for (const core::SceneNode* node = scene.getNodeById(id); + node != nullptr; + node = node->parent_id == core::NULL_NODE ? nullptr : scene.getNodeById(node->parent_id)) { + blocked.insert(node->id); + } + }; + + const std::string& training_model_name = scene.getTrainingModelNodeName(); + if (!training_model_name.empty()) { + const core::NodeId training_model_id = scene.getNodeIdByName(training_model_name); + if (training_model_id != core::NULL_NODE) { + mark_with_ancestors(training_model_id); + } + } + + for (const core::SceneNode* node : scene.getNodes()) { + if (node && + node->type == core::NodeType::CAMERA && + node->camera && + node->training_enabled) { + mark_with_ancestors(node->id); + } + } + + return blocked; + } + [[nodiscard]] bool ctrlDown() { const SDL_Keymod mod = SDL_GetModState(); return (mod & SDL_KMOD_CTRL) != 0 || (mod & SDL_KMOD_GUI) != 0; @@ -490,6 +533,14 @@ namespace lfs::vis::gui { void SceneGraphElement::OnUpdate() { claimRenameTextInputFocus(); ensureDom(); + const bool delete_training_node_allowed = [&] { + const auto* trainer = services().trainerOrNull(); + return !trainer || trainer->canPerform(TrainingAction::DeleteTrainingNode); + }(); + if (delete_training_node_allowed != last_delete_training_node_allowed_) { + last_delete_training_node_allowed_ = delete_training_node_allowed; + tree_rebuild_needed_ = true; + } bool rebuilt_tree = false; if (tree_rebuild_needed_) { if (auto* scene_manager = services().sceneOrNull()) { @@ -749,6 +800,7 @@ namespace lfs::vis::gui { const auto nodes = scene.getNodes(); const auto active_gaussian_counts = scene.getActiveGaussianCountsByNode(); + const auto delete_blocked_ids = buildTrainingDeleteBlockedNodeIds(scene, services().trainerOrNull()); snapshots.reserve(nodes.size()); root_ids.reserve(nodes.size()); @@ -792,6 +844,11 @@ namespace lfs::vis::gui { formatWithThousands(frame_count > 0 ? frame_count : node->children.size())); break; } + case core::NodeType::CAMERA_GROUP: + snapshot.label = std::format("{} ({})", + node->name, + formatWithThousands(node->children.size())); + break; case core::NodeType::KEYFRAME: if (node->keyframe) snapshot.label = std::format("Keyframe {} ({:.2f}s)", @@ -822,7 +879,10 @@ namespace lfs::vis::gui { const bool parent_is_dataset = parent_it != snapshots.end() && parent_it->second.type == core::NodeType::DATASET; snapshot.draggable = canDrag(snapshot.type, parent_is_dataset); - snapshot.deletable = isDeletable(snapshot.type, parent_is_dataset); + snapshot.can_delete = isDeletable(snapshot.type, parent_is_dataset); + snapshot.delete_enabled = snapshot.can_delete && !delete_blocked_ids.contains(id); + snapshot.can_rename = isRenamable(snapshot.type, parent_is_dataset); + snapshot.rename_enabled = snapshot.can_rename; snapshot.camera_frustum_container = (snapshot.type == core::NodeType::GROUP && std::ranges::any_of(snapshot.children, [&](const core::NodeId child_id) { @@ -878,7 +938,10 @@ namespace lfs::vis::gui { .encoded_label = encode(snapshot.label), .padding_left_dp = formatDp(4 + depth * 16), .has_mask = snapshot.has_mask, - .deletable = snapshot.deletable, + .can_delete = snapshot.can_delete, + .delete_enabled = snapshot.delete_enabled, + .can_rename = snapshot.can_rename, + .rename_enabled = snapshot.rename_enabled, .camera_loss_icon_color = snapshot.camera_loss_icon_color, }); if (snapshot.has_children && !collapsed_ids_.contains(snapshot.id)) @@ -1247,7 +1310,8 @@ namespace lfs::vis::gui { setCachedClass(slot.vis_icon, "icon-vis-off", !row.visible); setCachedAttribute(slot.delete_icon, "data-node-id", row.node_id_text); - setCachedProperty(slot.delete_icon, "display", row.deletable ? "inline" : "none"); + setCachedProperty(slot.delete_icon, "display", row.can_delete ? "inline" : "none"); + setCachedClass(slot.delete_icon, "trash-disabled", row.can_delete && !row.delete_enabled); const std::string_view icon_sprite = typeIconSprite(row.type); const std::string_view unicode = unicodeIcon(row.type); @@ -1453,7 +1517,7 @@ namespace lfs::vis::gui { void SceneGraphElement::beginRename(const core::NodeId node_id) { const auto it = node_snapshots_.find(node_id); - if (it == node_snapshots_.end() || !it->second.deletable) + if (it == node_snapshots_.end() || !it->second.rename_enabled) return; rename_node_id_ = node_id; rename_buffer_ = it->second.name; @@ -1474,6 +1538,10 @@ namespace lfs::vis::gui { const auto it = node_snapshots_.find(rename_node_id_); if (it == node_snapshots_.end()) return; + if (!it->second.rename_enabled) { + cancelRename(); + return; + } const std::string next_name = rename_buffer_.empty() ? it->second.name : rename_buffer_; if (next_name != it->second.name) { @@ -1531,6 +1599,11 @@ namespace lfs::vis::gui { .visible = !static_cast(node->visible)} .emit(); } else if (action == "delete") { + const auto snapshot_it = node_snapshots_.find(node_id); + if (snapshot_it == node_snapshots_.end() || !snapshot_it->second.delete_enabled) + return; + if (const auto result = scene_manager->canRemoveNode(node_id); !result) + return; cmd::RemoveNodeById{.node_id = static_cast(node_id), .keep_children = false}.emit(); tree_rebuild_needed_ = true; markStateDirty(); @@ -1905,12 +1978,12 @@ namespace lfs::vis::gui { drag_ghost_->SetProperty("display", "none"); } - std::vector SceneGraphElement::deletableSelectedNodeIds() const { + std::vector SceneGraphElement::deleteEnabledSelectedNodeIds() const { std::vector ids; ids.reserve(selected_ids_.size()); for (const core::NodeId id : selected_ids_) { const auto it = node_snapshots_.find(id); - if (it != node_snapshots_.end() && it->second.deletable) + if (it != node_snapshots_.end() && it->second.delete_enabled) ids.push_back(id); } return ids; @@ -1920,8 +1993,11 @@ namespace lfs::vis::gui { auto* scene_manager = services().sceneOrNull(); if (!scene_manager) return; - for (const core::NodeId id : deletableSelectedNodeIds()) + for (const core::NodeId id : deleteEnabledSelectedNodeIds()) { + if (const auto result = scene_manager->canRemoveNode(id); !result) + continue; cmd::RemoveNodeById{.node_id = static_cast(id), .keep_children = false}.emit(); + } } void SceneGraphElement::toggleChildrenTraining(const core::NodeId group_id, const bool enabled) { @@ -1993,10 +2069,10 @@ namespace lfs::vis::gui { prefixedAction("disable_all_selected_train"))); } - const auto deletable = deletableSelectedNodeIds(); - if (!deletable.empty()) { + const auto delete_enabled = deleteEnabledSelectedNodeIds(); + if (!delete_enabled.empty()) { items.push_back(makeAction( - std::format("{} ({})", tr(string_keys::Scene::DELETE_ITEM), deletable.size()), + std::format("{} ({})", tr(string_keys::Scene::DELETE_ITEM), delete_enabled.size()), prefixedAction("delete_selected"), !items.empty())); } @@ -2144,7 +2220,7 @@ namespace lfs::vis::gui { !items.empty())); } - if (node_snapshots_.at(node_id).deletable) { + if (node_snapshots_.at(node_id).rename_enabled) { items.push_back(makeAction( tr(string_keys::Scene::RENAME), prefixedAction(std::format("rename:{}", node_id)), @@ -2181,7 +2257,7 @@ namespace lfs::vis::gui { } } - if (node_snapshots_.at(node_id).deletable) { + if (node_snapshots_.at(node_id).delete_enabled) { items.push_back(makeAction( tr(string_keys::Scene::DELETE_ITEM), prefixedAction(std::format("delete:{}", node_id)), @@ -2268,12 +2344,22 @@ namespace lfs::vis::gui { toggleChildrenTraining(group_id, kind == "enable_all_train"); } else if (kind == "delete" && parts.size() >= 2) { core::NodeId node_id = core::NULL_NODE; - if (parseNodeId(parts[1], node_id)) + if (parseNodeId(parts[1], node_id)) { + const auto snapshot_it = node_snapshots_.find(node_id); + if (snapshot_it == node_snapshots_.end() || !snapshot_it->second.delete_enabled) + return; + if (const auto result = scene_manager->canRemoveNode(node_id); !result) + return; cmd::RemoveNodeById{.node_id = static_cast(node_id), .keep_children = false}.emit(); + } } else if (kind == "rename" && parts.size() >= 2) { core::NodeId node_id = core::NULL_NODE; - if (parseNodeId(parts[1], node_id)) + if (parseNodeId(parts[1], node_id)) { + const auto snapshot_it = node_snapshots_.find(node_id); + if (snapshot_it == node_snapshots_.end() || !snapshot_it->second.rename_enabled) + return; beginRename(node_id); + } } else if (kind == "duplicate" && parts.size() >= 2) { core::NodeId node_id = core::NULL_NODE; if (parseNodeId(parts[1], node_id)) diff --git a/src/visualizer/gui/rmlui/elements/scene_graph_element.hpp b/src/visualizer/gui/rmlui/elements/scene_graph_element.hpp index 69d110d9a..6695b3548 100644 --- a/src/visualizer/gui/rmlui/elements/scene_graph_element.hpp +++ b/src/visualizer/gui/rmlui/elements/scene_graph_element.hpp @@ -74,7 +74,10 @@ namespace lfs::vis::gui { std::string label; bool draggable = false; bool has_mask = false; - bool deletable = false; + bool can_delete = false; + bool delete_enabled = false; + bool can_rename = false; + bool rename_enabled = false; std::optional camera_loss_icon_color; }; @@ -93,7 +96,10 @@ namespace lfs::vis::gui { std::string encoded_label; std::string padding_left_dp; bool has_mask = false; - bool deletable = false; + bool can_delete = false; + bool delete_enabled = false; + bool can_rename = false; + bool rename_enabled = false; std::optional camera_loss_icon_color; }; @@ -166,7 +172,7 @@ namespace lfs::vis::gui { void showContextMenu(core::NodeId node_id, float mouse_x, float mouse_y); void showModelsHeaderContextMenu(float mouse_x, float mouse_y); bool isModelsHeaderTarget(Rml::Element* target) const; - std::vector deletableSelectedNodeIds() const; + std::vector deleteEnabledSelectedNodeIds() const; void deleteSelectedNodes(); void toggleChildrenTraining(core::NodeId group_id, bool enabled); void toggleSelectedTraining(bool enabled); @@ -208,6 +214,7 @@ namespace lfs::vis::gui { bool dom_dirty_ = true; bool tree_rebuild_needed_ = true; bool rename_focus_pending_ = false; + bool last_delete_training_node_allowed_ = true; uint64_t last_scene_generation_ = 0; uint64_t state_revision_ = 1; uint64_t last_bound_revision_ = 0; diff --git a/src/visualizer/gui/rmlui/resources/scene_tree.rcss b/src/visualizer/gui/rmlui/resources/scene_tree.rcss index 469c49b1e..cd58e1552 100644 --- a/src/visualizer/gui/rmlui/resources/scene_tree.rcss +++ b/src/visualizer/gui/rmlui/resources/scene_tree.rcss @@ -606,10 +606,19 @@ scene-graph, image-color: rgba(243, 139, 168, 230); } +.row-icon.trash-icon.trash-disabled, +.row-icon.trash-icon.trash-disabled:hover { + image-color: rgba(130, 130, 130, 115); +} + .tree-row.selected .row-icon.trash-icon { image-color: rgba(255, 153, 153, 230); } +.tree-row.selected .row-icon.trash-icon.trash-disabled { + image-color: rgba(150, 150, 150, 130); +} + .node-name { display: block; color: #cdd6f4; diff --git a/src/visualizer/gui/rmlui/resources/scene_tree.theme.rcss b/src/visualizer/gui/rmlui/resources/scene_tree.theme.rcss index 1e572b884..49260e2d1 100644 --- a/src/visualizer/gui/rmlui/resources/scene_tree.theme.rcss +++ b/src/visualizer/gui/rmlui/resources/scene_tree.theme.rcss @@ -87,6 +87,10 @@ image-color: @{error}; } +.tree-row.selected .row-icon.trash-icon.trash-disabled { + image-color: @{alpha(text_dim,0.55)}; +} + .rename-input { color: @{text}; background-color: @{surface}; diff --git a/src/visualizer/operator/ops/edit_ops.cpp b/src/visualizer/operator/ops/edit_ops.cpp index c3f89cc37..bd13ca2fd 100644 --- a/src/visualizer/operator/ops/edit_ops.cpp +++ b/src/visualizer/operator/ops/edit_ops.cpp @@ -106,10 +106,11 @@ namespace lfs::vis::op { } const bool keep_children = props.get_or("keep_children", false); - props.set("resolved_node_names", nodes); props.set("keep_children", keep_children); - for (const auto& name : nodes) { - ctx.scene().removePLY(name, keep_children); + props.set("resolved_node_names", nodes); + if (const auto result = ctx.scene().removeNodesWithResult(nodes, keep_children); !result) { + props.set("error", result.error()); + return OperatorResult::CANCELLED; } return OperatorResult::FINISHED; diff --git a/src/visualizer/scene/scene_manager.cpp b/src/visualizer/scene/scene_manager.cpp index c97680c1d..40127d96a 100644 --- a/src/visualizer/scene/scene_manager.cpp +++ b/src/visualizer/scene/scene_manager.cpp @@ -1206,6 +1206,14 @@ namespace lfs::vis { std::format("Cannot delete '{}': {}", name, trainer->getActionBlockedReason(TrainingAction::DeleteTrainingNode))); } + std::expected SceneManager::canRemoveNode(const core::NodeId id) const { + const auto* node = scene_.getNodeById(id); + if (!node) { + return {}; + } + return validateNodeRemoval(node->name, classifyTrainingRemovalImpact(node->name)); + } + std::expected SceneManager::removeNodeImpl(const std::string& name, const bool keep_children, const HistoryMode history_mode) { @@ -1263,12 +1271,14 @@ namespace lfs::vis { } const core::NodeId removed_id = scene_.getNodeIdByName(name); + bool removes_camera_nodes = false; std::vector ids_to_deselect; std::vector names_to_remove; if (removed_id != core::NULL_NODE && !keep_children) { std::function collect = [&](core::NodeId id) { ids_to_deselect.push_back(id); if (const auto* node = scene_.getNodeById(id)) { + removes_camera_nodes = removes_camera_nodes || node->type == core::NodeType::CAMERA; names_to_remove.push_back(node->name); for (const core::NodeId child_id : node->children) { collect(child_id); @@ -1279,6 +1289,7 @@ namespace lfs::vis { } else if (removed_id != core::NULL_NODE) { ids_to_deselect.push_back(removed_id); names_to_remove.push_back(name); + removes_camera_nodes = node_to_remove->type == core::NodeType::CAMERA; } drainGpuForTensorRelease(); @@ -1310,6 +1321,12 @@ namespace lfs::vis { } .emit(); + if (removes_camera_nodes) { + if (auto* gui = services().guiOrNull()) { + gui->asyncTasks().setImportNumImages(scene_.getAllCameras().size()); + } + } + if (scene_.getNodeCount() == 0) { resetToEmptyState(trainer_cleared); } @@ -1323,8 +1340,37 @@ namespace lfs::vis { return {}; } + std::expected SceneManager::removePLYWithResult(const std::string& name, const bool keep_children) { + return removeNodeImpl(name, keep_children, HistoryMode::Record); + } + + std::expected SceneManager::removeNodesWithResult(const std::vector& names, + const bool keep_children) { + std::vector> planned_removals; + planned_removals.reserve(names.size()); + + for (const auto& name : names) { + if (!scene_.getNode(name)) { + return std::unexpected("Node not found: " + name); + } + + const auto impact = classifyTrainingRemovalImpact(name); + if (const auto result = validateNodeRemoval(name, impact); !result) { + return result; + } + planned_removals.emplace_back(name, impact); + } + + for (const auto& [name, impact] : planned_removals) { + if (const auto result = removeNodeImpl(name, keep_children, HistoryMode::Record, impact); !result) { + return result; + } + } + return {}; + } + void SceneManager::removePLY(const std::string& name, const bool keep_children) { - if (const auto result = removeNodeImpl(name, keep_children, HistoryMode::Record); !result) { + if (const auto result = removePLYWithResult(name, keep_children); !result) { LOG_WARN("{}", result.error()); } } @@ -1362,10 +1408,17 @@ namespace lfs::vis { } void SceneManager::removeNode(const core::NodeId id, const bool keep_children) { + if (const auto result = removeNodeWithResult(id, keep_children); !result) { + LOG_WARN("{}", result.error()); + } + } + + std::expected SceneManager::removeNodeWithResult(const core::NodeId id, const bool keep_children) { const auto* node = scene_.getNodeById(id); - if (!node) - return; - removePLY(node->name, keep_children); + if (!node) { + return {}; + } + return removePLYWithResult(node->name, keep_children); } // ========== Node Selection ========== diff --git a/src/visualizer/scene/scene_manager.hpp b/src/visualizer/scene/scene_manager.hpp index d674c676f..d7438035e 100644 --- a/src/visualizer/scene/scene_manager.hpp +++ b/src/visualizer/scene/scene_manager.hpp @@ -119,8 +119,13 @@ namespace lfs::vis { bool select_new_node = true); size_t consolidateNodeModels(); + [[nodiscard]] std::expected canRemoveNode(core::NodeId id) const; + [[nodiscard]] std::expected removePLYWithResult(const std::string& name, bool keep_children = false); + [[nodiscard]] std::expected removeNodesWithResult(const std::vector& names, + bool keep_children = false); void removePLY(const std::string& name, bool keep_children = false); void setPLYVisibility(const std::string& name, bool visible); + [[nodiscard]] std::expected removeNodeWithResult(core::NodeId id, bool keep_children = false); void removeNode(core::NodeId id, bool keep_children = false); void setNodeVisibility(core::NodeId id, bool visible); diff --git a/tests/test_operator_registry_props.cpp b/tests/test_operator_registry_props.cpp index f88b8143b..6dac77e8b 100644 --- a/tests/test_operator_registry_props.cpp +++ b/tests/test_operator_registry_props.cpp @@ -3,6 +3,7 @@ #include "core/event_bridge/event_bridge.hpp" #include "core/event_bus.hpp" +#include "core/camera.hpp" #include "core/services.hpp" #include "core/splat_data.hpp" #include "core/tensor.hpp" @@ -19,6 +20,8 @@ #include "rendering/coordinate_conventions.hpp" #include "rendering/rendering_manager.hpp" #include "scene/scene_manager.hpp" +#include "training/trainer.hpp" +#include "training/training_manager.hpp" #include "visualizer/core/editor_context.hpp" #include "visualizer/gui_capabilities.hpp" #include "visualizer/scene_coordinate_utils.hpp" @@ -201,6 +204,58 @@ TEST_F(OperatorRegistryPropsTest, DeleteOperatorCanDeleteNamedNodeWithoutSelecti EXPECT_EQ(resolved->front(), "delete_me"); } +TEST_F(OperatorRegistryPropsTest, DeleteOperatorDeletesMultipleSelectedNodes) { + add_node("delete_a"); + add_node("delete_b"); + scene_manager_->selectNodes({"delete_a", "delete_b"}); + + lfs::vis::op::OperatorProperties props; + props.set("keep_children", false); + + const auto result = lfs::vis::op::operators().invoke(lfs::vis::op::BuiltinOp::Delete, &props); + ASSERT_TRUE(result.is_finished()); + EXPECT_EQ(scene_manager_->getScene().getNode("delete_a"), nullptr); + EXPECT_EQ(scene_manager_->getScene().getNode("delete_b"), nullptr); + + const auto resolved = props.get>("resolved_node_names"); + ASSERT_TRUE(resolved.has_value()); + EXPECT_EQ(*resolved, (std::vector{"delete_a", "delete_b"})); +} + +TEST_F(OperatorRegistryPropsTest, DeleteOperatorRejectsMixedTrainingBatchWithoutPartialRemoval) { + auto trainer_manager = std::make_unique(); + lfs::vis::services().set(trainer_manager.get()); + + auto& scene = scene_manager_->getScene(); + const auto model_id = scene.addSplat("Model", make_test_splat({0.0f, 0.0f, 0.0f})); + const auto unrelated_id = scene.addSplat("unrelated", make_test_splat({1.0f, 0.0f, 0.0f})); + const auto cameras_id = scene.addGroup("Cameras"); + const auto train_group_id = scene.addCameraGroup("Training", cameras_id, 1); + const auto val_group_id = scene.addCameraGroup("Validation", cameras_id, 1); + scene.addCamera("train.png", train_group_id, std::make_shared()); + const auto val_camera_id = scene.addCamera("val.png", val_group_id, std::make_shared()); + scene.setCameraTrainingEnabled(val_camera_id, false); + scene.setTrainingModelNode("Model"); + scene_manager_->changeContentType(lfs::vis::SceneManager::ContentType::Dataset); + + trainer_manager->setScene(&scene); + trainer_manager->setTrainerFromCheckpoint(std::make_unique(scene), 0); + ASSERT_FALSE(trainer_manager->canPerform(lfs::vis::TrainingAction::DeleteTrainingNode)); + + scene_manager_->selectNodes({"unrelated", "Model"}); + lfs::vis::op::OperatorProperties props; + props.set("keep_children", false); + + const auto result = lfs::vis::op::operators().invoke(lfs::vis::op::BuiltinOp::Delete, &props); + ASSERT_TRUE(result.is_cancelled()); + EXPECT_NE(scene.getNodeById(model_id), nullptr); + EXPECT_NE(scene.getNodeById(unrelated_id), nullptr); + + const auto error = props.get("error"); + ASSERT_TRUE(error.has_value()); + EXPECT_NE(error->find("Cannot delete 'Model'"), std::string::npos); +} + TEST_F(OperatorRegistryPropsTest, TransformTranslateOperatorUsesVisualizerWorldCoordinates) { add_node("move_me"); EXPECT_FALSE(scene_manager_->hasSelectedNode()); diff --git a/tests/test_undo_history.cpp b/tests/test_undo_history.cpp index 3091af424..9b4289ce0 100644 --- a/tests/test_undo_history.cpp +++ b/tests/test_undo_history.cpp @@ -4,6 +4,7 @@ #include "core/event_bridge/event_bridge.hpp" #include "core/event_bus.hpp" #include "core/events.hpp" +#include "core/camera.hpp" #include "core/services.hpp" #include "core/splat_data.hpp" #include "core/tensor.hpp" @@ -12,6 +13,8 @@ #include "operation/undo_history.hpp" #include "rendering/rendering_manager.hpp" #include "scene/scene_manager.hpp" +#include "training/trainer.hpp" +#include "training/training_manager.hpp" #include "visualizer/gui_capabilities.hpp" #include @@ -1732,6 +1735,52 @@ TEST_F(UndoHistoryTest, DeleteKeepChildrenRestoresHierarchyOnUndo) { EXPECT_EQ(child->parent_id, lfs::core::NULL_NODE); } +TEST_F(UndoHistoryTest, DeleteResultHonorsTrainingRemovalPolicy) { + auto scene_manager = std::make_unique(); + auto rendering_manager = std::make_unique(); + auto trainer_manager = std::make_unique(); + lfs::vis::services().set(scene_manager.get()); + lfs::vis::services().set(rendering_manager.get()); + lfs::vis::services().set(trainer_manager.get()); + + auto& scene = scene_manager->getScene(); + const auto root_id = scene.addGroup("root"); + const auto model_id = scene.addSplat("Model", make_test_splat({0.0f, 0.0f, 0.0f}), root_id); + const auto unrelated_id = scene.addSplat("unrelated", make_test_splat({1.0f, 0.0f, 0.0f})); + const auto cameras_id = scene.addGroup("Cameras"); + const auto train_group_id = scene.addCameraGroup("Training", cameras_id, 1); + const auto val_group_id = scene.addCameraGroup("Validation", cameras_id, 1); + const auto train_camera_id = scene.addCamera("train.png", train_group_id, std::make_shared()); + const auto val_camera_id = scene.addCamera("val.png", val_group_id, std::make_shared()); + scene.setCameraTrainingEnabled(val_camera_id, false); + scene.setTrainingModelNode("Model"); + scene_manager->changeContentType(lfs::vis::SceneManager::ContentType::Dataset); + + trainer_manager->setScene(&scene); + trainer_manager->setTrainerFromCheckpoint(std::make_unique(scene), 0); + ASSERT_FALSE(trainer_manager->canPerform(lfs::vis::TrainingAction::DeleteTrainingNode)); + + const auto model_result = scene_manager->removePLYWithResult("Model"); + EXPECT_FALSE(model_result); + EXPECT_NE(scene.getNodeById(model_id), nullptr); + + const auto ancestor_result = scene_manager->canRemoveNode(root_id); + EXPECT_FALSE(ancestor_result); + EXPECT_NE(scene.getNodeById(root_id), nullptr); + + const auto training_camera_result = scene_manager->removeNodeWithResult(train_camera_id); + EXPECT_FALSE(training_camera_result); + EXPECT_NE(scene.getNodeById(train_camera_id), nullptr); + + const auto validation_camera_result = scene_manager->removeNodeWithResult(val_camera_id); + EXPECT_TRUE(validation_camera_result); + EXPECT_EQ(scene.getNodeById(val_camera_id), nullptr); + + const auto unrelated_result = scene_manager->removeNodeWithResult(unrelated_id); + EXPECT_TRUE(unrelated_result); + EXPECT_EQ(scene.getNodeById(unrelated_id), nullptr); +} + TEST_F(UndoHistoryTest, RenameNodeCreatesUndoableSceneGraphEntry) { auto scene_manager = std::make_unique(); auto rendering_manager = std::make_unique();