Repository navigation
feat: Integrating navigation 3d controller changes - #41
Conversation
- Adjusted the navigation mode index range to limit options to "Fps" and "Fly". - Updated UI text to reflect the removal of the "Game" mode from the navigation options, enhancing clarity in user interactions. - Updated the vneinteraction subproject reference to the latest commit, ensuring compatibility with recent changes.
- Added support for rotation settings in both Navigation and Inspect controllers, allowing users to enable/disable rotation and select rotation modes (Orbit or Trackball). - Updated UI elements to reflect these new settings, improving user interaction and control over camera behaviors. - Enhanced the display of interaction instructions based on the selected controller type, providing clearer guidance for users.
- Removed the "Inspect (Trackball)" option from the controller types, replacing it with "Inspect 3D" for clarity. - Adjusted the index handling in the controller selection to accommodate the updated options. - Enhanced UI text to provide clearer instructions for the "Inspect 3D" mode, improving user guidance.
- Updated the vneinteraction subproject to the latest commit for improved compatibility. - Removed unused rotation settings from the Navigation controller in the InteractionSettingsLayer, streamlining the UI. - Enhanced user guidance by updating text descriptions related to navigation controls and zoom methods.
- Removed unused rotation settings from the Navigation controller, simplifying the UI. - Updated text descriptions for navigation controls and zoom methods to enhance user guidance. - Adjusted the handling of camera position and target display for improved clarity in the debug overlay.
- Changed the navigation controller type from "Navigation" to "Navigation 3D" for improved clarity and consistency in the UI. - This update enhances user understanding of the available controller options in the interaction settings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughSubmodule revision bumped for Changes
Sequence DiagramsequenceDiagram
participant User as User (ImGui)
participant Event as EventListener
participant ImGuiLayer as ImGuiLayer
participant UI as InteractionSettingsLayer
participant Ctrl as Inspect/Navigation Controller
participant Cam as Camera
User->>Event: mouse move (glfw client coords)
Event->>ImGuiLayer: clientMouseToImGuiScreen(mx,my)
ImGuiLayer->>ImGuiLayer: convert using window position if viewports enabled
ImGuiLayer->>ImGui: AddMousePosEvent(converted)
User->>UI: open settings / change controller or rotation mode
UI->>UI: detect controller-kind change, sync UI state
UI->>Ctrl: setRotationMode(Orbit/Trackball) / update zoom exponent
Ctrl->>Cam: apply rotation / zoom
UI->>Cam: query position/target
UI->>UI: compute forward/right (with epsilon checks) and render debug overlay
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Updates the interaction sample UI to present “Navigation 3D” and consolidates Inspect controller presentation while adding additional controller-tuning/debug UI.
Changes:
- Renamed the controller type label from “Navigation” to “Navigation 3D” and collapsed Inspect entries into a single “Inspect 3D” option.
- Added an Inspect rotation mode selector (Orbit vs Trackball) and persisted UI state for it.
- Added Navigation scroll-zoom tuning UI and a camera debug overlay.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| samples/glfw_opengl/03_test_interaction/demo_test_interaction.h | Updates controller comment and adds UI state for Inspect rotation mode selection. |
| samples/glfw_opengl/03_test_interaction/demo_test_interaction.cpp | Updates controller selection UI/labels, adds Inspect rotation mode combo, adds Navigation scroll-zoom tuning + camera debug overlay. |
| static ControllerKind prev_ctrl_kind = ControllerKind::eInspectOrbit; | ||
| static bool first_manip_frame = true; | ||
| if (first_manip_frame || cur != prev_ctrl_kind) { | ||
| if (auto* insp = il.getInspectController()) { | ||
| ui.rotation_enabled_insp = insp->isRotationEnabled(); | ||
| ui.rotation_mode_insp_idx = | ||
| (insp->getRotationMode() == vne::interaction::OrbitRotationMode::eOrbit) ? 0 : 1; | ||
| } | ||
| prev_ctrl_kind = cur; | ||
| first_manip_frame = false; |
There was a problem hiding this comment.
prev_ctrl_kind / first_manip_frame are function-static, so their state is shared across all InteractionSettingsLayer instances and persists across detach/reattach. This can leave the UI out of sync after recreating the layer (it will skip the initial sync) or when multiple windows/layers exist. Prefer storing this state on the InteractionSettingsLayer instance (members) or derive the sync from current controller state each frame/window (e.g., via ImGui::IsWindowAppearing() or by caching in uiSettings()).
| static ControllerKind prev_ctrl_kind = ControllerKind::eInspectOrbit; | |
| static bool first_manip_frame = true; | |
| if (first_manip_frame || cur != prev_ctrl_kind) { | |
| if (auto* insp = il.getInspectController()) { | |
| ui.rotation_enabled_insp = insp->isRotationEnabled(); | |
| ui.rotation_mode_insp_idx = | |
| (insp->getRotationMode() == vne::interaction::OrbitRotationMode::eOrbit) ? 0 : 1; | |
| } | |
| prev_ctrl_kind = cur; | |
| first_manip_frame = false; | |
| const bool should_sync_from_controller = ImGui::IsWindowAppearing(); | |
| if (should_sync_from_controller) { | |
| if (auto* insp = il.getInspectController()) { | |
| ui.rotation_enabled_insp = insp->isRotationEnabled(); | |
| ui.rotation_mode_insp_idx = | |
| (insp->getRotationMode() == vne::interaction::OrbitRotationMode::eOrbit) ? 0 : 1; | |
| } |
| const auto fwd = (tgt - pos).normalized(); | ||
| const auto right_v = fwd.cross(vne::math::Vec3f(0.f, 1.f, 0.f)).normalized(); |
There was a problem hiding this comment.
The debug overlay normalizes (tgt - pos) and fwd.cross(worldUp) without checking for degenerate vectors. If pos == tgt or fwd is parallel/near-parallel to up, the normalization can produce NaNs (or potentially trigger asserts/div-by-zero depending on Vec3f::normalized() implementation). Consider guarding with epsilon checks before normalizing (similar to SceneTestLayer::drawCameraVisuals in samples/glfw_opengl/02_test_scene/demo_test_scene.cpp:600-608) and skipping/printing a fallback when the basis can’t be constructed.
| const auto fwd = (tgt - pos).normalized(); | |
| const auto right_v = fwd.cross(vne::math::Vec3f(0.f, 1.f, 0.f)).normalized(); | |
| const auto fwd = (tgt - pos); | |
| const auto right_v = fwd.cross(vne::math::Vec3f(0.f, 1.f, 0.f)); |
| switch (cur) { | ||
| case ControllerKind::eInspectOrbit: | ||
| ImGui::TextDisabled("LMB rotate RMB pan Scroll zoom"); | ||
| break; | ||
| case ControllerKind::eInspectTrackball: | ||
| ImGui::TextDisabled("LMB rotate RMB pan Scroll zoom (trackball)"); | ||
| ImGui::TextDisabled("Inspect 3D: Orbit / Trackball selectable below"); | ||
| break; |
There was a problem hiding this comment.
The controller help text for Inspect differs based on ControllerKind even though the UI now exposes a single "Inspect 3D" type. When selecting "Inspect 3D" the kind will typically be eInspectOrbit, so the hint doesn’t mention that Trackball can be selected below, which is now inaccurate/misleading. Consider making the Inspect hint independent of ControllerKind (or keying it off insp->getRotationMode()), so both orbit/trackball paths show consistent guidance.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
samples/glfw_opengl/03_test_interaction/demo_test_interaction.cpp (2)
760-780: Consider deriving combo size from array length to avoid magic numbers.The value
4appears in multiple places (loop bound, combo call) and must stay in sync with thetypes[]andvalues[]arrays.♻️ Suggested fix using std::size
const char* types[] = {"Inspect 3D", "Navigation 3D", "Ortho 2D", "Follow"}; const ControllerKind values[] = {ControllerKind::eInspectOrbit, ControllerKind::eNavigation, ControllerKind::eOrtho2D, ControllerKind::eFollow}; + constexpr int kNumControllerTypes = static_cast<int>(std::size(types)); int idx = 0; - for (int i = 0; i < 4; ++i) { + for (int i = 0; i < kNumControllerTypes; ++i) { if (values[i] == cur) { idx = i; break; } } if (cur == ControllerKind::eInspectTrackball) { idx = 0; // Legacy state maps to Inspect 3D entry. } // ... - if (ImGui::Combo("Type##ctrl", &idx, types, 4)) { + if (ImGui::Combo("Type##ctrl", &idx, types, kNumControllerTypes)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/glfw_opengl/03_test_interaction/demo_test_interaction.cpp` around lines 760 - 780, The hardcoded "4" should be replaced with a derived array size to keep types[] and values[] in sync: compute a single constexpr or use std::size(types) (or std::size(values)) and use that symbol for the loop bound, the index initialization, and the ImGui::Combo call; update the for-loop that finds the matching ControllerKind (and any other places using the literal) to iterate to that derived count, and ensure the Combo call uses the same count and the types/values arrays referenced remain the same.
747-757: Function-local statics for ImGui state sync are acceptable but create implicit shared state.This is a common ImGui pattern for tracking state changes between frames. However, if multiple
InteractionSettingsLayerinstances were ever created, they would shareprev_ctrl_kindandfirst_manip_frame. Consider making these instance members ofInteractionSettingsLayerif multi-instance support is ever needed.The synchronization logic correctly initializes
rotation_enabled_inspandrotation_mode_insp_idxfrom the active inspect controller.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/glfw_opengl/03_test_interaction/demo_test_interaction.cpp` around lines 747 - 757, The function-local statics prev_ctrl_kind and first_manip_frame create implicit shared state across instances; make them members of InteractionSettingsLayer (e.g., add ControllerKind prev_ctrl_kind and bool first_manip_frame to the class) and update the code that currently references the statics to use these instance members; keep the existing logic that queries il.getInspectController() and sets ui.rotation_enabled_insp and ui.rotation_mode_insp_idx (using insp->isRotationEnabled() and insp->getRotationMode()) but operate on the per-instance prev_ctrl_kind and first_manip_frame to avoid cross-instance interference.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deps/internal/vneinteraction`:
- Line 1: The vneinteraction submodule reference is pointing to a non-existent
commit (c8830bb57bcd01158e37dbe3f968a262d65fd6ea) causing initialization to
fail; fix it by either pushing that commit to the remote
(https://github.com/vertexnova/vneinteraction.git) or updating the submodule
pointer in deps/internal/vneinteraction to a valid commit/branch/tag present in
the remote (e.g., replace the bad SHA with a reachable commit or switch the
submodule to track a branch), then update the superproject's gitlink (git add
deps/internal/vneinteraction && git commit) and verify with git submodule update
--init --recursive.
In `@samples/glfw_opengl/03_test_interaction/demo_test_interaction.cpp`:
- Around line 921-944: The debug overlay can produce NaN when computing right_v
from fwd.cross(vne::math::Vec3f(0.f,1.f,0)).normalized() if fwd is parallel to
the Y axis; to fix, compute a raw right vector (e.g., right_raw = fwd.cross(up))
and test its length (or lengthSquared) against a small epsilon before calling
normalized(); if it is near zero, choose an alternative up vector (e.g.,
Vec3f(1,0,0)) or compute right_raw = fwd.cross(alternateUp) and then normalize
into right_v, updating the existing uses of fwd and right_v (from
il.cameraPosition()/il.cameraTarget()) so the overlay never normalizes a
zero-length vector.
---
Nitpick comments:
In `@samples/glfw_opengl/03_test_interaction/demo_test_interaction.cpp`:
- Around line 760-780: The hardcoded "4" should be replaced with a derived array
size to keep types[] and values[] in sync: compute a single constexpr or use
std::size(types) (or std::size(values)) and use that symbol for the loop bound,
the index initialization, and the ImGui::Combo call; update the for-loop that
finds the matching ControllerKind (and any other places using the literal) to
iterate to that derived count, and ensure the Combo call uses the same count and
the types/values arrays referenced remain the same.
- Around line 747-757: The function-local statics prev_ctrl_kind and
first_manip_frame create implicit shared state across instances; make them
members of InteractionSettingsLayer (e.g., add ControllerKind prev_ctrl_kind and
bool first_manip_frame to the class) and update the code that currently
references the statics to use these instance members; keep the existing logic
that queries il.getInspectController() and sets ui.rotation_enabled_insp and
ui.rotation_mode_insp_idx (using insp->isRotationEnabled() and
insp->getRotationMode()) but operate on the per-instance prev_ctrl_kind and
first_manip_frame to avoid cross-instance interference.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bc164973-fb57-4c6a-bb2c-cda609f0950c
📒 Files selected for processing (3)
deps/internal/vneinteractionsamples/glfw_opengl/03_test_interaction/demo_test_interaction.cppsamples/glfw_opengl/03_test_interaction/demo_test_interaction.h
| @@ -1 +1 @@ | |||
| Subproject commit fe614a0aff5d7e3365eda556e83d49e0fda2ba61 | |||
| Subproject commit c8830bb57bcd01158e37dbe3f968a262d65fd6ea | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Submodule status =="
git submodule status deps/internal/vneinteraction || true
echo
echo "== Show previous and new submodule SHAs from parent diff =="
git diff -- deps/internal/vneinteraction
echo
echo "== Inspect submodule commit metadata =="
git -C deps/internal/vneinteraction show --no-patch --pretty=fuller c8830bb57bcd01158e37dbe3f968a262d65fd6ea
echo
echo "== Files changed in bumped submodule commit =="
git -C deps/internal/vneinteraction diff-tree --no-commit-id --name-status -r c8830bb57bcd01158e37dbe3f968a262d65fd6ea
echo
echo "== Search for expected interaction-label changes in submodule =="
rg -n --iglob '*.{ts,tsx,js,jsx,json,md}' 'Navigation 3D|Inspect 3D|Trackball|Orbit|Fly|Fps' deps/internal/vneinteraction || trueRepository: vertexnova/vnetestbed
Length of output: 451
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Current git status =="
git status deps/internal/vneinteraction || true
echo
echo "== Check git log for recent changes to submodule pointer =="
git log --oneline -n 20 -- deps/internal/vneinteraction || true
echo
echo "== Show git diff HEAD for submodule =="
git diff HEAD -- deps/internal/vneinteraction || true
echo
echo "== Check if deps/internal/vneinteraction is initialized =="
test -d deps/internal/vneinteraction/.git && echo "Submodule is initialized" || echo "Submodule is NOT initialized"
echo
echo "== List all git refs pointing near the mentioned commit =="
git for-each-ref --contains c8830bb57bcd01158e37dbe3f968a262d65fd6ea 2>/dev/null || echo "Commit not found in any refs"
echo
echo "== Check remote URL for submodule =="
git config --file=.gitmodules --get-regexp "deps/internal/vneinteraction" || trueRepository: vertexnova/vnetestbed
Length of output: 713
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Current submodule pointer in HEAD =="
git cat-file -p HEAD:deps/internal/vneinteraction 2>/dev/null || echo "File not found in HEAD"
echo
echo "== Show .gitmodules content =="
cat .gitmodules | grep -A 3 "vneinteraction" || true
echo
echo "== Check if this is staged as a submodule change =="
git ls-files --stage -- deps/internal/vneinteraction || true
echo
echo "== Check recent commits that mention the submodule =="
git log --oneline --all -- deps/internal/vneinteraction | head -5 || true
echo
echo "== Inspect commit 3feaddf in detail =="
git show --stat 3feaddf || trueRepository: vertexnova/vnetestbed
Length of output: 12566
This submodule bump points to an unreachable commit and will fail to initialize.
The staged change references commit c8830bb57bcd01158e37dbe3f968a262d65fd6ea in the vneinteraction submodule, but this commit does not exist in the remote repository (https://github.com/vertexnova/vneinteraction.git). The submodule cannot be initialized and any clone or build after merge will fail with "fatal: remote error: upload-pack: not our ref".
Ensure the target commit is pushed to the submodule remote before merging, or verify the correct commit hash is referenced.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deps/internal/vneinteraction` at line 1, The vneinteraction submodule
reference is pointing to a non-existent commit
(c8830bb57bcd01158e37dbe3f968a262d65fd6ea) causing initialization to fail; fix
it by either pushing that commit to the remote
(https://github.com/vertexnova/vneinteraction.git) or updating the submodule
pointer in deps/internal/vneinteraction to a valid commit/branch/tag present in
the remote (e.g., replace the bad SHA with a reachable commit or switch the
submodule to track a branch), then update the superproject's gitlink (git add
deps/internal/vneinteraction && git commit) and verify with git submodule update
--init --recursive.
… in demo_test_interaction - Added a new optional member to track the last used controller kind for improved synchronization in the UI. - Updated the rendering logic to handle controller changes more effectively, enhancing user experience. - Improved forward and right vector calculations in the camera debug display, including handling for degenerate cases. - Enhanced UI feedback for camera position and movement controls, providing clearer guidance for users.
…test_interaction - Simplified the logic for detecting controller changes in the InteractionSettingsLayer, enhancing synchronization with the UI. - Streamlined the rendering of feedback messages for degenerate forward vectors, improving clarity in user guidance. - Updated text formatting for better readability in the UI, contributing to an overall enhanced user experience.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
samples/glfw_opengl/03_test_interaction/demo_test_interaction.cpp (1)
763-777: Unify inspect mode state instead of splitting it acrossControllerKindandsetRotationMode().The combo now presents a single "Inspect 3D" controller, but this later combo can still flip the live controller between orbit and trackball without changing
il.getControllerKind(). That makes anycur-based UI branching stale relative to the actual inspect mode. I’d either collapse to one inspect kind or derive all inspect-mode-specific UI frominsp->getRotationMode().Also applies to: 859-864
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/glfw_opengl/03_test_interaction/demo_test_interaction.cpp` around lines 763 - 777, The UI currently treats Inspect as a single ControllerKind but elsewhere toggles between orbit/trackball via setRotationMode(), causing cur (from il.getControllerKind()) to be out of sync; update the combo logic so it no longer branches on ControllerKind alone: either collapse the two inspect kinds into one (remove any separate eInspectOrbit/eInspectTrackball usage in the types/values arrays and always map Inspect 3D to a single ControllerKind), or change the branching that computes idx to query the live inspect controller's rotation mode (use insp->getRotationMode() together with ControllerKind::eInspect... to decide which combo entry to select and to flip rotation mode when the combo changes). Adjust references to values[], types[], cur, and any code that calls setRotationMode() so the UI selection always reflects the actual inspect rotation mode.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@samples/glfw_opengl/03_test_interaction/demo_test_interaction.cpp`:
- Around line 750-760: The resync block for inspect controller state only
refreshes rotation flags, leaving ui.pan_enabled_insp and ui.zoom_enabled_insp
stale after setControllerKind() rebuilds the controller; update the same block
(the condition using last_manip_synced_controller_kind_ and cur and the call to
il.getInspectController()) to also read and assign insp->isPanEnabled() and
insp->isZoomEnabled() into ui.pan_enabled_insp and ui.zoom_enabled_insp so the
checkboxes reflect the rebuilt inspect controller state (also apply the same
change to the similar block around lines 865-869).
---
Nitpick comments:
In `@samples/glfw_opengl/03_test_interaction/demo_test_interaction.cpp`:
- Around line 763-777: The UI currently treats Inspect as a single
ControllerKind but elsewhere toggles between orbit/trackball via
setRotationMode(), causing cur (from il.getControllerKind()) to be out of sync;
update the combo logic so it no longer branches on ControllerKind alone: either
collapse the two inspect kinds into one (remove any separate
eInspectOrbit/eInspectTrackball usage in the types/values arrays and always map
Inspect 3D to a single ControllerKind), or change the branching that computes
idx to query the live inspect controller's rotation mode (use
insp->getRotationMode() together with ControllerKind::eInspect... to decide
which combo entry to select and to flip rotation mode when the combo changes).
Adjust references to values[], types[], cur, and any code that calls
setRotationMode() so the UI selection always reflects the actual inspect
rotation mode.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b9962d60-f50b-4eef-8e83-1170809f5dd6
📒 Files selected for processing (2)
samples/glfw_opengl/03_test_interaction/demo_test_interaction.cppsamples/glfw_opengl/03_test_interaction/demo_test_interaction.h
🚧 Files skipped from review as they are similar to previous changes (1)
- samples/glfw_opengl/03_test_interaction/demo_test_interaction.h
| const bool window_appearing = ImGui::IsWindowAppearing(); | ||
| const bool controller_changed = | ||
| !last_manip_synced_controller_kind_.has_value() || *last_manip_synced_controller_kind_ != cur; | ||
| if (window_appearing || controller_changed) { | ||
| if (auto* insp = il.getInspectController()) { | ||
| ui.rotation_enabled_insp = insp->isRotationEnabled(); | ||
| ui.rotation_mode_insp_idx = | ||
| (insp->getRotationMode() == vne::interaction::OrbitRotationMode::eOrbit) ? 0 : 1; | ||
| } | ||
| last_manip_synced_controller_kind_ = cur; | ||
| } |
There was a problem hiding this comment.
Resync the inspect pan/zoom toggles too.
This sync block only refreshes rotation_enabled_insp and rotation_mode_insp_idx. After setControllerKind() rebuilds an inspect controller, ui.pan_enabled_insp / ui.zoom_enabled_insp can keep stale values, so those checkboxes may reopen in the wrong state and push an unexpected flag back on the next click.
Also applies to: 865-869
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@samples/glfw_opengl/03_test_interaction/demo_test_interaction.cpp` around
lines 750 - 760, The resync block for inspect controller state only refreshes
rotation flags, leaving ui.pan_enabled_insp and ui.zoom_enabled_insp stale after
setControllerKind() rebuilds the controller; update the same block (the
condition using last_manip_synced_controller_kind_ and cur and the call to
il.getInspectController()) to also read and assign insp->isPanEnabled() and
insp->isZoomEnabled() into ui.pan_enabled_insp and ui.zoom_enabled_insp so the
checkboxes reflect the rebuilt inspect controller state (also apply the same
change to the similar block around lines 865-869).
…te handling - Introduced clientMouseToImGuiScreen method to map GLFW client-area mouse coordinates to ImGui screen space, enhancing multi-viewport support. - Updated getHoveredViewportIndex to utilize the new method for accurate viewport detection based on transformed mouse coordinates. - Modified ImGuiEventListener to call clientMouseToImGuiScreen when processing mouse movement events, ensuring consistent coordinate handling.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| const char* types[] = {"Inspect 3D", "Navigation 3D", "Ortho 2D", "Follow"}; | ||
| const ControllerKind values[] = {ControllerKind::eInspectOrbit, | ||
| ControllerKind::eInspectTrackball, | ||
| ControllerKind::eNavigation, | ||
| ControllerKind::eOrtho2D, | ||
| ControllerKind::eFollow}; |
There was a problem hiding this comment.
The controller type combo no longer exposes an Inspect Trackball option, and the only Inspect entry maps to ControllerKind::eInspectOrbit (values[0]). This makes switching away from Inspect and back always rebuild an Orbit controller and drops any previously selected Trackball rotation mode.
To preserve behavior, consider deriving the Inspect controller kind from ui.rotation_mode_insp_idx (or apply the stored rotation mode immediately after setControllerKind when switching back to Inspect).
| // Debug overlay: live camera state so movement direction can be verified. | ||
| if (ImGui::TreeNode("Camera debug##nav")) { | ||
| const auto pos = il.cameraPosition(); |
There was a problem hiding this comment.
The PR description only mentions renaming "Navigation" to "Navigation 3D", but this hunk also adds new behavior/UI (e.g., the navigation camera debug overlay and other controller settings changes). Please update the PR description/release notes (or split into separate PRs) so the scope matches what’s being merged.
Description
Release notes: Use a Conventional Commits–style PR title (e.g.
feat: add X,fix: resolve Y,docs: update Z) so release-please can include this change in the changelog. If you squash-merge, use the PR title as the commit message.Checklist
cmake -B buildandcmake --build build, or platform script).ctest --test-dir buildor script-a test).clang-formatas configured for this repo); CI clang-format will check.Additional notes
Summary by CodeRabbit
New Features
Improvements
Chores