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

feat-fix(server_openvr): Add timewarped compositing to Windows OpenVR compositor #2472

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

shinyquagsire23
Copy link
Contributor

Fixes juddery menus on Riven, and possibly judder on other OpenXR apps (HUDs, 2D menus, text, etc rendered at higher resolutions are the most likely candidates).

Before vs After

tl;dr on compositing

  • To composite with timewarp, we need three things: The exact view poses the layer frame was rendered at, the headset's per-eye view frustum tangents, and a renderer which can render a quad with positions/sizes in meters.
    • FrameRender is now passed the pose of each layer, the view tangents, and the eye transforms to make this happen.
  • The layer's quad is centered at 0,0 and extended 1 unit out in each direction, and then each direction is multiplied by the view tangents.
    • Each layer's quad is projected forward by 700m (to "infinity") and scaled proportionally, to remove small positional differences from IPD and the user's head rotation.
    • If the layer is at the exact current view position (which is always the case for layer 0), it will be projected pixel-perfect onto the framebuffer.
    • Layers indices >=1 only ever seem to appear in OpenXR-based titles, I think OpenVR started compositing its app layers into layer 0 on our behalf within the last 6 months (the dashboard used to be juddery as well).
  • For simplicity, and to make debugging a bit easier, I did all the vertex math on the CPU with the DirectX math functions (it's only 8 vertices and DirectX has SIMD support anyway).
    • I also split the draw command into two calls with different viewports/scissor rects, since the quads could cross between eyes after the transform
  • Also I removed the depth buffer for now, since we were not actually using it. If we ever decide to try and get fancy with depth buffers we can add it back I guess.

Copy link
Member

@zmerp zmerp left a comment

Choose a reason for hiding this comment

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

Sorry it took a bit, left some comments.

Comment on lines +735 to +741
#else
DirectX::XMVECTORF32 vertsL_VF32[4]
= { { { { -1.0f * depth * m, 1.0f * depth * m, -depth, 1.0f } } },
{ { { 1.0f * depth * m, -1.0f * depth * m, -depth, 1.0f } } },
{ { { 1.0f * depth * m, 1.0f * depth * m, -depth, 1.0f } } },
{ { { -1.0f * depth * m, -1.0f * depth * m, -depth, 1.0f } } } };
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Why did you hardcode disable this code with directives? Should it be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

could remove it yeah, wasn't sure if I'd have to come back to this code or not

Comment on lines +81 to +82
vr::HmdRect2_t m_viewProj[2];
vr::HmdMatrix34_t m_eyeToHead[2];
Copy link
Member

Choose a reason for hiding this comment

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

I'm not convinced about saving the view parameters in the DirectMode class. I think they should stay inside m_pD3DRender

Comment on lines +97 to +99
m_FrameRender->RenderFrame(
pTexture, bounds, poses, viewProj, eyeToHead, layerCount, recentering, message, debugText
);
Copy link
Member

Choose a reason for hiding this comment

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

I'm also not convinced why the renderer call should be forwarded from the encoder. It doesn't make sense that the renderer is "owned" by the encoder. I don't think this was like that initially but then the linux implementation was made like that and so also the windows impl got changed.
Don't bother fixing this in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants