godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.14k stars 93 forks source link

Add a Default Texture Filter project setting for BaseMaterial3D, SpriteBase3D and Label3D #5228

Open Calinou opened 2 years ago

Calinou commented 2 years ago

Describe the project you are working on

The Godot editor :slightly_smiling_face:

Describe the problem or limitation you are having in your project

In 4.0.alpha, there is no longer a way to set the default texture filter mode for all BaseMaterial3Ds used in a project. This is available in 2D as a project setting, but there is no such project setting in 3D yet.

There are two main use cases to changing the default texture filter in 3D:

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

Add a Default Texture Filter project setting for BaseMaterial3D, similar to what's already provided for CanvasItem in 2D:

image

I'm not sure if a Default Texture Repeat project setting makes sense for 3D, as repeat is highly specific on the use case for the texture.

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

I started work on adding a default BaseMaterial3D texture filter project setting: https://github.com/Calinou/godot/tree/basematerial3d-add-default-texture-filter

I don't have time to finish it, so please look into taking over this work if you want to see this in 4.0. Also, the way it's currently implemented requires restarting the editor/engine to apply. This is not a deal-breaker, but it would be best if changing this setting worked without having to restart.

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

In some cases, yes, by assigning this script to a MeshInstance3D node. Example for forcing nearest-neighbor filtering with anisotropic filtering enabled on all materials:

@tool
extends MeshInstance3D

func _ready() -> void:
    for index in mesh.get_surface_count():
        var material := mesh.surface_get_material(index)
        material.texture_filter = BaseMaterial3D.TEXTURE_FILTER_NEAREST_WITH_MIPMAPS_ANISOTROPIC

However, considering how often this will be used, a script shouldn't be required for this. Also, a different workaround is needed for 3D scenes (such as glTF files), as it should work recursively on all MeshInstance3D nodes within the scene.

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

This is about restoring 3D import workflow usability to the level of Godot 3.x.

elvisish commented 1 year ago

Is this planned for the next beta? Currently I'm not even sure how to set the filter on any 3D materials on import?

Calinou commented 1 year ago

Is this planned for the next beta? Currently I'm not even sure how to set the filter on any 3D materials on import?

I don't have time to work on this currently, so someone else will have to take over the work I did in the branch linked in OP. As a result, there is no ETA for this being completed.

Currently I'm not even sure how to set the filter on any 3D materials on import?

You can attach a script to an imported 3D scene's root node that sets the material filter mode on every mesh's material recursively. The script in OP only does this on a single MeshInstance3D node, but it should be relatively easy to modify it so that it iterates on child nodes instead.

elvisish commented 1 year ago

You can attach a script to an imported 3D scene's root node that sets the material filter mode on every mesh's material recursively. The script in OP only does this on a single MeshInstance3D node, but it should be relatively easy to modify it so that it iterates on child nodes instead.

Thanks, is there no setting in the importer anymore for this? Hopefully it can be implemented by 4.0 stable as it's a pretty important feature.

ztc0611 commented 1 year ago

I'm almost certain that if this doesn't make it by stable it's going to be in the top 10 most common complaints by 3D devs. Unfortunately as yet I've not managed to get much progress on attempting to fix that half finished PR, and neither has anyone else.

ztc0611 commented 1 year ago

@Calinou Any chance you can sync this branch with master? I wanna give it another shot.

Calinou commented 1 year ago

@Calinou Any chance you can sync this branch with master? I wanna give it another shot.

Done :slightly_smiling_face:

prominentdetail commented 1 year ago

Can we get customizeable defaults for all the properties of a Material, instead of just this?

Calinou commented 1 year ago

Can we get customizeable defaults for all the properties of a Material, instead of just this?

What properties do you have in mind here? This is technically feasible, but it would require adding a lot of "Default" values to material enum properties and turning some booleans into enums. I'm not sure if it's worth the added complexity.

There's been discussion about adding a way to override BaseMaterial3D with a custom shader entirely (which would fit your use case), but this needs to be done in a way that exposes the same API for importers to use.

prominentdetail commented 1 year ago

What properties do you have in mind here?

I'd be interested in these for the most part:

Transparency > Alpha Scissor Alpha Scissor Threshold Shading Mode > Unshaded Disable Ambient Light Vertex Color: Use as Albedo Sampling: Filter > Nearest Repeat Shadows > Disable recieve shadows

ztc0611 commented 1 year ago

@Calinou can you try to give your attempt at this code a quick compile? I just went in to attempt to fix it again, and it compiled without issue. Wasn't the problem it failing to compile?

Calinou commented 1 year ago

I've pushed a rebased version that compiles, but the setting fails to do anything even after restarting the editor (possibly because it's set too late?). Please look into it if you have time :slightly_smiling_face:

Testing project for the branch: test_basematerial3d_default_filter.zip

MrGenie151 commented 1 year ago

i need this PLEASE

guillermo-st commented 1 year ago

Just as a sidenote, this proposal would be incredibly beneficial for working with popular plugins that import quake maps such as Qodot or TBloader. Those maps's meshes sometimes have embedded textures in them, and their filter settings get reset every time the map is reimported by the plugins! This is a major PITA because you will have to set the proper filter setting on a texture-per-texture basis, only for all of that effort to be wasted when you need to make changes to the map and the texture settings get reset.

Because of this problem, if you are working with low res textures and these plugins in godot4, it's best to just work with an untextured/blurry map until all of the geometry is considered final (which isn't really practical :P), and only then add the textures and change their filter settings.

elvisish commented 1 year ago

Just as a sidenote, this proposal would be incredibly beneficial for working with popular plugins that import quake maps such as Qodot or TBloader. Those maps's meshes sometimes have embedded textures in them, and their filter settings get reset every time the map is reimported by the plugins! This is a major PITA because you will have to set the proper filter setting on a texture-per-texture basis, only for all of that effort to be wasted when you need to make changes to the map and the texture settings get reset.

Because of this problem, if you are working with low res textures and these plugins in godot4, it's best to just work with an untextured/blurry map until all of the geometry is considered final (which isn't really practical :P), and only then add the textures and change their filter settings.

Actually, TBLoader has a setting on the importer for this very reason (option filter nearest).

guillermo-st commented 1 year ago

Actually, TBLoader has a setting on the importer for this very reason (option filter nearest).

Is that a new feature? Couldn't find anything the last time I used it. That's a lifesaver, at least for maps, thank you!

elvisish commented 1 year ago

Actually, TBLoader has a setting on the importer for this very reason (option filter nearest).

Is that a new feature? Couldn't find anything the last time I used it. That's a lifesaver, at least for maps, thank you!

It's been there since I've used it at the beginning of December! ;)

MrGenie151 commented 1 year ago

Just as a sidenote, this proposal would be incredibly beneficial for working with popular plugins that import quake maps such as Qodot or TBloader. Those maps's meshes sometimes have embedded textures in them, and their filter settings get reset every time the map is reimported by the plugins! This is a major PITA because you will have to set the proper filter setting on a texture-per-texture basis, only for all of that effort to be wasted when you need to make changes to the map and the texture settings get reset.

Because of this problem, if you are working with low res textures and these plugins in godot4, it's best to just work with an untextured/blurry map until all of the geometry is considered final (which isn't really practical :P), and only then add the textures and change their filter settings.

sombody in the Godot discord server (thanks hoot.avi) wrote me a simple workaround. it's kinda scuffed, but it works.

extends Node3D

func _ready():
    getallnodes(self) 

#Recursive function to get all children and subchildren, and run "makeNearest" on that node
func getallnodes(node):
    for N in node.get_children():
        if N.get_child_count() > 0:
            makeNearest(N)
            getallnodes(N)
        else:
            makeNearest(N)

#Function that makes meshinstance3d textures filter as nearest neighbor
func makeNearest(node):

    #If node is a MeshInstance3D node, make texture_filter == nearest neighbor
    if node.get_class() == "MeshInstance3D":
        for f in node.get_mesh().get_surface_count():
            var material = node.get_active_material(f)
            if material != null:
                material.set_texture_filter(0)
    elif node.get_class() == "CSGMesh3D":
        var material = node.get_material()
        if material != null:
            material.set_texture_filter(0)
    else:
        pass
nukeop commented 1 year ago

Hi, is anyone working on this? Can contributors take this and fix it?

Calinou commented 1 year ago

Hi, is anyone working on this? Can contributors take this and fix it?

Feel free to salvage the branch I linked above, but expect this work to be nontrivial.