feat: Integrated new vneintraction code. Also, make vneinteraction by default on for the vnetestbed.#43
Open
ajeetsinghyadav wants to merge 5 commits intomainfrom
Open
Conversation
…improved compatibility
- Replaced references to behavior classes with manipulator classes for improved clarity and consistency in interaction management. - Updated navigation mode handling to utilize FreeLookMode instead of NavigateMode, enhancing the user experience. - Adjusted UI elements and documentation to reflect these changes, ensuring accurate representation of available controls and settings.
…action - Moved trackball projection settings UI elements to the correct conditional block based on the rotation mode, ensuring proper display and functionality. - Enhanced clarity in the interaction settings by maintaining consistent UI behavior for trackball projection options.
- Changed the default option for VNE_WITH_VNEINTRACTION to ON, indicating that the vneinteraction dependency is now expected for this repository. - Updated comments in VNEPrivateDeps.cmake to clarify the behavior of the vneinteraction dependency management, ensuring better understanding of its integration and testing options.
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR enables the vneinteraction submodule by default, bumps its submodule revision, and migrates sample interaction code from the behavior API to the manipulator API with renamed enums and updated signatures; CMake private-deps handling for vneinteraction cache variables was adjusted. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the GLFW/OpenGL interaction sample code to the newer vneinteraction API (mode/type renames and behavior→manipulator accessors) and makes vneinteraction enabled by default in the repo build.
Changes:
- Switch sample interaction code from
*Behavior()APIs /NavigateMode/OrbitRotationModeto*Manipulator()APIs /FreeLookMode/OrbitalRotationMode. - Default
VNE_WITH_VNEINTRACTIONtoONso thevneinteractionsubmodule is expected for typical builds. - Extend
VNEPrivateDeps.cmakecache var wiring to keepvneinteractiontests/examples/dev options disabled by default.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| samples/glfw_opengl/common/base_scene_layer.cpp | Update Inspect controller rotation mode enum name to match new interaction API. |
| samples/glfw_opengl/03_test_interaction/README.md | Update documentation to reference FreeLookMode (needs a small consistency fix). |
| samples/glfw_opengl/03_test_interaction/demo_test_interaction.h | Replace NavigateMode with FreeLookMode in the interaction demo interface/state. |
| samples/glfw_opengl/03_test_interaction/demo_test_interaction.cpp | Migrate demo implementation to manipulator APIs and updated interaction types. |
| CMakeLists.txt | Enable vneinteraction by default via VNE_WITH_VNEINTRACTION=ON. |
| cmake/VNEPrivateDeps.cmake | Ensure vneinteraction tests/examples/dev options stay disabled unless explicitly enabled. |
|
|
||
| 1. **Base scene**: Grid, axes, perspective or orthographic camera from `BaseSceneLayer`; settings can toggle grid/axes and switch projection. | ||
| 2. **Controllers**: Runtime selection among Inspect (Euler orbit or trackball), Navigation (FPS / Fly / Game via `NavigateMode`), Ortho 2D (`Ortho2DController`), and Follow; per-controller ImGui tuning (trackball projection when in trackball inspect mode, pivot mode, speeds, navigation multipliers, ortho/follow options). | ||
| 2. **Controllers**: Runtime selection among Inspect (Euler orbit or trackball), Navigation (FPS / Fly via `FreeLookMode`), Ortho 2D (`Ortho2DController`), and Follow; per-controller ImGui tuning (trackball projection when in trackball inspect mode, pivot mode, speeds, navigation multipliers, ortho/follow options). |
There was a problem hiding this comment.
The README still advertises Navigation3D as supporting “FPS/Fly/Game modes” in the intro (line 3), but the controller list and code now only reference FreeLookMode with FPS/Fly. Please update the intro text to match the current supported modes (or document how/where “Game” is selected if it still exists).
- Updated comments in VNEPrivateDeps.cmake to clarify the behavior of the SAVE_RESTORE mechanism for VNE_INTERACTION_* variables, addressing issues seen in CI with stale cache values. - Set VNETESTBED_VNEINTERACTION_EMBEDDED to ON to ensure proper integration of the vneinteraction tests while keeping its gtest suite out of vnetestbed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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