godotengine / godot

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

Editor crashes on textures import #85747

Open okla opened 10 months ago

okla commented 10 months ago

Godot version

v4.2.stable.official [46dc27791]

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 commented 10 months ago

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

https://github.com/godotengine/godot/blob/f8a2a9193662b2e8c1d04d65e647399dee94f31e/core/io/image.cpp#L2220

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.

https://github.com/godotengine/godot/blob/f8a2a9193662b2e8c1d04d65e647399dee94f31e/drivers/gles3/storage/texture_storage.cpp#L960

https://github.com/godotengine/godot/blob/f8a2a9193662b2e8c1d04d65e647399dee94f31e/drivers/gles3/storage/texture_storage.cpp#L988

okla commented 10 months ago

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

AThousandShips commented 10 months ago

See here

okla commented 9 months ago

In debugger I can see that p_image size here is already incorrect https://github.com/godotengine/godot/blob/2d0ee20ff30461b6b10f6fdfba87511a0ebc6642/drivers/gles3/storage/texture_storage.cpp#L746 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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 1 month ago

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.