Fix OpenXR depth submission data dropped

[brycehutchings]

After upgrading to 4.6 Beta 2, I noticed the depth buffers submitted to OpenXR were missing. I tracked it down to the OpenXRFrameSynthesisExtension extension being added which caused projection_views_extensions going from 0 to 1 and hit this code path that causes the next pointer to be nulled out. Over in the create_main_swapchains() function is where the next pointer is set on the projection_views structs to point to the depth_views so this next pointer needs to be preserved.

[m4gr3d]

Looks good to me!

[dsnopek]

Thanks!

However, I think a better fix might be having OpenXRCompositionLayerDepthExtension implement set_projection_views_and_get_next_pointer() in order to add its struct to the next pointer chain. That way what it's adding doesn't conflict with what other extensions may add via their own set_projection_views_and_get_next_pointer()

[brycehutchings]

Thanks!

However, I think a better fix might be having OpenXRCompositionLayerDepthExtension implement set_projection_views_and_get_next_pointer() in order to add its struct to the next pointer chain. That way what it's adding doesn't conflict with what other extensions may add via their own set_projection_views_and_get_next_pointer()

Thanks for this recommendation, it sounds like the Right Thing to do. I am pulling on this thread now and I think it may turn into a bigger change. OpenXRCompositionLayerDepthExtension doesn't do much right now and I don't see enough data exposed through OpenXRAPI / OpenXRExtensionWrapper to move implementation over there without adding some "internal to the engine" (sorry not sure what term is) functions for OpenXRAPI and OpenXRCompositionLayerDepthExtension to use. There's a bunch of levels of how far this can go:

1. Do we consider the depth swapchain something that is owned by the OpenXRCompositionLayerDepthExtension or is it just a consumer of it? In theory other extensions could use it, but right now it's only created if the depth extension is enabled.
2. I could move the RenderState::depth_views over to OpenXRCompositionLayerDepthExtension. This will more "internal to the engine" functions to coordinate between OpenXRAPI and OpenXRCompositionLayerDepthExtension.

Alternatively, I could do a smaller change, along the lines of what I have here, but instead of void *next_pointer = const_cast<void *>(render_state.projection_views[v].next); which creates the growing chain problem, just have it start pointing to the depth_view if depth is enabled.

[dsnopek]

Yeah, there would be some refactoring involved.

Alternatively, I could do a smaller change, along the lines of what I have here, but instead of void *next_pointer = const_cast<void *>(render_state.projection_views[v].next); which creates the growing chain problem, just have it start pointing to the depth_view if depth is enabled.

This alternative could be the "quick fix" for Godot 4.6, and we could try to do The Right Thing (tm) in another PR targeting Godot 4.7?

[brycehutchings]

Yeah, there would be some refactoring involved.

Alternatively, I could do a smaller change, along the lines of what I have here, but instead of void *next_pointer = const_cast<void *>(render_state.projection_views[v].next); which creates the growing chain problem, just have it start pointing to the depth_view if depth is enabled.

This alternative could be the "quick fix" for Godot 4.6, and we could try to do The Right Thing (tm) in another PR targeting Godot 4.7?

Yeah I think this is the safe thing to do. I think the refactor is too big to do so close to release.

[dsnopek]

Thanks! Looks good to me :-)

[akien-mga]

Thanks!