UV: Slice E — Projection unwrap modes (#463) - #776
Conversation
Expose view/box/cylinder/sphere/reset projections in the UV editor with undo, viewport camera integration, and unit tests for the core projector. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
More reviews will be available in 18 minutes and 33 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds UV geometric projection unwrap modes (Box, Cylinder, Sphere, View, ResetBox) via a new UV Projection Unwrap Modes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 413184e50a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| verts[static_cast<size_t>(vi)].uv = change.newUv; | ||
| verts[static_cast<size_t>(vi)].hasUV = true; |
There was a problem hiding this comment.
When projecting only a selected sub-entity/material on meshes that use Ogre shared vertex data, this writes the new UV only into that submesh's EditableMesh copy. EditableMesh::commitUvsToEntity() later merges every submesh using the shared buffer, so an unprojected later submesh can overwrite these UVs; existing UV edits avoid this by routing writes through applyWorkingMeshUv(), which fans the value out to all shared users. Please propagate each projected change to the other shared submeshes before committing, otherwise Projection buttons can appear to do nothing or commit stale UVs for shared-vertex meshes.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
src/UvProject_test.cpp (1)
90-150: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a
Sphereprojection test.This file covers Box/Cylinder/View/Reset only, so the new Sphere path can regress without CI noticing. A per-axis assertion would also catch the current
axisno-op inprojectSphere(). As per coding guidelines,src/**/*_test.cpp: Add Google Test unit tests for new functionality.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/UvProject_test.cpp` around lines 90 - 150, Add a Google Test case in UvProjectTest to cover the Sphere projection path in UvProject::project, since the current suite only exercises Box, Cylinder, View, and ResetBox. Use UvProject::Options with mode set to UvProject::Mode::Sphere and verify the report is applied and the resulting UVs are in range. Also include a per-axis assertion on the projected UV extents so the test fails if projectSphere() ignores opts.axis.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@qml/PropertiesPanel.qml`:
- Around line 2673-2678: The UV projection buttons are still hardcoding default
axis/scale values in the PropertiesPanel and matching detached editor UI, so
users cannot choose projection settings. Update the action handlers in
PropertiesPanel and the mirrored controls in UVEditorPanel to pass user-selected
axis and scale into UVEditorController.projectUvBox, projectUvCylinder, and
projectUvSphere instead of fixed constants. Wire the UI to expose X/Y/Z axis
selection and a configurable projection scale, and keep the button
labels/handlers aligned with those new parameters.
In `@src/mainwindow.cpp`:
- Around line 4409-4413: The fallback in mainwindow’s viewport selection only
checks mDockWidgetList.first(), which can miss a valid 3D viewport if the first
EditorViewport is stale or widget-less. Update the spaceCam lookup logic to
iterate through mDockWidgetList and pick the first usable EditorViewport with a
non-null Ogre widget and SpaceCamera, reusing the same null-safety checks
already used in the focus search path.
In `@src/mainwindow.h`:
- Around line 55-58: Make the Ogre::Matrix4 dependency explicit in mainwindow.h:
ViewportCameraSnapshot stores Ogre::Matrix4 by value, so add the direct
OgreMatrix4.h include in this header rather than relying on OgreFrameListener.h
transitively providing the type. Keep the struct definition unchanged and ensure
the header can compile independently with the matrix definition available.
In `@src/UVEditorController.cpp`:
- Around line 2298-2305: The success path in UVEditorController::apply
projection flow is updating m_statusText after the change notification already
goes out, leaving the QML status line stale; set the success status before
calling applyProjectionChanges(), or immediately re-emit the status update after
assigning m_statusText so the new message is published. Keep the fix within the
branch that uses modeToString, undoDescription, and the scoped selection check.
In `@src/UvProject_test.cpp`:
- Line 46: Replace the non-portable use of M_PI in the angle calculation with a
portable π constant in the sphere-related test setup, using the existing test
helper or a local constexpr so the build works on MSVC. Also expand
UvProject_test to add Sphere-mode coverage by invoking the Sphere path in the
UvProject tests, so projectSphere() is exercised and the axis-handling behavior
is validated.
In `@src/UvProject.cpp`:
- Around line 333-344: The UvProject::projectView path is being renormalized
again after it already returns viewport UVs, which changes the camera framing.
Update the post-processing in UvProject.cpp so normalizeUvMap(projected) is
skipped for Mode::View and only applied for the other projection modes
(Cylinder, Sphere, ResetBox), keeping projectView() output unchanged.
- Around line 161-173: The Sphere projection helper currently ignores the
selected axis, so all UVs are computed the same regardless of X/Y/Z. Update
projectSphere in UvProject.cpp to use the axis parameter when deriving the
spherical coordinates, and make sure the UV mapping differs per axis while
preserving the existing bounds-center based projection behavior.
- Around line 213-218: The box and cylinder UV mapping paths in UvProject
currently multiply by scale and then call normalizeUvMap on the full output,
which makes boxScale ineffective. Update the UV generation logic so scale
actually affects the final coordinates in the box/cylinder projection code (and
the related projectPositionOnBoxPlane / cylinder path), or remove the final
renormalization step if it cancels the scaling; keep the output in the intended
range without undoing boxScale.
In `@src/UvProject.h`:
- Around line 15-21: The public UvProject::Mode enum is missing the
smart/angle-threshold unwrap option, so add the required Smart UV projection
mode alongside the existing View, Box, Cylinder, Sphere, and ResetBox values.
Update any related UvProject API handling to recognize and route this new mode
wherever Mode is used so the workflow can be exposed and tested end to end.
---
Nitpick comments:
In `@src/UvProject_test.cpp`:
- Around line 90-150: Add a Google Test case in UvProjectTest to cover the
Sphere projection path in UvProject::project, since the current suite only
exercises Box, Cylinder, View, and ResetBox. Use UvProject::Options with mode
set to UvProject::Mode::Sphere and verify the report is applied and the
resulting UVs are in range. Also include a per-axis assertion on the projected
UV extents so the test fails if projectSphere() ignores opts.axis.
🪄 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: 84178be8-430a-4f87-9af4-74c4dc41ddce
📒 Files selected for processing (11)
qml/PropertiesPanel.qmlqml/UVEditorPanel.qmlsrc/CMakeLists.txtsrc/UVEditorController.cppsrc/UVEditorController.hsrc/UvProject.cppsrc/UvProject.hsrc/UvProject_test.cppsrc/mainwindow.cppsrc/mainwindow.htests/CMakeLists.txt
| model: [ | ||
| { label: "View", fn: function() { UVEditorController.projectUvFromView() } }, | ||
| { label: "Box", fn: function() { UVEditorController.projectUvBox(1.0) } }, | ||
| { label: "Cyl", fn: function() { UVEditorController.projectUvCylinder(1, 1.0) } }, | ||
| { label: "Sph", fn: function() { UVEditorController.projectUvSphere(1) } }, | ||
| { label: "Reset", fn: function() { UVEditorController.resetUvBox() } } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Expose projection axis/scale instead of hardcoding the defaults.
These buttons only surface projectUvBox(1.0), projectUvCylinder(1, 1.0), and projectUvSphere(1) here, and the detached editor mirrors the same constants in qml/UVEditorPanel.qml. That means users still cannot choose the requested X/Y/Z axis or configurable projection scale from the UI.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@qml/PropertiesPanel.qml` around lines 2673 - 2678, The UV projection buttons
are still hardcoding default axis/scale values in the PropertiesPanel and
matching detached editor UI, so users cannot choose projection settings. Update
the action handlers in PropertiesPanel and the mirrored controls in
UVEditorPanel to pass user-selected axis and scale into
UVEditorController.projectUvBox, projectUvCylinder, and projectUvSphere instead
of fixed constants. Wire the UI to expose X/Y/Z axis selection and a
configurable projection scale, and keep the button labels/handlers aligned with
those new parameters.
| enum class Mode { | ||
| View, | ||
| Box, | ||
| Cylinder, | ||
| Sphere, | ||
| ResetBox | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Missing the required Smart UV projection mode.
The public API still omits the smart/angle-threshold unwrap mode from the linked issue, so this PR cannot expose or test that required workflow yet.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/UvProject.h` around lines 15 - 21, The public UvProject::Mode enum is
missing the smart/angle-threshold unwrap option, so add the required Smart UV
projection mode alongside the existing View, Box, Cylinder, Sphere, and ResetBox
values. Update any related UvProject API handling to recognize and route this
new mode wherever Mode is used so the workflow can be exposed and tested end to
end.
Propagate projected UVs across shared vertex buffers, preserve view camera framing, honor sphere axis and box/cylinder scale, scan all viewports for camera fallback, and add sphere coverage tests. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Addressed review feedback in 9141a3e:
Intentionally not implemented: Smart UV was removed during development (it produced unusable layouts on test assets). Axis/scale UI spinboxes remain deferred — the API accepts params; QML uses defaults for this slice. |
|
1 participant
Summary
UvProjectwith view, box, cylinder, sphere, and reset-to-0–1 box projection modes scoped to the active UV selection (full sub-mesh when nothing is selected).UVEditCommandfor undo and recordsmesh.uv.projectSentry breadcrumbs.Test plan
./build_local/bin/UnitTests --gtest_filter="UvProject*"(box, cylinder, view guard, reset)(selection)Closes #463
Made with Cursor
Summary by CodeRabbit
New Features
Tests