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 resource reload bug #28761

Merged
merged 1 commit into from
May 13, 2019

Conversation

aqnuep
Copy link
Contributor

@aqnuep aqnuep commented May 8, 2019

If a non-imported texture resource file (e.g. DDS) gets updated the editor
doesn't reload it. The cause of the problem is two-fold:

First, the code of ImageTexture assumes that textures are always imported
from an image, but that's not the case for e.g. DDS. This change thus adds
code to issue a resource reload in case an image reload is not possible
(which is the case for non-imported texture resources).

Second, the code is filled with bogus calls to Image::get_image_data_size()
to determine the mipmap offset when that should be done using
Image::get_image_mipmap_offset(). Previous code literally passed the integer
mip level value to Image::get_image_data_size() where that actually expects
a boolean. Thus this part of the change might actually solve some other
issues as well.

To be pedantic, the texture_get_data() funciton of the rasterizer drivers is
still quite a mess, as it only ever returns the whole mipchain when
GLES_OVER_GL is set (practically only on desktop builds) but this change does
not attempt to resolve that.

If a non-imported texture resource file (e.g. DDS) gets updated the editor
doesn't reload it. The cause of the problem is two-fold:

First, the code of ImageTexture assumes that textures are always imported
from an image, but that's not the case for e.g. DDS. This change thus adds
code to issue a resource reload in case an image reload is not possible
(which is the case for non-imported texture resources).

Second, the code is filled with bogus calls to Image::get_image_data_size()
to determine the mipmap offset when that should be done using
Image::get_image_mipmap_offset(). Previous code literally passed the integer
mip level value to Image::get_image_data_size() where that actually expects
a boolean. Thus this part of the change might actually solve some other
issues as well.

To be pedantic, the texture_get_data() funciton of the rasterizer drivers is
still quite a mess, as it only ever returns the whole mipchain when
GLES_OVER_GL is set (practically only on desktop builds) but this change does
not attempt to resolve that.
@aqnuep aqnuep requested a review from reduz as a code owner May 8, 2019 15:32
@akien-mga akien-mga added this to the 3.2 milestone May 8, 2019
@akien-mga akien-mga merged commit 868ee3e into godotengine:master May 13, 2019
@akien-mga
Copy link
Member

Thanks!

@hpvb Should be fine to cherrypick for 3.1 according to @reduz.

@akien-mga
Copy link
Member

Cherry-picked for 3.1.2.

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.

2 participants