godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.12k stars 69 forks source link

Notify xr_interface about currently rendering viewport before get_render_target_size call #7306

Open mittorn opened 1 year ago

mittorn commented 1 year ago

Describe the project you are working on

OpenVR overlay app

Describe the problem or limitation you are having in your project

All viewports are forced to use size from get_render_target_size() and i cannot override size from specific viewports. xr_interface->pre_draw_viewport called after size is forced and viewports are sorted, so even patching engine in runtime will not help.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Pass viewport RID to get_render_target_size or call some interface callback to make xr interface know, which viewport is used

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

First way: break xr interface, adding an argument to get_render_target_size For scene application it will return eye viewport size, for overlay application it will return overlay viewport size

            if (xr_interface.is_valid()) {
                // Ignore update mode we have to commit frames to our XR interface
                visible = true;

                // Override our size, make sure it matches our required size and is created as a stereo target
                Size2 xr_size = xr_interface->get_render_target_size(vp);
                _viewport_set_size(vp, xr_size.width, xr_size.height, xr_interface->get_view_count());
            } else {

Second way: call pre_draw_viewport there"

            if (xr_interface.is_valid() && xr_interface->pre_draw_viewport(vp->render_target)) {
                // Ignore update mode we have to commit frames to our XR interface
                visible = true;

                // Override our size, make sure it matches our required size and is created as a stereo target
                Size2 xr_size = xr_interface->get_render_target_size();
                _viewport_set_size(vp, xr_size.width, xr_size.height, xr_interface->get_view_count());
            } else {
                // don't render anything
                visible = false;
                vp->size = Size2();
            }

and remove next call of this method. This allows to find viewport in XRInterface and return correct size

If this enhancement will not be used often, can it be worked around with a few lines of script?

I tried workaround it in different ways, even with binary patching, but i failed to override viewport size during render without glitches

Is there a reason why this should be core and not an add-on in the asset library?

It is impossible to implement in addon even with dirty hacks because overlays are resorted during rendering

BastiaanOlij commented 1 year ago

Hmm interesting issue. I know @beniwtv worked on this in the Godot 3 plugin, I can't remember exactly how he worked around the problem.

I think this is worth fixing, I think I would go for the second option and move _pre_draw_viewport forward.

mittorn commented 1 year ago

My workaround for godot3 now is this engine patch, just remove this line: https://github.com/mittorn/ovr-utils-dashboard/blob/d766debabfa0294fad4167aa429fc983e2d672b5/godot.patch But i hope, it maybe solved better way in upstream godot 4.x without need to build patched engine. I also tried to count get_render_targetsize calls during rendering and return correct size, but viewports are always rendered in different order, so i cannot even guess which viewport will be rendered after this call. It seems to be rearranged in inverse order in ViewportSort almost every frame, so i still getting random glitches. Another frustrating thing that i cannot even hook/patch this place in any portable way to make it work as addon to unpatched engine

BastiaanOlij commented 1 year ago

Hmm, that patch would disable part of the stereo rendering and break OpenXR, I guess in Godot 3 OpenVR we didn't create buffers in the plugin so you could get away with doing that.

I think its too big a breaking change to retrofit a proper solution back into Godot 3. If you're happy to migrate to Godot 4 I'd rather we concentrate on fixing this there. I take it you're using the Godot 4 OpenVR plugin, I haven't been keeping much of an eye on that seeing OpenXR took over, but overlays are a bit of a sore point and still better implemented in OpenVR.

Anyway if you want to put a PR in for your Godot 4 solution, I'll gladly review it.

mittorn commented 1 year ago

Stereo can be disabled in godot_openvr, my comment was related to removing viewport size change (first changed line). Patch was done before i patched godot_openvr, so it has remove stereo part. I looking for migrating to godot4. There is old port of OpenVR support to godot4 here: https://github.com/GodotVR/godot_openvr/tree/2.0-dev Maybe it will be enough for creating overlays. First i need find a way to render overlays with correct viewport size. Godot3 was used because i forked godot3-based project Godot3 arvr interface is stable and unlikely to be changed. It does not have pre_draw_viewport method which may help to workaround it, so this only related to godot4, as only way is change it in upstream branch and use patched engine for until port is done. Anyway i do not want to keep unused features like bullet in binary, but i want my project to load and work on original engine.

beniwtv commented 1 year ago

I planned on adding overlays support to OpenXR in Godot 4, but so far I did not look into it. For Godot 3 and OpenVR overlays arvr_interface.get_render_targetsize() and setting the Viewport size was enough.