godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.38k stars 21.26k forks source link

Scaled Sprite2D displays part of previous animation frame #81998

Open jackwilsdon opened 1 year ago

jackwilsdon commented 1 year ago

Godot version

v4.1.1.stable.official [bd6af8e0e]

System information

Godot v4.1.1.stable - Windows 10.0.19044 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 2060 SUPER (NVIDIA; 31.0.15.3713) - AMD Ryzen 5 3600X 6-Core Processor (12 Threads)

Issue description

When scaling a Sprite2D by an odd amount (e.g. 11), part the previous animation frame is incorrectly displayed.

For example, with this sprite sheet (scaled up 10x for GitHub):

Spritesheet

A sprite with a scale of 11 looks like this on the second frame, both in-editor (at 100% zoom) and in-game: Sprite showing issue

Scaling it by an even amount fixes the issue (12 in this case): Sprite not showing the issue

Disabling centering also fixes the issue, even with an odd scale (11 in this case): Sprite not showing the issue

Zooming out in the editor also fixes the issue: Sprite not showing the issue

Steps to reproduce

  1. Download and import the minimal reproduction project
  2. Switch to the 2D viewport
  3. Note that the bug is present on the oddly-scaled Sprite2D (the left one): Editor screenshot showing the bug
  4. Run the project
  5. Note that the bug is present on the oddly-scaled Sprite2D (the left one): In-game screenshot showing the bug

Minimal reproduction project

sprite2d-scaling-bug.zip

NibblyPig commented 1 year ago

I was going to open a similar bug but I'll put it here - it's not just the scaling, this also occurs for me when I'm using viewports and camera2ds with 4.1.1.Stable

Very basic setup of a Player ( AnimatedSprite2D and AnimationPlayer ). Loaded a sprite sheet into the AnimatedSprite2D and created a single animation with the AnimationPlayer, of one static frame.

When it's rendered within a Viewport, sometimes part of the adjacent sprite bleeds into it, even though it's imported as separate sprites and the cutting of the spritesheet is performed correctly.

image

For example, the bottom right man is the player sprite. He's cut out like this:

382714490_10167882335595534_2239151166780899859_n

Here's my game with two viewports side by side for splitscreen gaming. You can see that the same Player renders with and without the glitch in one of the viewports. As the character moves around, depending on its X position the glitch appears/disappears. When I use a sprite that doesn't have a close by neighbour on the spritesheet, the glitch goes away.

383076693_10167882271120534_7494576078714532553_n

Notice if I move one of the players:

382983841_10167882275210534_2923007766636228914_n

Scaling is set to 1 on everything. Camera is set to Zoom 4.

Minoxs commented 1 year ago

I am looking into fixing this and have made some progress (narrowed things down to RendererCanvasCull::_cull_canvas_item), although I'm unsure what the appropriate fix would be. It looks like it's not correctly limiting the sprite to the actual frame if it can't fit neatly into screen pixels or something like that, since just zooming can make the bleed appear and go away.

If the pixels were to fall outside the frame, they should just be discarded, right? or is it expected to have some sort of interpolation/snapping option?

Edit: Can confirm this still happens in the latest commit to date. Edit2: This is happening regardless of rendering type used (Forward+, Mobile, Compatibility), and all behave in the exact same way.

bs-mwoerner commented 1 year ago

Thanks for taking a look at this! I noticed this issue, too, after porting from Godot 3 to 4.

If the pixels were to fall outside the frame, they should just be discarded, right? or is it expected to have some sort of interpolation/snapping option?

Commenting purely from a engine user's point of view, I think there may be cases where I'd prefer clamping to discarding. If I understand the issue correctly, the problem appears when Godot maps a screen/viewport pixel to a texel that is outside the source atlas region (thus rendering texels from neighbouring regions). Treating these out-of-bounds texels as transparent would be great for isolated sprites, but if you were to place two sprites right next to each other so that they line up perfectly, then occasionally rendering transparent pixels at the border might lead to a flickering gap between these sprites. For these cases, repeating the nearest border texel from inside the source region might be preferable.

Minoxs commented 1 year ago

While poking around a bit more with op's project, I found this option hidden away that nearly completely stops the flickering.

image

However, it can still happen under some more specific circumstances (notice the sliver of blue on the right), so it's not really a fix imo.

image

Also, since a lot of people were mentioning ports from Godot 3 to 4, I tried replicating this on a fresh new Godot4 project. Didn't take very long to have it happen.

asdadsad.zip

NibblyPig commented 1 year ago

I found that option stops the movement of sprites from being smooth, I guess there is interpolation happening and it removes it.

bs-mwoerner commented 1 year ago

Yes, whether or not you want any of the snapping options heavily depends on the requirements of the game and the artistic look it's going for, so it's not necessarily a viable workaround, but might help to get to the root of this issue.

In the asdadsad-Example above, the source region is defined by setting the "Hframes" property to 4 and selecting frame 1. The same result can be achieved by instead using an AtlasTexture with a (x, y, w, h) region of (50, 0, 50, 50) and this gives the same result with the single red line at the left of the sprite. However, the AtlasTexture has a "Filter Clip" property that says "If true, the area outside the of the region is clipped to avoid bleeding of the surrounding texture pixels" and indeed this solves the problem for this example:

FilterClip

The Sprite's Animation parameters and SpriteFrames resources don't have a similar "Filter Clip" option, so this can only be used when working with actual AtlasTexture resources. However, it seems to be possible to populate a SpriteFrames with manually created AtlasTextures using a script. Here's a version that uses an AnimatedSprite2D with a SpriteFrames resource and a script that exports a "Filter Clip Frames" option. Toggling the option replaces the AtlasTexture in the SpriteFrames with a new one with or without the Filter Clip flag.

FilterClipSpriteFrames.zip

Minoxs commented 1 year ago

Good find! An Atlas texture can actually be used to fix this issue for both a Sprite2D and AnimatedSprite2D. No code required.

All you have to do is create an AtlasTexture resource and configure it image

And apply it to a Sprite2D image

Adding sprites to the SpriteFrames from an AtlasTexture also works correctly and removes the bleed. image image

Changing the frames in the animation section will now work as intended. This makes me believe that this, if intended, is more of a documentation problem since it's just a couple more clicks and (imo) makes sense having to specify what kind of texture you want. Probably should be handled automatically when porting from v3 too. The UI could be improved to make this more intuitive as well.

Edit: Upload test project. FixedSpriteBleed.zip

miv391 commented 1 year ago

I tested Sprite2D with AtlasTexture but the texture still doesn't work as expected. It seems to bug exactly like a normal texture. If the Sprite2D's y is *.5, there is bleed on top and cutted pixel on bottom.

kuva

System info: Godot v4.2.dev5 - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1060 6GB (NVIDIA; 31.0.15.3640) - Intel(R) Core(TM) i5-3570K CPU @ 3.40GHz (4 Threads)

bs-mwoerner commented 1 year ago

Ah, I didn't know you could use a saved AtlasTexture resource as a source for a SpriteFrames animation. That's good to know! Then there's indeed no code required.

To be fair, this approach appears to solve the issue for this example, but in my real-world project, Filter Clip and Snap 2D Vertices to Pixel reduce the sprite bleed but don't eliminate it entirely. Maybe this fix only works for the specific transformations we used in the example but not for others, or it may have to do with camera movement or with physics bodies not being in sync with rendering or with some other more complex issue. In any case, there's still potential for Godot to render texels from outside the source region. For now, I'm hoping for someone to stumble upon the root cause, but I may end up recreating all atlases with duplicated frame border pixels. I think that's what Godot 4 does internally for tilesets when "Use Texture Padding" is enabled (which it is by default).

Minoxs commented 1 year ago

@miv391 can you provide the project you used for that example? Having a reproducible example helps a lot when digging for the root cause, and I couldn't reproduce the bug anymore when using AtlasTextures.

bs-mwoerner commented 1 year ago

@miv391 From your screenshot, it looks like you're using (Linear?) filtering on the sprite. Seeing pixel-perfect cuts may only be a reasonable expectation for Nearest filtering. Reading from the middle of a texture with interpolation enabled, I think you'll always have some contribution from texels outside your source region unless you distort and resize your sprite to basically only render half of the border pixels and a quarter of the corner pixels.

I now managed to create a minimal reproduction project for one of my cases of "sprite bleed with Filter Clip enabled". StillBleeding.zip

It turns out, this is influenced by the Anti Aliasing/Quality/MSAA 2D setting. StillBleeding

I'm not sure if this is expected behaviour, but it certainly might be. I think I expected MSAA to take samples from the sprite texture and the background, but I suppose there's a world where jittered sprite texture samples are taken from outside the source region, so maybe MSAA 2D just isn't a very sensible choice when using texture atlases without separation.

Edit: Ah, and if you move the sprite to a vertical position of 483.09998 px, it'll show the red line even for MSAA 2D disabled and Filter Clip enabled...

NibblyPig commented 1 year ago

You should never get anything from outside your sprite texture area though. It feels like it's applying aliasing before it cuts the sprite out, which would be faster if you alias the whole spritesheet before cutting, but it's not good behaviour.

On Sun, 1 Oct 2023, 11:02 bs-mwoerner, @.***> wrote:

@miv391 https://github.com/miv391 From your screenshot, it looks like you're using (Linear?) filtering on the sprite. Seeing pixel-perfect cuts may only be a reasonable expectation for Nearest filtering. Reading from the middle of a texture with interpolation enabled, I think you'll always have some contribution from texels outside your source region unless you distort and resize your sprite to basically only render half of the border pixels and a quarter of the corner pixels.

I now managed to create a minimal reproduction project for one of my cases of "sprite bleed with Filter Clip" enabled. StillBleeding.zip https://github.com/godotengine/godot/files/12776863/StillBleeding.zip

It turns out, this is influenced by the Anti Aliasing/Quality/MSAA 2D setting. [image: StillBleeding] https://user-images.githubusercontent.com/35597337/271820784-606cf066-015c-425c-9e34-c5be04c1cfa0.png

I'm not sure if this is expected behaviour, but it certainly might be. I think I expected MSAA to take samples from the sprite texture and the background, but I suppose there's a world where jittered sprite texture samples are taken from outside the source region, so maybe MSAA 2D just isn't a very sensible choice when using texture atlases without separation.

— Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/issues/81998#issuecomment-1742028356, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMNW33NAELCXHYNYXIG55DX5E5SBANCNFSM6AAAAAA5AKFRDQ . You are receiving this because you commented.Message ID: @.***>

miv391 commented 1 year ago

@miv391 can you provide the project you used for that example? Having a reproducible example helps a lot when digging for the root cause, and I couldn't reproduce the bug anymore when using AtlasTextures.

All settings are default unless otherwise stated. Currently only adding transparent edge around the texture produces output that I expect. The numbers above columns state the decimal part of x and y coordinates. The image below is 300% from original screenshot.

kuva

g42_bleed_test.zip (fixed the bottom row to actually use filter clip)

miv391 commented 1 year ago

@miv391 From your screenshot, it looks like you're using (Linear?) filtering on the sprite.

Yes, in the above example I'm using the default Linear filter. Things get even worse when Nearest filter is used:

kuva

bs-mwoerner commented 1 year ago

All settings are default unless otherwise stated.

While I would argue that depending on how Filter Clip works, texture bleeding may actually not be unexpected with Linear filtering (since you're asking the GPU to interpolate between neighbouring pixels and Filter Clip would have to somehow stop the interpolation in the middle of the texture) and I think Filter Clip wasn't actually enabled on that atlas texture in the bottom row, I do get red lines here even with Nearest filtering and Filter Clip on.

The texture coordinates in use here are probably somewhat of a numerical worst case since 1/3 can't be exactly represented in binary and we may be hitting some edge case here. Moving the sprite by 1/100 unit in any direction makes the red line go away. Indeed, if I extend the texture to 96x96 px, so the region spans the 1/4-1/2 range, there are no red lines even at the original position. It would be interesting to know what Filter Clip actually does and whether there are some cases where things could be improved by using rounding instead of flooring, for example.

In any case, a takeaway from this example might be that it's a good idea to have the texture size be a power-of-two multiple of the cell size. If I change my "StillBleeding" example above to use a size of 8x8 cells instead of 5x5, that eliminates bleeding as well.

NibblyPig commented 1 year ago

None of that should matter. The code should cut out the source sprite from the sprite map meaning it's impossible for it to go outside the bounds of the spritesheet. After that, it doesn't matter at all what filtering, scaling, stretching, anti-aliasing etc is applied.

The best fix seems to be to cut your sprite sheet into individual frames outside of godot and import them that way.

Feels like the solution is to find in the source code where it extracts the sprite and to ensure its not doing any filtering or logic other than directly cutting it out. Perhaps it is trying to cache some of the texture filtering by applying it directly to the spritesheet, I don't know. But it seems futile to try and address this with any filtering, positioning or rendering settings, some of which produce a negative result on the outcome of the sprite.

miv391 commented 1 year ago

I think Filter Clip wasn't actually enabled on that atlas texture in the bottom row,

That's correct, I fixed the screenshots in previous comments. Results are still the same.

bs-mwoerner commented 1 year ago

The best fix seems to be to cut your sprite sheet into individual frames outside of godot and import them that way.

Yes, that's certainly possible, but that's actually a different use case than the one discussed here. When you use atlas textures or sprite regions, you don't create new textures but rather different views on one existing base texture. That means that only this single base texture will actually exist in video memory and when you render 100 different characters with different animation frames that all come from this texture, then the GPU can render all of these characters without any texture changes. This used to be a big deal. I don't know if modern GPUs care as much, but since there's still talk about "material batching", it may still be a factor. The downside of this is indeed that it can be a bit challenging to make sure that exactly the wanted region from that texture is displayed in all circumstances, because the GPU doesn't know about your regions and just works with the coordinates it receives after doing numerous transformations and floating point calculations.

The alternative approach is to create one individual texture per animation frame (either externally or via a script that chops up your sprite sheets into individual textures) and then, yes, there shouldn't be any of these issues, because these will all be individual objects on the GPU with individual sizes and clearly defined borders. On the downside, you'll have many more (albeit smaller) textures in video memory and the GPU will have to switch to a different texture whenever a new animation frame is needed.

Minoxs commented 1 year ago

The whole point of using spritesheets and texture atlases is to have less textures being sent to the GPU. For smaller games it would be no issue of course, but if you have hundreds of animations, each frame being on a different texture would add up quickly... thus, chopping textures for individual frames in not feasible imo. The engine must have a way to deal with this in a sensible way.

@bs-mwoerner Interesting that you mention power of 2 cells, since GPUs really like that sort of things. Perhaps the AtlasTexture could be converted automatically when it's first loaded, if no better solution exists.

miv391 commented 1 year ago

I tested with 128x128 atlas texture from where a 32x32 region was used. If Nearest filter is used, there is no bleed. With Linear filter, there is bleed.

If I were making a pixel perfect game, the simplest fix would be to use "Snap 2D Transforms to Pixel" project setting. But for a game where the sprites move smoothly, there is no other fix than padding the sprite textures with transparent edges. Aside of fixing how the sprites are drawn, of course.

bs-mwoerner commented 1 year ago

Interesting that you mention power of 2 cells, since GPUs really like that sort of things.

And that doesn't even have to result in a power-of-two texture size: My sprites are 16x20, and so far, using Filter Clip, disabling MSAA, and using a 128x160 texture size (8x8 cells) gives very good results. This way, each cell starts on an "clean" uv coordinate (in binary), which probably makes rounding errors much less of a concern.

But for a game where the sprites move smoothly, there is no other fix than padding the sprite textures with transparent edges.

Now, that depends on your definition of "smooth". To me, that would mean pixel-art sprites that can be drawn at arbitrary viewport pixel locations (which is sub-pixel resolution in the world of the pixel art assets). Obviously, you can't really move more smoothly than your viewport resolution, but you can certainly start interpolating, filling viewport pixels with colors averaged from different texels. However: If you want the GPU to draw colors mixed from different texels when you place the edge of your sprite at a non-integral viewport pixel location, then you need to come up with some idea of what the GPU should add to this mix for partial pixels on the "outside" side. "Transparency" is one option, but certainly not the only one. If you want your sprite edge to fade to transparent when interpolating across the edge, then you indeed need to explicitly put this transparency into the texture the sprite is sampled from. If you instead pack your sprite sheet as tighly as you would with pixel art assets, then the mix color for partial outside pixels is whatever happens to lie just outside your source region and in your case, that's just "red".

If you put the sprite into its own texture instead, then you can select what should happen at the border with the texture "Repeat" option. The default is "Disabled", which probably means the GPU will repeat the closest pixel from inside the texture, so you might want to have a transparent border here as well if you want your edge to fade to transparent.

miv391 commented 1 year ago

Now, that depends on your definition of "smooth".

My definition of smooth:

  1. create new project
  2. add Node2D as a root node
  3. add Sprite2D with icon.svg as a texture
  4. add code to Sprite2D:
extends Sprite2D

func _process(delta: float) -> void:
    position += Vector2(5, 5) * delta

I expect the sprite to move smoothly without flickering edges.

bs-mwoerner commented 1 year ago

I expect the sprite to move smoothly without flickering edges.

So this would establish what would not be "smooth", but then what would be "smooth"? I can think of at least two alternative interpretations, which would lead to two different feature requests:

  1. "I would like the default value for "Snap 2D Vertices to Pixel" to be changed to On, so that my test sprite is only drawn at integral positions", or
  2. "I would like the "icon.svg" default resource to include a transparent border, so that I my test sprite is antialiased with transparency instead of the dark blue border color when placed at non-integral positions."

To make this a bit clearer, here are some images. Assume your _process code produces non-integral sprite coordinates like this (black lines indicating the viewport pixels): Smooth1

Now, we need to somehow fill the viewport pixels to represent this sprite position. Variant 1 is just moving the whole sprite so that sprite and viewport pixels align. This is what "Snap 2D Vertices to Pixel" does: The sprite is always crisp, but sprites at 0.0, 0.25, or 0.5 pixels all look the same. Smooth2

The second method is filling the viewport pixels covered by the sprite with interpolated colors. This is what the default settings in your examples do. Inside edges are smoothed depending on the fractional positions, but the sprite gets a bit blurry and the outside edges still jump from viewport pixel to viewport pixel. Smooth3

The third method is the same but mixes the border pixels with transparency depending on how much of their viewport pixel they cover. This is what giving the sprite a 1 px transparent border does. Inside and outside edges are smoothed, but both the sprite and its edges are blurry and when placing two sprites right next to each other and moving them together, a transparent seam would appear and disappear between them as they move. Smooth4

All of these approaches have their uses depending on the application and the desired visual style, so it's not easy to come up with a default behaviour that pleases everyone. I have a feeling you'd be most happy with the transparent border option, but if that were the default, it wouldn't be long before someone would file a bug report on the flickering seam between two neighbouring sprites.

miv391 commented 1 year ago

The second method is filling the viewport pixels covered by the sprite with interpolated colors. This is what the default settings in your examples do. Inside edges are smoothed depending on the fractional positions, but the sprite gets a bit blurry and the outside edges still jump from viewport pixel to viewport pixel. Smooth3

But that's not how the default settings work. Instead, this is what happens:

kuva

Only the top and left edges are filled with interpolated colors. From the bottom and right edges one pixel row and column is cut away.

I find it very difficult to believe that there are real use cases for this kind of behaviour at least for a single sprite. When multiple sprites are used to construct a larger image (tiling), I guess this behaviour is useful too.

I would be happy if the three different cases you presented were possible. But currently only the first one is. Second one isn't (as far as I know), neither isn't the third one without adding transparent edge pixels to the textures.

bs-mwoerner commented 1 year ago

Only the top and left edges are filled with interpolated colors. From the bottom and right edges one pixel row and column is cut away.

Yes, I was simplifying a bit in the hope of not opening another can of worms, but you're right that this means that my example doesn't match what you see in Godot, which isn't ideal. So here's the full story:

The sprite resource has a size of 5x5 pixels, so when drawn at a 100 % scale, it should cover a 5x5 area on the viewport. When moving diagonally towards the bottom right, we would expect to see the sprite cover the area (0, 0)-(4, 4), then later the area (1, 1)-(5, 5), then later the area (2, 2)-(6, 6) etc. Also, we want smooth interpolation, so we want the contents of the sprite to be a mix of different texels to simulate it being drawn at fractional pixel offsets to visually smooth the transition between the individual physical positions.

My example showed the sprite at a (1.5, 1.5) position and for rasterization, I made it cover the (1, 1)-(6, 6) region, actually enlarging it to 6x6 pixels. However, that would mean that the ".5" is sometimes rounded down (from 1.5 to 1) and sometimes rounded up (from 6.5 to 7), which is not what happens in reality. Instead, ".5" is rounded down on all sides, so the sprite covers the (1, 1)-(5, 5) region as shown in your example and keeps its size of 5x5 pixels. In combination with the texel colors mixed based on a virtual origin at (1.5, 1.5), this means that you see more of the dark blue edge on the left and top sides and less of it on the right and bottom sides.

Second one isn't (as far as I know)

I suppose you could create the effect in my image with shaders, but the sprite covering larger or smaller areas as it moves is probably not what you're after anyway. Your image is more faithful to what method 2 looks like for such minuscule sprites.

neither isn't the third one without adding transparent edge pixels to the textures.

Well, for this particular example, you could just enable MSAA 2D in the project settings and be done with it, but this issue was originally concerned with bleeding issues in sprite sheets before taking this extended detour. That's quite a different topic, and apparently, using MSAA by its nature isn't a great idea when you're trying to work with precise UV coordinates within a texture.