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

GLES3: Reset anisotropic filtering when changing texture filtering mode #79568

Merged
merged 1 commit into from
Jul 21, 2023

Conversation

LRFLEW
Copy link
Contributor

@LRFLEW LRFLEW commented Jul 17, 2023

This fixes #79567. This is simply the part of the code change from #79367 that relates to anisotropic filtering. The existing implementation of Texture::gl_set_filter() only calls glTexParameterf with _GL_TEXTURE_MAX_ANISOTROPY_EXT when anisotropic filter is enabled. This has the problem that it won't ever disable anisotropic filtering when switching to a filtering mode that doesn't use it. This PR simply makes it so that it calls it with a value of 1.0f when disabling anisotropic filtering.

This PR targets the master branch, but it should be safe to cherry-pick for a 4.1.X release.

Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@clayjohn clayjohn added the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Jul 19, 2023
@LRFLEW
Copy link
Contributor Author

LRFLEW commented Jul 20, 2023

I just noticed something. This is more stylistic than functional, so it isn't a major issue, but I still wanted to bring it up. Should I change float anisotropy to GLfloat anisotropy? Using GLfloat would match the other variables in the function, but both would work basically the same in this context (even for platforms where they aren't the same type, which I don't think Godot even supports). Notably, config->anisotropic_level is a float, not GLfloat, so either way there would be an implicit conversion from float to GLfloat.

@LRFLEW
Copy link
Contributor Author

LRFLEW commented Jul 20, 2023

Just a heads up, this PR may interact with #79685, and may need a rebase if that gets merged first.

@clayjohn
Copy link
Member

I just noticed something. This is more stylistic than functional, so it isn't a major issue, but I still wanted to bring it up. Should I change float anisotropy to GLfloat anisotropy? Using GLfloat would match the other variables in the function, but both would work basically the same in this context (even for platforms where they aren't the same type, which I don't think Godot even supports). Notably, config->anisotropic_level is a float, not GLfloat, so either way there would be an implicit conversion from float to GLfloat.

That's fine GLfloat is just a typdef for float.

@YuriSizov YuriSizov changed the title GLES3: reset anisotropic filtering when changing texture filtering mode GLES3: Reset anisotropic filtering when changing texture filtering mode Jul 21, 2023
@YuriSizov YuriSizov merged commit 4acb8c6 into godotengine:master Jul 21, 2023
@YuriSizov
Copy link
Contributor

Thanks! And congrats on your first merged Godot PR!

@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.2.

@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Aug 31, 2023
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.

Anisotropic Filtering not disabled when switching filtering mode in Compatibility renderer
3 participants