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

Unexpected visual artifacts #4

Closed
stephanemagnenat opened this issue Apr 23, 2022 · 29 comments · Fixed by #7
Closed

Unexpected visual artifacts #4

stephanemagnenat opened this issue Apr 23, 2022 · 29 comments · Fixed by #7

Comments

@stephanemagnenat
Copy link

stephanemagnenat commented Apr 23, 2022

In a simple program using this package, I am seeing unexpected visual artifacts, as can be seen in the green line:

visual artefact

MSAA is disabled, and the map is zoomed enough so that the problem is not likely to be related to unwanted mip-mapping in the texture atlas. The problem can be reproduced using this repository with the following command:

cargo run --bin mapviewer maps/varied.g1m

One can move the camera with arrow keys and zoom with PageUp/PageDown.

I suspect that the bug is related to some border casing in the chunking mechanism, as when I zoom out a lot (my map is 1024x1024 tiles), there seems to be a pattern from the visual artifacts that would match the 64 size of a chunk:

big map

@stephanemagnenat
Copy link
Author

Regarding hardware, I am using a NVIDIA GeForce RTX 2060 SUPER with proprietary drivers version 470.103.01 on Ubuntu 20.04.4

@stephanemagnenat
Copy link
Author

It seems that the issue is not strictly related to chunking, as it also appears more often in some cases (here when I load the map, but not after I zoom:

more artefacts

@Droggelbecher
Copy link

I seem to have a similar issue:
grafik
Note the pink color is the background color of unused tiles. The distance of these artifacts seems to depend on the zoom level (camera transform scale)

@Droggelbecher
Copy link

grafik

@stephanemagnenat
Copy link
Author

I tested my code on two different machines, the issue appears on a Linux with an Nvidia RTX 2060 Super with proprietary drivers, but not on an iMac 2015 with MacOS 12.3.1 with a AMD Radeon R9 M395X 4 GB. Probably unrelated, but maybe worth mentioning, on the iMac the application sometimes, but not always, crashes on start, with error:

thread 'Compute Task Pool (2)' has overflowed its stack
fatal runtime error: stack overflow
Abort trap: 6

@Droggelbecher
Copy link

Mine was on WSL2 with a RTX 3080

@forbjok
Copy link
Owner

forbjok commented May 18, 2022

I tried to run the reproduction, but I'm always getting

warning: `glob1rs` (lib) generated 2 warnings
    Finished dev [optimized + debuginfo] target(s) in 0.31s
     Running `target\debug\mapviewer.exe maps/varied.g1m`
2022-05-18T13:50:55.346930Z  INFO bevy_render::renderer: AdapterInfo { name: "NVIDIA GeForce RTX 2070", vendor: 4318, device: 7943, device_type: DiscreteGpu, backend: Vulkan }

thread 'main' has overflowed its stack
error: process didn't exit successfully: `target\debug\mapviewer.exe maps/varied.g1m` (exit code: 0xc000041d)

@forbjok
Copy link
Owner

forbjok commented May 18, 2022

I did some testing.
Might be the same as this, which can be seen in the benchmark example arbitrarily when zooming or moving around?

benchmark_iSipVORJdc

If so, I've actually seen this before, but I always assumed it was some quirk in the way the zoom/translation is done in Bevy.
I poked around a bit and tried to figure out if it could be some dodgy rounding going on somewhere in my code, but I couldn't really find any evidence of that.

In the image above, I made a local change for testing purposes that puts space in between the chunks, and as you can see the artifact lines are appearing in the middle of chunks, and their positions change arbitrarily when moving the camera around or zooming.

@stephanemagnenat
Copy link
Author

Thanks for trying to reproduce. I might allocate a too large data structure on the stack, I must check that.

I've run the example benchmark, and I also see the issue with a newly-created window:

benchmark

...but, surprisingly, not when the window is maximized (the problem re-appears if I un-maximize it).

@forbjok
Copy link
Owner

forbjok commented May 18, 2022

I have never seen it happen before, unless the zoom is changed. Can you try again with the latest commit I pushed to the master branch a few minutes ago, which adds 1 pixel padding in the example sprite sheet?

I've seen similar issues to this before caused by pixels from adjacent sprites bleeding over, mainly when MSAA is enabled.
It's not the cause of all the line artifacts (I'm still seeing them when zooming out in the benchmark even with the padding), but it would be interesting to see if it could be what is causing it to happen at the default scaling.

@stephanemagnenat
Copy link
Author

stephanemagnenat commented May 18, 2022

Here it is (cargo run --example benchmark) with the new version, freshly launched, default window size, no user input:

benchmark2

If I maximize, the problem goes away as before.

@forbjok
Copy link
Owner

forbjok commented May 18, 2022

I'm now pretty sure the issue is caused by adjacent pixels bleeding over. I did an experiment where I replaced the 1 pixel transparent padding with a pinkish color, and sure enough the line artifacts turned pink.

Testing spritesheet: (you can try replacing tilesheet.png with this and see if you get pink lines as well)
tilesheet

Result:
benchmark_g8EZQrBFPI

Why it's happening, I couldn't say though. I'm guessing it comes down to some sort of UV rounding error occurring somewhere, possibly in driver or platform-specific code, since it doesn't seem to be consistent.

@Droggelbecher
Copy link

Do we happen to know whether other tilemap impls for bevy (eg. bevy_ecs_tilemap) have the same issue? If no, that would at least hint at a way to circumvent it?

@Droggelbecher
Copy link

Answer to self: Seems ECS tilemap has the same problem: StarArawn/bevy_ecs_tilemap#184

@forbjok
Copy link
Owner

forbjok commented May 20, 2022

Answer to self: Seems ECS tilemap has the same problem: StarArawn/bevy_ecs_tilemap#184

Interesting. I was thinking the solution to this might be to ditch TextureAtlas and use a texture array instead, but if bevy_ecs_tilemap, which I believe is already using texture arrays also has these issues then that might not be it.

EDIT: Reading the thread in that issue, it seems like texture arrays will indeed solve the problem, as the user having it was using an optional atlas feature and disabling that fixed it.

EDIT: I did some searching about similar issues involving texture atlases, and it seems like the issues may be caused by automatically generated mipmaps for the sprite sheet texture. If there is a way to disable mipmapping for the texture used in the atlas, that might be another possible solution.

@forbjok
Copy link
Owner

forbjok commented May 28, 2022

I've implemented using texture arrays based on the TextureArrayCache code from bevy_ecs_tilemap (with some minor refactoring and bugfixes), but while this works fine running natively, it does not currently work when compiled to WASM running in a browser, and therefore will not be going into a release in its current state. I don't know for sure whether or not it will be possible to get this method to work in WASM, or whether it will be necessary to implement fallback support for texture atlas in order to support it. This is also a breaking API change, since it no longer uses TextureAtlas.

For the time being, it's available for testing in this branch: https://github.com/forbjok/bevy_simple_tilemap/tree/texture-array

EDIT:
From what I understand, talking to cwfitzgerald in the Bevy discord, this is supposed to be possible in WASM, but isn't currently fully implemented in wgpu. Hopefully this means that in a future version of wgpu this will work.

@fossegutten
Copy link

Maybe this fixed it?
bevyengine/bevy#4085

@forbjok
Copy link
Owner

forbjok commented Jun 1, 2022

Maybe this fixed it? bevyengine/bevy#4085

Interesting. I will try running against the Bevy main branch at some point to see if that prevents the issue from happening. If this fixes it, then the texture array change might not even be needed.

@fossegutten
Copy link

Interesting. I will try running against the Bevy main branch at some point to see if that prevents the issue from happening.

Cool, hope so. I recommend to test at random camera zoom/scales that aren't power of 2 values. From experience this will cause glitches more often

@forbjok
Copy link
Owner

forbjok commented Jun 1, 2022

Unfortunately, bevyengine/bevy#4085 does not seem to fix the issue.

@fossegutten
Copy link

A common workaround to this in other engines is to draw the tilemap to a texture and just scale that texture. Seems like bevy main branch just added support for easy use of render targets, this should be trivial in 0.8 release

@stephanemagnenat
Copy link
Author

Do you mean rendering the final tilemap? In that case, it won't scale. The map I draw has 1024x1024 tiles of 32x32, so it would be 4 GB of texture memory just for the map.

@fossegutten
Copy link

fossegutten commented Jun 2, 2022

Do you mean rendering the final tilemap?

No,

  1. You can draw the tilemap onto a texture/render target that is slightly larger than your main camera view rectangle (1 pixel should be enough if you got the math correct, and don't plan to rotate your camera)
  2. Offset the render target / texture along with the main camera and draw it to the main render target
  3. you can now zoom your main camera and avoid atlas bleeding

If you plan to zoom far out, you need a larger texture obviously.

@zicklag
Copy link
Contributor

zicklag commented Jun 24, 2022

I think I might have found a solution with a PR I made to bevy_ecs_tilemap: StarArawn/bevy_ecs_tilemap#197.

If you guys could see if that fixes the issue on your hardware that would be great.

If it does, it should be simple to implement a similar fix here.

@forbjok
Copy link
Owner

forbjok commented Jun 27, 2022

If it does, it should be simple to implement a similar fix here.

I tried implementing the fix of chopping off 0.5 pixel on each side in the UVs, but while it does seem to fix the pixel bleed issue, it causes tiles to look distorted when rendered upscaled to a higher resolution.

Has this been tested with StarArawn/bevy_ecs_tilemap#197?
It's possible that there is something wrong with the way I've implemented it.

The experimental fix is available in this branch: https://github.com/forbjok/bevy_simple_tilemap/tree/fix/texture-atlas-pixel-bleed

Example before fix:
image

Example after fix:
image

@zicklag
Copy link
Contributor

zicklag commented Jun 27, 2022

Yeah, cutting of 0.5 pixels in the UV in the vertex attribute doesn't help because it will only show half of the pixel on the edges, like you are seeing in the image above. The issue is that the vertex attribute is interpolated between the vertices evenly, so if you just cut off 0.5 pixels on each side in the vertex data, it will just result in stretching the texture out and cutting off the edges.

Instead you have to have the fragment shader decide to stop following the UV all the way to the edge and only modify the UV at the edges, leaving the whole rest of the tile's UV unchanged.

I'll see if I can open a PR real quick.

@zicklag
Copy link
Contributor

zicklag commented Jun 27, 2022

Voilà, I opened #7 with the fix.

I tested by inserting bright pink pixels in the current transparent 1px gap in the demo texture, verified that on master I get the bleeding at different window sizes, and then verified that the bleeding disappears with the fix. Hope this helps!

I ended up making more changes than I thought, just because we needed some extra data in the shader uniforms and vertex data, but it's all straight-forward and shouldn't be too bad.

This specific bleeding issue has been like my arch nemesis since I first got into rendering a couple years ago and wrote a few of my own tile renderers. I was so frustrated that I couldn't figure it out, it made me feel better that I wasn't the only one who had the problem. Finally I think this actually fixes it! Feels good to help other running into the same problem. :D

@stephanemagnenat
Copy link
Author

stephanemagnenat commented Jun 27, 2022

Great that we have a fix, thanks a lot!

Out of curiosity, are people here aware of this discussion on webglfundamentals.org? Because their solution seems to be slightly different and not involve a conditional test.

@zicklag
Copy link
Contributor

zicklag commented Jun 27, 2022

I haven't seen that before. I have somewhat limited internet access so I can't follow that link now, but I'll look into that once I get the chance.

I almost managed to get a solution that worked without a conditional, but it was much harder to read, because it used a somewhat cryptic-looking combination of multiplication, floor and max to accomplish the same thing as the if statement.

I'm not yet sure of how good/bad it is to branch inside of a fragment shader but the owner of bevy_ecs_tilemap said he wasn't worried about it in this case, so I didn't bother trying too hard to figure out a way around it.

Still, I'm definitely going to look at your link once I can, it could very possibly be a better solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants