Skip to content

Commit

Permalink
Use FloatOrd for sprite Z comparison and ignore sprites with NaN (#…
Browse files Browse the repository at this point in the history
…15267)

# Objective

Fixes #15258

## Solution

If my understanding is correct, sprites with NaN anywhere in their
transform won't even get onto the screen, so should not generate pick
events. This PR filters sprites with NaN in their transforms before
sorting by depth, then uses `FloatOrd` to simplify the comparison. Since
we're guaranteed to not have NaN values, it's technically unnecessary,
and we could instead sort with `a.partial_cmp(&b).unwrap()`, or even
`unwrap_unchecked()`.

## Testing

I ran the picking example to ensure Z sorting was working as intended.
  • Loading branch information
jnhyatt authored Sep 17, 2024
1 parent e0d38a4 commit 8d78c37
Showing 1 changed file with 7 additions and 8 deletions.
15 changes: 7 additions & 8 deletions crates/bevy_sprite/src/picking_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
//! sprites with arbitrary transforms. Picking is done based on sprite bounds, not visible pixels.
//! This means a partially transparent sprite is pickable even in its transparent areas.

use std::cmp::Ordering;
use std::cmp::Reverse;

use crate::{Sprite, TextureAtlas, TextureAtlasLayout};
use bevy_app::prelude::*;
use bevy_asset::prelude::*;
use bevy_ecs::prelude::*;
use bevy_math::{prelude::*, FloatExt};
use bevy_math::{prelude::*, FloatExt, FloatOrd};
use bevy_picking::backend::prelude::*;
use bevy_render::prelude::*;
use bevy_transform::prelude::*;
Expand Down Expand Up @@ -43,12 +43,11 @@ pub fn sprite_picking(
>,
mut output: EventWriter<PointerHits>,
) {
let mut sorted_sprites: Vec<_> = sprite_query.iter().collect();
sorted_sprites.sort_by(|a, b| {
(b.4.translation().z)
.partial_cmp(&a.4.translation().z)
.unwrap_or(Ordering::Equal)
});
let mut sorted_sprites: Vec<_> = sprite_query
.iter()
.filter(|x| !x.4.affine().is_nan())
.collect();
sorted_sprites.sort_by_key(|x| Reverse(FloatOrd(x.4.translation().z)));

let primary_window = primary_window.get_single().ok();

Expand Down

0 comments on commit 8d78c37

Please sign in to comment.