Soft selection heat map and logarithmic radius slider - #283
Conversation
- Vertex overlay now shows weight-based heat map colors when soft selection is active: red (1.0) → orange → yellow → green → cyan → blue (0.0) - Extract weightToColor() static helper for testability - Radius slider uses logarithmic mapping for fine precision at small values (0.01) and coarser control at large values (20.0) - Bump version to 2.25.1 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
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)
📝 WalkthroughWalkthroughVersion bumped to 2.25.1; the soft-selection radius slider was changed to a normalized logarithmic control and synchronized with Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PropertiesPanel as QML\\ PropertiesPanel
participant EditMode as EditModeController
participant Renderer as VertexOverlay
User->>PropertiesPanel: adjust normalized slider (0.0–1.0)
PropertiesPanel->>EditMode: set softSelectionRadius (log-mapped from slider)
EditMode->>EditMode: compute/update per-vertex soft-selection weights
EditMode->>Renderer: provide per-vertex (index, weight) pairs
Renderer->>Renderer: map weight -> color via weightToColor()
Renderer->>User: render colored vertices in overlay
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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)
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: 035a8d2805
ℹ️ 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".
| } | ||
| Connections { | ||
| target: EditModeController | ||
| function onSoftSelectionRadiusChanged() { |
There was a problem hiding this comment.
Connections is wired to onSoftSelectionRadiusChanged, but EditModeController does not emit a softSelectionRadiusChanged signal (its softSelectionRadius property uses NOTIFY softSelectionChanged in src/EditModeController.h). As a result this handler never runs, so if the radius is changed from anywhere other than this slider (e.g. restored settings or programmatic calls), the slider/value label will not update and the UI drifts out of sync with controller state.
Useful? React with 👍 / 👎.
| return Ogre::ColourValue(1.0f, 0.6f + 0.4f * (1.0f - t), 0.0f, 1.0f); | ||
| } else if (weight > 0.4f) { | ||
| float t = (weight - 0.4f) / 0.2f; | ||
| return Ogre::ColourValue(1.0f - t, 1.0f, 0.0f, 1.0f); |
There was a problem hiding this comment.
The 0.4–0.6 branch computes red as 1.0f - t, so increasing weight in that interval decreases red (yellow→green) instead of increasing it (green→yellow) as described by the function comment. This also creates a visible color discontinuity at the 0.6 boundary when the next branch jumps red back up, producing incorrect heat-map gradients for common soft-selection weights.
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: 2
🧹 Nitpick comments (1)
src/EditModeController_test.cpp (1)
130-169: Add seam-point assertions for the piecewise gradient.Nice coverage overall. Please add explicit checks at
0.2/0.4/0.6/0.8to lock continuity and prevent transition regressions.As per coding guidelines: "Add Google Test unit tests for new functionality. Test files live alongside source in src/ with _test.cpp suffix (e.g., Manager_test.cpp)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/EditModeController_test.cpp` around lines 130 - 169, Add explicit unit assertions for the seam points of the piecewise gradient by adding tests that call EditModeController::weightToColor at weights 0.2, 0.4, 0.6, and 0.8 and assert the expected R/G/B/A continuity (e.g., monotonic relationship across adjacent seam points and alpha == 1.0). Create new TEST cases (or extend WeightToColorGradientMonotonic) that compute colors at these seam weights and compare them with neighboring values using EXPECT_LE/EXPECT_GE for the channels that should be monotonic and EXPECT_FLOAT_EQ(c.a, 1.0f) for alpha to lock the transitions and prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@qml/PropertiesPanel.qml`:
- Around line 363-369: The QML Connections block listens to EditModeController
but uses the wrong handler name (onSoftSelectionRadiusChanged) so updates from
the C++ NOTIFY signal are missed; change the handler to onSoftSelectionChanged
and keep the body that sets softRadiusSlider.updating/value/updating to
true/false using EditModeController.softSelectionRadius, and audit other
handlers (e.g., onSoftSelectionFalloffChanged, onSoftSelectionEnabledChanged)
for similar mismatches against the C++ NOTIFY names.
In `@src/EditModeController.cpp`:
- Around line 502-516: weightToColor has discontinuities at segment boundaries
and the 0.4–0.6 segment interpolates r in the wrong direction; fix by clamping
weight to [0,1], compute a normalized t = (weight - segmentLow) / (segmentHigh -
segmentLow) for each branch, and interpolate from the correct startColor to
endColor so that each boundary value yields identical colours (no jumps) and the
0.4–0.6 branch increases r as weight increases; update the branches in
weightToColor to use these normalized t values and explicit start/end colours
for each segment to ensure continuous, correctly directed interpolation.
---
Nitpick comments:
In `@src/EditModeController_test.cpp`:
- Around line 130-169: Add explicit unit assertions for the seam points of the
piecewise gradient by adding tests that call EditModeController::weightToColor
at weights 0.2, 0.4, 0.6, and 0.8 and assert the expected R/G/B/A continuity
(e.g., monotonic relationship across adjacent seam points and alpha == 1.0).
Create new TEST cases (or extend WeightToColorGradientMonotonic) that compute
colors at these seam weights and compare them with neighboring values using
EXPECT_LE/EXPECT_GE for the channels that should be monotonic and
EXPECT_FLOAT_EQ(c.a, 1.0f) for alpha to lock the transitions and prevent
regressions.
🪄 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: 0ff7aaf9-c052-4989-a583-44dec38c9e9a
📒 Files selected for processing (5)
CMakeLists.txtqml/PropertiesPanel.qmlsrc/EditModeController.cppsrc/EditModeController.hsrc/EditModeController_test.cpp
- Fix weightToColor() segment direction for 0.4-0.6 range and add std::clamp; rewrite as ascending low-to-high for clarity - Fix QML Connections handler: onSoftSelectionRadiusChanged → onSoftSelectionChanged to match the C++ NOTIFY signal - Add boundary continuity test for weightToColor Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/EditModeController.cpp`:
- Around line 1335-1345: The code always calls getSoftSelectionWeights() and
maps weights to colours, causing selected vertices to be heat-mapped even when
soft selection is off; change the logic to branch on m_softSelectionEnabled:
when true, use getSoftSelectionWeights() and colour via weightToColor(weight) as
now, but when false, iterate the selected vertex indices (use the same
globalToLocal(...) loop over the selection set) and call
m_overlayVertices->colour(...) with the legacy fixed selection colour (replace
weightToColor call with the fixed selection colour used elsewhere in the
codebase), keeping m_overlayVertices->position(...) and the bounds checks on
m_editableMesh unchanged.
🪄 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: 15eaae28-9f4d-4512-b61b-fce7dc9d2d9f
📒 Files selected for processing (3)
qml/PropertiesPanel.qmlsrc/EditModeController.cppsrc/EditModeController_test.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
- qml/PropertiesPanel.qml
- src/EditModeController_test.cpp
Heat map colors only apply when soft selection is enabled. With soft selection off, selected vertices use the original fixed orange color. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
1 participant
Summary
weightToColor()static helper with unit testsTest plan
weightToColor()heat map (5 new tests, all pass)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Version