From e4f8475ce81e2bff18d10ba25ceec6dcbe6f1f73 Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Tue, 14 Jul 2026 13:53:30 +1000 Subject: [PATCH] bonsaiviewer: View Selected Model from the models panel context menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Right-clicking a model in the models panel now offers "View Selected Model", which frames the camera on just that model's geometry — View All, scoped to one model. With several models selected the action reads "View Selected Models" and frames their union, matching how the panel's existing Move to Group already treats a multi-selection. The AABB fold behind viewAll moves into InstanceCompose, which exists so this kind of logic is unit-testable without a Qt window or a wgpu device (populating ViewportCore's model map needs a real GPU, so the fold was previously untestable in place). It splits in two: - sceneWorldAabb — every VISIBLE model, what viewAll frames. - modelsWorldAabb — only the named models, hidden or not. A model the caller named explicitly is framed even if hidden; second-guessing that is worse than honouring it. Models with no loaded geometry contribute nothing, and if none of them do the camera is left alone rather than flying to the origin. Both are covered by six new cases in test_instance_compose (131 total). Co-Authored-By: Claude Opus 4.8 --- src/bonsaiviewer/modules/models/Commands.cpp | 24 ++++ src/bonsaiviewer/modules/models/Commands.h | 5 + src/bonsaiviewer/modules/models/Panel.cpp | 15 ++- src/ifcviewer/InstanceCompose.cpp | 51 +++++++++ src/ifcviewer/InstanceCompose.h | 18 +++ src/ifcviewer/ViewportCore.cpp | 31 +++--- src/ifcviewer/ViewportCore.h | 12 ++ src/ifcviewer/ViewportWindow.cpp | 3 + src/ifcviewer/ViewportWindow.h | 5 + src/ifcviewer/tests/test_instance_compose.cpp | 103 ++++++++++++++++++ 10 files changed, 249 insertions(+), 18 deletions(-) diff --git a/src/bonsaiviewer/modules/models/Commands.cpp b/src/bonsaiviewer/modules/models/Commands.cpp index ca75273200..954f0dffb6 100644 --- a/src/bonsaiviewer/modules/models/Commands.cpp +++ b/src/bonsaiviewer/modules/models/Commands.cpp @@ -262,6 +262,30 @@ void removeModel(SessionState& session, ViewportWindow& viewport, QWidget& host, session.setStatusMessage("Models", "Model removed"); } +void viewModels(SessionState& session, ViewportWindow& viewport, const QStringList& model_ids) { + // Federation ids are the panel's currency; the viewport speaks session + // model ids. sessionModelIdForModelId returns 0 for a model the viewport + // has never been given geometry for — skip those rather than framing id 0. + std::vector session_model_ids; + session_model_ids.reserve(std::size_t(model_ids.size())); + for (const QString& model_id : model_ids) { + const uint32_t session_model_id = session.sessionModelIdForModelId(model_id); + if (session_model_id != 0) session_model_ids.push_back(session_model_id); + } + + if (!viewport.viewModels(session_model_ids)) { + session.setStatusMessage("Models", "Nothing to view — no loaded geometry"); + return; + } + + QString label = QString("%1 models").arg(model_ids.size()); + if (model_ids.size() == 1) { + const Federation::Model* model = session.federation()->findById(model_ids.front()); + label = model ? model->display_name : model_ids.front(); + } + session.setStatusMessage("Models", QString("Viewing %1").arg(label)); +} + namespace detail { void loadModels(SessionState& session, const QStringList& paths, const QStringList& model_ids) { diff --git a/src/bonsaiviewer/modules/models/Commands.h b/src/bonsaiviewer/modules/models/Commands.h index 15be7e77ed..788f4aa1a7 100644 --- a/src/bonsaiviewer/modules/models/Commands.h +++ b/src/bonsaiviewer/modules/models/Commands.h @@ -60,6 +60,11 @@ void moveGroup(SessionState& session, const QString& id, const QString& parent_g void moveModels(SessionState& session, const QStringList& ids, const QString& parent_group_id); void removeGroup(SessionState& session, QWidget& host, const QString& group_id); void removeModel(SessionState& session, ViewportWindow& viewport, QWidget& host, const QString& model_id); +// "View Selected Model" — frame the camera on just these models' geometry, the +// way View All frames the whole federation. Models that carry no loaded +// geometry (never loaded, or still streaming their metadata) contribute +// nothing; if none of them do, the camera is left where it is. +void viewModels(SessionState& session, ViewportWindow& viewport, const QStringList& model_ids); void addModel(SessionState& session, QWidget& host); // Connector picker → pull_models_interactive → addCloudModel + load. // Reachable from AddModelDialog's CloudModel button; the underlying call diff --git a/src/bonsaiviewer/modules/models/Panel.cpp b/src/bonsaiviewer/modules/models/Panel.cpp index f6874acc9a..ca19970fd7 100644 --- a/src/bonsaiviewer/modules/models/Panel.cpp +++ b/src/bonsaiviewer/modules/models/Panel.cpp @@ -321,6 +321,19 @@ ModelsPanel::ModelsPanel(bonsaiviewer::SessionState* session_state, parent_group_id = idOf(parent_index); } + const QStringList selected_model_ids = selectedModelIdsAt(tree_, index); + + // Frame the camera on just these models — View All, scoped. Right + // above Rename so the two "do something with this model" actions + // that need no dialog sit together at the top. + QAction* view_models = menu.addAction( + components::icons::makeSvgIcon(":/icons/cube-scan.svg"), + selected_model_ids.size() > 1 ? "View Selected Models" + : "View Selected Model"); + connect(view_models, &QAction::triggered, this, [this, selected_model_ids]() { + commands::viewModels(*session_state_, *viewport_, selected_model_ids); + }); + QAction* rename = menu.addAction( components::icons::makeSvgIcon(":/icons/cube.svg"), "Rename"); connect(rename, &QAction::triggered, this, [this, id]() { @@ -333,8 +346,6 @@ ModelsPanel::ModelsPanel(bonsaiviewer::SessionState* session_state, commands::addGroup(*session_state_, *this, parent_group_id); }); - const QStringList selected_model_ids = selectedModelIdsAt(tree_, index); - QMenu* move_menu = menu.addMenu("Move to Group"); QAction* move_root = move_menu->addAction("(Root)"); connect(move_root, &QAction::triggered, this, [this, selected_model_ids]() { diff --git a/src/ifcviewer/InstanceCompose.cpp b/src/ifcviewer/InstanceCompose.cpp index 15fc167e87..a46b1e03c2 100644 --- a/src/ifcviewer/InstanceCompose.cpp +++ b/src/ifcviewer/InstanceCompose.cpp @@ -95,4 +95,55 @@ bool findInstanceInModels( return false; } +namespace { + +// Start an AABB accumulator empty, so the first fold sets both corners. +void resetAabb(float mn[3], float mx[3]) { + for (int i = 0; i < 3; ++i) { + mn[i] = std::numeric_limits::infinity(); + mx[i] = -std::numeric_limits::infinity(); + } +} + +// Fold one model's instance world AABBs into an accumulator already reset. +// Returns whether the model contributed anything (an instance-less model — one +// whose metadata is up but whose geometry has not landed — contributes nothing). +bool foldModelAabb(const ModelGpuData& model_data, float mn[3], float mx[3]) { + bool any = false; + for (const InstanceInfo& instance : model_data.instances) { + for (int i = 0; i < 3; ++i) { + mn[i] = std::min(mn[i], instance.world_aabb_min[i]); + mx[i] = std::max(mx[i], instance.world_aabb_max[i]); + } + any = true; + } + return any; +} + +} // namespace + +bool sceneWorldAabb(const std::unordered_map& models, + float world_min_out[3], float world_max_out[3]) { + resetAabb(world_min_out, world_max_out); + bool any = false; + for (const auto& [session_model_id, model_data] : models) { + if (model_data.hidden) continue; + any |= foldModelAabb(model_data, world_min_out, world_max_out); + } + return any; +} + +bool modelsWorldAabb(const std::unordered_map& models, + const std::vector& session_model_ids, + float world_min_out[3], float world_max_out[3]) { + resetAabb(world_min_out, world_max_out); + bool any = false; + for (uint32_t session_model_id : session_model_ids) { + auto it = models.find(session_model_id); + if (it == models.end()) continue; + any |= foldModelAabb(it->second, world_min_out, world_max_out); + } + return any; +} + } // namespace InstanceCompose diff --git a/src/ifcviewer/InstanceCompose.h b/src/ifcviewer/InstanceCompose.h index 302e16e49b..732b045193 100644 --- a/src/ifcviewer/InstanceCompose.h +++ b/src/ifcviewer/InstanceCompose.h @@ -30,6 +30,7 @@ #include #include +#include #include "ModelGpuData.h" @@ -85,6 +86,23 @@ bool findInstanceInModels( const std::unordered_map& models, InstanceLookup& out); +// Union of every instance's world AABB across every VISIBLE model — the box +// viewAll frames. Model-hidden models are excluded (framing them would fly the +// camera at geometry you cannot see). +// +// Returns false when nothing contributed, in which case [mn, mx] is left as the +// empty box (min = +inf, max = -inf) and the caller must not use it. +bool sceneWorldAabb(const std::unordered_map& models, + float world_min_out[3], float world_max_out[3]); + +// The same union restricted to the named models — the box "view selected model" +// frames. Ids naming a model that isn't loaded contribute nothing. Hidden +// models are NOT skipped here: the caller named these specifically, so honour +// the request rather than second-guessing it. +bool modelsWorldAabb(const std::unordered_map& models, + const std::vector& session_model_ids, + float world_min_out[3], float world_max_out[3]); + } // namespace InstanceCompose #endif // INSTANCECOMPOSE_H diff --git a/src/ifcviewer/ViewportCore.cpp b/src/ifcviewer/ViewportCore.cpp index 4602ada26d..b2ad8ec1bd 100644 --- a/src/ifcviewer/ViewportCore.cpp +++ b/src/ifcviewer/ViewportCore.cpp @@ -193,22 +193,12 @@ void ViewportCore::buildViewProj(Eigen::Matrix4f& view_out, } bool ViewportCore::computeSceneAabb(float mn[3], float mx[3]) const { - bool any = false; - for (int i = 0; i < 3; ++i) { - mn[i] = std::numeric_limits::infinity(); - mx[i] = -std::numeric_limits::infinity(); - } - for (const auto& [session_model_id, m] : models_gpu_) { - if (m.hidden) continue; - for (const auto& inst : m.instances) { - for (int i = 0; i < 3; ++i) { - mn[i] = std::min(mn[i], inst.world_aabb_min[i]); - mx[i] = std::max(mx[i], inst.world_aabb_max[i]); - } - any = true; - } - } - return any; + return InstanceCompose::sceneWorldAabb(models_gpu_, mn, mx); +} + +bool ViewportCore::computeModelsAabb(const std::vector& session_model_ids, + float mn[3], float mx[3]) const { + return InstanceCompose::modelsWorldAabb(models_gpu_, session_model_ids, mn, mx); } float ViewportCore::chunkScreenAreaPx(const ModelGpuData::Chunk& c, @@ -438,6 +428,15 @@ void ViewportCore::viewAll() { cx, cy, cz, camera_distance_, radius); } +bool ViewportCore::viewModels(const std::vector& session_model_ids) { + float mn[3], mx[3]; + if (!computeModelsAabb(session_model_ids, mn, mx)) return false; + frameAabb(mn, mx, 1.10f); // same padding as viewAll + Log::info().noquote().nospace() + << "[wgpu] viewModels framed " << session_model_ids.size() << " model(s)"; + return true; +} + void ViewportCore::setCamera(float tx, float ty, float tz, float dist, float yaw_deg, float pitch_deg) { camera_target_[0] = tx; diff --git a/src/ifcviewer/ViewportCore.h b/src/ifcviewer/ViewportCore.h index 4825e9157e..5f6ffda6f6 100644 --- a/src/ifcviewer/ViewportCore.h +++ b/src/ifcviewer/ViewportCore.h @@ -179,6 +179,12 @@ public: void buildViewProj(Eigen::Matrix4f& view_out, Eigen::Matrix4f& proj_out) const; bool computeSceneAabb(float mn[3], float mx[3]) const; + // The same union restricted to `session_model_ids`. Unknown ids contribute + // nothing; false means none of them resolved to any geometry. Unlike + // computeSceneAabb this does NOT skip hidden models — the caller asked for + // these models specifically. + bool computeModelsAabb(const std::vector& session_model_ids, + float mn[3], float mx[3]) const; float chunkScreenAreaPx(const ModelGpuData::Chunk& c, const Eigen::Matrix4f& vp_mat) const; @@ -195,6 +201,12 @@ public: // ---- Camera mutators / getters ------------------------------------------ void viewAll(); + // viewAll scoped to specific models: frame the union of their world AABBs + // with the same 1.10 padding, so "view this model" and "view everything" + // sit the camera the same way. Returns whether it framed anything (an + // unloaded or empty model leaves the camera alone rather than flying it to + // the origin). + bool viewModels(const std::vector& session_model_ids); void setCamera(float tx, float ty, float tz, float dist, float yaw_deg, float pitch_deg); void setStandardView(float yaw_deg, float pitch_deg); diff --git a/src/ifcviewer/ViewportWindow.cpp b/src/ifcviewer/ViewportWindow.cpp index 6e70280173..53a08625b3 100644 --- a/src/ifcviewer/ViewportWindow.cpp +++ b/src/ifcviewer/ViewportWindow.cpp @@ -1402,6 +1402,9 @@ void ViewportWindow::setCamera(float tx, float ty, float tz, // viewAll / frameAabb / computeObjectAabb moved to ViewportCore (#84-i). void ViewportWindow::viewAll() { core_.viewAll(); } +bool ViewportWindow::viewModels(const std::vector& session_model_ids) { + return core_.viewModels(session_model_ids); +} void ViewportWindow::frameAabb(const float mn[3], const float mx[3], float padding) { core_.frameAabb(mn, mx, padding); } diff --git a/src/ifcviewer/ViewportWindow.h b/src/ifcviewer/ViewportWindow.h index 746811a2cf..6d544e277b 100644 --- a/src/ifcviewer/ViewportWindow.h +++ b/src/ifcviewer/ViewportWindow.h @@ -165,6 +165,11 @@ public: // setCamera was already invoked); clients can re-invoke to re-frame. void viewAll(); + // viewAll scoped to specific models — "view selected model" in a federation + // browser. Returns whether it framed anything (unloaded / empty models + // leave the camera where it was). + bool viewModels(const std::vector& session_model_ids); + // Explicit camera state, mirroring the GL ViewportWindow API. Suppresses // the auto-viewAll on first load so a script-driven camera survives // model loading. Parameters match the GL --camera tx,ty,tz,dist,yaw,pitch diff --git a/src/ifcviewer/tests/test_instance_compose.cpp b/src/ifcviewer/tests/test_instance_compose.cpp index 3cbcf7b3f4..11a2954310 100644 --- a/src/ifcviewer/tests/test_instance_compose.cpp +++ b/src/ifcviewer/tests/test_instance_compose.cpp @@ -342,3 +342,106 @@ TEST_CASE("findInstanceInModels skips a corrupt instance-index entry", "[instanc InstanceCompose::InstanceLookup out; REQUIRE_FALSE(InstanceCompose::findInstanceInModels(42u, models, out)); } + +// --------------------------------------------------------------------------- +// sceneWorldAabb / modelsWorldAabb — the boxes viewAll and "View Selected +// Model" frame. Both fold per-instance world AABBs; the difference is which +// models they fold, and that difference is exactly what these pin down. +// --------------------------------------------------------------------------- + +namespace { + +// A model holding one instance whose world AABB is the unit box translated to +// (tx, 0, 0), so each model occupies a distinct, easily-checked slab of space. +ModelGpuData make_model_at(float tx, bool hidden = false) { + ModelGpuData m; + m.hidden = hidden; + InstanceInfo inst; + inst.world_aabb_min[0] = tx - 1.0f; + inst.world_aabb_min[1] = -1.0f; + inst.world_aabb_min[2] = -1.0f; + inst.world_aabb_max[0] = tx + 1.0f; + inst.world_aabb_max[1] = 1.0f; + inst.world_aabb_max[2] = 1.0f; + m.instances.push_back(inst); + return m; +} + +} // namespace + +TEST_CASE("sceneWorldAabb unions every visible model", "[instance_compose][aabb]") { + std::unordered_map models; + models.emplace(1u, make_model_at(0.0f)); + models.emplace(2u, make_model_at(10.0f)); + + float mn[3], mx[3]; + REQUIRE(InstanceCompose::sceneWorldAabb(models, mn, mx)); + REQUIRE(mn[0] == -1.0f); // model 1's left face + REQUIRE(mx[0] == 11.0f); // model 2's right face +} + +TEST_CASE("sceneWorldAabb skips hidden models", "[instance_compose][aabb]") { + std::unordered_map models; + models.emplace(1u, make_model_at(0.0f)); + models.emplace(2u, make_model_at(10.0f, /*hidden*/true)); + + float mn[3], mx[3]; + REQUIRE(InstanceCompose::sceneWorldAabb(models, mn, mx)); + REQUIRE(mx[0] == 1.0f); // the hidden model at x=10 contributed nothing +} + +TEST_CASE("sceneWorldAabb reports empty for a scene with no geometry", + "[instance_compose][aabb]") { + const std::unordered_map empty; + float mn[3], mx[3]; + REQUIRE_FALSE(InstanceCompose::sceneWorldAabb(empty, mn, mx)); + + // A model whose metadata is up but whose instances haven't landed yet is + // just as empty — the caller must not frame it. + std::unordered_map no_instances; + no_instances.emplace(1u, ModelGpuData{}); + REQUIRE_FALSE(InstanceCompose::sceneWorldAabb(no_instances, mn, mx)); +} + +TEST_CASE("modelsWorldAabb folds only the named models", "[instance_compose][aabb]") { + std::unordered_map models; + models.emplace(1u, make_model_at(0.0f)); + models.emplace(2u, make_model_at(10.0f)); + models.emplace(3u, make_model_at(20.0f)); + + float mn[3], mx[3]; + REQUIRE(InstanceCompose::modelsWorldAabb(models, {2u}, mn, mx)); + REQUIRE(mn[0] == 9.0f); + REQUIRE(mx[0] == 11.0f); // model 2 alone — not the whole scene + + // Several at once unions just those. + REQUIRE(InstanceCompose::modelsWorldAabb(models, {1u, 3u}, mn, mx)); + REQUIRE(mn[0] == -1.0f); + REQUIRE(mx[0] == 21.0f); +} + +TEST_CASE("modelsWorldAabb honours a hidden model the caller named", + "[instance_compose][aabb]") { + // Unlike sceneWorldAabb: "view this model" was asked for explicitly, so a + // hidden model still frames rather than silently reporting nothing. + std::unordered_map models; + models.emplace(1u, make_model_at(10.0f, /*hidden*/true)); + + float mn[3], mx[3]; + REQUIRE(InstanceCompose::modelsWorldAabb(models, {1u}, mn, mx)); + REQUIRE(mx[0] == 11.0f); +} + +TEST_CASE("modelsWorldAabb reports empty for unloaded / unnamed models", + "[instance_compose][aabb]") { + std::unordered_map models; + models.emplace(1u, make_model_at(0.0f)); + + float mn[3], mx[3]; + REQUIRE_FALSE(InstanceCompose::modelsWorldAabb(models, {}, mn, mx)); // nothing named + REQUIRE_FALSE(InstanceCompose::modelsWorldAabb(models, {99u}, mn, mx)); // not loaded + + // A partially-resolvable list still frames what it can. + REQUIRE(InstanceCompose::modelsWorldAabb(models, {99u, 1u}, mn, mx)); + REQUIRE(mx[0] == 1.0f); +}