Skip to content

Axis View: dragging respects Orbit around Origin (#17245) - #17256

Merged
bherbruck merged 2 commits into
tangency-labs:mainfrom
cfaulkingham:codex/fix-axis-origin-17245
Nov 15, 2026
Merged

bherbruck merged 2 commits into
tangency-labs:mainfrom
cfaulkingham:codex/fix-axis-origin-17245

Conversation

@cfaulkingham

@cfaulkingham cfaulkingham commented Nov 15, 2026 •

Copy link
Copy Markdown
Contributor

Change

Dragging the Axis View widget now keeps the world origin fixed when Navigation preferences select "Orbit around: Origin".

Fixes #17245.

The widget previously ignored that preference and turned around the camera target. Panning the view made the error visible.

The widget now uses the existing Origin orbit handling, including Shift snapping. View centre and Cursor retain their current widget behavior.

This fixes the Axis View drag described in the issue body. I have not reproduced the separate Home/Fit framing problem in the title. PR #16774 covers View centre orbit behavior.

Changes a default: no

Validation

  • Reproduced twice on unmodified main at c71236a40 with the real UI harness.
  • A 48-pixel widget drag moved the origin's camera coordinates by 13.7604 mm before the fix.
  • The regression failed before the fix. All three focused view_axis_drag tests pass after the fix.
  • Coverage includes Free and Turntable orbit, Shift snapping, consecutive gestures, gesture cleanup, and unchanged undo history.
  • The tests also check that View centre and Cursor keep the existing widget pivot.

cargo xtask ci completed all 113 test binaries, WebAssembly checks for 30 crates, and documentation tests. Formatting, repository checks, all 16 shared-crate checks, and workspace Clippy passed.

The gate exited 1. Thirteen failing tests match tests/KNOWN_FAILING_TESTS. The UI suite had 5,632 passes, 22 ignored tests, and one additional failure: gpu::tests::blur_steady_frame_benchmark.

That benchmark failed a source-cache counter assertion. It then passed three isolated runs on both unmodified main and the fixed build under matching settings. The full suite is not reported as green.

The separate web-offline-test passed. It ran separately because the test failures prevented the gate from reaching that step.

The naming corpus failure also reproduced with binaries built before the claim. All five fixture diagnostics matched exactly.

Screenshots of the behavior

Reproduction uses a synthetic 20 mm box:

  1. Select "Orbit around: Origin" in Navigation preferences.
  2. Choose Front view and pan until the origin is away from the view center.
  3. Drag the Axis View widget horizontally.

Expected: the world origin stays at the same screen position while the view turns.

Before: the origin moves about 58 pixels right. After: the origin stays at the same pixel.

The maximum measured camera-coordinate drift after the fix is 1.43e-14 mm. Both repeated runs match.

I captured and inspected actual before/after GPU-rendered screenshots locally. The available browser upload tools could not attach the images. The screenshots are preserved for attachment; no generated media is committed.

Integration status

Based on upstream main c71236a40de9f36713194df624663a252d0f71a3. No conflicts required resolution.

The claim preceded implementation. Review and maintainer-triggered CI are pending. No conflicting open claim was found.

Summary by CodeRabbit

  • Bug Fixes
    • Axis-view dragging now respects the selected orbit pivot. With Origin selected, the origin remains fixed during free and snapped orbits; with ViewCentre or Cursor selected, the camera target remains fixed.
    • Orbiting after panning preserves the camera’s distance and does not modify the design.

@coderabbitai

coderabbitai Bot commented Nov 15, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Axis-view dragging now follows the OrbitAround preference. When Origin is selected, the drag uses the origin pivot; otherwise, it retains the camera-target pivot. Regression tests cover Origin, ViewCentre, and Cursor modes.

Changes: Axis-view orbit behavior

Layer / File(s) Summary
Select the axis-view orbit pivot
crates/ui-egui/src/navigation.rs, crates/ui-egui/src/viewport.rs
The documentation describes the selected pivot. axis_view_orbit selects the origin pivot when OrbitAround::Origin is active and the camera-target pivot otherwise.
Verify pivot behavior
crates/ui-egui/src/viewport.rs
Tests cover free and snapped orbits in Origin mode, plus ViewCentre and Cursor modes. They check camera position or target, distance, gesture state, FOV, and undo depth.

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Low

Merge Risk: Minimal · up to 4e0ff

Axis View dragging now keeps the origin fixed when Orbit around is set to Origin, and other modes behave as before. No merge-blocking risk was found; the full CI gate is not reported green, but the author attributes the extra failure to a benchmark that passed in isolated runs.

Security architecture risk: Minimal · up to 4e0ff

The widget now honors the existing Origin preference through the existing navigation logic. No introduced security, ownership, rollback, or failure-containment risk was identified in the inspected change.

Pre-merge checks: Passed 4 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Linked Issues check Inconclusive Issue #17245 has one direct link. Its Expected Behavior asks that both the Orbit tool and the Axis View widget respect "Orbit around → Origin" and keep the origin centred. The diff implements the widg… Confirm whether the standard Orbit tool already keeps the origin fixed at base c71236a when Orbit around is Origin. Provide evidence, such as a focused test or code path, for that baseline. Also confirm whether the Home/Fit framing proble…
Passed Passed checks: 4 passed

Check name Status Explanation
Title check Passed The title names the main change: Axis View dragging now respects the Orbit around Origin preference. It matches the code change in viewport.rs and navigation.rs and the linked issue #17245. It is spec…
Description check Passed The description covers the change, the linked issue, and the default-behaviour line. It includes detailed validation and before/after screenshot steps. It also reports a failing CI gate and explains t…
Out of Scope Changes check Passed The whole-PR diff against merge base c71236a changes two areas. First, crates/ui-egui/src/viewport.rs changes axis_view_orbit to use the selected origin pivot and updates its doc comment. Second, it…
Existing Defaults Unchanged Passed The PR changes two Rust files: crates/ui-egui/src/viewport.rs and crates/ui-egui/src/navigation.rs. The viewport change passes around_origin = (orbit_around == Origin) into orbit_drag in `axis…
Full details: Title check

Explanation

The title names the main change: Axis View dragging now respects the Orbit around Origin preference. It matches the code change in viewport.rs and navigation.rs and the linked issue #17245. It is specific and concise.

Full details: Description check

Explanation

The description covers the change, the linked issue, and the default-behaviour line. It includes detailed validation and before/after screenshot steps. It also reports a failing CI gate and explains the known-failure status. The Change section states the problem and the resulting behavior in plain words. The Screenshots section describes the reproduction and the expected/actual behavior. However, the template asks for actual screenshots or a recording in the description, and the author states that no media is attached. The description also omits the Test change line, which is only required for kernel changes, so it does not apply here. The gap in screenshots is the main shortfall, but the rest of the template is substantially complete.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff against merge base c71236a changes two areas. First, crates/ui-egui/src/viewport.rs changes axis_view_orbit to use the selected origin pivot and updates its doc comment. Second, it adds two tests for the widget drag behaviour. The navigation.rs change is a comment update that describes the same behaviour. All changes relate to the Axis View drag described in #17245. The PR does not add unrelated features or refactors in the reviewed diff. The PR description says View centre orbit work belongs to PR #16774, and the diff does not include that work.

Full details: Existing Defaults Unchanged

Explanation

The PR changes two Rust files: crates/ui-egui/src/viewport.rs and crates/ui-egui/src/navigation.rs. The viewport change passes around_origin = (orbit_around == Origin) into orbit_drag in axis_view_orbit, where it previously passed false. The orbit_drag pivot logic (navigation.rs ~1437-1443) only uses the flag to select Some(Vec3::ZERO) for Origin. For ViewCentre and Cursor, the flag is false and the pivot is None, as before. The only changed behaviour is the Axis View drag when the user has explicitly selected "Orbit around: Origin". The UX default is unchanged. OrbitAround defaults to ViewCentre (prefs.rs ~654-661). A test in prefs.rs asserts "the view centre stays the default". With default preferences, the Axis View drag behaves exactly as before. No preference default, shortcut, layout, or command default changes. The PR description's "Changes a default: no" matches the code. Users who chose Origin do see a different widget drag. Before, the widget turned about the camera target and ignored the setting. After, it keeps the origin fixed on screen, as the Origin option already does for the Orbit tool. This is a fix to a non-default preference path, not a change to a default that users rely on. The check therefore passes.

Full details: Linked Issues check

Explanation

Issue #17245 has one direct link. Its Expected Behavior asks that both the Orbit tool and the Axis View widget respect "Orbit around → Origin" and keep the origin centred. The diff implements the widget part. In viewport.rs, axis_view_orbit now passes around_origin = (orbit_around == Origin) to orbit_drag, where it previously passed false. Two new tests cover Free and Turntable orbit, Shift snapping, consecutive gestures, and the unchanged ViewCentre and Cursor pivots. The Orbit tool path was not changed and was not inspected at the base, so its compliance is not established. The issue title also names a Home/Fit framing problem. The PR states that this problem was not reproduced and is not addressed. The issue text does not state Home/Fit as a separate expected behaviour, so the scope of that item is unclear.

Resolution

Confirm whether the standard Orbit tool already keeps the origin fixed at base c71236a when Orbit around is Origin. Provide evidence, such as a focused test or code path, for that baseline. Also confirm whether the Home/Fit framing problem is in scope for #17245. If it is in scope, provide a fix or evidence that it is resolved. If it is not in scope, the issue scope should be narrowed or split.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between c71236a and 4e0fff4.

Files selected for processing: 2

  • crates/ui-egui/src/navigation.rs
  • crates/ui-egui/src/viewport.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

@cfaulkingham
cfaulkingham marked this pull request as ready for review November 15, 2026 11:57
@bherbruck bherbruck added ready to merge reviewed, every ask addressed; merge once CI is green approved-for-ci Reviewed: run the full PR CI (build, tests, goldens, Clippy) labels Nov 15, 2026
@bherbruck
bherbruck enabled auto-merge November 15, 2026 16:26
@bherbruck
bherbruck added this pull request to the merge queue Nov 15, 2026
Merged via the queue into tangency-labs:main with commit 275f20e Nov 15, 2026
110 of 114 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved-for-ci Reviewed: run the full PR CI (build, tests, goldens, Clippy) ready to merge reviewed, every ask addressed; merge once CI is green

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Camera framing (Home / Fit) remains off-center to the right in v0.42.0, and dragging the Axis Widget ignores "Orbit around: Origin"

2 participants