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

Improve xr gdnative #49301

Closed

Conversation

BastiaanOlij
Copy link
Contributor

@BastiaanOlij BastiaanOlij commented Jun 4, 2021

Depends on #48207

This PR adds additional changes to XRInterfaceGDNative and related structures to enable writing XR plugins for Godot 4.

Breaking changes (some of these I'm still working on, some coming from 48207):

  • Output now based on Vulkan renderer using commit_views
  • is_stereo was replaced by get_view_count
  • get_name now has String as parameter, not return value
  • godot_xr_get_worldscale will be removed (use XRServer)
  • 'godot_xr_get_reference_framewill be removed (useXRServer`)
  • godot_xr_*_controller* will be removed (use XRPositionalTracker)

Currently deprecated but will be re-introduced in another form:

  • get_external_texture_for_eye
  • get_external_depth_for_eye
  • 'commit_for_eye' (maybe, depends on the requirements of the OpenGL backend)

@@ -1593,6 +1593,9 @@
<member name="rendering/textures/vram_compression/import_s3tc" type="bool" setter="" getter="" default="true">
If [code]true[/code], the texture importer will import VRAM-compressed textures using the S3 Texture Compression algorithm. This algorithm is only supported on desktop platforms and consoles.
</member>
<member name="rendering/vr/enabled" type="bool" setter="" getter="" default="false">
If [code]true[/code], VR support is enabled in Godot, this ensures required shaders are compiled.
Copy link
Member

Choose a reason for hiding this comment

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

Is it VR or XR?

Copy link
Contributor Author

@BastiaanOlij BastiaanOlij Jun 6, 2021

Choose a reason for hiding this comment

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

This change comes from #48207, basically this enabled stereoscopic rendering which I guess has a purpose for both AR and VR. Possibly renaming this setting to XR might be more future proof.

@BastiaanOlij
Copy link
Contributor Author

Rebase issues not withstanding, this now does enough for the OpenVR plugin to work. There are a number of shortcuts in place so I'll be prettying up things in the next couple of days.

@BastiaanOlij BastiaanOlij force-pushed the improve_xr_gdnative branch from 35da7c2 to 89025ff Compare June 8, 2021 02:28
@fire
Copy link
Member

fire commented Jun 8, 2021

I can verify that it works. However, I can see that rendering is not enabled?

https://github.com/GodotVR/godot_openvr/pull/123/files#diff-b8924b2ef5b728671d43c41693816e9cac2d13358f330b0af011522d7939747dR213-R217

void godot_arvr_commit_views(void *p_data, godot_rid *p_render_target, godot_rect2 *p_screen_rect) {

Had to convert Transform to Transform3D. Quat to Quaternion. Some of the GDNative XR interface changed.

@BastiaanOlij BastiaanOlij force-pushed the improve_xr_gdnative branch from 89025ff to 044ff50 Compare June 10, 2021 01:46
@@ -8318,3 +8328,23 @@ RenderingDeviceVulkan::~RenderingDeviceVulkan() {
context->local_device_free(local_device);
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@reduz I'm not happy about exposing these this way, but I don't see any other way atm. Open to suggestions. Note that this is purely so we can send this data to the GDNative XR plugin. OpenVR needs this, OpenXR likely as well.

@@ -946,6 +946,9 @@ class RenderingDeviceVulkan : public RenderingDevice {
virtual Error texture_clear(RID p_texture, const Color &p_color, uint32_t p_base_mipmap, uint32_t p_mipmaps, uint32_t p_base_layer, uint32_t p_layers, uint32_t p_post_barrier = BARRIER_MASK_ALL);
virtual Error texture_resolve_multisample(RID p_from_texture, RID p_to_texture, uint32_t p_post_barrier = BARRIER_MASK_ALL);

// for now to get access to VkImage
virtual void *texture_get_vulkan_image(const RID p_texture);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@reduz similarly this one, I need access to VkImage (which is a pointer), I'm casting it to void * so the rest of the engine doesn't need to know. Interestingly enough OpenVR ends up casting it to uint64_t before submitting it to the VR Compositor.

"major": 1,
"minor": 1
"major": 4,
"minor": 0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renumbered this to 4.0, might be better to just name it 2.0, as we're breaking the API to something entirely new for Vulkan anyway it didn't make sense to keep this at 1.1.

@@ -48,7 +48,7 @@ typedef struct {
godot_gdnative_api_version version; /* version of our API */
void *(*constructor)(godot_object *);
void (*destructor)(void *);
godot_string (*get_name)(const void *);
void (*get_name)(const void *, godot_string *);
godot_int (*get_capabilities)(const void *);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

May end up making similar changes in a few other API calls after discussing with @vnen. Seeing string allocates memory this seems to be a safer way to ensure there are no memory leaks.

godot_real_t GDAPI godot_xr_get_worldscale();
godot_transform3d GDAPI godot_xr_get_reference_frame();

// helper functions for rendering
// helper functions for rendering (deprecated)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Many of these functions will be removed before I'm done with this.

ERR_FAIL_COND_V(interface == nullptr, blit_to_screen);

// commit_views can do callbacks to `godot_xr_blit_layer` to blit output to screen. This is ignored if this isn't the main view
interface->commit_views(data, (void *)&blit_to_screen, (const godot_rid *)&p_render_target, (godot_rect2 *)&p_screen_rect);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thinking of using a RID for blit_to_screen to enable these callbacks for preview output.

const Rect2 *rect = (const Rect2 *)p_rect;

BlitToScreen blit;
blit.render_target = *render_target;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to add a source rect option to BlitToScreen, as we're mostly using this for previewing an image on screen that is properly displayed inside of the headset, it's often only part of the image we're showing.

@BastiaanOlij
Copy link
Contributor Author

I can verify that it works. However, I can see that rendering is not enabled?

https://github.com/GodotVR/godot_openvr/pull/123/files#diff-b8924b2ef5b728671d43c41693816e9cac2d13358f330b0af011522d7939747dR213-R217

void godot_arvr_commit_views(void *p_data, godot_rid *p_render_target, godot_rect2 *p_screen_rect) {

Had to convert Transform to Transform3D. Quat to Quaternion. Some of the GDNative XR interface changed.

Yeah did all those but as I was without internet didn't have a chance to push it all in yet. Those are all changes on godot-cpp so also make sure you're on the correct version of that.
These are all brand new renames upstream so.

@BastiaanOlij BastiaanOlij force-pushed the improve_xr_gdnative branch 5 times, most recently from 6ffc8a8 to a2dfdb6 Compare June 19, 2021 10:22
@BastiaanOlij
Copy link
Contributor Author

#warning TODO check for support via RenderingDevice to enable on mobile when possible
 ^
drivers/vulkan/rendering_device_vulkan.cpp:3250:9: error: cannot initialize return object of type 'void *' with an lvalue of type 'VkImage' (aka 'unsigned long long')
        return tex->image;

So on some implementations VkImage is a *VkImage_t, and on some it's a uint64_t, OpenVR actually works with uint64_t on all platforms so might change the functions to return that instead of void *.

@BastiaanOlij
Copy link
Contributor Author

Leaving this to harvest stuff from, but with GDNative being retired it is being superseeded by #52003

@BastiaanOlij
Copy link
Contributor Author

Now that we've merged the GD Extensions changes I've moved the remaining changes into #52203 and #52205

@BastiaanOlij
Copy link
Contributor Author

Now that we've merged the GD Extensions changes I've moved the remaining changes into

@BastiaanOlij BastiaanOlij deleted the improve_xr_gdnative branch August 29, 2021 05:08
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.

3 participants