Skip to content

GLES3: always make gl calls when setting filter and repeat on texture#79367

Closed
LRFLEW wants to merge 1 commit into
godotengine:masterfrom
LRFLEW:glstale
Closed

GLES3: always make gl calls when setting filter and repeat on texture#79367
LRFLEW wants to merge 1 commit into
godotengine:masterfrom
LRFLEW:glstale

Conversation

@LRFLEW

@LRFLEW LRFLEW commented Jul 12, 2023

Copy link
Copy Markdown
Contributor

This is intended as a fix for #79315. Sometimes the repeat property of a texture doesn't get updated, usually when the texture has been seen recently. While looking through the source code, I noticed that it avoids calling glTexParameteri if it thinks it's redundant, and figured this is likely a culprit. Changing this so it always updates the texture parameters resolves the issue from my testing. While I was at it, I also made the change for updating the filtering mode (including anisotropic filtering), since it uses the same caching system, and could have similar issues with it. This isn't an elegant solution, as it doesn't directly address the original cause of the stale state, but it's a viable one.

As to why the bug exists in the first place, I'm assuming it's down to something related to the re-use of data and/or IDs in TextureStorage. How exactly it happens, I didn't test. One option is that the values for state_filter and state_repeat are leftovers from an unrelated texture, so the engine thinks it's already set the parameters for the current texture when it hasn't. (If this is the case, then the bug should also apply to the filtering mode, though I didn't really test that). Another option is that some of the texture parameters are reset when certain actions are performed on an existing texture (such as changing the pixel data).

This PR should be safe to cherry-pick for a 4.1.x release. I will also understand if you'd rather take a different approach to resolving this issue, since this is a bit of a blunt solution.

@LRFLEW LRFLEW requested a review from a team as a code owner July 12, 2023 10:56
@AThousandShips AThousandShips added this to the 4.x milestone Jul 12, 2023
Comment thread drivers/gles3/storage/texture_storage.h Outdated
Comment thread drivers/gles3/storage/texture_storage.h Outdated
@clayjohn

Copy link
Copy Markdown
Member

Thanks for taking a stab at this!

While this PR appears to be a helpful workaround for #79315, it is not a solution that we can consider merging into the engine. Instead, of removing the caching system entirely, the proper fix will be to fix the case that is triggering a false positive in the caching system

@LRFLEW

LRFLEW commented Jul 17, 2023

Copy link
Copy Markdown
Contributor Author

I'm closing this, as its fixes are better covered by #79566 and #79568

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 Repeat sometimes ignored on Compatibility renderer

4 participants