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

Editor crashes on textures import #85747

Closed
okla opened this issue Dec 4, 2023 · 11 comments
Closed

Editor crashes on textures import #85747

okla opened this issue Dec 4, 2023 · 11 comments

Comments

@okla
Copy link

okla commented Dec 4, 2023

Godot version

v4.2.stable.official [46dc277]

System information

Fedora 39 KDE Plasma 5.27.9

Issue description

Since upgrading my project from Godot 3 to 4, there is a recurrent bug with textures import that is present on some machines/OSes and not present on others (there is no system in its behavior as I can see). On my machine the editor prints this error for every imported texture and then crashes (sometimes it doesn't crash):

ERROR: Expected Image data size of 182x242x1 (DXT1 RGB8 with 7 mipmaps) = 30120 bytes, got 30216 bytes instead.
   at: initialize_data (core/io/image.cpp:2219)

On my coworkers' machines (both Linux and Windows) it may work fine for some time, but then suddenly it starts to show these errors too.

Steps to reproduce

  1. Launch the attached project

Minimal reproduction project

Bug.zip

@W4RH4WK
Copy link
Contributor

W4RH4WK commented Dec 15, 2023

Errors still encountered on current master (f8a2a91), it's emitted here:

ERR_FAIL_MSG(vformat("Expected Image data size of %s = %d bytes, got %d bytes instead.", description, size, p_data.size()));

The size discrepancy originates between texture->alloc_width, texture->alloc_height vs. texture->width, texture->height.

Values are 184x244 for allocation, 182x242 for actual size.

int data_size = Image::get_image_data_size(texture->alloc_width, texture->alloc_height, texture->real_format, texture->mipmaps > 1);

image = Image::create_from_data(texture->width, texture->height, texture->mipmaps > 1, texture->real_format, data);

@okla
Copy link
Author

okla commented Dec 16, 2023

I could try to debug this. Is there any instruction on how to build and debug the editor?

@AThousandShips
Copy link
Member

See here

@okla
Copy link
Author

okla commented Dec 17, 2023

In debugger I can see that p_image size here is already incorrect

texture.width = p_image->get_width();

Unfortunately this function is called kinda indirectly in a CommandQueueMT instance and I can't find where it's (and its arguments including the image) added to it.

@W4RH4WK
Copy link
Contributor

W4RH4WK commented Dec 18, 2023

If I understand correctly, the size is different because of the ETC compression used for the image which uses a block size of 4. Taking the width for example, 182 is not divisible by 4, but 184 is. Image::_get_dst_image_size does the corresponding calculation.

I am still not sure how exactly Godot should handle this.

@Calinou
Copy link
Member

Calinou commented Dec 19, 2023

If I understand correctly, the size is different because of the ETC compression used for the image which uses a block size of 4. Taking the width for example, 182 is not divisible by 4, but 184 is. Image::_get_dst_image_size does the corresponding calculation.

I am still not sure how exactly Godot should handle this.

The usual way to resolve this is to resize the image to a slightly greater size (i.e. from 182 to 184 pixels, preserving aspect ratio). We can use Lanczos scaling during this operation to ensure crispness remains as close as possible to the original.

@W4RH4WK
Copy link
Contributor

W4RH4WK commented Dec 20, 2023

The usual way to resolve this is to resize the image to a slightly greater size (i.e. from 182 to 184 pixels, preserving aspect ratio). We can use Lanczos scaling during this operation to ensure crispness remains as close as possible to the original.

Should this happen during the initial import of image? And should the width/height be updated accordingly?

@Calinou
Copy link
Member

Calinou commented Dec 21, 2023

Should this happen during the initial import of image? And should the width/height be updated accordingly?

This should be done on import type, or at runtime if using Image.compress(). The width/height will have to be updated accordingly, although we could set a size override in 2D so that the physical sprite size doesn't change (we have a method/property for this currently not exposed in the inspector).

@okla
Copy link
Author

okla commented Sep 4, 2024

Can confirm that resizing source images to sizes divisible by 4 resolves the issue. Would be great if Godot could do this for us though.

@BlueCube3310
Copy link
Contributor

This may have been fixed in 4.4-beta3, I'm unable to reproduce the error

@akien-mga akien-mga added this to the 4.4 milestone Feb 10, 2025
@akien-mga
Copy link
Member

Confirmed fixed in 4.4.dev3 onward, possibly by #97325.

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

No branches or pull requests

7 participants