Skip to content

[DRAFT] DrawableTextures #105701

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ColinSORourke
Copy link

@ColinSORourke ColinSORourke commented Apr 24, 2025

godotengine/godot-proposals#7379

This PR is an implementation of @reduz's proposal for Drawable Textures in the Godot Engine. Authored primarily by me, with some much appreciated assistance from @clayjohn and @BastiaanOlij. This is still a WORK-IN-PROGRESS, very open to feedback and suggestions.

See Demo Vid:

DrawableTexturesDemoVid.mov

Download Demo Project: DrawableTextureDemoProj.zip

The implementation follows Reduz’s specification pretty strictly, with a few departures based on feedback from Rendering Contributor Meetings. Most importantly, we decided to leave implementation of Blit_Polygon functions for a future pull request due to complexity. We also added a p_material parameter to Blit_Rect & Blit_Rect_Multi on the user exposed resource to allow users to pass their own materials into the resource (if left blank, still defaults to 'default material').

Currently this PR implements:
The Rendering Server API for DrawableTextures

enum TextureDrawableFormat {
   TEXTURE_DRAWABLE_FORMAT_RGBA8,
   TEXTURE_DRAWABLE_FORMAT_RGBA8_SRGB, // Use this if you want to read the result from both 2D (non-hdr) and 3D.
   TEXTURE_DRAWABLE_FORMAT_RGBAH,
   TEXTURE_DRAWABLE_FORMAT_RGBAF,
};

// Create
RID texture_drawable_create(int p_width, int p_height, TextureDrawableFormat p_format,bool p_with_mipmaps = false);

// Blit a rect
void texture_drawable_blit_rect(const Vector<RID> &p_textures, const Rect2i& p_rect, RID p_material,const Color& p_modulate,const Vector<RID>& p_source_textures,int p_to_mipmap=0); 

// Utility functions
RID texture_drawable_get_default_material(); // To use with simplified functions in DrawableTexture2D

The new ShaderType, TextureBlit Shaders

shader_type texture_blit
// Optional render modes:
render_mode blend_mix /* default , also available: blend_add, blend_sub, blend_mul */;

uniform sampler2D source_texture : hint_blit_source;
uniform sampler2D source_texture2 : hint_blit_source2;
// up to 4 sources

void blit() {
     // Read sources and blit
     COLOR = texture(source_texture,UV) * MODULATE;
     COLOR2 = texture(source_texture2,UV);
     // up to COLOR4
}

And several user-exposed Resource classes to interact with DrawableTextures, most importantly of course, DrawableTexture2D

class DrawableTexture2D : public Texture2D {
   enum DrawableFormat {
       DRAWABLE_FORMAT_RGBA8,
       DRAWABLE_FORMAT_RGBA8_SRGB,
       DRAWABLE_FORMAT_RGBAH,
       DRAWABLE_FORMAT_RGBAF,
   };
    void setup(int p_width, int p_height, DrawableFormat p_format,bool p_use_mipmaps=false);
    void blit_rect(const Rect2i p_rect, const Ref<Texture2D>& p_source, const Color& p_modulate,int p_mipmap=0, const Ref<Material> &p_material = Ref<Material>());
    void blit_rect_multi(const Rect2i p_rect, const TypedArray<Texture2D>& p_sources, const TypedArray<DrawableTexture2D>& p_extra_targets, const Color& p_modulate,int p_mipmap=0, const Ref<Material> &p_material = Ref<Material>());

   void generate_mipmaps();
};

However all in the Compatibility Renderer (Drivers/GLES3) only. All of the code for the Forward+ Renderer (Servers/Rendering/Renderer_RD) is non-functional in several ways at the moment, do not expect it to work or look correct.

The files with the most relevant new code are:

  • drivers/gles3/shaders/tex_blit.glsl
  • drivers/gles3/storage/texture_storage (.h & .cpp)
  • drivers/gles3/storage/material_storage (.h & .cpp)
  • scene/resources/drawable_texture_2d (.h & .cpp)

But I made a detailed listing of the currently modified files in a spreadsheet here

TODO Before I think this can be considered complete:

  1. Implement mipmap functionality
  2. Add guardrails to catch & throw appropriate errors
  3. Implement all behaviors in the Forward+ Renderer
  4. Address any major comments & feedback on this & the original proposal
  5. Write documentation

Bugsquad edit: Implements and fixes godotengine/godot-proposals#7379

@ColinSORourke ColinSORourke requested review from a team as code owners April 24, 2025 06:34
@Chaosus Chaosus added this to the 4.x milestone Apr 24, 2025
@ColinSORourke ColinSORourke marked this pull request as draft April 24, 2025 07:17
@@ -1370,6 +1594,15 @@ Vector<Ref<Image>> TextureStorage::texture_3d_get(RID p_texture) const {
return ret;
}

void TextureStorage::texture_drawable_generate_mipmaps(RID p_texture) {
// TODO: Generate mipmaps code
Copy link
Contributor

Choose a reason for hiding this comment

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

Just call glGenerateMipmap ;)

Copy link
Member

@Calinou Calinou Apr 24, 2025

Choose a reason for hiding this comment

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

Just call glGenerateMipmap ;)

Isn't that pretty slow? We typically need a dedicated approach to generate mipmaps in real-time with reasonable performance. This is the reason why ViewportTexture doesn't feature mipmap generation, but screen_texture does.

This also applies to modern low-level APIs which have no direct equivalent of glGenerateMipmap() to my knowledge.

@@ -1414,6 +1586,11 @@ void TextureStorage::texture_proxy_update(RID p_texture, RID p_proxy_to) {
}
}

// Output textures in p_textures must ALL BE THE SAME SIZE
void TextureStorage::texture_drawable_blit_rect(const TypedArray<RID> &p_textures, const Rect2i &p_rect, RID p_material, const Color &p_modulate, const TypedArray<RID> &p_source_textures, int p_to_mipmap) {
// TODO: Write this code for Renderer_RD
Copy link
Contributor

Choose a reason for hiding this comment

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

So for RD this is a little more involved than with OpenGL but the functionality already exists. If I recall correctly it's a matter of adding some extra logic when initialising the texture to create texture views for each miplayer.

Then here you can loop through the layers and use the copyeffect to scale down each layer. There might even be a function that already handles this loop for you. I'll see if I can find an example of this, pretty sure there are a few examples around the radiance map code.

@@ -390,7 +394,10 @@ const ShaderLanguage::KeyWord ShaderLanguage::keyword_list[] = {
{ TK_HINT_SCREEN_TEXTURE, "hint_screen_texture", CF_UNSPECIFIED, {}, {} },
{ TK_HINT_NORMAL_ROUGHNESS_TEXTURE, "hint_normal_roughness_texture", CF_UNSPECIFIED, {}, {} },
{ TK_HINT_DEPTH_TEXTURE, "hint_depth_texture", CF_UNSPECIFIED, {}, {} },

{ TK_HINT_BLIT_SOURCE, "hint_blit_source", CF_UNSPECIFIED, {}, {} },
Copy link
Contributor

Choose a reason for hiding this comment

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

This is just thinking out load, there might simply be too much of an issue in the shader compiler not being able to handle this. But could we just have one hint (e.g. hint_blit_source) and have it work on a first come, first serve basis?
E.g. the first uniform with hint_blit_source becomes our first source, the second uniform becomes our second source, etc.
With a compiler error if more than 4 exist.

Copy link
Author

Choose a reason for hiding this comment

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

If I understand correctly, the relevant code to try that approach is in shader_compiler.cpp,

_dump_node_code, lines 931->939

Which at a cursory glance only operates on a single token or line at a time? Those lines would need to read/write to some context variable that says how many 'sources' have already been set, and I'm unfamiliar with whether that function is supposed to access any outside context.

@BastiaanOlij
Copy link
Contributor

I had a first cursory glance at the implementation, already witnessing this being build as I was helping @ColinSORourke out, in broad strokes this looks really good. I need time to dive into the detail and do some proper stress testing with it and see if I can find any issues.

@RodZill4 , you might want to take a look at this as well, I think this would be very valuable for Material Maker. That would be a good stress test for writing to the 4 textures as I can imagine that generating texture ORMs this way could be nice.

@ColinSORourke one thing I'm wondering about as well, but this may be worth a follow up PR, instead of supplying 4 drawable textures to write too, we should also support supplying a texture array of up to 4 layers.

@ColinSORourke
Copy link
Author

ColinSORourke commented Apr 24, 2025

@ColinSORourke one thing I'm wondering about as well, but this may be worth a follow up PR, instead of supplying 4 drawable textures to write too, we should also support supplying a texture array of up to 4 layers.

@BastiaanOlij Just making sure I understand correctly, do you mean
A) To change the code at the base, GLSL layer, to support uniform sampler2D name[4] instead of 4x uniform sampler2D
OR
B) to change the code at the user exposed abstraction to support passing a TextureLayered resource (AKA Texture2DArray) as a parameter instead of TypedArray<Texture2D>

Edit: I'll also note that the user exposed abstraction is currently always called from 1 of the output textures in the first place, and accepts up to 3 'other' output textures. So we would either need to perform the operation to merge the 'original' output into the TextureLayered 3 others, or design a new abstraction that can be called independently from any target.

@BastiaanOlij
Copy link
Contributor

B) to change the code at the user exposed abstraction to support passing a TextureLayered resource (AKA Texture2DArray) as a parameter instead of TypedArray

This one, though it's really an edge case we might want to reserve until we have a proper use case for it. I can see there might be some benefits to doing so when manipulating stereo images.

@Bonkahe
Copy link
Contributor

Bonkahe commented Apr 24, 2025

After some testing on the example project, firstly awesome work! second are uniforms meant to be unsupported? I tested around with sending custom data to COLOR instead of the blited texture, and one thing I tried was a uniform of source color, and that threw an error.
Shader in question:

shader_type texture_blit;
render_mode blend_mix;

uniform vec4 ColorMod : source_color;

uniform sampler2D source_texture : hint_blit_source;
uniform sampler2D source_texture2 : hint_blit_source2;
uniform sampler2D source_texture3 : hint_blit_source3;
uniform sampler2D source_texture4 : hint_blit_source4;

void blit() {
	// Copies from each whole source texture to a rect on each output texture.
	//COLOR = texture(source_texture, UV) * MODULATE;
	//COLOR = vec4(UV.x, UV.y, 0.0, 1.0);
	COLOR = ColorMod;
	COLOR2 = texture(source_texture2, UV);
	COLOR3 = texture(source_texture3, UV);
	COLOR4 = texture(source_texture4, UV);
}

Errors:

E 0:00:01:684   DrawTexture.gd:32 @ blitRandom(): TexBlitShaderGLES3: Fragment shader compilation failed:
0(72) : error C1503: undefined variable "m_ColorMod"

  <C++ Source>  drivers\gles3\shader_gles3.cpp:265 @ _display_error_with_code()
  <Stack Trace> DrawTexture.gd:32 @ blitRandom()
E 0:00:01:684   DrawTexture.gd:32 @ blitRandom(): Method/function failed.
  <C++ Source>  drivers\gles3\shader_gles3.cpp:407 @ ShaderGLES3::_compile_specialization()
  <Stack Trace> DrawTexture.gd:32 @ blitRandom()

As an aside, would it be possible to sample the current texture/destination UV we are copying into inside of the blit shader? or is that a no go? I assume it's not possible but I just wanted to be sure.

EDIT:
Blend modes other than mix do not seem to be working? seemingly no visual difference happened when changing it.

EDIT2:
Realized FragCoord works, just to get the UV scale is dependent on dividing it by the resolution of the target texture, only real need now would be able to sample the destinations current value, tried screentexture but it obviously doesn't work.

@RodZill4
Copy link
Contributor

@RodZill4 , you might want to take a look at this as well, I think this would be very valuable for Material Maker. That would be a good stress test for writing to the 4 textures as I can imagine that generating texture ORMs this way could be nice.

Hmm I moved all Material Maker rendering to compute shaders, and already write to several textures at once (when painting).
I can see a use for this in other projects, though (the VR scene edit one), but will this be forward+ only?

@ColinSORourke
Copy link
Author

ColinSORourke commented Apr 24, 2025

@Bonkahe

second are uniforms meant to be unsupported? I tested around with sending custom data to COLOR instead of the blited texture, and one thing I tried was a uniform of source color, and that threw an error.

Yes they are meant to be supported, great catch. I do see the bits of GLSL I'm missing, and am going to get to work on that.

As an aside, would it be possible to sample the current texture/destination UV we are copying into inside of the blit shader? or is that a no go? I assume it's not possible but I just wanted to be sure.

I can definitely see how that would be helpful, I don't think it would be too difficult, I'll just investigate the best way to surface those values to the user.

@RodZill4

but will this be forward+ only?

Funnily enough, currently it's Compatibility renderer only! Learning how to correctly compile Shaders in Forward+ tripped me up a scooch, so I focused on implementing in Compatibility Renderer to get a working implementation sooner. By the end, this feature should be equally available in both Render pipelines.

@ColinSORourke
Copy link
Author

Just pushed (4/26)

  • Fix for using Uniforms from custom materials
  • Fix for correct blending behaviors based on Blend Mode

@Bonkahe
Copy link
Contributor

Bonkahe commented Apr 27, 2025

Just pushed (4/26)

  • Fix for using Uniforms from custom materials
  • Fix for correct blending behaviors based on Blend Mode

Looks good, uniforms working just fine, as well as blend modes.

Did notice one problem, I was unable to build until I added a return value on line 1727 in \servers\rendering\renderer_rd\storage_rd\texture_storage.cpp

I just added this: return RID(); obviously this is just placeholder but something to be aware of, regardless looking great!

@ColinSORourke
Copy link
Author

Just pushed (5/16)

Forward+ Renderer implementation! Demo Project is unchanged, you should be able to run the same demo project on either renderer by swapping Renderer in Project settings.

Note:

  • Due to this issue, Blend_Sub & Blend_Mul don't behave identically between renderer settings.

Still to-do for 'completion'

  • Forward+ MipMap function
  • Rigorous Testing
  • Documentation
  • Feedback?

A few more talking points (I'll bring these up next Rendering Meeting as well)

  • Near nothing about the actual DrawableTexture resource is handled differently than a regular Texture2D - I'm curious if 'Drawable' could just be a flag on any Texture2D instead of a distinct resource?
  • Given that the Vertex Shader portion is not exposed to users, and we are not implementing blit_polygon at the moment, would it be worthwhile to add a Rotation Parameter to blit_rect, to rotate the target rect?
  • A pretty common use case will be sticking with all the defaults for this - what else should the Default TextureBlit shader cover, and what should be the default setup for a DrawableTexture
  • Should DrawableTextures and Blit_Rect support more Color & Blend Formats? What would need to be done for this to be allowed?

@beicause
Copy link
Contributor

beicause commented May 17, 2025

I tried this PR and the demo project. It works normally.

I'm bit hard to understand the usage of DrawableTexture2D.blit_rect_multi. Why it is needed instead of using 4 drawable textures separately and calling DrawableTexture2D.blit_rect on each?

Also why does DrawableTexture2D only accept 4 sources instead of using them as material uniforms? Custom uniform textures seem don't work and are always black.

@ColinSORourke
Copy link
Author

ColinSORourke commented May 17, 2025

@beicause

I'm bit hard to understand the usage of DrawableTexture2D.blit_rect_multi. Why it is needed instead of using 4 drawable textures separately and calling DrawableTexture2D.blit_rect on each?

Performance reasons - faster to make a single call to the GPU with 8 textures, than 4 calls to the GPU with 2 textures each. You can still call 4x Blit_Rects individually if you so choose

Also why does DrawableTexture2D only accept 4 sources instead of using them as material uniforms? Custom uniform textures seem don't work and are always black.

Extra Material Uniforms with textures should work - I'll investigate and fix.

@ColinSORourke
Copy link
Author

Just Pushed (5/17)

Fixes for binding custom Texture Uniforms from User Materials.

@beicause

@CarpenterBlue
Copy link

Would it be possible to add a way to erase from the textures? Erasing from textures is nearly impossible in whole of Godot sans using blend_mode substract which is sadly unusable in my case. I would have usecase for that right now.

I am building custom light system for my game because the built-in one is ugly, lacks features and is buggy and I am constantly running into same issue of needing to remove alpha from texture. It's really hard, Godot offers almost no ways of achieving that.
I made my own custom LightOccluders but I need a way to carve out the polygons from the light texture, my current, suboptimal solution is adding the background color on top of the light texture. Which make the lights blendable if they are in certain order but also leaves shadow leaks if the order is incorrect. as demonstrated in the video.

Godot_v4.4.1-stable_win64_0FnPjEcd77.mp4

If I could erase alpha from the drawable texture, I could build lights instead of shadows. which would stop the shadow leaking.

@Calinou
Copy link
Member

Calinou commented May 22, 2025

Erasing from textures is nearly impossible in whole of Godot sans using blend_mode substract which is sadly unusable in my case.

You may be interested in #102366, which exposes many more blend factors for use in custom shaders.

@beicause
Copy link
Contributor

I think we could expose a argument like RDPipelineColorBlendState (or a new data class for this) godotengine/godot-proposals#12443 (comment). So you can do custom blend by recreating the pipeline instead of creating a new shader.

@CarpenterBlue
Copy link

CarpenterBlue commented May 23, 2025

Erasing from textures is nearly impossible in whole of Godot sans using blend_mode substract which is sadly unusable in my case.

You may be interested in #102366, which exposes many more blend factors for use in custom shaders.

Actually, as far as I understand shaders, blend modes are completely useless for me in this regard as they affect whole texture, I need to partially erase texture which is possible only with viewports or canvas groups. Having viewport per light is not good.

I need to be able to carve out polygons from the texture's alpha, using blend_mode would affect other textures which is not desired. Notice in the following demonstration that I can change background color.

Photoshop_Tr98OoMy7R.mp4

In this image, the mask in photoshop is applied AFTER the texture. Erasing the alpha. This is not replicable with clip_children.

image

Here is video demonstration of the desired and undesired behavior. Godot still does not have an answer for it.
I hope drawable textures would finally address it.

Photoshop_ernf5Y6nzY.mp4

Here are the lights.

Masked Light

Implementing DrawableTextures based on: godotengine/godot-proposals#7379
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.

Implement Drawable Textures
8 participants