viewport: move first-model false-origin guess out of refresh()

Loading a model whose first placement sits at the world origin
stack-overflowed BonsaiViewer instantly on Windows (and macOS).
WinDbg trace was a 5-frame Qt signal-slot cycle hitting the guard
page ~1400 levels deep; Linux escaped only because that machine's
iterator order put a non-origin instance first, which made the guess
return a non-default value and naturally terminated the recursion
after one step.

Root cause is the architecture, not the specific guard inside the
guess function. `ViewportView::refresh()` was connected to six
SessionState signals (projectReset, projectOpened, modelsChanged,
federationChanged, visibilityChanged, modelGeometryReady) and was
calling `maybeGuessFederatedFalseOrigin` on every model on every
fire. That helper called `session_state_->notifyFederationChanged()`
unconditionally after the mutation, which re-emitted
SessionState::federationChanged, which re-entered refresh(), which
re-entered the guess — a hidden emit-in-slot loop. The "current ==
defaults" guard at the top of the guess prevented further mutations
once the value moved off defaults, but on machines where the guess
itself returned defaults the guard never fired and the loop ran
forever.

Cleanup:

* refresh() is now terminal: it reads federation state, pushes it to
  the viewport, and returns. No mutations, no signal emissions.
  maybeGuessFederatedFalseOrigin is removed from its for-loop.

* The guess is renamed to `tryGuessFirstModelFalseOrigin(uint32_t)`
  and is now invoked only from the modelGeometryReady connection,
  not from refresh(). Conditions:
    1. modelIds().size() == 1 (the just-loaded model is the only
       model — i.e. this is the "first model added" edge)
    2. federation->federatedFalseOrigin() == defaults (nobody has
       set the origin yet — possibly because the previous attempt
       guessed defaults and no-op'd, in which case we deliberately
       want to retry next time a model lands)
  No one-shot flag: add→remove→add cycles re-attempt the guess
  precisely while the origin is still default, which is the right
  semantics.

* SessionState now relays Federation::federatedFalseOriginChanged
  onto its own bus via notifyFederationChanged. This replaces the
  manual `session_state_->notifyFederationChanged()` call the old
  guess made post-mutation. With the relay in place, any future
  mutation site (commands, settings dialog, project load) will
  propagate to views automatically — the emit point lives at the
  data change, not at every caller. Views still subscribe to
  SessionState only; Federation stays a back-end detail.

Reproduced with ISSUE_053_20181220Holter_Tower_10.ifcview on
Windows (build 21d3945, WinDbg `kn30` showed the recurring cycle
explicitly). Diagnosis confirmed on Linux by adding tracing prints
in refresh() and the guess body: same model, same code, but the
iterator's first-placement happened to be non-origin so the loop
terminated after one step.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Dion Moult
2026-06-03 11:27:35 +10:00
parent ddee88bed3
commit cc54237f51
3 changed files with 39 additions and 10 deletions
+7
View File
@@ -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) {