GodotVR / godot_openvr

GDNative based Open VR module
MIT License
227 stars 35 forks source link

Rendering in headset breaks after moving window #133

Open gyroninja opened 2 years ago

gyroninja commented 2 years ago

I am using: Linux with bspwm Godot 3.3 Godot OpenVR 1.1.0 SteamVR 1.19.7 (latest non beta release at time of writing)

I am not sure how exactly to reproduce this bug since it just seemingly randomly started happening to me. What happens is that if I move the window that Godot creates, rendering in the headset stops working. Most of the time it the picture in the headset freezes with the right eye's contents for both eyes, but one time just my left eye was frozen, but my right eye still was working. The regular desktop window remains updating just fine.

gyroninja commented 2 years ago

I can confirm that Submit is returning vr::VRCompositorError_None https://github.com/GodotVR/godot_openvr/blob/master/src/ARVRInterface.cpp#L263

Since I can easily reproduce it myself right now, feel free to provide me a patch to test for you.

gyroninja commented 2 years ago

I found that commenting out these two lines prevents the bug from happening https://github.com/godotengine/godot/blob/3.x/platform/x11/os_x11.cpp#L2272

After following what happens when the size of the window changes it seems the problem is resizing the render_target. https://github.com/godotengine/godot/blob/3.x/servers/visual/visual_server_viewport.cpp#L395

gyroninja commented 2 years ago

Okay this problem effects both the GLES2 and GLES3 renderer and seems to be a bug in the engine itself. If you replace the resize here with

VSG::storage->render_target_set_size(vp->render_target, 1000, 1000);
VSG::storage->render_target_set_size(vp->render_target, vp->size.x, vp->size.y);

It will instantly break. This would be equivalent to someone resizing the window to 1000x1000 every frame.

It seems that commenting out this glDeleteTextures prevent this bug from happening. If it's significant the texture that it is trying to delete is a texture being used (or at least was submitted to) by openvr.

gyroninja commented 2 years ago

Okay this seems to be a quirk of OpenVR https://github.com/ValveSoftware/openvr/issues/1255

The runtime caches information about the texture after the first Submit and it does not like you modifying the texture. Commenting out glDeleteTextures caused a different texture id to be returned which prevented OpenVR from using the cached information. This is the same upstream issue that is causing #55

BastiaanOlij commented 2 years ago

Owh interesting find! I was never able to reproduce 55

BastiaanOlij commented 2 years ago

I wonder what we can do about this because Godot doesn't have a mechanism for keeping the texture around for a bit longer. I wonder if there is a way we can detect the size change on the plugin side and clear whatever hold it has on the old texture.

Godots behavior is correct, the size of the viewport changes so it needs to create new textures. It's not designed to reuse texture id's I think.

BastiaanOlij commented 2 years ago

Btw, one way to work around the resize issue is to render to a separate viewport like the demo does. That way window resizing doesn't effect the render target for the HMD.

gyroninja commented 2 years ago

One way to work around it from within the engine is to make sure the new texture has a different id than the old one. Although not as elegant / clean you can probably just move the glDeleteTextures for getting rid of the old texture until later.

If you wanted it to be taken care of by the plugin I wonder if you could just have a second texture that you just keep around for invalidating the cached texture. You could listen for the signal for the resize happening and then use that second texture for the next frame (of each eye?) before switching back to the actual one.

BastiaanOlij commented 2 years ago

This should be fixed in Godot 3.5, probably even in the 3.4.2 stable release, needs to be tested.