OSVR / SteamVR-OSVR

An OSVR plugin for SteamVR, providing SteamVR support for OSVR HMDs.
Apache License 2.0
138 stars 57 forks source link

Judder introduced by the driver #110

Closed 0x1100 closed 7 years ago

0x1100 commented 7 years ago

Tracking had always been terrible compared to a native OSVR application.

The root cause of this problem is the fact that the update of the OSVR context is done in the runframe() method of the server driver. This method is called at an even lower frequency than the framerate so SteamVR tries to extrapolate much more that it should have to. @mdigkin's pull request #103 helps a lot because, without it, SteamVR cannot extrapolate the orientation at all. But it still doesn't fix the problem.

The context should instead be updated in a separate thread, as advised by Valve. This reduces the need for extrapolation to a minimum and, along with #103, completely fixes the judder problem.

bagleman commented 7 years ago

Are valve and Sensics/Razer aware of this problem?

russell-taylor commented 7 years ago

Sensics is now aware of it, and we're looking into it. Thanks!

0x1100 commented 7 years ago

Getting it to work is pretty straightforward. We quickly patched it to release the AIO installer without having to wait a new build.

diff --git a/src/ServerDriver_OSVR.cpp b/src/ServerDriver_OSVR.cpp
index af5cb92..d97b014 100644
--- a/src/ServerDriver_OSVR.cpp
+++ b/src/ServerDriver_OSVR.cpp
@@ -42,6 +42,23 @@
 #include <cstring>                  // for std::strcmp
 #include <string>                   // for std::string

+#include <chrono>
+#include <thread>
+#include <atomic>
+
+namespace {
+static std::thread      gs_thread;
+static std::atomic<int> gs_thread_quit = 0;
+static std::atomic<int> gs_thread_ms_wait = 1;
+static void gs_thread_work(osvr::clientkit::ClientContext ctx) {
+    while (!gs_thread_quit.load()) {
+        ctx.update();
+        std::this_thread::sleep_for(std::chrono::milliseconds(gs_thread_ms_wait.load()));
+    }
+    gs_thread_quit = 0;
+}
+} // anonymous namespace
+
 vr::EVRInitError ServerDriver_OSVR::Init(vr::IDriverLog* driver_log, vr::IServerDriverHost* driver_host, const char* user_driver_config_dir, const char* driver_install_dir)
 {
     if (driver_log)
@@ -52,11 +69,15 @@ vr::EVRInitError ServerDriver_OSVR::Init(vr::IDriverLog* driver_log, vr::IServer
     trackedDevices_.emplace_back(std::make_unique<OSVRTrackedDevice>(*(context_.get()), driver_host));
     trackedDevices_.emplace_back(std::make_unique<OSVRTrackingReference>(*(context_.get()), driver_host));

+    gs_thread = std::thread(gs_thread_work, context_->get());
+
     return vr::VRInitError_None;
 }

 void ServerDriver_OSVR::Cleanup()
 {
+    gs_thread_quit.store(1);
+    gs_thread.join();
     trackedDevices_.clear();
     context_.reset();
 }
@@ -101,7 +122,10 @@ vr::ITrackedDeviceServerDriver* ServerDriver_OSVR::FindTrackedDeviceDriver(const

 void ServerDriver_OSVR::RunFrame()
 {
-    context_->update();
+    // In a real driver, this should happen from some pose tracking thread.
+    // The RunFrame interval is unspecified and can be very irregular if some other
+    // driver blocks it for some periodic task.
+    //context_->update();
 }

 bool ServerDriver_OSVR::ShouldBlockStandbyMode()
@@ -112,11 +136,13 @@ bool ServerDriver_OSVR::ShouldBlockStandbyMode()
 void ServerDriver_OSVR::EnterStandby()
 {
     // TODO
+    gs_thread_ms_wait.store(100);
 }

 void ServerDriver_OSVR::LeaveStandby()
 {
     // TODO
+    gs_thread_ms_wait.store(1);
 }

 std::string ServerDriver_OSVR::getDeviceId(vr::ITrackedDeviceServerDriver* device)
godbyk commented 7 years ago

I've incorporated a version of this patch into the update-v1.0.6 branch.

Thanks!