Expose a function to create textures from a native handle in the compatibility renderer #96928
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dsnopek
force-pushed
the
rename-and-expose-texture-create-external
branch
5 times, most recently
from
a3fe3a4 to
290570a
Compare
BastiaanOlij
approved these changes
Sep 18, 2024
| @@ -1108,6 +1108,195 @@ void TextureStorage::texture_proxy_initialize(RID p_texture, RID p_base) { | |||
| tex->proxies.push_back(p_texture); | |||
| } | |||
|
|
|||
| // Note: We make some big assumptions about format and usage. If developers need more control, | |||
| // they should use RD::texture_create_from_extension() instead. | |||
There was a problem hiding this comment.
Is it correct this comment refers to texture_create_from_extension ?
There was a problem hiding this comment.
Yep! This PR adds RenderingServer::texture_create_from_native_handle() which is implemented by both OpenGL and the rendering device renderers, but if you're using rendering device you can get more control by using the pre-existing RenderingDevice::texture_create_from_extension() which has more options. I put a similar note in the docs.
devloglogan
approved these changes
Sep 19, 2024
…atibility renderer
dsnopek
force-pushed
the
rename-and-expose-texture-create-external
branch
from
290570a to
7d56b09
Compare
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There is already a function in the compatibility renderer to create a texture from a native handle, however, it is unexposed and badly named, which is
GLES3::TextureStorage::texture_create_external().We need to expose it in order to implement some OpenXR extensions via GDExtension in the 'godot_openxr_vendor' repo.
But it is badly named, because there is an effort to add support for
GL_TEXTURE_EXTERNAL_OES(see PR #96705 and #83519), and since that is from official GLES extensions, it would be better to call that an "external texture", and rename the existing function to something else.We discussed a number of names for this over here, but I ended up going with something else in this PR:
RenderingServer::texture_create_from_native_handle()(which I picked because it pairs withRenderingServer::texture_get_native_handle()).I'm happy to continue discussing names for this if folks don't like it!
NOTE: There is already function for doing this when using rendering device,
RenderingDevice::texture_create_from_extension()(also not named very well :-)), which allows configuring many RD-specific options. Since this PR adds a function toRenderingServer, we also need to implement it for the RD renderer, but it just callsRenderingDevice::texture_create_from_extension()while making a bunch of assumptions for the RD-specific options. Most folks using the RD render would probably be better off using the RD-specific function, rather than the generic one onRenderingServer, which I've noted in the docs.