Skip to content

Fix possible crash - #2391

Open
Erwan MATHIEU (wawanbreton) wants to merge 1 commit into
5.14from
CURA-13357_slicing-crash-with-paint-on-support
Open

Erwan MATHIEU (wawanbreton) wants to merge 1 commit into
5.14from
CURA-13357_slicing-crash-with-paint-on-support

Conversation

@wawanbreton

Copy link
Copy Markdown
Contributor

When using paint-on-support, the feature_value can be either 0, 1 or 2. With the value_or call, the argument is always evaluated, even if not used. However if the printer has only 1 extruder, the scene.extruders contains only 1 entry, and then the argument evaluation will crash. Using a ternary operator instead of value_or fixes it, because the operator is lazily evaluated.

CURA-13357

CURA-13357
When using paint-on-support, the feature_value can be either 0, 1 or 2. With the value_or, the argument is always evaluated, even if not used. However if the printer has only 1 extruder, the scene.extruders contains only 1 entry, and then the argument evaluation will crash.
Using a ternary operator instead of value_or fixes it, because the operator is lazily evaluated.
Copilot AI balanced review requested due to automatic review settings September 29, 2026 08:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The focused change correctly prevents evaluation of the unsafe fallback without altering intended behavior.

Review effort: Balanced
Findings: None

What changed in this PR

Prevents paint-on-support crashes by avoiding eager evaluation of invalid extruder settings.

Changes:

  • Lazily selects mesh settings using a conditional expression.
  • Avoids out-of-range extruder access when support values exceed the extruder count.
File Description
src/​MeshMaterialSplitter.cpp Safely selects settings for generated modifier meshes.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

31 tests  ±0   31 ✅ ±0   5s ⏱️ ±0s
 1 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit c53ed42. ± Comparison against base commit 8e53208.

♻️ This comment has been updated with latest results.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'C++ Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.

Benchmark suite Current: c53ed42 Previous: 1fecafe Ratio
InfillTest/Infill_generate_connect/0/400 5.740188083940878 ms/iter 3.694871895238196 ms/iter 1.55
InfillTest/Infill_generate_connect/1/1200 440.62116333331386 ms/iter 292.31364849999864 ms/iter 1.51
SimplifyTestFixture/simplify_slot_noplugin 3.741669309789577 ns/iter 2.1659779888967523 ns/iter 1.73

This comment was automatically generated by workflow using github-action-benchmark.

CC: Jelle Spijker (@jellespijker) Erwan MATHIEU (@wawanbreton) Casper Lamboo (@casperlamboo) HellAholic

@rburema Remco Burema (rburema) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants