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

Clean up texture filtering settings #13683

Merged
merged 2 commits into from Aug 24, 2023

Conversation

grorp
Copy link
Member

@grorp grorp commented Jul 23, 2023

Closes #13108, probably closes #8171. This PR makes the following changes:

I decided against exposing GL_NEAREST_MIPMAP_LINEAR in this PR because the terms "bilinear filtering" and "trilinear filtering" seem to be relatively well known. I didn't see enough additional value in GL_NEAREST_MIPMAP_LINEAR to justify making the settings more complicated for players.

To do

This PR is a Ready for Review. After this PR is merged, the SMaterialLayer::setFiltersMinetest method should be removed from IrrlichtMt.

How to test

For example: Enable bilinear or trilinear filtering and verify that nothing appears blurry when magnified anymore.

@sfan5
Copy link
Member

sfan5 commented Jul 29, 2023

Even though I like and use mipmapping in my config I'm not sure if it should be enabled by default because it does this:
screenshot_20230729_174745
This is from a moderate view distance and definitely noticeable with rails.

Plus the whole issue with that game authors should have a say on visual direction and thus this kind of settings, I guess that's for another day.

@grorp
Copy link
Member Author

grorp commented Jul 29, 2023

Even though I like and use mipmapping in my config I'm not sure if it should be enabled by default because it does this: [...]

Disabled by default again 😢

It looks a bit better if you also enable anisotropic filtering:
rails mipmap + aniso
But the real solution would be to allow mod authors to disable mipmapping for specific textures, because there will always be some cases where it looks bad.

Plus the whole issue with that game authors should have a say on visual direction and thus this kind of settings, I guess that's for another day.

👍

@grorp
Copy link
Member Author

grorp commented Jul 29, 2023

Oh no, the CI failed with a vcpkg download error again ☠️

Copy link
Member

@Desour Desour left a comment

Choose a reason for hiding this comment

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

Works. Thanks!

Just have a small suggestion to make NEAREST_MIPMAP_LINEAR available. Feel free to dismiss.

src/client/mesh.cpp Show resolved Hide resolved
@Desour
Copy link
Member

Desour commented Jul 29, 2023

mipmap default

Should we maybe enable mipmapping and anisotropic filtering by default. At least on my machine there's no noticeable performance penalty through these in minetest.

(Should be another PR though.)

@Iniquitatis
Copy link

I'm one of those who always uses this option. Not every game/texture pack uses pixel art, and even when it's the case, it's still useful to enable texture filtering along with cranking up the current texture_min_size to prevent excessive aliasing, while still preserving the pixel art's crispiness. Best of two worlds.
There's also a similar option in GZDoom (which also deals with those big chunky pixels) with the difference that it scales all textures by the same factor instead of to the common base resolution.

@grorp
Copy link
Member Author

grorp commented Aug 22, 2023

I'm one of those who always uses this option. Not every game/texture pack uses pixel art, and even when it's the case, it's still useful to enable texture filtering along with cranking up the current texture_min_size to prevent excessive aliasing, while still preserving the pixel art's crispiness. Best of two worlds.

There's also a similar option in GZDoom (which also deals with those big chunky pixels) with the difference that it scales all textures by the same factor instead of to the common base resolution.

After this PR, the "bilinear filtering", "trilinear filtering" and "anisotropic filtering" settings will still help against aliasing as much as before. They're still applied during minification, where they prevent aliasing.

They're just not applied during magnification anymore, where bilinear filtering causes blurriness and is usually used as a stylistic device rather than to avoid aliasing. Because of technical limitations in Irrlicht, Minetest could until recently only enable bilinear filtering on both minification and magnification together. texture_min_size is a workaround that hides the effect of bilinear filtering on magnification. Because we can now disable bilinear filtering for magnification (while keeping it enabled for minification), this workaround is no longer necessary.

It's true that some games made with Minetest may want to enable bilinear filtering on magnification as a part of their (non-pixel art) style, but that's something for another PR.

@grorp grorp requested a review from Desour August 22, 2023 08:20
@grorp grorp merged commit 72ef908 into minetest:master Aug 24, 2023
11 of 13 checks passed
erlehmann added a commit to erlehmann/minetest that referenced this pull request Nov 13, 2023
This reverts commit 72ef908.

The reverted commit caused several regressions in rendering textures
using bilinear, trilinear or anisotropic filtering. In particular:

• Textures with a height or width lower than “texture_min_size” were no
  longer first upscaled and then filtered. This change affected how all
  small textures (i.e. almost all Minetest textures) looked if filtering
  was enabled – they were not filtered at all. To users this looked like
  bilinear and trilinear filtering no longer working. Anisotropic filter
  effects were affected in differently, depending on vendor (see below).

• On Intel GPUs (and possibly others), anisotropic filtering would blur
  textures with small height or width extremely due to GL_NEAREST being
  set. The effect of that combination depends on driver internals – see
  <KhronosGroup/Vulkan-Docs#1361> and Vulkan
  1.3.249 spec update for details. Note that Khronos Group explicitly
  warned against using GL_NEAREST in this scenario, as it can not be
  expected to result in a consistent look with different GPUs, ever.
erlehmann added a commit to erlehmann/minetest that referenced this pull request Nov 16, 2023
This reverts commit 72ef908.

The reverted commit caused several regressions in rendering textures
using bilinear, trilinear or anisotropic filtering. In particular:

• Textures with a height or width lower than “texture_min_size” were no
  longer first upscaled and then filtered. This change affected how all
  small textures (i.e. almost all Minetest textures) looked if filtering
  was enabled – they were not filtered at all. To users this looked like
  bilinear and trilinear filtering no longer working. Anisotropic filter
  effects were affected in differently, depending on vendor (see below).

• On Intel GPUs (and possibly others), anisotropic filtering would blur
  textures with small height or width extremely due to GL_NEAREST being
  set. The effect of that combination depends on driver internals – see
  <KhronosGroup/Vulkan-Docs#1361> and Vulkan
  1.3.249 spec update for details. Note that Khronos Group explicitly
  warned against using GL_NEAREST in this scenario, as it can not be
  expected to result in a consistent look with different GPUs, ever.
erlehmann added a commit to erlehmann/irrlichtmt that referenced this pull request Nov 17, 2023
This reverts commit 1d4672b.

The “now unused” code is needed again, as the Minetest PR that obsoleted
it <minetest/minetest#13683> had to be reverted
due to causing several regressions in Minetest's texture filtering.
erlehmann added a commit to erlehmann/minetest that referenced this pull request Nov 17, 2023
This reverts commit 72ef908.

The reverted commit caused several regressions in rendering textures
using bilinear, trilinear or anisotropic filtering. In particular:

• Textures with a height or width lower than “texture_min_size” were no
  longer first upscaled and then filtered. This change affected how all
  small textures (i.e. almost all Minetest textures) looked if filtering
  was enabled – they were not filtered at all. To users this looked like
  bilinear and trilinear filtering were no longer working. Anisotropic
  filter effects were affected in different ways, depending on vendor.

• On Intel GPUs (and possibly others), anisotropic filtering would blur
  textures with small height or width extremely due to GL_NEAREST being
  set. The effect of that combination depends on driver internals – see
  <KhronosGroup/Vulkan-Docs#1361> and Vulkan
  1.3.249 spec update for details. Note that Khronos Group explicitly
  warned against using GL_NEAREST in this scenario, as it can not be
  expected to result in a consistent look with different GPUs, ever.
erlehmann added a commit to erlehmann/minetest that referenced this pull request Nov 17, 2023
This reverts commit 72ef908.

The reverted commit caused several regressions in rendering textures
using bilinear, trilinear or anisotropic filtering. In particular:

• Textures with a height or width lower than “texture_min_size” were no
  longer upscaled and then filtered – textures were not filtered at all
  when being upscaled instead, to achieve a crisper look. To users this
  looked like bilinear and trilinear filtering options were not working,
  as most textures in Minetest are rather small and thus often upscaled.
  Anisotropic filter effects were affected in different ways; depending
  on vendor filtering could happen when upscaling textures (see below).

• On Intel GPUs (and possibly others), anisotropic filtering would blur
  textures with small height or width extremely due to GL_NEAREST being
  set. The effect of that combination depends on driver internals – see
  <KhronosGroup/Vulkan-Docs#1361> and Vulkan
  1.3.249 spec update for details. Note that Khronos Group explicitly
  warned against using GL_NEAREST in this scenario, as it can not be
  expected to result in a consistent look with different GPUs, ever.
@grorp grorp deleted the texfilter-settings-cleanup branch December 18, 2023 16:37
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.

Remove/redo bilinear filter settings Wield mesh texturing regression.
5 participants