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

Metal: Implement texture_create_from_extension #97304

Merged

Conversation

kroketio
Copy link
Contributor

@kroketio kroketio commented Sep 21, 2024

This PR implements texture_create_from_extension() for Metal.

The Vulkan device driver implements texture_create_from_extension() to handle externally backed textures (OpenXR module, or otherwise), which takes an incoming VkImage, creates a view, and returns it as TextureID.

RDD::TextureID RenderingDeviceDriverVulkan::texture_create_from_extension(uint64_t p_native_texture, TextureType p_type, DataFormat p_format, uint32_t p_array_layers, bool p_depth_stencil) {
VkImage vk_image = (VkImage)p_native_texture;
// We only need to create a view into the already existing natively-provided texture.
VkImageViewCreateInfo image_view_create_info = {};

For Metal, there is no exact equivalent to Vulkan's VkImageView, so for this driver we only need to cast the incoming handle to id<MTLTexture>, then return as TextureID.

Parameters p_type, p_format, p_array_layers, p_depth_stencil are ignored - MTLTexture (and the callee) already have this information and is only relevant when reinterpreting or remaping the texture in different ways.

The use of __bridge with void* in the casting is to signal we are not transferring ownership, and is a safe cast to bridge from C-style memory to Objective-C memory management (but correct me if I'm wrong).

loosely related to #96860

Parameters p_type, p_format, p_array_layers, p_depth_stencil are
ignored - MTLTexture (and the callee) already have this information
and is only relevant when reinterpreting or remaping the texture in
different ways.
Copy link
Contributor

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

I don't know much about Metal, in general, but lately I've been spending a bunch of time in this part of the RenderingDevice and RenderingServer API, and this seems about right to me.

However, it would be good for @stuartcarnie and the rendering team to take a look

@stuartcarnie
Copy link
Contributor

This may not be relevant here, as the client can create the new view over the texture themselves. In Metal there is a the ability to create a new texture view for an existing id<MTLTexture>:

- (id<MTLTexture])newTextureViewWithPixelFormat:(MTLPixelFormat)pixelFormat 
                                    textureType:(MTLTextureType)textureType 
                                         levels:(NSRange)levelRange 
                                         slices:(NSRange)sliceRange 
                                        swizzle:(MTLTextureSwizzleChannels)swizzle;

@kroketio
Copy link
Contributor Author

This is the reason why this PR does not add a view but simply casts and returns, as opposed to the Vulkan implementation where it adds a view. For context, I have a foreign MTLTexture (from a different rendering engine) and to get that texture usable in Godot, is to go through RenderingDevice::texture_create_from_extension() which for Metal will end up calling RenderingDeviceDriverMetal::texture_create_from_extension() - which is expected to return TextureID.

@akien-mga akien-mga changed the title Metal: implement texture_create_from_extension Metal: Implement texture_create_from_extension Sep 23, 2024
@akien-mga akien-mga changed the title Metal: Implement texture_create_from_extension Metal: Implement texture_create_from_extension Sep 23, 2024
@akien-mga akien-mga modified the milestones: 4.x, 4.4 Sep 23, 2024
@akien-mga akien-mga merged commit f032af7 into godotengine:master Oct 4, 2024
19 checks passed
@akien-mga
Copy link
Member

Thanks!

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.

5 participants