Skip to content

Handle texture filtering sanely to avoid blurriness#16034

Merged
sfan5 merged 1 commit into
luanti-org:masterfrom
sfan5:abolish-blur
Apr 21, 2025
Merged

Handle texture filtering sanely to avoid blurriness#16034
sfan5 merged 1 commit into
luanti-org:masterfrom
sfan5:abolish-blur

Conversation

@sfan5

@sfan5 sfan5 commented Apr 17, 2025

Copy link
Copy Markdown
Member

tl;dr enabling "bilinear filter" or "trilinear filter" is no longer an automatic blurry knob

fixes #15604 as proposed
relates to #13108

To do

This PR is Ready for Review.

How to test

  1. test various settings, texture sizes/packs, mods, ...
  2. confirm everything looks nice

@sfan5 sfan5 added this to the 5.12.0 milestone Apr 17, 2025
@sfan5
sfan5 force-pushed the abolish-blur branch 2 times, most recently from 0d8370a to 1c23ba4 Compare April 17, 2025 14:39
@sfan5 sfan5 added the Feature ✨ PRs that add or enhance a feature label Apr 17, 2025
Comment thread builtin/settingtypes.txt Outdated
# This is also used as the base node texture size for world-aligned
# texture autoscaling.
texture_min_size (Base texture size) int 64 1 32768
texture_min_size (Base texture size) int 192 1 16384

@lhofhansl lhofhansl Apr 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Where does 192 come from?
Also, if we really enforce a min-limit in the code (see comment below), then this should be 192 192 16384 here as well. (But I think it makes no sense to enforce a min-limit)

@sfan5 sfan5 Apr 17, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

oops, missed that

192 is just based on my personal testing. the lowest resolution where everything is non-blurry enough that you could give this to an unsuspecting player in good conscience (IMO).

Comment thread src/nodedef.cpp
translucent_liquids = g_settings->getBool("translucent_liquids");
enable_minimap = g_settings->getBool("enable_minimap");
node_texture_size = std::max<u16>(g_settings->getU16("texture_min_size"), 1);
node_texture_size = rangelim(g_settings->getU16("texture_min_size"),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why enforce a min-limit in the code? The default is already set to 192. If a user wants this lower, why not let them?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

the idea is: the user should not be able to ruin the artistic choice of the game author

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see. One could then argue why have a setting like this at all? I.e. the server should be in control.
Having a good default, but letting a user set whatever they want makes more sense to me (it's already in the "advanced" section).

@sfan5 sfan5 Apr 17, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think larger sizes help with moire even more and make it less blurry (obviously), so it sort of still makes sense.
Don't want to break someone's use of this setting :)

@grorp grorp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the approach seems sane 👍

the only thing I can think of that could cause "backwards compat" complaints here is if someone has a lower-than-192px texture pack that they meant to be blurry / bilinearly filtered. e.g. "SharpNet Photo Realism 64px" (with all the filtering settings enabled) would change from this:
screenshot before, scene 1
screenshot before, scene 2

to this:
screenshot after, scene 1
screenshot after, scene 2

the new version actually looks preferable to me though 🤷

EDIT: my example doesn't work anyway because looking at the package screenshots of "SharpNet Photo Realism 64px", it doesn't look like bilinear was enabled, so the author likely didn't intend it to be blurry.
I can't really think of any case where this would be a concern.

Comment thread doc/lua_api.md Outdated
Comment thread src/client/content_cao.cpp
Comment thread builtin/settingtypes.txt
@sfan5 sfan5 added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Apr 19, 2025
@sfan5 sfan5 removed the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Apr 19, 2025

@grorp grorp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The new changes look good

const core::dimension2d<u32> dim = baseimg->getDimension();
if (filter) {
const f32 scaleto = rangelim(g_settings->getU16("texture_min_size"),
TEXTURE_FILTER_MIN_SIZE, 16384);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what happened to "2 tabs for continuation lines" btw? (also elsewhere)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

idk I usually never do that

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.

bilinear_filter and trilinear_filter are a disaster

3 participants