Skip to content

Fix turntable pitch escape from polar views - #227

Merged
SchrodingersCattt merged 5 commits into
mainfrom
fix/tui-polar-escape
Oct 25, 2026
Merged

SchrodingersCattt merged 5 commits into
mainfrom
fix/tui-polar-escape

Conversation

@SchrodingersCattt

@SchrodingersCattt SchrodingersCattt commented Oct 25, 2026 •

Copy link
Copy Markdown
Owner

Problem and change

An exact top-down camera starts at elevation 90°, beyond the default 89° turntable limit. The old sampled clamp treated the first inward step as a new violation, freezing orbit(pitch_deg=-30) at 90°. Both ±30° requests now leave the pole and end at approximately 60°, in opposite horizontal directions.

Compute the first outward boundary crossing analytically from the rotation arc. Preserve narrow caps near 90°, very small positive limits, tangency, and repeated outward requests at an already-clipped boundary. Use complementary angular comparisons with roundoff scaled to the polar gap, retaining inclusive tangency without erasing narrow caps. Use the measured arc amplitude for limits near zero.

Add 51 regression cases and update the TUI API contract.

Also isolate the surface-picking browser regression in a fresh server fixture. The prior orthographic-drag test persisted a DAP-4 camera in the shared module server, putting the surface-picking fixtures out of view. The unchanged sequence reproduced the CI failure locally; with independent server state, both tests pass. Picking assertions are unchanged. The branch includes main's #226 Camera WebGL settle-race fix (e4663b4).

Validation

Final-head GitHub CI: 9/9 checks passed, including Camera WebGL and Python 3.10/3.11/3.12 full suites. Run 40176981439. The earlier surface-picking failure reproduced twice in CI and in the local drag-then-pick sequence; the fixture-isolation fix passed the sequence locally and this final CI run.

  • Browser order regression: orthographic drag followed by surface picking reproduced the failure before fixture isolation; the same sequence passes both tests after isolation (2 passed in 92.76s).

  • Current head 4bd1585c35f70995c713bf7534afd8914b4d387f: all 61 new/existing camera tests pass locally; changed Python files pass Ruff.

  • Public JSONL smoke check of the polar escape: synchronized orbit(yaw=30, pitch=-30) moves all three fixture cameras from 90° to approximately 60° and produces distinct candidate frames.

  • Local full suite before the concurrent numerical follow-up (test-isolation patch included): 1847 passed, 50 skipped, 23 failed. The previous 22 baseline failures remain; the additional upload concurrency test fails with the same Windows temporary-directory PermissionError on both patched and unmodified baseline runs.

  • The initial implementation's local full suite on 0258900 reported 1819 passed, 50 skipped, and 22 failures. All 22 failures reproduced on the unmodified same base. Whole-repository Ruff's 266 findings also matched the baseline; none were in the changed Python files.

Experimental provenance and the earlier deterministic checks are archived in the chirality campaign. The campaign used an older pinned revision; its counts are separate from this PR's final-head CI.

@SchrodingersCattt SchrodingersCattt left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: approve-ready (COMMENT — GitHub blocks self-APPROVE on author PR)

Focused fix for a real turntable bug: sampled elevation clamp treated an already-polar view as an outward violation, so the first inward pitch stayed stuck at ±90°. Replacing that with analytic Rodrigues z(t) roots and clamping only on outward boundary crossings is the right model.

What looks solid

  • Early return when already beyond the cap and the pitch derivative would push further out — this is the escape-vs-freeze distinction, and the docstring/API note match it.
  • Cap on |requested| at 180° + analytic roots still prevent tunneling through a narrow polar band (covered by test_narrow_cap_cannot_be_skipped_by_large_request).
  • Parametrized cases cover both poles, ±pitch escape, near-pole inward/outward, normal-view clip, and huge requests; opposite-direction check at the north pole is a good chirality guard.
  • Honest PR note that the 22 full-suite failures reproduce on unmodified 0258900 — not introduced here.
  • Call sites stay in TUI/orbit_turntable (controller.py + camera tests); scope matches the stated bug.

Process notes (non-blocking)

  1. Rebase onto current main after #226 (e4663b4, scene-rename settle). This branch is still based on 0258900, so Camera WebGL on the PR can still hit the rename flake until rebased; the math change itself does not touch that path.
  2. If a Dash/WebGL orbit path ever diverges from orbit_turntable, that would be a separate follow-up (not required for this PR).

Happy to merge once CI is green on a #226-inclusive base.

@SchrodingersCattt
SchrodingersCattt merged commit a6c10d0 into main Oct 25, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant