-
-
Notifications
You must be signed in to change notification settings - Fork 22k
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: Multiview support #98803
Metal: Multiview support #98803
Conversation
@BastiaanOlij and @RevoluPowered might be interested in this PR |
Amazing stuart! I'm going to see how far I get with implementing XR_KHR_metal_enable ontop of this, that should be a great test for it. |
f5cb156
to
00ad0d7
Compare
Thanks! I can't review the actual rendering code (it's beyond me) but I tested this with a simple XR project that I threw together using However, with verbose output turned on, I'm getting this debug message spammed: And in the console, it's spamming blank lines (probably one per frame?) So, there's something wrong with that |
00ad0d7
to
e0cb42d
Compare
Thanks @dsnopek it's now removed! |
drivers/metal/metal_objects.mm
Outdated
@@ -96,6 +96,9 @@ | |||
MDRenderPipeline *rp = (MDRenderPipeline *)p; | |||
|
|||
if (render.encoder == nil) { | |||
// This error would happen if the render pass failed. | |||
ERR_FAIL_NULL_MSG(render.desc, "Render pass descriptor is null"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ERR_FAIL_NULL_MSG(render.desc, "Render pass descriptor is null"); | |
ERR_FAIL_NULL_MSG(render.desc, "Render pass descriptor is null."); |
drivers/metal/metal_objects.mm
Outdated
@@ -649,7 +671,9 @@ | |||
|
|||
MDAttachment const &attachment = pass.attachments[idx]; | |||
|
|||
id<MTLTexture> tex = fb.textures[idx]; | |||
id<MTLTexture> tex = fb.get_texture(idx); | |||
ERR_FAIL_NULL_MSG(tex, "Frame buffer color texture is null"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ERR_FAIL_NULL_MSG(tex, "Frame buffer color texture is null"); | |
ERR_FAIL_NULL_MSG(tex, "Frame buffer color texture is null."); |
drivers/metal/metal_objects.mm
Outdated
@@ -662,7 +686,8 @@ | |||
attachmentCount += 1; | |||
uint32_t idx = subpass.depth_stencil_reference.attachment; | |||
MDAttachment const &attachment = pass.attachments[idx]; | |||
id<MTLTexture> tex = fb.textures[idx]; | |||
id<MTLTexture> tex = fb.get_texture(idx); | |||
ERR_FAIL_NULL_MSG(tex, "Frame buffer depth / stencil texture is null"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ERR_FAIL_NULL_MSG(tex, "Frame buffer depth / stencil texture is null"); | |
ERR_FAIL_NULL_MSG(tex, "Frame buffer depth / stencil texture is null."); |
drivers/metal/metal_objects.mm
Outdated
@@ -702,8 +727,15 @@ | |||
uint32_t p_base_vertex, | |||
uint32_t p_first_instance) { | |||
DEV_ASSERT(type == MDCommandBufferStateType::Render); | |||
ERR_FAIL_NULL_MSG(render.pipeline, "No pipeline set for render command buffer"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ERR_FAIL_NULL_MSG(render.pipeline, "No pipeline set for render command buffer"); | |
ERR_FAIL_NULL_MSG(render.pipeline, "No pipeline set for render command buffer."); |
drivers/metal/metal_objects.mm
Outdated
@@ -751,8 +783,15 @@ | |||
int32_t p_vertex_offset, | |||
uint32_t p_first_instance) { | |||
DEV_ASSERT(type == MDCommandBufferStateType::Render); | |||
ERR_FAIL_NULL_MSG(render.pipeline, "No pipeline set for render command buffer"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ERR_FAIL_NULL_MSG(render.pipeline, "No pipeline set for render command buffer"); | |
ERR_FAIL_NULL_MSG(render.pipeline, "No pipeline set for render command buffer."); |
drivers/metal/metal_objects.mm
Outdated
@@ -770,6 +809,8 @@ | |||
|
|||
void MDCommandBuffer::render_draw_indexed_indirect(RDD::BufferID p_indirect_buffer, uint64_t p_offset, uint32_t p_draw_count, uint32_t p_stride) { | |||
DEV_ASSERT(type == MDCommandBufferStateType::Render); | |||
ERR_FAIL_NULL_MSG(render.pipeline, "No pipeline set for render command buffer"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ERR_FAIL_NULL_MSG(render.pipeline, "No pipeline set for render command buffer"); | |
ERR_FAIL_NULL_MSG(render.pipeline, "No pipeline set for render command buffer."); |
drivers/metal/metal_objects.mm
Outdated
@@ -794,6 +835,8 @@ | |||
|
|||
void MDCommandBuffer::render_draw_indirect(RDD::BufferID p_indirect_buffer, uint64_t p_offset, uint32_t p_draw_count, uint32_t p_stride) { | |||
DEV_ASSERT(type == MDCommandBufferStateType::Render); | |||
ERR_FAIL_NULL_MSG(render.pipeline, "No pipeline set for render command buffer"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ERR_FAIL_NULL_MSG(render.pipeline, "No pipeline set for render command buffer"); | |
ERR_FAIL_NULL_MSG(render.pipeline, "No pipeline set for render command buffer."); |
@@ -1811,6 +1817,20 @@ void deserialize(BufReader &p_reader) { | |||
} | |||
} | |||
|
|||
for (auto f : resources.builtin_inputs) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a proper value, auto
is not allowed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably be:
for (auto f : resources.builtin_inputs) { | |
for (const BuiltInResource &f : resources.builtin_inputs) { |
From the files
} | ||
|
||
if (!r_shader_meta.has_multiview) { | ||
for (auto f : resources.builtin_outputs) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for (auto f : resources.builtin_outputs) { | |
for (const BuiltInResource &f : resources.builtin_outputs) { |
* Adds support for multiview * Returns native handles for more driver resources
e0cb42d
to
3dac388
Compare
Thanks @AThousandShips – resolved all your feedback |
Thanks! |
This PR adds multi-view support for the Metal rendering device driver.
Testing
GodotStereoDemo
A screenshot of XR enabled and rendering multiple views, thanks to @BastiaanOlij
Clearing attachments
It is not easy to test clearing a subset of an attachment when multi-view is enabled in Godot outside of XR, so I validated it by creating a multi-view frame buffer and clearing it and then validating the output in the Metal Debugger.
The following screenshot has the following properties:
{ x=10, y=10, w=50, h=50 }
Color.AQUAMARINE
0.5
In this screenshot, we can see the following:
The GDScript to set up the multi-view frame buffer:
and some code to clear it: