fix: Rust 1.81+ sort_by total order compliance#2
Conversation
Rust 1.81+ enforces strict total ordering checks in sort_by(). The SimHeaven tiebreaker violated transitivity: when comparing a SimHeaven pack to a non-SimHeaven pack at the same score tier, Equal was returned, but two SimHeaven packs with different layers returned Less/Greater. This caused a panic at runtime. Add else-branches for the mixed case (one SimHeaven, one not) to group SimHeaven packs before non-SimHeaven at the same score tier. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Review Summary by QodoFix Rust 1.81+ sort_by total order compliance in sorter and damage region sorting
WalkthroughsDescription• Fixed Rust 1.81+ strict total ordering compliance in two critical locations that caused runtime panics • **sorter.rs**: Resolved SimHeaven tiebreaker transitivity violation by adding else-branches for mixed SimHeaven/non-SimHeaven pack comparisons at the same score tier • **patches/iced_graphics/src/damage.rs**: Fixed NaN handling in damage region sorting by replacing partial_cmp().unwrap_or(Equal) with total_cmp() for f32 distance comparisons • Added complete local iced_graphics 0.13.0 patch under patches/iced_graphics/ as a workaround for upstream issue (iced-rs/iced#2650), to be removed when upgrading to iced 0.14+ • All 167 tests pass; GUI successfully completes full scenery scan (1480 packs) without panic Diagramflowchart LR
A["Rust 1.81+ strict<br/>total ordering checks"]
B["sorter.rs<br/>SimHeaven comparator"]
C["iced_graphics damage.rs<br/>f32 distance sorting"]
D["Add else-branches<br/>for mixed cases"]
E["Replace partial_cmp<br/>with total_cmp"]
F["Compliance achieved<br/>No panics"]
A --> B
A --> C
B --> D
C --> E
D --> F
E --> F
File Changes1. crates/x-adox-core/src/scenery/sorter.rs
|
Code Review by Qodo
1. Paragraph logs full text
|
Backport of iced-rs/iced#2651 (merged into iced 0.14). Since Rust 1.81, sort_by() panics when the comparator violates total ordering. iced_graphics 0.13's damage::group() uses partial_cmp() on f32 distances with unwrap_or(Equal), which breaks transitivity when NaN values appear. This crashes the app on the tiny_skia fallback renderer path (GL backend / Wayland dmabuf failures). Fix: replace partial_cmp().unwrap_or(Equal) with total_cmp() in patches/iced_graphics/src/damage.rs (line 53-55). This is the exact same fix as upstream PR #2651. The patched crate is vendored under patches/iced_graphics/ and referenced via [patch.crates-io] in the workspace Cargo.toml. Remove this patch when upgrading to iced 0.14+. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
cc75db5 to
fb29698
Compare
Includes PR #2 from @mmaechtel: sorter.rs total-order fix and iced_graphics 0.13 total_cmp() backport from iced 0.14. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Thank you for this! Really appreciate the assistance. |
Summary
Rust 1.81+ enforces strict total ordering checks in
sort_by()comparators, causing runtime panics in two locations:sorter.rs— The SimHeaven tiebreaker violated transitivity: comparing a SimHeaven pack to a non-SimHeaven pack at the same score tier returnedEqual, but two SimHeaven packs with different layers returnedLess/Greater. Fixed by addingelse-branches for the mixed case (+4 lines).iced_graphics::damage::group()— Usespartial_cmp().unwrap_or(Equal)onf32distances, which breaks transitivity when NaN values appear. This is a known upstream issue (iced-rs/iced#2650), fixed in iced 0.14 via PR #2651. Backported here as a local[patch.crates-io]vendor underpatches/iced_graphics/— the only change ispartial_cmp(...).unwrap_or(Equal)→total_cmp(...)indamage.rs. Remove this patch when upgrading to iced 0.14+.Test plan
cargo testtests pass🤖 Generated with Claude Code