diff --git a/src/bonsaiviewer/SessionState.cpp b/src/bonsaiviewer/SessionState.cpp index 56699f59af..eb41396ec9 100644 --- a/src/bonsaiviewer/SessionState.cpp +++ b/src/bonsaiviewer/SessionState.cpp @@ -33,6 +33,13 @@ SessionState::SessionState(QObject* parent) , element_registry_(new ElementRegistry(this)) , connector_registry_(new modules::connectors::ConnectorRegistry(this)) { + // Relay specific Federation mutations onto the SessionState bus. Views + // subscribe to SessionState signals only — Federation stays a back-end + // detail. Doing the relay here (instead of having every mutation site + // manually call notifyFederationChanged) keeps the "emit point" at one + // hop from the data change and rules out emit-in-slot recursion bugs. + connect(federation_, &Federation::federatedFalseOriginChanged, + this, &SessionState::notifyFederationChanged); } void SessionState::createLoader(WgpuViewportWindow* viewport) { diff --git a/src/bonsaiviewer/modules/viewport/View.cpp b/src/bonsaiviewer/modules/viewport/View.cpp index f17a880d71..f52474f52f 100644 --- a/src/bonsaiviewer/modules/viewport/View.cpp +++ b/src/bonsaiviewer/modules/viewport/View.cpp @@ -54,9 +54,17 @@ ViewportView::ViewportView(bonsaiviewer::SessionState* session_state, connect(session_state_, &SessionState::projectReset, this, &ViewportView::refresh); connect(session_state_, &SessionState::projectOpened, this, [this](const QString&) { refresh(); }); connect(session_state_, &SessionState::modelsChanged, this, &ViewportView::refresh); - connect(session_state_, &SessionState::federationChanged, this, &ViewportView::refresh); - connect(session_state_, &SessionState::visibilityChanged, this, &ViewportView::refresh); - connect(session_state_, &SessionState::modelGeometryReady, this, [this](uint32_t) { 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. + connect(session_state_, &SessionState::modelGeometryReady, this, [this](uint32_t mid) { + tryGuessFirstModelFalseOrigin(mid); + refresh(); + }); // Measurement tools — input-driven, share the View's lifetime. connect(viewport_, &WgpuViewportWindow::surfacePickedInTool, this, @@ -123,7 +131,6 @@ void ViewportView::refresh() { for (uint32_t mid : session_state_->modelIds()) { applyCoordinateOperation(mid); applyModelVisibility(mid); - maybeGuessFederatedFalseOrigin(mid); } } @@ -173,24 +180,39 @@ void ViewportView::applyModelVisibility(uint32_t mid) { } } -void ViewportView::maybeGuessFederatedFalseOrigin(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. +// +// 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 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 +// (federatedFalseOriginChanged → notifyFederationChanged) without +// re-entering this function. +void ViewportView::tryGuessFirstModelFalseOrigin(uint32_t mid) { + if (session_state_->modelIds().size() != 1) return; + Federation* federation = session_state_->federation(); - SceneLoader* loader = session_state_->loader(); if (!federation->filePath().isEmpty()) return; const FederatedFalseOrigin& current = federation->federatedFalseOrigin(); 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; federation->setFederatedFalseOrigin(guessFederatedFalseOrigin( *placement, *georef, federation->config())); - // The federation mutation above will emit its own signal, but to keep - // views off the Federation bus we re-emit through SessionState. - session_state_->notifyFederationChanged(); } void ViewportView::updateVolumeReadout() { diff --git a/src/bonsaiviewer/modules/viewport/View.h b/src/bonsaiviewer/modules/viewport/View.h index 09b42452d3..67bf71d983 100644 --- a/src/bonsaiviewer/modules/viewport/View.h +++ b/src/bonsaiviewer/modules/viewport/View.h @@ -53,7 +53,7 @@ private: void applyCoordinateOperation(uint32_t mid); void applyModelTransformation(uint32_t mid); void applyModelVisibility(uint32_t mid); - void maybeGuessFederatedFalseOrigin(uint32_t mid); + void tryGuessFirstModelFalseOrigin(uint32_t mid); void updateVolumeReadout(); bonsaiviewer::SessionState* session_state_ = nullptr;