godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.17k stars 98 forks source link

Allow specific materials to use bilinear mipmap filtering instead of trilinear #7411

Open ttencate opened 1 year ago

ttencate commented 1 year ago

Describe the project you are working on

The GUI of a game, containing TextureRects with line art, whose displayed size is not known ahead of time.

Describe the problem or limitation you are having in your project

I want these textures to scale to arbitrary sizes, but look as crisp as possible without being pixelated. If the texture is displayed at close to its native size, TEXTURE_FILTER_LINEAR does the job nicely. However, if it's scaled down too much, then edges become jagged.

The standard solution is of course to use mipmaps. In Godot we can do this using the TEXTURE_FILTER_LINEAR_WITH_MIPMAPS filter mode. But this results in a more blurred look, because it doesn't just interpolate between four adjacent texels, it also interpolates between two adjacent mipmap levels (trilinear interpolation).

Describe the feature / enhancement and how it helps to overcome the problem or limitation

The best look would be obtained by selecting the nearest mipmap, and then interpolating within that mipmap. In OpenGL, this is what GL_LINEAR_MIPMAP_NEAREST does, which has been available for literally 30 years. But somehow Godot doesn't expose it.

In 3D, this filter mode would result in a sharp edge between mipmap levels. But in 2D, as long as the scale of the texture doesn't change dynamically, it's a faster and better looking solution than trilinear interpolation. The difference is subtle but noticeable:

2023-07-30T13:24:10_599x271

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

We already have these: TEXTURE_FILTER_NEAREST_WITH_MIPMAPS = GL_NEAREST_MIPMAP_NEAREST TEXTURE_FILTER_LINEAR_WITH_MIPMAPS = GL_LINEAR_MIPMAP_LINEAR

The proposal is to add: TEXTURE_FILTER_NEAREST_WITH_LINEAR_MIPMAPS = GL_NEAREST_MIPMAP_LINEAR TEXTURE_FILTER_LINEAR_WITH_NEAREST_MIPMAP = GL_LINEAR_MIPMAP_NEAREST

In shaders, the hint format for sampler2D could be extended to:

filter[_nearest, _linear][_mipmap][_nearest, _linear][_anisotropic]
                                  ^^^^^^^^^^^^^^^^^^^

The second nearest/linear indicates the interpolation between mipmap levels. If omitted, it defaults to the first nearest/linear for backwards compatibility.

If this enhancement will not be used often, can it be worked around with a few lines of script?

I suppose it could be done using a custom shader, but this would cost performance because the interpolation is done in shader code instead of native hardware.

Is there a reason why this should be core and not an add-on in the asset library?

Seems like a trivial addition when done in core, but a hassle when attempted as a library.

Mickeon commented 1 year ago

While I do understand the proposal, I wonder if there is a way to make this less verbose, more accessible. I find the whole name TEXTURE_FILTER_NEAREST_WITH_LINEAR_MIPMAPS and the opposite to be quite a mouthful to display and write.

ttencate commented 1 year ago

I think the long names aren't too bad because they're not used frequently. They'll often be selected from a list in the editor too, be it the inspector or autocomplete. I can't think of a way to shorten them without losing clarity.

The OpenGL constants like GL_NEAREST_MIPMAP_LINEAR are shorter, but I'm always confused which part refers to the mipmap and which part to the interpolation within the mipmap. Godot's current naming fortunately prevents this already.

It could also be split up into two orthogonal settings, i.e.

enum TextureFilter {
  TEXTURE_FILTER_NEAREST,
  TEXTURE_FILTER_LINEAR,
};
enum MipmapFilter {
  MIPMAP_FILTER_NONE,
  MIPMAP_FILTER_NEAREST,
  MIPMAP_FILTER_LINEAR,
};

Doing this in a non-confusing backwards compatible way would be a challenge, though.

clayjohn commented 1 year ago

Allow me to introduce you to https://docs.godotengine.org/en/latest/classes/class_projectsettings.html#class-projectsettings-property-rendering-textures-default-filters-use-nearest-mipmap-filter

ttencate commented 1 year ago

I did not know about that! But it's too limiting because it's project-wide; the 3D parts of the game do benefit from trilinear filtering.

Calinou commented 1 year ago

I did not know about that! But it's too limiting because it's project-wide; the 3D parts of the game do benefit from trilinear filtering.

We can probably add an equivalent project setting that only affects 2D samplers' mipmap filtering mode. This way, compatibility with existing projects is preserved.

ttencate commented 1 year ago

What if someone wants to use linear-within-nearest-mipmap filtering with some 2D textures but not others? For example, if some textures have their scale animated, you'd want to use trilinear on those, but not on the others. My proposal would allow this.

Are there any objections to the proposal, apart from the difficulty of picking names for the new constants?

Calinou commented 1 year ago

Are there any objections to the proposal, apart from the difficulty of picking names for the new constants?

Edit: An alternative way is to expose a separate boolean property, aptly named Use Nearest Mipmap Filter below the filter mode property in BaseMaterial3D and CanvasItemMaterial. It can be hidden when the chosen filter mode has no mipmaps. In custom shaders, sampler filter modes would still use a _bilinear suffix explicitly in the name (with existing trilinear ones kept as-is). This may be a better way to go, actually.

I've left the old answer below for posterity.


The issue is that adding new sampler modes will require adding them at the end of the enum to avoid breaking compatibility, which looks awkward. There's also high potential for confusion with users picking the "wrong" option for their project, as we usually go for the approach where the last options in the enums are the highest quality (and most expensive) ones.

Imagine we have the following:

I can picture a lot of users picking Linear Mipmaps Anisotropic (Bilinear) instead of Linear Mipmaps Anisotropic for realistic-looking 3D scene setups. We could explicitly mention (Trilinear) in the existing mipmaps options, but not everyone knows that trilinear filtering is higher-quality than bilinear filtering. (Remember that most modern games don't give you a choice nowadays, so a lot of younger users will never have seen the difference.)

The Use Nearest Mipmap Filter project setting name would also become a bit ambiguous, as it would effectively become a "force nearest mipmap filter" setting instead. It's not the end of the world though.

ttencate commented 1 year ago

Thank you for considering this!

The issue is that adding new sampler modes will require adding them at the end of the enum to avoid breaking compatibility, which looks awkward. There's also high potential for confusion with users picking the "wrong" option for their project, as we usually go for the approach where the last options in the enums are the highest quality (and most expensive) ones.

Agreed that ordering is important. Are enums in the editor and documentation ordered by their integer value, or by the order in which BIND_ENUM_CONSTANT is called? If it's the latter, the new modes can be inserted into the list wherever we please.

An alternative way is to expose a separate boolean property, aptly named Use Nearest Mipmap Filter below the filter mode property in BaseMaterial3D and CanvasItemMaterial.

What would the default value be? To preserve backwards compatibility, when Nearest With Mipmaps is selected, it should be true, but when Linear With Mipmaps is selected, it should be false.

Mickeon commented 1 year ago

Are enums in the editor and documentation ordered by their integer value, or by the order in which BIND_ENUM_CONSTANT is called? If it's the latter, the new modes can be inserted into the list wherever we please.

The latter.

Calinou commented 1 year ago

What would the default value be? To preserve backwards compatibility, when Nearest With Mipmaps is selected, it should be true, but when Linear With Mipmaps is selected, it should be false.

By default, we always use trilinear filtering, even when using the Nearest Mipmaps filter mode on a material. It's important to make the distinction between texture filtering (nearest/linear) and mipmap filtering (bilinear/trilinear – which you may actually interpret as "nearest" and "linear" respectively). Mipmap filtering refers to the way mips are blended between each other, rather than the texture filtering for distant mipmaps.

This means we can keep that new boolean property false by default, and have the Use Nearest Mipmap Filter project setting always act as if it was set to true.

I have a testing project here: https://github.com/Calinou/godot-mipmaps-test It uses linear + mipmaps by default, but if you change MovingPlane's material to use nearest mipmaps, it will still transition smoothly between mip levels when you run the project (even though the individual textures are nearest-neighbor filtered).

ttencate commented 1 year ago

I was basing myself on the naming and documentation, which says:

TEXTURE_FILTER_NEAREST_WITH_MIPMAPS
The texture filter reads from the nearest pixel in the nearest mipmap. The fastest way to read from textures with mipmaps.

Your test project shows that that is only correct if Use Nearest Mipmap Filter is enabled project-wide, which is not the default.

It's all very confusing :(

Calinou commented 1 year ago

It's all very confusing :(

That documentation is incorrect, so I fixed it: https://github.com/godotengine/godot/pull/83907