godot_openvr icon indicating copy to clipboard operation
godot_openvr copied to clipboard

Rendering in headset breaks after moving window

Open gyroninja opened this issue 2 years ago • 9 comments

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 avatar Sep 27 '21 09:09 gyroninja

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 avatar Sep 27 '21 11:09 gyroninja

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 avatar Sep 28 '21 06:09 gyroninja

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 avatar Sep 28 '21 08:09 gyroninja

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

gyroninja avatar Sep 28 '21 09:09 gyroninja

Owh interesting find! I was never able to reproduce 55

BastiaanOlij avatar Oct 04 '21 13:10 BastiaanOlij

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 avatar Oct 04 '21 13:10 BastiaanOlij

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.

BastiaanOlij avatar Oct 04 '21 13:10 BastiaanOlij

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.

gyroninja avatar Oct 04 '21 13:10 gyroninja

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

BastiaanOlij avatar Jan 26 '22 04:01 BastiaanOlij