Skip to content

Fix panic due to non-total ordering in Area::compare_order() - #5569

Merged
emilk merged 2 commits into
emilk:masterfrom
HactarCE:patch-2
Jan 7, 2025
Merged

emilk merged 2 commits into
emilk:masterfrom
HactarCE:patch-2

Conversation

@HactarCE

@HactarCE HactarCE commented Jan 2, 2025

Copy link
Copy Markdown
Contributor

Area::compare_order() is not a total ordering. If three layers A, B, and C have the same order field but only A and B are present in order_map, then A==C and B==C but A!=C. This can cause a panic in the stdlib sort function, and does in my app although it's very difficult to reproduce.

  • I have self-reviewed this PR and run ./scripts/check.sh
  • I have followed the instructions in the PR template

@github-actions

github-actions Bot commented Jan 2, 2025

Copy link
Copy Markdown

Preview available at https://egui-pr-preview.github.io/pr/5569-patch-2
Note that it might take a couple seconds for the update to show up after the preview_build workflow has completed.

Comment thread crates/egui/src/memory/mod.rs Outdated
@emilk emilk added bug Something is broken egui crash crash, panic, segfault, freeze, … labels Jan 2, 2025

@emilk emilk left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for the unit-test!

@emilk
emilk merged commit 329c8f2 into emilk:master Jan 7, 2025
twvd pushed a commit to twvd/snow-egui that referenced this pull request Jul 20, 2025
…#5569)

[Area::compare_order()](https://github.com/emilk/egui/blob/ee4ab08c8a208f4044a8c571326c9414e7a1c8a6/crates/egui/src/memory/mod.rs#L1174-L1183)
is not a total ordering. If three layers A, B, and C have the same
`order` field but only A and B are present in `order_map`, then `A==C`
and `B==C` but `A!=C`. This can cause a panic in the stdlib sort
function, and does in [my
app](https://github.com/HactarCE/Hyperspeedcube/tree/v2.0) although it's
very difficult to reproduce.

* [x] I have self-reviewed this PR and run `./scripts/check.sh`
* [x] I have followed the instructions in the PR template
twvd added a commit to twvd/snow that referenced this pull request Jul 20, 2025
This happened sporadically when vigorously navigating the
menu's and is caused by an issue in egui 0.30 (fixed in 0.31),
triggered by a change in Rust 1.81:
https://blog.rust-lang.org/2024/09/05/Rust-1.81.0/#new-sort-implementations

See upstream PR: emilk/egui#5569

This commit bumps Snow's egui fork to a version with the fix
cherry picked.
twvd added a commit to twvd/snow that referenced this pull request Jul 20, 2025
This happened sporadically when vigorously navigating the
menu's and is caused by an issue in egui 0.30 (fixed in 0.31),
triggered by a change in Rust 1.81:
https://blog.rust-lang.org/2024/09/05/Rust-1.81.0/#new-sort-implementations

See upstream PR: emilk/egui#5569

This commit bumps Snow's egui fork to a version with the fix
cherry picked.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something is broken crash crash, panic, segfault, freeze, … egui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants