Texture filtering does not update properly

[genacvid]

Tested versions

-Reproducible in: v4.4.dev4.official [36e6207bb], v4.4.dev5.official [9e6098432] -Not reproducible in: v4.4.dev3.official [f4af8201b]

System information

Godot v4.4.dev5 - Manjaro Linux #1 SMP PREEMPT_DYNAMIC Tue Oct 8 03:11:08 UTC 2024 on X11 - X11 display driver, Multi-window, 3 monitors - Vulkan (Forward+) - dedicated AMD Radeon RX 5500 XT (RADV NAVI14) - AMD Ryzen 5 3600 6-Core Processor (12 threads)

Issue description

When changing texture filtering, changes are not updated at runtime. In the editor, the current scene needs to be switched back and forth.

Intended behavior (in 4.4dev3 and earlier)

https://github.com/user-attachments/assets/e8bc5217-a2bb-4236-a198-4ca7796053f1

Broken behavior (in 4.4dev4 and later)

https://github.com/user-attachments/assets/190e36fe-fbd5-47f7-addb-1ede93a133dc

Steps to reproduce

1. Create a new mesh and assign a new StandardMaterial3D
2. Change the texture_filter property on the material
3. Observe the mesh - texture filtering does not change until the scene is switched in the editor

Attempting to change the mesh at runtime is also broken in affected versions.

The attached scene in the MRP has a button to toggle "texture smoothing". This button works in 4.4dev3 and prior, but not 4.4dev4 and 5.

Minimal reproduction project (MRP)

texture_filter_mrp.zip

[BlackShift]

Bisected to [e2c6daf7eff6e0b7e2e8d967e95a9ad56e948231] Implement asynchronous transfer queues, thread guards on RenderingDevice. Add ubershaders and rework pipeline caches for Forward+ and Mobile.

# good: [f4af8201bac157b9d47e336203d3e8a8ef729de2] Merge pull request #96309 from Geometror/fix-ge-cs-crash
git bisect good f4af8201bac157b9d47e336203d3e8a8ef729de2
# bad: [7815ccbdd554eb7836c7a2cca2dda5c3003e4a23] Merge pull request #98294 from Calinou/texture-placeholders-use-shared-copy
git bisect bad 7815ccbdd554eb7836c7a2cca2dda5c3003e4a23
# bad: [54f6a1bf64c22b3d513b0bf1fe68f83d5ea41c12] Fix lookup symbol for enum members to search a correct code definition
git bisect bad 54f6a1bf64c22b3d513b0bf1fe68f83d5ea41c12
# bad: [84768aba8a55ebfba4abc6b9a220161feb2d2892] Merge pull request #97514 from bruvzg/font_contour_info
git bisect bad 84768aba8a55ebfba4abc6b9a220161feb2d2892
# good: [903c3bc154cebf9d002f651edbccf25924411df9] Merge pull request #97727 from SlugFiller/llvm-computed-goto
git bisect good 903c3bc154cebf9d002f651edbccf25924411df9
# bad: [5314793ac7d80eb33b1b71abd97cf230fcfc6386] Merge pull request #97519 from timothyqiu/itemlist-at
git bisect bad 5314793ac7d80eb33b1b71abd97cf230fcfc6386
# bad: [f3694a6933ec729f0b433f5f73e89c3ae6cd6878] Merge pull request #95389 from Gaktan/graph_node_stretch_fix
git bisect bad f3694a6933ec729f0b433f5f73e89c3ae6cd6878
# good: [2e144928793f17ebd70e1475bb7a7f4fd1095484] Merge pull request #97739 from adamscott/fix-get-bus-null
git bisect good 2e144928793f17ebd70e1475bb7a7f4fd1095484
# good: [3b41f731057f481455ac24ed60bc6b368f560cfa] Merge pull request #80473 from KoBeWi/they're_the_same_picture
git bisect good 3b41f731057f481455ac24ed60bc6b368f560cfa
# bad: [98deb2a0005cf654e667679cd72904d9b5d4c734] Merge pull request #90400 from DarioSamo/transfer_and_pipelines
git bisect bad 98deb2a0005cf654e667679cd72904d9b5d4c734

[AThousandShips]

CC @DarioSamo

[BlackShift]

I found a work around,

Setting certain properties, marks the material as dirty, but doesn't call _update_shader() by getting the the RID of the material, you can force this update to occur. Changing the MRP's code will work to include this fix in-game, but the editor would need a tool script.

extends Node3D

func _on_check_button_toggled(toggled_on: bool) -> void:
    if toggled_on:
        $MeshInstance3D.get_surface_override_material(0).texture_filter = StandardMaterial3D.TEXTURE_FILTER_LINEAR_WITH_MIPMAPS
    else:
        $MeshInstance3D.get_surface_override_material(0).texture_filter = StandardMaterial3D.TEXTURE_FILTER_NEAREST
    $MeshInstance3D.get_surface_override_material(0).get_rid()

When would it be desirable to call this update?

[DarioSamo]

There should be no need to do that, I'll investigate how to fix the regression during the week.

[DarioSamo]

Setting certain properties, marks the material as dirty, but doesn't call _update_shader()

That's pretty much the reason behind the regression yes. I took out the calls to _update_shader() as we needed to defer shader updates to the moment the RID is requested. However, it seems not all properties equally fire off an event to notify others that the material has been updated.

Calling notify_property_list_changed(); afterwards does fix it, but I feel that carries a heavier cost than it seems to be at first, and there seems to be some pattern behind whether the callback is called or not on the other setters.

I'll see what options we have to fix this.

[DarioSamo]

You can check out https://github.com/godotengine/godot/pull/99716 and see if it fixes the issue and doesn't introduce any others.

[BlackShift]

I tested the patch and it does fix it for the MRP.

However, it seems not all properties equally fire off an event to notify others that the material has been updated.

I would imagine the reason behind this is due to a difference of a bound uniform vs code change in the shader output. This might also be something that we expose via bind_methods() for the user to control. But I'm worried it would break backward compatibility.