-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add optional transparency passthrough for sprite backend with bevy_picking #16388
Open
vandie
wants to merge
8
commits into
bevyengine:main
Choose a base branch
from
vandie:ignore_transparancy_on_sprite_picking
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+59
−3
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
c5dd17a
Add optional transparancy passthrough for sprite backend
vandie bba576b
set correct default
vandie f9088c7
fix typo
vandie d1013cf
fix too many arguments
vandie 1924e02
fix doc typo
vandie 42c156b
clean up final check into match
vandie 3ca1039
fix docs and naming
vandie 7038101
change transparency to alpha in variable names
vandie File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,25 +11,52 @@ use bevy_ecs::prelude::*; | |
use bevy_image::Image; | ||
use bevy_math::{prelude::*, FloatExt, FloatOrd}; | ||
use bevy_picking::backend::prelude::*; | ||
use bevy_reflect::prelude::*; | ||
use bevy_render::prelude::*; | ||
use bevy_transform::prelude::*; | ||
use bevy_window::PrimaryWindow; | ||
|
||
/// Runtime settings for the [`SpritePickingPlugin`]. | ||
#[derive(Resource, Reflect)] | ||
#[reflect(Resource, Default)] | ||
pub struct SpriteBackendSettings { | ||
/// When set to `true` picking will ignore any part of a sprite which has an alpha lower than the cutoff | ||
/// Off by default for backwards compatibility. This setting is provided to give you fine-grained | ||
/// control over if transparency on sprites is ignored. | ||
pub alpha_passthrough: bool, | ||
/// How Opaque does part of a sprite need to be in order count as none-transparent (defaults to 10) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you elaborate in the docs what the units are? 10 what? What is a valid range I could use, as a user? |
||
/// | ||
/// This is on a scale from 0 - 255 representing the alpha channel value you'd get in most art programs. | ||
pub alpha_cutoff: u8, | ||
} | ||
|
||
impl Default for SpriteBackendSettings { | ||
fn default() -> Self { | ||
Self { | ||
alpha_passthrough: false, | ||
alpha_cutoff: 10, | ||
} | ||
} | ||
} | ||
|
||
#[derive(Clone)] | ||
pub struct SpritePickingPlugin; | ||
|
||
impl Plugin for SpritePickingPlugin { | ||
fn build(&self, app: &mut App) { | ||
app.add_systems(PreUpdate, sprite_picking.in_set(PickSet::Backend)); | ||
app.init_resource::<SpriteBackendSettings>() | ||
.add_systems(PreUpdate, sprite_picking.in_set(PickSet::Backend)); | ||
} | ||
} | ||
|
||
#[allow(clippy::too_many_arguments)] | ||
pub fn sprite_picking( | ||
pointers: Query<(&PointerId, &PointerLocation)>, | ||
cameras: Query<(Entity, &Camera, &GlobalTransform, &OrthographicProjection)>, | ||
primary_window: Query<Entity, With<PrimaryWindow>>, | ||
images: Res<Assets<Image>>, | ||
texture_atlas_layout: Res<Assets<TextureAtlasLayout>>, | ||
settings: Res<SpriteBackendSettings>, | ||
sprite_query: Query<( | ||
Entity, | ||
&Sprite, | ||
|
@@ -130,12 +157,41 @@ pub fn sprite_picking( | |
|
||
let is_cursor_in_sprite = rect.contains(cursor_pos_sprite); | ||
|
||
blocked = is_cursor_in_sprite | ||
let cursor_in_valid_pixels_of_sprite = is_cursor_in_sprite | ||
&& (!settings.alpha_passthrough || { | ||
let texture: &Image = images.get(&sprite.image)?; | ||
// If using a texture atlas, grab the offset of the current sprite index. (0,0) otherwise | ||
let texture_rect = sprite | ||
.texture_atlas | ||
.as_ref() | ||
.and_then(|atlas| { | ||
texture_atlas_layout | ||
.get(&atlas.layout) | ||
.map(|f| f.textures[atlas.index]) | ||
}) | ||
.or(Some(URect::new(0, 0, texture.width(), texture.height())))?; | ||
// get mouse position on texture | ||
let texture_position = (texture_rect.center().as_vec2() | ||
+ cursor_pos_sprite.trunc()) | ||
.as_uvec2(); | ||
// grab pixel | ||
let pixel_index = | ||
(texture_position.y * texture.width() + texture_position.x) as usize; | ||
// check transparency | ||
match texture.data.get(pixel_index * 4..(pixel_index * 4 + 4)) { | ||
// If possible check the alpha bit is above cutoff | ||
Some(pixel_data) if pixel_data[3] > settings.alpha_cutoff => true, | ||
// If not possible, it's not in the sprite | ||
_ => false, | ||
} | ||
}); | ||
|
||
blocked = cursor_in_valid_pixels_of_sprite | ||
&& picking_behavior | ||
.map(|p| p.should_block_lower) | ||
.unwrap_or(true); | ||
|
||
is_cursor_in_sprite.then(|| { | ||
cursor_in_valid_pixels_of_sprite.then(|| { | ||
let hit_pos_world = | ||
sprite_transform.transform_point(cursor_pos_sprite.extend(0.0)); | ||
// Transform point from world to camera space to get the Z distance | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is mainly off by default for performance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest with you, I didn't want it to change the default behaviour in a way that would break existing use cases. I tend to favour opt in than opt out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just saw/remembered per the linked issue @alice-i-cecile seems to suggest it is on by default with a way to turn off. As a user, I want this OOTB myself so on by default makes sense to me, for what it’s worth. To avoid churn feel free to wait for other reviews before changing your code.