bonsaiviewer: rework add-models false-origin guess + drop fly-mode input lag

Two independent threads that landed on this branch.

## 1. Federation false-origin guess: arm-on-add + frame-on-origin

The cc54237f5 fix moved the guess off the refresh() fan-in (terminating
the stack-overflow recursion on Holter Tower), but the gating was still
`modelIds().size() == 1` — broken for batch-add-into-empty-session
because the session registry is populated synchronously by all
addModel calls before any modelGeometryReady lands. Result: load 3
files at once → none of the per-model geometry-ready events ever
finds `size() == 1` → guess never fires → camera + federation origin
stay at surveyor coords.

Rework:

* **Arm/consume at the command boundary.** modules/models/Commands
  exposes `armFederatedFalseOriginGuess` / `consumeFederatedFalseOrigin
  Guess` (function pair — arming is a one-shot, raw-bool would let a
  peek-without-clear silently break the contract). `addModel` and the
  cloud-callback in `addModelFromCloud` arm if `modelIds().isEmpty()`
  at the moment they're about to register federation entries. The
  first geometry-ready then consumes the arm and runs the guess —
  batch add or single, works the same way.

* **Lazy first geometry point from mesh AABB centre.** Drop
  SceneLoader's `firstPlacement(mid)` / `first_placement` /
  `has_first_placement` and the two capture sites entirely. The
  viewport keeps CPU-side MeshInfo + InstanceCpu for picking /
  measurement; compute the anchor on demand via new const accessor
  `WgpuViewportWindow::firstGeometryPointWorldM(mid, out)` =
  instance0.placement × meshes[instance0.mesh_id].aabb_centre.
  This is more representative than the placement translation
  (placements often live far from the actual geometry due to long
  ObjectPlacement chains / intermediate local frames), and lighter
  storage-wise (lazy, vs. 128 B per model held just-in-case).

* **`guessFederatedFalseOrigin` math signature: Matrix4d → Vector3d.**
  The function only ever consumed `.block<3,1>(0,3)`; the Matrix4d
  API surface was a strictly-larger-than-necessary contract. Vector3d
  matches what the function actually needs.

* **`WgpuViewportWindow::frameOnFederatedOrigin(mid, max_distance_m)`**
  replaces the post-shift use of viewAll() that the ViewportView
  almost reached for. The federated false origin sits at (0,0,0) in
  post-shift space by construction, so the camera targets there
  directly; distance fits the model's post-shift AABB diagonal with
  viewAll's padding math, clamped to `max_distance_m` so a model
  with one crazy-coord outlier vertex can't pull the camera back so
  far the real geometry becomes a pixel. Called with 100 m cap from
  the guess. Unlike viewAll() this only iterates the one model the
  guess fired for — the "load 10 models, viewAll shows nothing"
  failure mode is structurally avoided.

* **ViewportView::tryGuessFirstModelFalseOrigin** renamed to
  `guessFederatedFalseOriginFromFirstModel` and the body restructured
  to consume the arm, look up the anchor via the viewport, mutate
  the federation origin (which propagates through SessionState's
  federatedFalseOriginChanged relay → refresh() → recompose all
  instance world AABBs), then `frameOnFederatedOrigin(mid, 100)`.

* **Internal guards preserved.** filePath skip (project files own
  the origin), current==defaults skip (don't clobber a user who
  set the origin manually then removed the model), placement /
  georef availability checks — defense-in-depth around the arm, not
  the primary gate. The arm-only flow means re-arming on add-into-
  empty-session is naturally re-firable: add → remove → add will
  retry if the previous guess returned defaults.

## 2. wgpu present-mode: prefer Immediate above FifoRelaxed

On Linux Vulkan stacks where the driver / compositor doesn't advertise
Mailbox (confirmed on the user's setup — capability log added in this
patch reports just `fifo, fifo_relaxed, immediate`), Fifo's 2–3 frame
queue doubles input-to-photon latency the moment WASD activates
(~16 ms render-body fully consumes the budget, so the queue is held
deep). On a 60 Hz display this reads as "less smooth than the 100 fps
HUD suggests" during fly-mode mouse-look-while-moving — confirmed by
WGPU_FLY_DEBUG dt traces (rock-solid 16-17 ms cadence, so it's not
frame pacing — it's latency).

Promote Immediate above FifoRelaxed in the preference order so that
when Mailbox is unavailable we pick the no-queue option (can tear
under fast motion, but tearing on architectural geometry is usually
invisible while the latency win is immediately felt). Also log the
full advertised capability list on first configure so future "why
isn't Mailbox available?" diagnostics don't need a code patch.

Mailbox remains first preference; Fifo remains the spec-guaranteed
final fallback.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Dion Moult
2026-06-04 10:02:53 +10:00
parent 2b91e41fc4
commit 7f87408b78
10 changed files with 270 additions and 76 deletions
@@ -61,6 +61,24 @@
#include <memory>
namespace bonsaiviewer::modules::models {
namespace {
bool should_guess_federated_false_origin_ = false;
} // namespace
void armFederatedFalseOriginGuess() {
should_guess_federated_false_origin_ = true;
}
bool consumeFederatedFalseOriginGuess() {
const bool armed = should_guess_federated_false_origin_;
should_guess_federated_false_origin_ = false;
return armed;
}
} // namespace bonsaiviewer::modules::models
namespace bonsaiviewer::modules::models::commands {
namespace {
@@ -247,6 +265,15 @@ void addModel(SessionState& s, QWidget& host) {
return;
}
// Arm the false-origin guess if we're adding into a session with no
// models yet — the first model that finishes loading will set the
// origin via ViewportView. Checked here (before federation->addModel)
// because federation->addModel doesn't yet populate SessionState's
// model mapping; modelIds() reflects pre-add state at this point.
if (s.modelIds().isEmpty()) {
armFederatedFalseOriginGuess();
}
QStringList accepted_paths;
QStringList accepted_fed_ids;
for (const auto& path : paths) {
@@ -290,6 +317,12 @@ void addModelFromCloud(SessionState& s, QWidget& host) {
proc->call("pull_models_interactive", QJsonValue(),
[sguard, connector_id](const QJsonValue& result) {
if (!sguard) return;
// Arm before the first addCloudModel — modelIds() reflects the
// session state at the moment the connector returns, which is
// when the user's "add into empty session" intent applies.
if (sguard->modelIds().isEmpty()) {
armFederatedFalseOriginGuess();
}
const QJsonArray arr = result.toArray();
QStringList paths;
QStringList fed_ids;
@@ -31,6 +31,23 @@ class QWidget;
class WgpuViewportWindow;
namespace bonsaiviewer { class SessionState; }
namespace bonsaiviewer::modules::models {
// Arm/consume pair for "the next model load should auto-guess the
// federation's false origin." Add-model commands arm this when they're
// about to add into an empty session; ViewportView consumes it on the
// next modelGeometryReady. The flag has consume-on-read semantics so
// the API is functions, not a raw bool (a peek-without-clear would
// silently break the one-shot guarantee).
//
// Module-scoped (not on SessionState) because it's add-command intent —
// SessionState shouldn't grow a field for every module's per-command
// state. Lives next to the commands that arm it.
void armFederatedFalseOriginGuess();
bool consumeFederatedFalseOriginGuess();
} // namespace bonsaiviewer::modules::models
namespace bonsaiviewer::modules::models::commands {
// User-facing commands. Each one is responsible for emitting any notify()
+48 -26
View File
@@ -22,6 +22,7 @@
#include "../../ViewerSettings.h"
#include "../../SessionState.h"
#include "../models/Commands.h"
#include "../../../ifcviewer/Federation.h"
#include "../../../ifcviewer/SceneLoader.h"
#include "../../../ifcviewer-wgpu/WgpuViewportWindow.h"
@@ -56,13 +57,18 @@ ViewportView::ViewportView(bonsaiviewer::SessionState* session_state,
connect(session_state_, &SessionState::modelsChanged, this, &ViewportView::refresh);
connect(session_state_, &SessionState::federationChanged, this, &ViewportView::refresh);
connect(session_state_, &SessionState::visibilityChanged, this, &ViewportView::refresh);
// modelGeometryReady is the right hook for "first model just finished
// loading" — count is already in modelIds() by the time the geometry
// signal lands. We try to auto-guess the false origin here (and only
// here — refresh() stays terminal) so a slot can't accidentally re-emit
// into itself through federationChanged.
// modelGeometryReady is the right hook for "a model finished loading"
// — by this point the loader has populated firstPlacement/modelGeoref,
// so the guess has the data it needs. Whether to actually guess is
// gated by the arm-flag set by the add-model commands when they're
// adding into an empty session (so batch-adds work too: arm once,
// first geometry-ready consumes the arm). refresh() stays terminal —
// any federation mutation from the guess propagates through
// SessionState's federatedFalseOriginChanged relay.
connect(session_state_, &SessionState::modelGeometryReady, this, [this](uint32_t mid) {
tryGuessFirstModelFalseOrigin(mid);
if (modules::models::consumeFederatedFalseOriginGuess()) {
guessFederatedFalseOriginFromFirstModel(mid);
}
refresh();
});
@@ -180,25 +186,28 @@ void ViewportView::applyModelVisibility(uint32_t mid) {
}
}
// Auto-guess the federation's false origin when a model finishes loading,
// scoped to the precise case where it's actually wanted: the just-loaded
// model is the *only* model in the session and the false origin hasn't
// been set yet. Re-firable: if the user removes the model and adds a new
// one, and the previous load's guess didn't change the origin from
// defaults (e.g. first-placement happened to land at the world origin —
// see Holter Tower), the next load will retry.
// Apply the federation false-origin guess from this model's first
// placement. Called only when the add-model command armed the guess
// (i.e. it was adding into an empty session); see the modelGeometryReady
// connection above and modules::models::armFederatedFalseOriginGuess().
//
// This deliberately lives off the refresh() fan-in. refresh() is connected
// to half a dozen signals; calling a federation mutator from inside it
// stack-overflowed BonsaiViewer once the guess returned defaults, because
// The internal guards (filePath, default origin, placement/georef
// presence) are defense-in-depth, not the primary gate — that's the arm.
// Order of checks matters: filePath skip protects project files
// (federation owns origin); current==defaults skip protects a user who
// previously set the origin manually and only later removed the model;
// nullptr placement/georef skip handles loads where the loader hasn't
// surfaced data yet (rare given the modelGeometryReady contract).
//
// Lives off the refresh() fan-in deliberately. refresh() is connected to
// half a dozen signals; calling a federation mutator from inside it
// stack-overflowed BonsaiViewer once the guess returned defaults because
// the mutation re-emitted through SessionState → re-entered refresh().
// Guessing only on the modelGeometryReady edge means a Federation
// mutation here propagates through SessionState's normal relay
// mutation here propagates through SessionState's federation relay
// (federatedFalseOriginChanged → notifyFederationChanged) without
// re-entering this function.
void ViewportView::tryGuessFirstModelFalseOrigin(uint32_t mid) {
if (session_state_->modelIds().size() != 1) return;
void ViewportView::guessFederatedFalseOriginFromFirstModel(uint32_t mid) {
Federation* federation = session_state_->federation();
if (!federation->filePath().isEmpty()) return;
@@ -206,13 +215,26 @@ void ViewportView::tryGuessFirstModelFalseOrigin(uint32_t mid) {
const FederatedFalseOrigin defaults;
if (current.xyz != defaults.xyz || current.rz_deg != defaults.rz_deg) return;
SceneLoader* loader = session_state_->loader();
const Eigen::Matrix4d* placement = loader->firstPlacement(mid);
const ModelGeoref* georef = loader->modelGeoref(mid);
if (placement == nullptr || georef == nullptr) return;
Eigen::Vector3d first_geometry_point_m;
if (!viewport_->firstGeometryPointWorldM(mid, first_geometry_point_m)) return;
federation->setFederatedFalseOrigin(guessFederatedFalseOrigin(
*placement, *georef, federation->config()));
SceneLoader* loader = session_state_->loader();
const ModelGeoref* georef = loader->modelGeoref(mid);
if (georef == nullptr) return;
federation->setFederatedFalseOrigin(::guessFederatedFalseOrigin(
first_geometry_point_m, *georef, federation->config()));
// applyCachedModel's auto-viewAll already framed the camera against
// the pre-shift (surveyor) coordinates. The setFederatedFalseOrigin
// call above propagated through SessionState's federation relay →
// refresh() → viewport->setFederatedFalseOrigin → recomposeAndUpload
// (all synchronous Qt direct connections), which rewrote every
// instance's world AABB to its post-shift position. Re-target on
// (0,0,0) — the federated false origin in render space — capped at
// 100 m so a model with crazy-coord geometry can't pull the camera
// back into nothing.
viewport_->frameOnFederatedOrigin(mid, 100.0f);
}
void ViewportView::updateVolumeReadout() {
+1 -1
View File
@@ -53,7 +53,7 @@ private:
void applyCoordinateOperation(uint32_t mid);
void applyModelTransformation(uint32_t mid);
void applyModelVisibility(uint32_t mid);
void tryGuessFirstModelFalseOrigin(uint32_t mid);
void guessFederatedFalseOriginFromFirstModel(uint32_t mid);
void updateVolumeReadout();
bonsaiviewer::SessionState* session_state_ = nullptr;