Allow changing the anisotropic filter level at run-time per Viewport #88313
Conversation
|
I can't speak for the GLES3 changes but the RD ones look good to me, glad to see the effort of splitting the samplers into being per-viewport was continued like this. The change makes sense to me. |
| @@ -131,6 +131,7 @@ void RenderSceneBuffersGLES3::configure(const RenderSceneBuffersConfiguration *p | |||
| scaling_3d_mode = p_config->get_scaling_3d_mode(); | |||
| //fsr_sharpness = p_config->get_fsr_sharpness(); | |||
| //texture_mipmap_bias = p_config->get_texture_mipmap_bias(); | |||
| //anisotropic_filtering_level = p_config->get_anisotropic_filtering_level(); | |||
There was a problem hiding this comment.
Is there a purpose to the commented code being introduced here?
There was a problem hiding this comment.
Seems to be that it doesn't fill in any of the fields since all other viewport lines are also commented out, I added that one there just in case the commented lines are there for future change kinda thing
There was a problem hiding this comment.
While FSR1 probably won't be implemented in Compatibility, anisotropic filtering level change can be done on all platforms (including OpenGL ES) while mipmap LOD bias is limited to desktop OpenGL (it was never part of OpenGL ES).
There was a problem hiding this comment.
Tested locally (rebased on top of master 907db8e), it works as expected.
Testing project: test_pr_88313.zip
simplescreenrecorder-2024-02-14_19.29.05.mp4
There was a problem hiding this comment.
Code looks good to me. Please squash commits together into a single commit so this can be merged.
wagnerfs
force-pushed
the
run-time-anisotropic-filtering
branch
from
e652bce to
356e285
Compare
|
Please clean up your commit message, you don't need all the original messages, just a short message describing what's done |
wagnerfs
force-pushed
the
run-time-anisotropic-filtering
branch
from
17d88a6 to
7b1a6f5
Compare
|
Should be good to go now! |
|
You'll need to run |
wagnerfs
force-pushed
the
run-time-anisotropic-filtering
branch
from
8326c2a to
64a7460
Compare
|
@akien-mga never dealt with docs before, thanks for the help |
wagnerfs
force-pushed
the
run-time-anisotropic-filtering
branch
from
39fe10d to
9c4bc20
Compare
wagnerfs
force-pushed
the
run-time-anisotropic-filtering
branch
from
b911fa4 to
2093462
Compare
|
Fixes #69379 |
There was a problem hiding this comment.
Approving. The code looks great!
I am still skeptical about the need to expose this on a per-viewport basis. I am especially wary that the main reason this PR exposes anisotropy on a per-viewport basis was simply because it was easier than the alternative (because it can piggyback on our mip offset code). However, enough people have commented on the PR with reasons why they might need this exposed on a per viewport basis that I won't fight against it.
Overall, the code additions are small and this won't add much maintenance burden beyond what we have already accepted for the mip level offset
|
Thanks! And congrats for your first merged Godot contribution 🎉 |
Fixes godotengine/godot-proposals#4673
Changes to anisotropic filtering on project settings are real time now and don't need an engine restart, also adds
anisotropic_filtering_levelproperty to viewports and can be changed separately.Added the additional lines do the docs on viewports as well, I'm not sure what to do on the
ProjectSettings.xmlthough besides removing the note about not being able to change at run-time.A new enum had to be added though similar to MSAA
enum AnisotropicFiltering { ANISOTROPY_DISABLED, ANISOTROPY_2X, ANISOTROPY_4X, ANISOTROPY_8X, ANISOTROPY_16X, ANISOTROPY_MAX };