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

Images with big AtlasTextures are broken in systems with AMD GPUs #87119

Closed
LakshayaG73 opened this issue Jan 12, 2024 · 11 comments · Fixed by #87145
Closed

Images with big AtlasTextures are broken in systems with AMD GPUs #87119

LakshayaG73 opened this issue Jan 12, 2024 · 11 comments · Fixed by #87145

Comments

@LakshayaG73
Copy link

LakshayaG73 commented Jan 12, 2024

Tested versions

4.2 stable

System information

Windows 11 - 4.2 stable - AMD 7900xtx

Issue description

For sprites embedded within large atlas textures, the visual gets corrupted.

This issue only seems to occur with AMD GPUs, and not on any NVIDIA GPUs that have been tested.

I've only been able to test and repro this with 'debug export' builds.

Please export attached project and test on AMD GPU to repro.

See attached video
gpu_specs

Steps to reproduce

Sprite.Test.Glitch.mp4

pc_specs

Minimal reproduction project (MRP)

Sprite_Test.zip

@rsubtil
Copy link
Contributor

rsubtil commented Jan 12, 2024

Can confirm on my AMD setup, but also on a lowend Nvidia laptop.

I think this might actually be a hardware limit. Most graphics card can only go up to 16k resolution, and your image surpasses that. Both my GPUs can only work with 16k textures max. But I don't know how Godot handles this, whether it "cuts" image resolution down or not when working with atlas textures.

@quirkylemon
Copy link
Contributor

if this is just a limitation of some graphics cards then it might be a good idea to add a warning in the editor.
Also you might be able to fix the issue by making the texture atlas wider it is currently 2048 w x 16800 h

@AThousandShips
Copy link
Member

AThousandShips commented Jan 12, 2024

The editor wouldn't work for this as it isn't helpful to just know the limitations of your hardware, you might end up running things on weaker hardware for example

See here for one recommendation, which your texture goes far over, the editor could warn about this limit, but it would again depend on the target

OP: Could you check the result of RenderingServer.get_rendering_device().limit_get(LIMIT_MAX_TEXTURE_SIZE_2D)? It should return 16384

I'd suggest reducing the size of your textures, and consider other methods, atlas textures are, as far as I recall, not necessarily the huge deal that they once were, with modern rendering hardware, but I suspect you don't actually need this huge atlases in your project, ensure that you limit how much data you're using at any one time and make sure it's relevant to what you're doing, if you are needing this much data consider using other approaches, this is pushing a lot of limits

@LakshayaG73
Copy link
Author

LakshayaG73 commented Jan 13, 2024

This is interesting.

The result I get is 32768

I mainly use these huge atlases for the sake of convenience. One file, one enemy, done. That sort of a thing.

The other thing is, the bigger the AtlasTexture, the bigger the VRAM savings. Perhaps not by a huge margin, but still. Once we have AtlasTexture imports working consistently with Mesh2Ds, we will be in a better place.

I can split these up, the VRAM usage shouldn't increase too much. Let me try that.

@AThousandShips
Copy link
Member

But you need to keep in mind the range of hardware you could run it on, granted the 8k suggestion is seemingly a bit out of date, but it's still a good safe suggestion I'd say, note that the numbers there aren't always representative due to duplicates etc.

So I'd suggest to at least restrict yourself to the 16k, and preferably lower to be able to cover more targets, don't rely on the things your own hardware can manage, it's hard to plan and test, but it's good to be conservative when you're able

@rsubtil
Copy link
Contributor

rsubtil commented Jan 13, 2024

To complement @AThousandShips's point, 16k textures are absolutely massive even for today's standards. Unity only supports 16k as maximum size too, and Unreal Engine is even lower at 8k textures.

Using the AtlasTexture is not the wrong approach; your base image resolution is simply too big.

@AThousandShips
Copy link
Member

AThousandShips commented Jan 13, 2024

Didn't realize that this was related to the generation of atlas textures from multiple input files, so this might be something to look into as a configuration thing to allow wider but less tall textures, as the current setup limits the width to 2048, that would be something to discuss as an actionable change here

Edit: I've done some testing and while it's not (currently) possible to add this to the import itself, as group files can't have common settings of this kind, it can be a project setting, will do some further testing and see about opening a PR for this

@AThousandShips
Copy link
Member

AThousandShips commented Jan 13, 2024

Opened a draft PR for this, it works basically well, but needs some testing and discussion, let's keep the general topic here

With this PR and modifying the setting to 4096 the resulting texture is much more useful, forget the exact result but the height was somewhere around 7770

Edit: doing so also fixed the rendering in my testing

@Calinou
Copy link
Member

Calinou commented Jan 13, 2024

The 2048 limitation is definitely out of date nowadays as even low-end mobile devices support 4096×4096 textures, but I believe we have to keep it nonetheless to preserve compatibility.

@LakshayaG73
Copy link
Author

@AThousandShips possible regression for this in 4.3?

Some of my user's atlas textures are not respecting the new property that was introduced.

@AThousandShips
Copy link
Member

Please open a new report, as this would be a different issue

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

Successfully merging a pull request may close this issue.

5 participants