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

OpenVR and OpenXR symmetric and other projection matrix options #196

Merged

Conversation

mrbelowski
Copy link
Contributor

@mrbelowski mrbelowski commented Feb 15, 2024

Big PR (sorry...)

This adds various projection matrix tweaks to the Compatibility page - horizontallly symmetric, horizontally mirrored, vertically symmetric, vertically matched, and combinations. It also has an option to extend the render target to accommodate the required cropping of the eye images (so the use can set up, say, symmetric projection and choose whether to keep the image quality but sacrifice some performance, or vice-versa). The changes are applied immediately.

This works for OpenVR and OpenXR - I've tested OpenVR and OpenXR implementations with a Valve Index and a Quest3.

Symmetric projection fixes minor eye differences on some surfaces (seems to be more common when the surface is angled near to the z-axis), and some reflection and other shader disparities. The extent of the improvement depends on the game but something like The Forgotten City is noticably more comfortable with symmetric projection (horizontally symmetric has the biggest impact).

src/mods/vr/D3D11Component.cpp Outdated Show resolved Hide resolved
src/mods/vr/runtimes/OpenVR.cpp Outdated Show resolved Hide resolved
src/mods/vr/runtimes/OpenXR.cpp Outdated Show resolved Hide resolved
src/mods/vr/runtimes/OpenVR.cpp Outdated Show resolved Hide resolved
src/mods/vr/runtimes/OpenVR.cpp Outdated Show resolved Hide resolved
src/mods/vr/runtimes/OpenXR.cpp Outdated Show resolved Hide resolved
src/mods/vr/runtimes/OpenXR.cpp Outdated Show resolved Hide resolved
src/mods/VR.cpp Outdated Show resolved Hide resolved
this->eyes[i][3] = Vector4f{*(Vector3f*)&pose.position, 1.0f};
// if we've not yet derived an eye projection matrix, or we've changed the projection, derive it here
// Hacky way to check for an uninitialised eye matrix - is there something better, is this necessary?
if (this->should_recalculate_eye_projections || this->projections[0][2][3] == 0) {
Copy link
Owner

@praydog praydog Feb 16, 2024

Choose a reason for hiding this comment

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

You can stop the execution of this entire function if should_recalculate_eye_projections is false, not just this block at the bottom.

After every call to respective runtime's VRRuntime::synchronize_frame(), should_recalculate_eye_projections could get set back to true. Doing this once per frame isn't bad at all, which synchronize_frame essentially signals. Or even once every N frames could work too.

i.e.:

VRRuntime::Error OpenXR::update_matrices(float nearz, float farz) {
    if (!this->should_recalculate_eye_projections) {
        return VRRuntime::Error::SUCCESS;
    }
...
}

VRRuntime::Error OpenXR::synchronize_frame(std::optional<uint32_t> frame_count) {
    ....
    this->should_recalculate_eye_projections = true;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it worth leaving the guard in so we're not repeating the eye matrix derivation on every tick, and adding another guard that's unset on sync-frame, which skips the entire function?

src/mods/vr/runtimes/VRRuntime.hpp Show resolved Hide resolved
@keton
Copy link
Contributor

keton commented Mar 12, 2024

Just in case keton@eb17662 rebases cleanly on latest UEVR nightlies and has some minor incompatibilities fixed.

I've tried to save the attribution but needed to make some changes for it to compile. Feel free to git format-patch and git apply to remove my name.

I'd appreciate this getting merged for AC7 community among others. Currently I need latest plugin API from nightlies and symmetric projection this PR provides.

@mrbelowski
Copy link
Contributor Author

I'm not precious or fussy about this PR - if you want a clean Git history perhaps cherry picking Keton's changes is the simplest solution? Attribution isn't important. Alternatively I'm happy to abandon this PR and raise another for a new branch with a single commit. Given the history, rebasing / squashing the commits in this PR is awkward enough to feel like a bad idea

@praydog praydog merged commit 9074c01 into praydog:master Mar 20, 2024
1 check passed
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.

3 participants