Skip to content
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

Bevy 0.5 Sprite Sheet Animation Sprite Leak #1949

Open
selfup opened this issue Apr 17, 2021 · 22 comments
Open

Bevy 0.5 Sprite Sheet Animation Sprite Leak #1949

selfup opened this issue Apr 17, 2021 · 22 comments
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior C-Regression Functionality that used to work but no longer does. Add a test for this!

Comments

@selfup
Copy link

selfup commented Apr 17, 2021

Bevy version

bevy = "0.5"

Operating system & version

Windows 10

Can test on Ubuntu 20.04 as well as MacOS Catalina

What you did

Upgrade to Bevy 0.5 from 0.4

What you expected to happen

Expect sprite sheet animation to work the same as before

What actually happened

Once upgraded, when I full screen the window the sprite animation has artifacts from another position in the sprite sheet

Additional information

Here in my player_setup.rs: https://github.com/selfup/ionized_earth_rpg/blob/main/src/systems/player_setup.rs#L13

let texture_atlas = TextureAtlas::from_grid(texture_handle, Vec2::new(16.0, 16.0), 5, 5);

This now has to be changed to:

let texture_atlas = TextureAtlas::from_grid(texture_handle, Vec2::new(16.1, 16.1), 5, 5);

To avoid the other sprite from leaking in.

Here is a small screenshot of the feet leaking in above the player. Looks like two black bars:

image

Short video showing the artifacts leaking through when moving around.

2021-04-17.01-59-52.mp4
@selfup selfup added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Apr 17, 2021
@Moxinilian Moxinilian added A-Rendering Drawing game state to the screen C-Regression Functionality that used to work but no longer does. Add a test for this! and removed S-Needs-Triage This issue needs to be labelled labels Apr 17, 2021
@mockersf
Copy link
Member

I could not reproduce, either using your code or trying a smaller example 😞

@cart
Copy link
Member

cart commented Apr 19, 2021

Hmm could you both share your graphics cards (and driver if relevant)

@selfup
Copy link
Author

selfup commented Apr 20, 2021

@mockersf

I have updated my repo to have the latest 0.5 changes so you can try it out without checking out to a different branch:

https://github.com/selfup/ionized_earth_rpg


@cart

GPU: 1660ti

Currently on 461.72 - Going to upgrade to 466.11

image

Edit: Same issue with new driver.


Rust version and target info on Windows 10:

stable-x86_64-pc-windows-msvc - rustc 1.51.0 (2fd73fabe 2021-03-23)

Tried 1080p max window, same issue.

Fine when window is in the original size.

Let me know if you need any additional information!

@cart
Copy link
Member

cart commented Apr 20, 2021

I'm able to repro on my computer (nvidia gtx 1070, arch linux, proprietary drivers), but only if I vertically resize the window. Some vertical scales produce the artifacts (identical to the screenshot above), and others don't. This bug is identical to one that this commit resolved, which was later removed. Re-adding that offset resolves the issue in ionized_earth_rpg. Ideally shaders shouldn't need to compensate for this (as custom user shaders won't be aware of the need). We should try to fix the problem in a "global" way instead of re-adding the shader hack.

@CptPotato
Copy link
Contributor

CptPotato commented Apr 23, 2021

Not sure if I'm understanding the source of the problem correctly.. but if it's the surrounding sprites bleeding into the current one then I think 1px of transparent padding and TextureAtlas::from_grid_with_padding() might be the way to go here.

@tatref
Copy link

tatref commented Feb 16, 2022

I'm running into the same issue. Win 10, last updates, Nvidia RTX 2060, drivers 511.79.

The issue is happening regardless of window resize and of sprite size.

The leaking edge is not always the same, depending on the position of the sprite. I made a video with adjacent tiles colored pink:

out.mp4

Any idea how we could help to track down this issue?

@cart
Copy link
Member

cart commented Feb 16, 2022

Yeah padding sprite sheets is a "generally accepted" resolution to this. I'd like to sort out a fix for unpadded sheets, but resolving these rounding errors has been pretty nasty (and sometimes platform specific?). As mentioned in StarArawn/bevy_ecs_tilemap#123, one option is to move to a texture array representation internally, but off the top of my head idk what the full implications of that move would be. For example, im pretty sure the array would need each layer to be the same size, so each sprite would need to take up the space of the "largest" sprite in the array. TextureAtlases can be more "compressed".

Pulling in @StarArawn as this is relevant to what they're doing.

@StarArawn
Copy link
Contributor

StarArawn commented Feb 17, 2022

Texture atlas bleeding

Causes

  • Floating point precision issues - f32 is notoriously bad at precision and on GPU's its almost worse. When you start to add zooming and differing screen resolutions to this issue it becomes even worse than before because the texture pixels that can fit in the small triangle need to be squashed together.
  • Texture filtering - This is probably the biggest culprit. There are two main types of filtering techniques used by GPU's: Nearest Neighbor and Linear. Both techniques work by sampling surrounding pixels in the texture. Nearest uses the pixels directly next to the current pixel where as Linear uses the 4 closest pixels to the current pixel being processed. When you have a sprite or tile directly next to one another you end up sampling into the neighboring pixels regardless of what filtering method you use. A good way around that is to add spacing. Usually you can get away with 1 pixel of spacing but if you require Linear filtering you'll likely need way more.
  • Mipmapping - Bevy doesn't have mipmapping yet, but that's soon to change so I think its important to add some info here. As soon as you need mipmapping with an atlas I would advise to run away from atlases completely. The reason being that there is no ideal way of sampling an atlas at a given spot. For example typically mipmapping works by halving the size of the texture continuously until you are left with a 1x1 pixel. How can a 1x1 pixel represent multiple tiles or sprites in an atlas? You can manually generate mipmaps up until a certain point so you have roughly 1 pixel per item in the atlas but its messy to do. Further more the same texture filtering issue from before manifests itself in an even worse way as filtering occurs across the mipmap layers. In the past this was bypassed by increasing the size of your padding in some cases you had to increase it to more than double the original size of the texture in the atlas. There's obvious downsides to this approach.

Array Textures(Also known as Texture Arrays if you've used D3D)

Array textures can be best described as a texture that has layers. The texture's size remains constant throughout the layers which is one of the biggest disadvantages of using them. The advantages are you can filter and mipmap them just like you would a single texture meaning you don't need padding, you don't need weird filtering rules/setups or even manually filtering in shader(ew), and depending on your art style you may want linear filtering!

My sprites/tiles are different sizes can I still use an array texture?

Yes! The cost of using the array texture with differing sizes is that every texture has to be the max size of your largest sprite. This leads to some funky sizing of sprites as well since you'll want the size of the sprite to match the size of the array texture not the actual sprite texture size. Since the extra pixels are transparent this usually isn't an issue. The increased memory is a bummer but fairly minimal when working with 2D!

Wait why do I want mipmapping for 2D?

You probably don't, but you might? If you implement any sort of zooming you'll soon see artifacts where the GPU has a hard time picking a good texture pixel for a given screen pixel. This results in all sorts of weird looking artifacts like this:
image

A hybrid solution?

In bevy_ecs_tilemap I use a hybrid approach where I support both atlases and texture arrays. I think it should be possible to do this in your own code or even in bevy code. There's no reason why we can't have both! :)

@StarArawn
Copy link
Contributor

I meant to say something about pixel perfect cameras which can also help with these things especially reducing floating point inaccuracies for 2D games.

@james7132
Copy link
Member

Gentle ping about this since the 0.6 renderer rework landed since this issue was opened. Is this still desirable/reproducible?

@selfup
Copy link
Author

selfup commented Mar 21, 2022

I have upgraded bevy to 0.6 then 0.6.1 and I still have the issue on Windows when maximizing the window. I'll have to check on my other machines. This seems to affect different GPUs/OSs differently so I was just confirming that my initial report is still accurate. Thanks for asking!

@wilk10
Copy link
Contributor

wilk10 commented Apr 9, 2022

Just as FYI, the issue has been reported in a discussion a few days ago: #4424 and i've also run into it separately. There was also a brief exchange on discord

@selfup
Copy link
Author

selfup commented Apr 10, 2022

Thank you for the cross reference!

@wilk10
Copy link
Contributor

wilk10 commented Apr 12, 2022

An update:

I’m trying to make some progress on this because in my opinion it has a relatively high impact for 2D games using sprite sheets. At the same time I'm definitely not über knowledgeable, which makes things tricky at best.

After some discussion on Discord with Rob Swain and Nical it seems that the general agreed upon solution is (quote):

clamping the UVs in the shader to not go past the 0.5 texel border in order to not incur sampling from texels outside the sprite bounds

which is what Webrender does.

As Rob said (quote):

the clamping should happen between (sprite.offset + vec2<f32>(0.5)) * texel_size and (sprite.offset + sprite.size - vec2<f32>(0.5)) * texel_size

My understanding is that sprite offset and sprite size (for sprites from texture atlases) is not data available in the sprite shader (it is shared for all sprites, regardless if they come from an atlas or not). Is this correct?

If that’s correct (it might not be), it leads to these possibilities, and i’m not sure what’s best:

  • the above size and offset data is passed to the shader via an uniform, which would mean that’s available for all sprites, but for the majority of them totally useless (because for normal sprites, sprite size will match textureDimensions(sprite_texture) and sprite offset is zero anyway)
  • it can be handled with shader defs, but that would mean in the sprite shader we’d have 4 possibilities already: coloured or not, sprite sheet or not, so it starts to get complicated
  • it would mean (re)introducing a separate shader for sprite sheets

What are your thoughts on this? Is the above accurate? Thanks a lot

@superdump
Copy link
Contributor

As the information is only needed for texture atlas sprites it feels like there should be a way to make a uniform for that case and only use it in that case. Coming up with a good overall solution will need a bit more thought that I don’t have time for in this moment. I’ll try to remember to revisit this but if I forget, poke me on Discord.

@flipbit03
Copy link

Found the same off by one behavior where a tile leaks if I resize my window. Usually happens if you have an odd-sized window, and periodically fixes itself at multiples of 2 or 8 or so.

I have descaled my textures while I was debugging this (and pulling some hairs out thinking I was doing something very stupid 😬 ) and the behavior is the same.

Additional Info:

  • The problem happens both horizontally and vertically
  • Our "terrain" tile set is 32x32, and it was scaled 4.0 (did not make any difference, but still happens). I have descaled it in these examples
  • Window Title has current (last) WindowResized event data:

Bug Happening (leaking the LAST ROW of the tile ABOVE the grass tile)

image

Bug Happening
(leaking the RIGHTMOST ROW of the tile LEFT the grass tile)
(a little difficult to see because the tile at the left is a dark one.)

image

Both happening at the same time:

image

The bug is fully resolved by having an even-sized resolution at both dimensions:
image

@flipbit03
Copy link

Another bit of info about my report above:

My desktop has a little bit of scaling applied to it, for making things a little bit bigger. So, I don't know if it's my desktop scaling that triggers this, but in my Bevy game, if I don't set a scale_factor_override to Some(1.0), then the temporary fix I explained above of setting the window resolution to an even number becomes basically impossible. The resolution becomes very... floating point-y 😬 like this screenshot is showing in the title bar: 798.5455 x 547.2 😢

image

@spalfs
Copy link

spalfs commented Aug 1, 2022

hey just maybe worth noting that I did not have this issue on 0.5.0, but did with 0.7.0 and 0.8.0
as referenced here:
#5510

@zicklag
Copy link
Member

zicklag commented Aug 1, 2022

I think it's very device specific and small tweaks to shaders can fix it in one place and break it in other places so that might be what's happening between Bevy 0.5 and the later versions on your graphics card.

I've submitted fixes for this on a couple Bevy tilemap renderers that have seemed to work ( StarArawn/bevy_ecs_tilemap#197 & forbjok/bevy_simple_tilemap#7 ). I also might be able to do a better version without branching in the shader, but I'll have to test it out.

If I get the chance I'll try to see what fixing this in Bevy's sprite renderer might look like.

@knuxyl
Copy link

knuxyl commented Jul 31, 2023

I can confirm this issue is still present in latest (0.11)
When using from_grid() pixels from tiles underneath the rows are showing up on the upper tiles. This happens when maximizing the window. I suspect its something with the scaling with f32. I think the images need to be actually extracted first from tilesheet before applying any scaling or probably any modification. Im also using nearest interpolation. Adding padding may work, vut I'm concerned if the interpolation will actually show transparency and therefore the clear color instead of ignoring it. I dont know if nearest considers transparency like that.

System
Intel W-1290T / Radeon Pro WX4100
Debian Testing / Gnome / Wayland (although it's building for x11, so xwayland
bevy *

@MeoMix
Copy link

MeoMix commented Oct 16, 2023

I experience this as well.

I mitigated the issue very slightly by adjusting padding with from_grid but not to the point that I eliminated it entirely. So I think I am going to revert back to loading individual sprites rather than a sheet for now.

It's affected by zooming the projection's scale. Sometimes the lines disappear from specific areas when I zoom in/out.

WASM/Chrome testing generated from an x86_64-unknown-linux-gnu build.

image

@MeoMix
Copy link

MeoMix commented Dec 2, 2023

I was able to sufficiently mitigate this issue for my use case without modifying the underlying assets. Here's my current understanding just in case it helps anyone going forward.

  • This issue is caused by a combination of projection.scale + bleeding. An individual sprite may look correct at a given projection.scale and incorrect at another due to how these interact. For me, since I render a tile grid which is fit to the window width, and offer the ability to pan/zoom on this grid, I was seeing bleed flickering as I zoomed in/out and when I resized my window.

  • Since the issue is caused by bleeding between sprites, until a fix for the underlying issue is implemented, the goal is to bleed gracefully. This means that the bled-into pixel color should match the desired pixel color. This means you don't want to surround sprites with transparency. You want to surround them with their neighboring colors.

  • For complex sprites, the solution to this would be to "extrude" the colors of each sprite, increasing the size of the sprites, and allow the bleed to occur. The bleed effect reads the extruded color value, which matches the neighboring color value, and results in the desired visual effect.

  • For simple sprites, you don't necessarily need to modify your sprite assets. Instead, you can achieve a similar effect by adjusting the from_grid call.

Consider the sprite sheet below. It is one column of 16 sprites. Each sprite is 128x128. 15 of the sprites have a thick, black border along one or more of their edges. There is no padding between them.

image

The "correct" way to generate a texture atlas from this asset is:

    texture_atlases.add(TextureAtlas::from_grid(
        texture_handle,
        Vec2::splat(128.0),
        1,
        16,
        None,
        None,
    ))

However, this exhibits the undesired bleed effect. For my scenario, this occurs with these settings:

  • I am rendering 128x128 textures on sprites whose default size is overridden via custom_size = Some(Vec2::Splat(1.0)).
  • I have a 2D tilemap grid of these sprites. My grid is 144x144. By default, it renders at 144px * 144px.
  • I want my grid to visually fill the viewport. So, I adjust my Camera's projection.scale. This results in a projection.scale of 0.0525. The specific scale isn't important - just providing context.

Here is how this renders. Note the black border bleeding into various sprites. Also note that if I resize the window then the affected sprites change.

image

Now, consider an adjusted call to from_grid of the form:

    texture_atlases.add(TextureAtlas::from_grid(
        texture_handle,
        Vec2::new(128.0, 120.0),
        1,
        16,
        Some(Vec2::new(0.0, 8.0)),
        Some(Vec2::new(0.0, 4.0)),
    ))

Here I keep my x-axis as-is because I am working with a single column sprite sheet. I reduce the size of my sprite along the y-axis, then add an equal amount to y-padding (not half!), and add half the amount to y-offset.

Here is the result:

image

This eliminates the bleed artifacts.

Note that using a sufficiently large reduction in tile_size is necessary to achieve the desired result. Originally, I thought this approach was not a valid solution because I reduced tile_size by just 2 and still saw bleeding on a full-size window. Then, I reduced by 4, setting my values to 124.0 // 4.0 // 2.0, which worked for a full-size window, but did not hold for a resized window. This is because my resized window had different subpixel rounding due to a varied projection.scale.

The final solution was to work my way down through reduced tile_size values until I found an aggressive enough setting to eliminate artifacts at all resolutions I intend to support.

UPDATE:

As my awareness of this issue increases, so does the length of this post :)

I ended up moving away from a homegrown tilemap solution for performance reasons. My 144x144 tilemap crashed on my test iOS device where it did not crash before the introduction of sprite sheets. The lowest hanging fruit performance fix was to adopt "meshing" in which multiple sprites are combined into larger mesh chunks. This functionality is implemented already in bevy_ecs_tilemap (https://github.com/StarArawn/bevy_ecs_tilemap/tree/main/src/render) and bevy_simple_tilemap. I tried using the simple version first, but tiles aren't entities in its implementation which made updates too challenging. So, I adopted bevy_ecs_tilemap.

bevy_ecs_tilemap, when running in WASM targeting WebGL2, makes use of atlases internally, but its external interface expects texture: Handle<Image>, i.e. the raw sprite sheet not a TextureAtlas. bevy_ecs_tilemap exposes TilemapSpacing, but making use of it did not prevent bleeding.

After talking with the maintainer of bevy_ecs_tilemap I was instructed to disable Msaa:

app.insert_resource(Msaa::Off);

This fixed my bleed issue without any padding hacks.

So, I would encourage others to use this approach first and only to explore more complex, padded solutions afterward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior C-Regression Functionality that used to work but no longer does. Add a test for this!
Projects
None yet
Development

No branches or pull requests