Replace FloatOrd with ordered_float crate

[BeastLe9enD]

Objective

The FloatOrd type is only available for f32, in some cases it is helpful to have a f64 ordered float.

Solution

I replaced FloatOrd in bevy_utils iwth OrderedFloat<T> from the ordered_float crate, which is generic and can be used with f32 and f64. I think it is okay to use an external crate for it, since it is already in version 3.0 and will not change anymore no unnecessary oter features :)

---

Changelog

This section is optional. If this was a trivial fix, or has no externally-visible impact, you can delete this section.

• What changed as a result of this PR?
• If applicable, organize changes under "Added", "Changed", or "Fixed" sub-headings
• Stick to one or two sentences. If more detail is needed for a particular change, consider adding it to the "Solution" section
  • If you can't summarize the work, your change may be unreasonably large / unrelated. Consider splitting your PR to make it easier to review and merge!

Migration Guide

This section is optional. If there are no breaking changes, you can delete this section.

• If this PR is a breaking change (relative to the last release of Bevy), describe how a user might need to migrate their code to support these changes
• Simply adding new functionality is not a breaking change.
• Fixing behavior that was definitely a bug, rather than a questionable design choice is not a breaking change.

[tim-blackbird]

This changes the way NaN is handled from "sorting NaN as less than all other numbers" to "NaN is sorted as greater than all other values" (from the respective docs).
I'm not sure how important that is.

[BeastLe9enD]

This changes the way NaN is handled from "sorting NaN as less than all other numbers" to "NaN is sorted as greater than all other values" (from the respective docs). I'm not sure how important that is.

I am sure this won't be a problem in the code I changed, not sure if "sorting NaN as less than all other numbers" is desired at all.

[mockersf]

Last time we tried to change that trait this had unexpected performances impact. I can't find a comment on #5208 but I remember loss of performances when rendering.

Could you get a tracy trace before and after on one of the examples with a lot of items to render (in the stress_tests folder) and compare them?

[james7132]

Last time we tried to change that trait this had unexpected performances impact. I can't find a comment on #5208 but I remember loss of performances when rendering.

This might have changed after #5049, since we're now using radix sorts instead of FloatOrd's comparison. This will probably have an impact on batched phases, like sprites, which still use that stable sort. It's probably better to check against 2D rendering in particular, in this case.

[BeastLe9enD]

I tested the version against the latest branch using the many_sprites stress test. I compared both versions with the tracy compare function

FloatOrd	OrderedFloat(f32)
Mean time: 30.09ms	Mean time: 29.87ms
Median time: 28.61ms	Median time: 28.43ms

[inodentry]

Could you also try bevymark? It has different characteristics to many_sprites, because it doesn't have hierarchies and transform propagation overhead.

Try something like cargo run --release --example bevymark -- 10000 10

[BeastLe9enD]

I tested the bevymark as @inodentry suggested. There is no difference in performance between FloatOrd & OrderedFloat<f32>.
I've also looked at both implementations, and they really don't differ. Therefore, in my opinion, no differences should be expected.
The only thing that might be a good question is whether there are long-term plans to use f32::total_cmp instead of the current workaround. However, ordered_float currently uses the same implementation as FloatOrd at the moment.

[mockersf]

Thinking in terms of "is it worth it to add another dependency", I'm actually not sure:

• this doesn't change anything inside Bevy, the trait is only used with f32
• this doesn't unblock users from using this crate for their own sort keys as there is actually no block

Do you have a scenario currently where you are blocked of using ordered_float with f64?

[BeastLe9enD]

Of course, I can understand what you mean. Bevy is not benefiting from it, at least at this point. F64 types are not used in Bevy. Since I use it myself, I just thought that it would be nice to use, since FloatOrd is also exported by bevy_utils.
If it is not desired, of course, is perfectly fine. I just thought that it might be nice to have both types under one generic type in the utils library, as I can well imagine that it will also find a use in the future :)

[james7132]

https://crates.io/crates/ordered-float shows it's a pretty commonly used crate so it likely wouldn't cause that big of a conflict, and it would make FloatOrd uses type-compatible with those external use cases.

That said this code is pretty small and are definitely not on the bottleneck path for compilation.

Overall, I would rather us just use something from the std/core lib for this if possible over an ecosystem crate.