bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Add optional transparency passthrough for sprite backend with bevy_picking

Open vandie opened this issue 1 year ago • 14 comments

Objective

  • Allow bevy_sprite_picking backend to pass through transparent sections of the sprite.
  • Fixes #14929

Solution

  • After sprite picking detects the cursor is within a sprites rect, check the pixel at that location on the texture and check that it meets an optional transparency cutoff. Change originally created for mod_picking on bevy 0.14 (https://github.com/aevyrie/bevy_mod_picking/pull/373)

Testing

  • Ran Sprite Picking example to check it was working both with transparency enabled and disabled
  • ModPicking version is currently in use in my own isometric game where this has been an extremely noticeable issue

Showcase

Sprite Picking Text

Migration Guide

Sprite picking now ignores transparent regions (with an alpha value less than or equal to 0.1). To configure this, modify the SpriteBackendSettings resource.

vandie avatar Nov 14 '24 09:11 vandie

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

github-actions[bot] avatar Nov 14 '24 09:11 github-actions[bot]

@vandie also see https://github.com/bevyengine/bevy/issues/14929 I think your PR fixes it.

mgi388 avatar Nov 14 '24 10:11 mgi388

@vandie also see #14929 I think your PR fixes it.

I think it does yeah.

vandie avatar Nov 14 '24 10:11 vandie

@vandie also see #14929 I think your PR fixes it.

I think it does yeah.

You can put “Fixes #{id}” in your PR description so merging this PR closes the issue as fixed. 😌

mgi388 avatar Nov 14 '24 10:11 mgi388

Seems everyone agrees that this should be enabled by default so I've updated that and introduced a new SpriteBackendAlphaPassthrough enum so this can be controlled by a single setting rather than multiple.

vandie avatar Nov 17 '24 09:11 vandie

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy? It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

github-actions[bot] avatar Nov 17 '24 13:11 github-actions[bot]

Can you add a small migration guide explaining that this is now on by default? Definitely the better default, but could break existing users.

alice-i-cecile avatar Nov 17 '24 13:11 alice-i-cecile

Can you add a small migration guide explaining that this is now on by default? Definitely the better default, but could break existing users.

Added. Hope that's ok and clear?

vandie avatar Nov 17 '24 14:11 vandie

The migration guide was clear and effective, but the tone / target audience was a bit weird. The goal is to communicate "why is my stuff broken and how do I fix it", but your initial draft was closer to a change log :) I've quickly edited the PR description incorporating the information you've added. These get automatically scraped / compiled, so it's good to clean them up here.

alice-i-cecile avatar Nov 17 '24 17:11 alice-i-cecile

Testing the changes got this error after a crash:

2024-11-22T07:39:32.137594Z  INFO bevy_winit::system: Creating new window "App" (0v1#4294967296)
2024-11-22T07:39:32.137772Z  INFO winit::platform_impl::platform::x11::window: Guessed window scale factor: 1
2024-11-22T07:39:32.326951Z  WARN bevy_render::view::window: Couldn't get swap chain texture after configuring. Cause: 'The underlying surface has changed, and therefore the swap chain must be updated'
thread 'Compute Task Pool (4)' panicked at /home/adrian/bevy_testing/bevy/crates/bevy_image/src/image.rs:974:36:
range end index 8 out of range for slice of length 4
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Encountered a panic in system `bevy_sprite::picking_backend::sprite_picking`!
Encountered a panic in system `bevy_app::main_schedule::Main::run_main`!

I'm using 0.15 release branch with the changes applied on top. Running the sprite_picking example The sprite on the right works fine, but the squares and bevy logos works quite bad and make the app crash. The test run on linux with x11 as you can see in the errors

MonforteAdrian avatar Nov 22 '24 07:11 MonforteAdrian

Testing the changes got this error after a crash:

I verified this on Windows and on main as well. Looks like there's something wrong with Image::get_color_at. Dang! I'll file another issue.

andriyDev avatar Nov 22 '24 08:11 andriyDev

Testing the changes got this error after a crash:

I verified this on Windows and on main as well. Looks like there's something wrong with Image::get_color_at. Dang! I'll file another issue.

edit: Oh wait, irritating, this looks like a different panic...

vandie avatar Nov 22 '24 10:11 vandie

I tested the sprite_picking example, looks like the black squares are offset by half its width and height. Color sprites are actually a 1x1 image, so it looks like we're offset by half a pixel somewhere.

Edit: nevermind I completely misunderstood what was happening, looks like the color sprites are off by half width/height which in this case is 64x64

Edit 2: DOUBLE NEVERMIND I WAS CONFUSED, we pass a custom size to the color sprite, which makes it 64x64, but the image size is 1x1.

andriyDev avatar Nov 23 '24 01:11 andriyDev

@vandie When you have time, could you take a look at https://github.com/vandie/bevy/pull/2 ? I tested it and it looks like it solves a lot of issues! (just wanna make sure it doesn't get lost)

andriyDev avatar Nov 23 '24 18:11 andriyDev

Thank you to everyone involved with the authoring or reviewing of this PR! This work is relatively important and needs release notes! Head over to https://github.com/bevyengine/bevy-website/issues/1961 if you'd like to help out.

alice-i-cecile avatar Mar 25 '25 20:03 alice-i-cecile

Thank you to everyone involved with the authoring or reviewing of this PR! This work is relatively important and needs release notes! Head over to https://github.com/bevyengine/bevy-website/issues/1966 if you'd like to help out.

alice-i-cecile avatar Mar 25 '25 20:03 alice-i-cecile