Skip to content

Conversation

@chocola-mint
Copy link
Contributor

@chocola-mint chocola-mint commented Feb 25, 2025

Closes godotengine/godot-proposals#3266

godot-sprite3d-custom-shader-demo.mp4

This PR lets custom shaders (via Material Override) access the sprite texture used by SpriteBase3D. (and its child classes Sprite3D and AnimatedSprite3D by extension)

Validation is implemented to warn the user if a Material Override without the necessary uniforms (namely uniform sampler2D texture_albedo and uniform ivec2 albedo_texture_size) is being used. The "Flags" group properties of SpriteBase3D are also hidden when a Material Override is being used, as those properties are used in the default automatic material generation. (StandardMaterial3D::get_material_for_2d()) The user can reimplement those features in the custom shader code anyway, so they have been hidden.

Side note: This pattern of requiring the shader to have uniforms of specific names and types can be turned into a reusable "shader interface/trait" system if needed. Users could make their own high-level 3D nodes that send data to shaders like SpriteBase3D does. (Maybe a more advanced version of Sprite3D that assigns normal maps and SDF texture maps as uniforms) The main benefit of such a system would be to reduce the amount of builtin shader types, though. (No need to add a new sprite_3d shader type) Though this probably would have to be a separate proposal.


This PR was originally made to add custom shader support for AnimatedSprite3D in particular, but it is evident that implementing the support for SpriteBase3D makes more sense here.

@chocola-mint chocola-mint requested a review from a team as a code owner February 25, 2025 10:14
@AThousandShips AThousandShips added this to the 4.x milestone Feb 25, 2025
@TokageItLab TokageItLab requested review from a team and clayjohn February 26, 2025 08:45
@chocola-mint chocola-mint force-pushed the sprite-3d-custom-shaders branch from 8330368 to 36e80b5 Compare March 4, 2025 04:55
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally, it works as expected.

Testing project: test_pr_103274.zip

Some feedback:

  • There should be a Sprite3D shader template you can choose from, with the correct uniforms already in place:

image

The shader template could also have render_mode unshaded; below shader_mode spatial;, as most Sprite3Ds are used in an unshaded manner (this is their default property).

Note that if #103312 is merged before this PR, the cache key should vary depending on the custom shader used. It should be possible for meshes with the same ShaderMaterial resource to be reused, but different shaders will require a different cache key.

@chocola-mint chocola-mint force-pushed the sprite-3d-custom-shaders branch from 36e80b5 to 79b7adf Compare March 6, 2025 08:01
@chocola-mint chocola-mint requested a review from a team as a code owner March 6, 2025 08:01
@chocola-mint chocola-mint force-pushed the sprite-3d-custom-shaders branch from 79b7adf to 9be33cc Compare March 6, 2025 08:05
@chocola-mint
Copy link
Contributor Author

chocola-mint commented Mar 6, 2025

Tested locally, it works as expected.

Testing project: test_pr_103274.zip

Some feedback:

  • There should be a Sprite3D shader template you can choose from, with the correct uniforms already in place:

image

The shader template could also have render_mode unshaded; below shader_mode spatial;, as most Sprite3Ds are used in an unshaded manner (this is their default property).

godot-sprite3d-custom-shader-template.mp4
  • The Sprite3D shader template is only shown when the shader mode is set to Spatial.
  • A minimal example that uses vertex color (thus supporting SpriteBase3D.modulate) and the albedo texture is attached.
  • The required uniforms are properly commented as such.

Note that ShaderCreateDialog doesn't seem to support adding custom shader templates in the first place, so I've taken the liberty to make it a bit easier to add hardcoded shader templates. Perhaps a future feature proposal could expand on this and make it more similar to script templates in terms of ergonomics.

@chocola-mint chocola-mint force-pushed the sprite-3d-custom-shaders branch from 9be33cc to 3f13a5a Compare March 20, 2025 05:11
@BastiaanOlij
Copy link
Contributor

Just looking at this because it came up during the rendering meeting, with clay on holiday don't want to make an in-depth judgement call.

But my question would be, why make this work with a special uniform? Why not make this work with the normal albedo color hint so the code is the same as with normal geometry? What makes this a different situation?

@chocola-mint
Copy link
Contributor Author

Just looking at this because it came up during the rendering meeting, with clay on holiday don't want to make an in-depth judgement call.

But my question would be, why make this work with a special uniform? Why not make this work with the normal albedo color hint so the code is the same as with normal geometry? What makes this a different situation?

Oh, I was admittedly not aware of the source_color hint's existence.

The design here was so the code path is consistent between both cases. texture_albedo and texture_albedo_size are both uniforms that are configured for the default Sprite3D material. (StandardMaterial3D::get_material_for_2d)

I see how it's possible to replace texture_albedo by looking for a sampler2D uniform with HINT_SOURCE_COLOR instead. There isn't an equivalent hint for texture_albedo_size, however.

Though texture_albedo_size is only used to calculate the MSDF and is technically optional. (My example there didn't use it either) So we could just not set texture_albedo_size for custom materials. What do you think?

@chocola-mint chocola-mint force-pushed the sprite-3d-custom-shaders branch from 3f13a5a to 71dc166 Compare June 6, 2025 15:52
@chocola-mint
Copy link
Contributor Author

@BastiaanOlij

(Rebased to fix conflict with ad9b661 )

Follow up: I think there's an actual argument against using HINT_SOURCE_COLOR (corresponding to the source_color hint) here.

  • A quick search into the engine's source code shows that there's no existing code that looks for a uniform with a specific hint and assigns values to it.
  • HINT_SOURCE_COLOR is mostly used so Godot knows to make a texture work as a color texture.
    • It's not even possible to retrieve this information at the moment. ShaderLanguage::uniform_to_property_info (used by RenderingServer::get_shader_parameter_list) is unable to include this information and we can't tell if a sampler2D uniform is source_color, hint_normal, or some other hint.
    • Even if we could, this would mean that uniform order now matters and if someone has multiple source_color uniforms, they must be aware that the first will be assigned the sprite texture by Sprite3D.

As such, I think the current approach is fine as-is.

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.

Add support for custom shaders on AnimatedSprite3D

4 participants