Skip to content

Automatically assign PBR textures from base texture name in BaseMaterial3D#99838

Open
Calinou wants to merge 1 commit intogodotengine:masterfrom
Calinou:material-infer-from-texture-name
Open

Automatically assign PBR textures from base texture name in BaseMaterial3D#99838
Calinou wants to merge 1 commit intogodotengine:masterfrom
Calinou:material-infer-from-texture-name

Conversation

@Calinou
Copy link
Copy Markdown
Member

@Calinou Calinou commented Nov 29, 2024

This works based on a common set of texture name conventions that most texture sites out there follow. It works with both StandardMaterial3D and ORMMaterial3D. If mismatched textures are detected (compared to the current material type), a notice is displayed.

Since this is implemented in EditorInspectorPluginMaterial, this only affects textures set in the inspector, similar to the existing behavior where roughness and metallic are automatically set to 1.0 when assigning a texture to them.

Testing project: test_pbr_autofill.zip

Preview

editor_pbr_autofill.mp4

TODO

  • Move the BaseMaterial3D code to a static method, so that we don't need to create an instance of it when assigning textures.
  • Implement this for the Decal node as well (could be done in a separate PR).

@Calinou Calinou added this to the 4.x milestone Nov 29, 2024
@Calinou Calinou requested review from a team as code owners November 29, 2024 15:16
@fire fire requested a review from a team November 29, 2024 15:27
@lander-vr
Copy link
Copy Markdown
Contributor

Love this! Does this also work when drag-dropping from the FileSystem?

@fire
Copy link
Copy Markdown
Member

fire commented Nov 29, 2024

Since this is a hidden feature, can you add some documentation for it in the editor?

Comment thread scene/resources/material.cpp Outdated
@Calinou Calinou force-pushed the material-infer-from-texture-name branch from aa445d7 to d436854 Compare November 29, 2024 17:17
@Calinou Calinou requested a review from a team as a code owner November 29, 2024 17:17
@Calinou
Copy link
Copy Markdown
Member Author

Calinou commented Nov 29, 2024

Love this! Does this also work when drag-dropping from the FileSystem?

This currently doesn't apply to either drag-and-dropping onto an object in the 3D viewport, or drag-and-dropping onto a material property in the inspector. It only applies to drag-and-dropping onto a texture property in a material resource in the inspector.

I remember editing the drag-and-drop code at some point, so maybe it should be feasible to implement.

Since this is a hidden feature, can you add some documentation for it in the editor?

Done.

Comment thread doc/classes/BaseMaterial3D.xml Outdated
@Calinou Calinou force-pushed the material-infer-from-texture-name branch 2 times, most recently from 4b7201f to 9083c87 Compare November 29, 2024 17:40
@lander-vr
Copy link
Copy Markdown
Contributor

It only applies to drag-and-dropping onto a texture property in a material resource in the inspector.

Perfect! That's what I meant! :)

... so maybe it should be feasible to implement.

I assume here you mean drag dropping a single texture onto a meshinstance or material property, and it setting up a StandarMaterial3D/ORM for you with the complete textureset?

If that's the case, I assume it would be a unique material every time? (Seems like a hard problem to solve otherwise)
Considering that, I imagine it would lead to users who are not entirely familiar with this behavior finding themselves in a situation where they've dragged-and-dropped the same texture onto various meshes in a scene, assuming it would apply the same material. I imagine there are performance drawbacks to that, compared to having a single resource that's re-used, but even just considering UX it would probably not be the most intuitive.

@smix8
Copy link
Copy Markdown
Contributor

smix8 commented Nov 29, 2024

Sounds neat but please not as a hidden miracle.

Make this texture auto behavior an Editor setting that can be enabled by default but has the option to be disabled.

Some people get annoyed by the extra clicks of adding textures but I get at least equally annoyed when I have to stare at a frozen Godot due to texture load and add that blocks me from doing anything while firing its selective convenience feature that I dont need. I regularly only want to drag in the albedo texture for a quick model test, not loading all the other textures on purpose because it is so slow. I dont want to rename all those files just to avoid a hidden miracle feature.

@Calinou
Copy link
Copy Markdown
Member Author

Calinou commented Nov 29, 2024

If that's the case, I assume it would be a unique material every time?

Yes, but it's already the case with how the feature works currently.

@Calinou Calinou force-pushed the material-infer-from-texture-name branch from 9083c87 to 10d1f02 Compare November 29, 2024 18:38
@Calinou
Copy link
Copy Markdown
Member Author

Calinou commented Nov 29, 2024

Make this texture auto behavior an Editor setting that can be enabled by default but has the option to be disabled.

Done:

image

@Calinou Calinou force-pushed the material-infer-from-texture-name branch 2 times, most recently from 1bb24da to ea23454 Compare November 30, 2024 20:41
Comment thread doc/classes/BaseMaterial3D.xml Outdated
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.

This note is so much. It would be so INSANELY annoying to read and localize for absolutely everything where this applies. I also am not sure if most of this is necessary, given the setting is enabled by default, and the editor tells you about this action happening. I think the information of what naming standards automatically enable this feature need a better place to exist.

Comment thread doc/classes/EditorSettings.xml Outdated
@Calinou Calinou force-pushed the material-infer-from-texture-name branch from ea23454 to 9635cf0 Compare December 1, 2024 18:25
…ial3D

This works based on a common set of texture name conventions that most
texture sites out there follow. It works with both StandardMaterial3D
and ORMMaterial3D. If mismatched textures are detected (compared
to the current material type), a notice is displayed.

Since this is implemented in EditorInspectorPluginMaterial,
this only affects textures set in the inspector, similar to the existing
behavior where `roughness` and `metallic` are automatically set to `1.0`
when assigning a texture to them.
@Calinou Calinou force-pushed the material-infer-from-texture-name branch from 9635cf0 to 3349749 Compare December 2, 2024 23:40
@Geometror
Copy link
Copy Markdown
Member

Geometror commented Dec 18, 2024

I agree with @smix8 that this feature shouldn't be a hidden miracle, but disabling it by default hurts discoverability a lot.
What about adding an inspector button (e.g., "Auto-assign PBR textures") right below the material preview that opens a file dialog or directly assigns the textures if one of them was already set? This also has the advantage compared to the current approach that you have more control and can decide whether you need it on a case by case basis.

image

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.

Automatically assign PBR textures from base texture name in BaseMaterial3D

7 participants