Skip to content
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

Add a Texture2D wrapper for RenderingDevice raw textures #6964

Closed
joemarshall opened this issue May 28, 2023 · 11 comments
Closed

Add a Texture2D wrapper for RenderingDevice raw textures #6964

joemarshall opened this issue May 28, 2023 · 11 comments

Comments

@joemarshall
Copy link

Describe the project you are working on

Mobile low level glsl shaders for e.g. responsive water

Describe the problem or limitation you are having in your project

I'm building some shaders which need specific texture formats, access bits etc. and using the low level renderdevice stuff to create draw lists etc.

It all works fine and dandy in itself, I can run shaders against textures, output things into framebuffer textures. But then everything is stuck in renderdevice land. There's as far as I can see no simple way to get the output textures from my things into Godot texture2d objects. I can go the other way by calling texture_get_rd_texture in renderingserver, but inevitably ways of creating texture2d objects (imagetexture etc.) don't use the texture creation flags I want (e.g. they can't be bound to a framebuffer, or don't support copying from other textures or whatever).

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

I want a way to take an existing renderdevice texture and create a texture2d object from it.

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

In renderingserver, add a method texture_wrap_rd_texture to make a Texture object pointing at a particular render device texture.

Add 'create_from_rd_texture' static method in ImageTexture which uses the texture wrapping.

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

In theory one can do most things with viewports and shaders, but there are right now some limitations on what one can do with them (you can't set texture formats etc ), and I always end up at some point getting texture data to CPU and loading to an imagetexture for anything too complex which destroys performance.

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

It would allow the low level usage of GPU features in the same way that unity and unreal do it (raw access to draw lists, framebuffers, textures). We've got 99% of the features here, just no simple way of getting low level shader code back into the core Godot infrastructure which risks any use of low level shader stuff forcing you to move other stuff into low level code and generally adding work that you wouldn't have to do in other engines.

@Calinou Calinou changed the title Texture2d wrapper for renderdevice raw texture Add a Texture2D wrapper for RenderingDevice raw textures May 28, 2023
@joemarshall
Copy link
Author

I've done some further thinking on this -

I think this is staged in terms of what to do - first thing was to make multithreaded rendering work at all, which the fix to
godotengine/godot#78786 I think does.

Second, quite minimal stage in this would be to enable copying from low level textures into Godot imagetextures. That way if you have a process drawing into a renderdevice texture, you can copy back using renderingdevice.texture_copy

Third is the thing I suggest above, a wrapper class to either a)make a Texture object which you can set the underlying renderingdevice parameters of, or b)make a Texture object that wraps an existing renderingdevice texture. Suggestions on which way would be welcome

@clayjohn
Copy link
Member

clayjohn commented Jun 28, 2023

I don't have the brainpower to dive into this right now. But we have thought about the API for how we share textures between the RD and the built in renderers a little bit. Here is a previous post outlining a chat reduz and I had on the topic (the surrounding discussion is also useful as well).

We have been delaying actually working out the API until we finish properly implementing multithreading in the RD. Right now multithreading mostly works because we add mutexes to all the problematic functions (as you did in godotengine/godot#78794). While this works as a temporary measure, it is suboptimal for performance, and doesn't scale well to many cores.

Ideally the RD API would take advantage of where DX12 and Vulkan are thread safe and have as few locks as possible while allowing users to call RD functions from any thread. We want to have that done (or at least designed) before fully committing to a resource-sharing solution as the multithreading design may conflict with how easily resources can be shared.

@joemarshall
Copy link
Author

@clayjohn I've done a whole load of looking/messing around with this and I think there may be fewer issues with the mutex approach as used currently than you might think. Right now, it locks everything during submission of draw lists to the queue, and unlocks once draw_list_end is called. But the catch with this is that submitting commands is really quick in vulkan. As long as people follow sensible practices with their draw list code - i.e. setup all the relevant data, then called draw_list_begin, draw_list_draw etc., there is minimal overhead of the fact you can't actually do those calls in parallel right now, whereas you can generate the source data in parallel perfectly well. In theory I guess you could be using some complex cpu code to generate multiple vertex buffers on the fly or something, but that's a bit of an edge case.

@joemarshall
Copy link
Author

Will put more details over in:
#7163

@clayjohn
Copy link
Member

I think you are missing all our calls to _THREAD_SAFE_METHOD_. Pretty much all functions relating to resource creation/update are also locked behind the same mutex. The mutex makes sense

We really should allow users to create resources from any thread (and have them asynchronously transfer, using the transfer queue if available). To me, that is the thread safety piece that is blocking having a tighter access to RD textures (that being said, your notes on draw list thread safety in #7163 are also very good and something worth exploring more)

@clayjohn
Copy link
Member

The following is a suggestion from reduz:

Custom texture API in RenderingServer

API has to be added to RenderingServer for custom texture API registration:

// in RenderingServer

	virtual RID texture_rd_create(const RID& p_rd_texture) = 0;

This takes actual RenderingDevice textures so they can be easily used as regular textures in materials, etc.

If the texture was not created for sampling, it should error (we should add API in RenderingDevice to query texture type, size, usage flags, etc to make it easier to use with this).

Texture2DRD, TextureCubeRD, Texture3DRD, etc.

Higher level resources that take a RenderingDevice RID and call internally texture_rd_create (and texture_replace if set again).

This allows users to create custom textures in RenderingDevice, use them for compute, etc. Then simply instantiate a Texture2DRD, set the texture and use it in any material, node, etc..

@joemarshall
Copy link
Author

joemarshall commented Jun 30, 2023

I think you are missing all our calls to _THREAD_SAFE_METHOD_. Pretty much all functions relating to resource creation/update are also locked behind the same mutex. The mutex makes sense

We really should allow users to create resources from any thread (and have them asynchronously transfer, using the transfer queue if available). To me, that is the thread safety piece that is blocking having a tighter access to RD textures (that being said, your notes on draw list thread safety in #7163 are also very good and something worth exploring more)

Ah yes - I was primarily thinking of / working with draw lists because in the context I'm in I'm basically wanting to keep everything on GPU, so texture upload is not an issue, but I agree that async texture upload and download would be great. Texture upload I can see isn't an api change, but texture download presumably would need a callback mechanism in godotscript?

I think it is quite doable anyway - essentially if draw lists add barriers for: a) framebuf textures, b) storage textures, and c) any textures used in uniform sets, then texture upload operations could be fire and forget. I think the barriers will be needed anyway if draw list things split out to make multiple command buffers .

The only question in my head in the upload case is how to handle hanging on to the buffer data, ideally without copying, and for the download case how to callback into godotscript with the data nicely.

Having said that, it seems to me that there is a clear two stage process here -

  1. update drawlists to use command buffers and make that work correctly,
  2. implement async vertex buffer upload
  3. implement async texture upload / download (which depends on (1) being done correctly)

@sesores
Copy link

sesores commented Sep 14, 2023

Hi everyone!

It's a fanatastic feature thet we will be able to share data between RenderingDevices! Thank you!

But how does it work the other way without the CPU roundtrip? So how can I sample a regular Texture2D/3D/Whatever in my compute shader? I don't need to modify just read the texture.

I just can't pass - or at least I don't know how - the texture RID to a compute sampler uniform.

@sesores
Copy link

sesores commented Sep 14, 2023

Ohhh. Maybe I can pass the RID returned by RenderingServer.texture_get_rd_texture()? 😅

@clayjohn
Copy link
Member

Implemented by godotengine/godot#79288

@clayjohn
Copy link
Member

Ohhh. Maybe I can pass the RID returned by RenderingServer.texture_get_rd_texture()? 😅

Yep!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants