Repository navigation
fix WeatherParticleMesh assertion failure on high precipitation settings - #561
Merged
nschimme merged 4 commits intoOct 1, 2026
Conversation
- Create docs/web.md as a landing page for MMapper Web matching desktop platforms. - Provide direct link to launch MMapper Web and a collapsible Web Beta section. - Add PWA installation instructions for iOS (Safari) and Android (Chrome). - Update homepage (docs/index.md) Web platform link to point to /web.html.
In Debug builds, setting Precipitation Intensity in Preferences above 50 (up to 100) caused an assertion crash in WeatherParticleMesh::setIntensity() because target intensities were calculated up to 2.0 (setting / 50.0). Updated WeatherParticleMesh::setIntensity() to assert non-negativity instead of clamping to 1.0f, and clamped the particle instance count in virt_render() to m_numParticles (512) to ensure buffer safety.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThe PR fixes high-precipitation assertion failures by accepting nonnegative intensity values above 1.0 and bounding the resulting rendered particle count to the allocated particle buffer. Flow diagram for bounded weather particle renderingflowchart TD
A[setIntensity intensity] --> B{intensity >= 0}
B -->|No| C[Assertion failure]
B -->|Yes| D[m_intensity = max 0 intensity]
D --> E{m_intensity > 0}
E -->|Yes| F[count = clamp intensity times maxCount between 1 and maxCount]
F --> G[Render bounded particle instances]
E -->|No| H[Skip particle rendering]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/opengl/legacy/WeatherMeshes.cpp" line_range="178-179" />
<code_context>
- static_cast<GLsizei>(m_intensity
- * static_cast<float>(maxCount)));
+ const auto count = std::clamp<GLsizei>(static_cast<GLsizei>(m_intensity
+ * static_cast<float>(maxCount)),
+ 1,
+ maxCount);
</code_context>
<issue_to_address>
**issue (bug_risk):** The particle count is converted to `GLsizei` before it is clamped. A finite intensity large enough that `intensity * maxCount` exceeds `GLsizei`'s range causes an out-of-range floating-to-integer conversion, which has undefined behavior instead of safely selecting `maxCount`.
**Triggers:** When `setIntensity()` is called with a finite value greater than approximately 4.2 million for the current 512-particle buffer, including a future caller that bypasses the preferences' 0–100 limit.
**Suggested fix:** Clamp the intensity or the floating-point count before converting it to `GLsizei`, or handle `m_intensity >= 1.0f` by assigning `maxCount` directly.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: src/opengl/legacy/WeatherMeshes.cpp:179
In Debug builds, setting Precipitation Intensity in Preferences above 50 (up to 100) caused an assertion crash in WeatherParticleMesh::setIntensity() because target intensities were calculated up to 2.0 (setting / 50.0). Updated WeatherParticleMesh::setIntensity() to assert non-negativity instead of clamping to 1.0f, and updated virt_render() to assign maxCount directly when m_intensity >= 1.0f to avoid float overflow prior to integer casting.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #561 +/- ##
==========================================
- Coverage 27.91% 27.91% -0.01%
==========================================
Files 563 563
Lines 46169 46171 +2
Branches 4884 4884
==========================================
Hits 12889 12889
- Misses 33280 33282 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Prevent debug assertion failures for precipitation intensity values above 50 while keeping particle rendering within the available particle buffer.
Summary by Sourcery
Bug Fixes: