From de5eb9641f7fce78138d8edc766b16b500ec0643 Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Mon, 4 May 2026 12:22:14 +1000 Subject: [PATCH] ifcviewer-full: hide and remove model actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Right-click a model root in the Elements tree to get Hide/Show and Remove. Hide flips the federation's per-model visible flag (already round-tripped to .ifcfed), pushes ViewportWindow::hideModel/showModel, and italicises + greys the tree root as a visual cue. Remove drops the model from the viewport, the SceneLoader (streamer + caches), the MainWindow UI maps and tree, and the Federation — disabled while the model is the active load. Visibility is reapplied on each model's load completion (sidecar or stream), so a federation saved with hidden models opens with them hidden. clearScene() now also drops SceneLoader state so streamers no longer leak across federation transitions. API additions: - Federation::setModelVisible + modelVisibilityChanged signal - SceneLoader::removeModel + isLoadingModel Tests cover the setter (dirty + signal + idempotence + unknown id); extends the existing round-trip test to actually exercise the visibility load/save it always claimed to. Co-Authored-By: Claude Opus 4.7 --- src/ifcviewer-full/MainWindow.cpp | 87 +++++++++++++++++++++++++ src/ifcviewer-full/MainWindow.h | 10 +++ src/ifcviewer/Federation.cpp | 11 ++++ src/ifcviewer/Federation.h | 2 + src/ifcviewer/SceneLoader.cpp | 19 ++++++ src/ifcviewer/SceneLoader.h | 7 ++ src/ifcviewer/tests/test_federation.cpp | 47 +++++++++++++ 7 files changed, 183 insertions(+) diff --git a/src/ifcviewer-full/MainWindow.cpp b/src/ifcviewer-full/MainWindow.cpp index 84f13ea908..bcff5b6890 100644 --- a/src/ifcviewer-full/MainWindow.cpp +++ b/src/ifcviewer-full/MainWindow.cpp @@ -28,9 +28,11 @@ #include #include +#include #include #include #include +#include #include #include #include @@ -69,6 +71,13 @@ MainWindow::MainWindow(QWidget* parent) applyModelTransformationToViewport(it->second); } }); + connect(federation_, &Federation::modelVisibilityChanged, + this, [this](const QString& fed_id, bool /*visible*/) { + auto it = fed_id_to_model_id_.find(fed_id); + if (it != fed_id_to_model_id_.end()) { + applyModelVisibilityToViewport(it->second); + } + }); connect(federation_, &Federation::dirtyChanged, this, [this](bool dirty) { setWindowModified(dirty); @@ -148,7 +157,10 @@ void MainWindow::setupUi() { element_tree_->setColumnWidth(0, 200); element_tree_->setColumnWidth(1, 120); element_tree_->setSelectionMode(QAbstractItemView::SingleSelection); + element_tree_->setContextMenuPolicy(Qt::CustomContextMenu); connect(element_tree_, &QTreeWidget::itemSelectionChanged, this, &MainWindow::onTreeSelectionChanged); + connect(element_tree_, &QTreeWidget::customContextMenuRequested, + this, &MainWindow::onTreeContextMenu); tree_dock->setWidget(element_tree_); addDockWidget(Qt::LeftDockWidgetArea, tree_dock); @@ -432,6 +444,7 @@ void MainWindow::clearScene() { while (!tree_roots_.empty()) { uint32_t mid = tree_roots_.begin()->first; viewport_->removeModel(mid); + loader_->removeModel(mid); removeModelUi(mid); } fed_id_to_model_id_.clear(); @@ -645,6 +658,7 @@ void MainWindow::onLoadedFromSidecar(uint32_t mid, qint64 elapsed_ms) { // ModelTransformation immediately rather than waiting for the // (possibly never-arriving) data-source load. applyCoordinateOperationToViewport(mid); + applyModelVisibilityToViewport(mid); maybeGuessFederatedFalseOrigin(mid); } @@ -755,6 +769,7 @@ void MainWindow::onLoadedFromStream(uint32_t mid, qint64 elapsed_ms) { .arg(formatElapsed(elapsed_ms))); applyCoordinateOperationToViewport(mid); + applyModelVisibilityToViewport(mid); maybeGuessFederatedFalseOrigin(mid); writeSidecarForModel(mid); @@ -882,3 +897,75 @@ QString MainWindow::formatElapsed(qint64 ms) const { ? QString::number(ms / 1000.0, 'f', 2) + " s" : QString::number(ms) + " ms"; } + +uint32_t MainWindow::modelIdForRoot(QTreeWidgetItem* item) const { + if (!item) return 0; + for (const auto& kv : tree_roots_) { + if (kv.second == item) return kv.first; + } + return 0; +} + +void MainWindow::applyModelVisibilityToViewport(uint32_t mid) { + auto fed_it = model_id_to_fed_id_.find(mid); + if (fed_it == model_id_to_fed_id_.end()) return; + const Federation::Model* m = federation_->findById(fed_it->second); + if (!m) return; + if (m->visible) viewport_->showModel(mid); + else viewport_->hideModel(mid); + + // Tree-side cue: italicise + grey out the model root when hidden. + auto root_it = tree_roots_.find(mid); + if (root_it != tree_roots_.end()) { + QFont f = root_it->second->font(0); + f.setItalic(!m->visible); + for (int col = 0; col < element_tree_->columnCount(); ++col) { + root_it->second->setFont(col, f); + root_it->second->setForeground( + col, + m->visible ? element_tree_->palette().color(QPalette::Text) + : element_tree_->palette().color(QPalette::Disabled, + QPalette::Text)); + } + } +} + +void MainWindow::onTreeContextMenu(const QPoint& pos) { + QTreeWidgetItem* item = element_tree_->itemAt(pos); + uint32_t mid = modelIdForRoot(item); + if (mid == 0) return; // not a model root — only roots get the menu + + auto fed_it = model_id_to_fed_id_.find(mid); + if (fed_it == model_id_to_fed_id_.end()) return; + const Federation::Model* m = federation_->findById(fed_it->second); + if (!m) return; + + const bool currently_loading = loader_->isLoadingModel(mid); + + QMenu menu(this); + QAction* hide_show = menu.addAction(m->visible ? "Hide" : "Show"); + QAction* remove = menu.addAction("Remove"); + remove->setEnabled(!currently_loading); + + QAction* chosen = menu.exec(element_tree_->viewport()->mapToGlobal(pos)); + if (!chosen) return; + if (chosen == hide_show) { + federation_->setModelVisible(fed_it->second, !m->visible); + } else if (chosen == remove) { + removeModel(mid); + } +} + +void MainWindow::removeModel(uint32_t mid) { + if (loader_->isLoadingModel(mid)) return; + + QString fed_id; + auto fed_it = model_id_to_fed_id_.find(mid); + if (fed_it != model_id_to_fed_id_.end()) fed_id = fed_it->second; + + viewport_->removeModel(mid); + loader_->removeModel(mid); + removeModelUi(mid); + if (!fed_id.isEmpty()) federation_->removeModel(fed_id); + updateWindowTitle(); +} diff --git a/src/ifcviewer-full/MainWindow.h b/src/ifcviewer-full/MainWindow.h index 3910f4fd62..99e694de53 100644 --- a/src/ifcviewer-full/MainWindow.h +++ b/src/ifcviewer-full/MainWindow.h @@ -67,6 +67,7 @@ private slots: void onModelTransformations(); void onObjectPicked(uint32_t object_id); void onTreeSelectionChanged(); + void onTreeContextMenu(const QPoint& pos); void onLoadStarted(uint32_t mid, QString display_name); void onLoadProgressChanged(int percent); @@ -99,6 +100,15 @@ private: const std::string& type); void writeSidecarForModel(uint32_t mid); void removeModelUi(uint32_t mid); + void removeModel(uint32_t mid); + // Returns the model_id whose tree root is `item`, or 0 if `item` is not + // a model root (i.e. an element row, or null). + uint32_t modelIdForRoot(QTreeWidgetItem* item) const; + // Push the federation's `visible` flag for `mid` onto the viewport. + // No-op if `mid` is not in the federation map. Idempotent — safe to + // call before the model is finalised on the viewport (hideModel is a + // lookup-and-set on models_gpu_; missing entries are skipped). + void applyModelVisibilityToViewport(uint32_t mid); void applyPendingBenchmark(); // Push a model's CoordinateOperation matrix to the viewport (or diff --git a/src/ifcviewer/Federation.cpp b/src/ifcviewer/Federation.cpp index 892266e3a7..d32e066617 100644 --- a/src/ifcviewer/Federation.cpp +++ b/src/ifcviewer/Federation.cpp @@ -254,6 +254,17 @@ void Federation::setModelTransformation(const QString& fed_id, } } +void Federation::setModelVisible(const QString& fed_id, bool visible) { + for (auto& m : models_) { + if (m.id != fed_id) continue; + if (m.visible == visible) return; + m.visible = visible; + setDirty(true); + emit modelVisibilityChanged(fed_id, visible); + return; + } +} + void Federation::markClean() { setDirty(false); } diff --git a/src/ifcviewer/Federation.h b/src/ifcviewer/Federation.h index ce0e715df0..ddc5173e3f 100644 --- a/src/ifcviewer/Federation.h +++ b/src/ifcviewer/Federation.h @@ -210,6 +210,7 @@ public: void setConfig(const FederationConfig&); void setFederatedFalseOrigin(const FederatedFalseOrigin&); void setModelTransformation(const QString& fed_id, const ModelTransformation&); + void setModelVisible(const QString& fed_id, bool visible); // Accessors const std::vector& models() const { return models_; } @@ -232,6 +233,7 @@ signals: void configChanged(); void federatedFalseOriginChanged(); void modelTransformationChanged(const QString& fed_id); + void modelVisibilityChanged(const QString& fed_id, bool visible); private: void setDirty(bool d); diff --git a/src/ifcviewer/SceneLoader.cpp b/src/ifcviewer/SceneLoader.cpp index b37c4cb6f1..62ae291513 100644 --- a/src/ifcviewer/SceneLoader.cpp +++ b/src/ifcviewer/SceneLoader.cpp @@ -123,6 +123,25 @@ void SceneLoader::connectStreamer(GeometryStreamer* streamer) { this, &SceneLoader::onStreamerError, Qt::QueuedConnection); } +void SceneLoader::removeModel(uint32_t mid) { + // Refuse while the model is the active load: the streamer thread is still + // running and would race with the deleteLater(). UI gates Remove on + // isLoading(), but guard here too. + if (loading_model_id_ == mid) return; + + for (auto it = load_queue_.begin(); it != load_queue_.end();) { + if (*it == mid) it = load_queue_.erase(it); + else ++it; + } + + auto it = models_.find(mid); + if (it == models_.end()) return; + if (it->second.streamer) { + it->second.streamer->deleteLater(); + } + models_.erase(it); +} + void SceneLoader::cancelCurrentLoad() { if (loading_model_id_ == 0) return; auto it = models_.find(loading_model_id_); diff --git a/src/ifcviewer/SceneLoader.h b/src/ifcviewer/SceneLoader.h index 71459f8d6a..992b7def3f 100644 --- a/src/ifcviewer/SceneLoader.h +++ b/src/ifcviewer/SceneLoader.h @@ -60,8 +60,15 @@ public: std::vector addFiles(const QStringList& paths); void cancelCurrentLoad(); bool isLoading() const { return loading_model_id_ != 0 || !load_queue_.empty(); } + bool isLoadingModel(uint32_t mid) const { return loading_model_id_ == mid; } size_t modelCount() const { return models_.size(); } + // Drop the loader's tracking for `mid` — its streamer, file path, georef + // cache, and queue slot if still pending. Caller is responsible for the + // viewport / UI cleanup; this only releases the loader's own state. + // Refuses while the model is the active load (use cancelCurrentLoad first). + void removeModel(uint32_t mid); + QString filePath(uint32_t mid) const; QString displayName(uint32_t mid) const; ifcopenshell::file* ifcFile(uint32_t mid) const; diff --git a/src/ifcviewer/tests/test_federation.cpp b/src/ifcviewer/tests/test_federation.cpp index 223075c66f..0f90eb36e2 100644 --- a/src/ifcviewer/tests/test_federation.cpp +++ b/src/ifcviewer/tests/test_federation.cpp @@ -145,6 +145,48 @@ TEST_CASE("setHomeView / clearHomeView toggle dirty + has_home_view", "[federati REQUIRE(spy.count() == 0); } +TEST_CASE("setModelVisible toggles flag, dirty, and signal; idempotent", "[federation]") { + ensureQApp(); + QTemporaryDir tmp; + REQUIRE(tmp.isValid()); + + Federation fed; + QString id = fed.addModel(writeStubFile(tmp.filePath("a.ifc"))); + REQUIRE_FALSE(id.isEmpty()); + REQUIRE(fed.findById(id)->visible); // visible by default + fed.markClean(); + + QSignalSpy dirty_spy(&fed, &Federation::dirtyChanged); + QSignalSpy vis_spy(&fed, &Federation::modelVisibilityChanged); + + fed.setModelVisible(id, false); + REQUIRE_FALSE(fed.findById(id)->visible); + REQUIRE(fed.isDirty()); + REQUIRE(dirty_spy.count() == 1); + REQUIRE(vis_spy.count() == 1); + REQUIRE(vis_spy.takeFirst().at(0).toString() == id); + + // Idempotent: same value, no signal, dirty unchanged. + fed.markClean(); + dirty_spy.clear(); + vis_spy.clear(); + fed.setModelVisible(id, false); + REQUIRE_FALSE(fed.isDirty()); + REQUIRE(dirty_spy.count() == 0); + REQUIRE(vis_spy.count() == 0); + + // Unknown fed_id is a no-op (no crash, no signal). + fed.setModelVisible("not-a-real-id", false); + REQUIRE_FALSE(fed.isDirty()); + REQUIRE(vis_spy.count() == 0); + + // Toggle back on. + fed.setModelVisible(id, true); + REQUIRE(fed.findById(id)->visible); + REQUIRE(fed.isDirty()); + REQUIRE(vis_spy.count() == 1); +} + TEST_CASE("save then load round-trips models, transform, visibility, home view", "[federation]") { ensureQApp(); QTemporaryDir tmp; @@ -160,6 +202,9 @@ TEST_CASE("save then load round-trips models, transform, visibility, home view", REQUIRE_FALSE(id1.isEmpty()); REQUIRE_FALSE(id2.isEmpty()); + // Hide the second model — exercises the visibility round-trip. + src.setModelVisible(id2, false); + Federation::HomeView hv; hv.target = QVector3D(10, 20, 30); hv.distance = 77.0f; @@ -183,9 +228,11 @@ TEST_CASE("save then load round-trips models, transform, visibility, home view", REQUIRE(dst.models()[0].id == id1); REQUIRE(dst.models()[0].display_name == "Wall"); REQUIRE(dst.models()[0].source_path == src1); + REQUIRE(dst.models()[0].visible); REQUIRE(dst.models()[1].id == id2); REQUIRE(dst.models()[1].display_name == "slab.ifc"); REQUIRE(dst.models()[1].source_path == src2); + REQUIRE_FALSE(dst.models()[1].visible); REQUIRE(dst.hasHomeView()); REQUIRE(dst.homeView().target == QVector3D(10, 20, 30));