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

Fix texture atlas generation when source sprite is larger than generated atlas #55095

Merged
merged 1 commit into from
Nov 18, 2021

Conversation

Giwayume
Copy link
Contributor

@Giwayume Giwayume commented Nov 18, 2021

Fixes #41414.

The problem: atlas images getting cut off.

image

https://github.com/godotengine/godot/blob/3.4/editor/import/resource_importer_texture_atlas.cpp#L134

In this line, "xf" starts at the left margin in the original sprite frame (source) image. Which in my case is a 512x512 image with the eyeball in the middle and a lot of empty space around it. This 512x512 dimension is set to the src_width and src_height variables respectively.

image

The variables width and height in this function refer to the final atlas texture's width and height, which is 256x252 in this case. So the entire generated texture atlas (256x252) is smaller than a single source animation frame (512x512). That exposes this bug.

It looks like in this loop it's limiting by the generated atlas's width when it's actually looping through the source image's pixels.

for (int xi = (xf > 0 ? int(xf) : 0); xi < (xt < width ? xt : width - 1); xi++) {

So instead it should limit by the source image's width.

for (int xi = (xf > 0 ? int(xf) : 0); xi < (xt < src_width ? xt : src_width - 1); xi++) {

This indeed produces the correct results. And it makes sense why the images were getting cut off before.

image

There's already code that guarantees you're not drawing outside of the destination atlas texture's dimensions.

https://github.com/godotengine/godot/blob/3.4/editor/import/resource_importer_texture_atlas.cpp#L147

This width/src_width variable swap seems to have been a type-o. I don't think the same coding problem is present on the y axis, because my sprites would have been clipped off vertically as well.

@Giwayume Giwayume requested a review from a team as a code owner November 18, 2021 15:09
@akien-mga akien-mga added this to the 4.0 milestone Nov 18, 2021
@akien-mga akien-mga changed the title Fix texture atlas generation when source sprite is larger than generated atlas 4.0 Fix texture atlas generation when source sprite is larger than generated atlas Nov 18, 2021
@akien-mga akien-mga merged commit 84f59c6 into godotengine:master Nov 18, 2021
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@boruok
Copy link
Contributor

boruok commented Nov 22, 2021

I met some graphical issue: region is fine, but it cutout right size of texture.
here example, right red border doesn't render
fence_grass

@Giwayume
Copy link
Contributor Author

@boruok Can you send the source images?

@boruok
Copy link
Contributor

boruok commented Nov 22, 2021

@Giwayume sure
New Game Project.zip

@Giwayume
Copy link
Contributor Author

Uh, I'm not sure I understand. is fence_grass.png supposed to be a generated texture atlas? That's the only image I see. What about the images you generated it from?

@boruok
Copy link
Contributor

boruok commented Nov 22, 2021

@Giwayume if you generate atlas from given texture you can see that right red border doesn't render.

@Giwayume
Copy link
Contributor Author

Interesting edge case.

@Giwayume
Copy link
Contributor Author

Giwayume commented Nov 22, 2021

The atlas generation algorithm even in 3.4.stable is adding unnecessary extra space in the atlas in this case. There's more than 1 pre-existing problem here.

image

EDIT:

Eh... now that I think about it, it's rounding up to the nearest power of 2. Probably designed to do that for memory management.

@boruok
Copy link
Contributor

boruok commented Nov 22, 2021

@Giwayume im pretty sure that is src_width - 1

for (int xi = (xf > 0 ? int(xf) : 0); xi < (xt < src_width ? xt : src_width - 1); xi++) {
...
for (int xi = (xf < src_width ? int(xf) : src_width - 1); xi >= (xt > 0 ? xt : 0); xi--) {
...

@Giwayume
Copy link
Contributor Author

Giwayume commented Nov 22, 2021

The original is (atlas_)width - 1, curious if this problem occurs on a 256px wide image in 3.4.stable.

Actually, it adds 4px padding to everything so probably can't reproduce the problem in stable due to that. Bug saved by padding.

It's gonna take me an hour to rebuild Godot after switching branches to find out either way. =(

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

Successfully merging this pull request may close these issues.

Texture altas import breaking sometimes - Cutting off sprite
3 participants