From 5e729a0e82aed699c437f673154b0b10cbf96246 Mon Sep 17 00:00:00 2001 From: Mads Buvik Sandvei Date: Tue, 21 Jul 2020 12:28:39 +0200 Subject: [PATCH] Improved XR event processing logic. Particularly, handling session stop/start in a more predictable, less crashy manner. Added disabling of rendering when XR session is not running. --- apps/openmw/mwvr/openxrmanager.cpp | 17 +++ apps/openmw/mwvr/openxrmanager.hpp | 9 ++ apps/openmw/mwvr/openxrmanagerimpl.cpp | 137 ++++++++++++++++++----- apps/openmw/mwvr/openxrmanagerimpl.hpp | 16 ++- apps/openmw/mwvr/openxrswapchainimpl.cpp | 6 + apps/openmw/mwvr/vrsession.cpp | 15 +-- apps/openmw/mwvr/vrsession.hpp | 1 - apps/openmw/mwvr/vrview.cpp | 39 ++++--- apps/openmw/mwvr/vrview.hpp | 7 +- apps/openmw/mwvr/vrviewer.cpp | 2 +- 10 files changed, 189 insertions(+), 60 deletions(-) diff --git a/apps/openmw/mwvr/openxrmanager.cpp b/apps/openmw/mwvr/openxrmanager.cpp index 213237b7b..c9a7f3b05 100644 --- a/apps/openmw/mwvr/openxrmanager.cpp +++ b/apps/openmw/mwvr/openxrmanager.cpp @@ -32,6 +32,13 @@ namespace MWVR return false; } + bool OpenXRManager::frameShouldRender() + { + if (realized()) + return impl().frameShouldRender(); + return false; + } + void OpenXRManager::handleEvents() { if (realized()) @@ -86,6 +93,16 @@ namespace MWVR return impl().disablePredictions(); } + void OpenXRManager::xrResourceAcquired() + { + return impl().xrResourceAcquired(); + } + + void OpenXRManager::xrResourceReleased() + { + return impl().xrResourceReleased(); + } + std::array OpenXRManager::getPredictedViews(int64_t predictedDisplayTime, ReferenceSpace space) { return impl().getPredictedViews(predictedDisplayTime, space); diff --git a/apps/openmw/mwvr/openxrmanager.hpp b/apps/openmw/mwvr/openxrmanager.hpp index a7ee8d73b..cde9aab15 100644 --- a/apps/openmw/mwvr/openxrmanager.hpp +++ b/apps/openmw/mwvr/openxrmanager.hpp @@ -56,6 +56,9 @@ namespace MWVR //! Whether the openxr session is currently in a running state bool xrSessionRunning(); + //! Whether frames should be rendered in the current state + bool frameShouldRender(); + //! Process all openxr events void handleEvents(); @@ -68,6 +71,12 @@ namespace MWVR //! Disable pose predictions. void disablePredictions(); + //! Must be called every time an openxr resource is acquired to keep track + void xrResourceAcquired(); + + //! Must be called every time an openxr resource is released to keep track + void xrResourceReleased(); + //! Get poses and fov of both eyes at the predicted time, relative to the given reference space. \note Will throw if predictions are disabled. std::array getPredictedViews(int64_t predictedDisplayTime, ReferenceSpace space); diff --git a/apps/openmw/mwvr/openxrmanagerimpl.cpp b/apps/openmw/mwvr/openxrmanagerimpl.cpp index 40ac7f815..0e33d9f7d 100644 --- a/apps/openmw/mwvr/openxrmanagerimpl.cpp +++ b/apps/openmw/mwvr/openxrmanagerimpl.cpp @@ -214,7 +214,7 @@ namespace MWVR inline XrResult CheckXrResult(XrResult res, const char* originator, const char* sourceLocation) { if (XR_FAILED(res)) { std::stringstream ss; - ss << sourceLocation << ": OpenXR[" << to_string(res) << "]: " << originator; + ss << sourceLocation << ": OpenXR[Error: " << to_string(res) << "][Thread: " << std::this_thread::get_id() << "][DC: " << wglGetCurrentDC() << "][GLRC: " << wglGetCurrentContext() << "]: " << originator; Log(Debug::Error) << ss.str(); throw std::runtime_error(ss.str().c_str()); } @@ -471,39 +471,58 @@ namespace MWVR { std::unique_lock lock(mEventMutex); - // React to events + xrQueueEvents(); + while (auto* event = nextEvent()) { - Log(Debug::Verbose) << "OpenXR: Event received: " << to_string(event->type); - switch (event->type) + if (!processEvent(event)) { - case XR_TYPE_EVENT_DATA_SESSION_STATE_CHANGED: - { - const auto* stateChangeEvent = reinterpret_cast(event); - HandleSessionStateChanged(*stateChangeEvent); - break; - } - case XR_TYPE_EVENT_DATA_INSTANCE_LOSS_PENDING: - case XR_TYPE_EVENT_DATA_INTERACTION_PROFILE_CHANGED: - case XR_TYPE_EVENT_DATA_REFERENCE_SPACE_CHANGE_PENDING: - default: { - Log(Debug::Verbose) << "OpenXR: Event ignored"; - break; - } + // Do not consider processing an event optional. + // Retry once per frame until every event has been successfully processed + return; } + popEvent(); } } - void - OpenXRManagerImpl::HandleSessionStateChanged( + const XrEventDataBaseHeader* OpenXRManagerImpl::nextEvent() + { + if (mEventQueue.size() > 0) + return reinterpret_cast (&mEventQueue.front()); + return nullptr; + } + + bool OpenXRManagerImpl::processEvent(const XrEventDataBaseHeader* header) + { + Log(Debug::Verbose) << "OpenXR: Event received: " << to_string(header->type); + switch (header->type) + { + case XR_TYPE_EVENT_DATA_SESSION_STATE_CHANGED: + { + const auto* stateChangeEvent = reinterpret_cast(header); + return handleSessionStateChanged(*stateChangeEvent); + break; + } + case XR_TYPE_EVENT_DATA_INSTANCE_LOSS_PENDING: + case XR_TYPE_EVENT_DATA_INTERACTION_PROFILE_CHANGED: + case XR_TYPE_EVENT_DATA_REFERENCE_SPACE_CHANGE_PENDING: + default: + { + Log(Debug::Verbose) << "OpenXR: Event ignored"; + break; + } + } + return true; + } + + bool + OpenXRManagerImpl::handleSessionStateChanged( const XrEventDataSessionStateChanged& stateChangedEvent) { auto oldState = mSessionState; auto newState = stateChangedEvent.state; - mSessionState = newState; - Log(Debug::Verbose) << "XrEventDataSessionStateChanged: state " << to_string(oldState) << "->" << to_string(newState); - + bool success = true; switch (newState) { @@ -517,21 +536,45 @@ namespace MWVR } case XR_SESSION_STATE_STOPPING: { - mSessionRunning = false; - CHECK_XRCMD(xrEndSession(mSession)); + if (checkStopCondition()) + { + CHECK_XRCMD(xrEndSession(mSession)); + mSessionStopRequested = false; + mSessionRunning = false; + } + else + { + mSessionStopRequested = true; + success = false; + } break; } default: - Log(Debug::Verbose) << "XrEventDataSessionStateChanged: Ignoring new strate " << to_string(newState); + Log(Debug::Verbose) << "XrEventDataSessionStateChanged: Ignoring new state " << to_string(newState); } + + if (success) + { + mSessionState = newState; + } + else + { + Log(Debug::Verbose) << "XrEventDataSessionStateChanged: Conditions for state " << to_string(newState) << " not met, retrying next frame"; + } + + return success; } - const XrEventDataBaseHeader* - OpenXRManagerImpl::nextEvent() + bool OpenXRManagerImpl::checkStopCondition() { - XrEventDataBaseHeader* baseHeader = reinterpret_cast(&mEventDataBuffer); + return mAcquiredResources == 0; + } + + bool OpenXRManagerImpl::xrNextEvent(XrEventDataBuffer& eventBuffer) + { + XrEventDataBaseHeader* baseHeader = reinterpret_cast(&eventBuffer); *baseHeader = { XR_TYPE_EVENT_DATA_BUFFER }; - const XrResult result = xrPollEvent(mInstance, &mEventDataBuffer); + const XrResult result = xrPollEvent(mInstance, &eventBuffer); if (result == XR_SUCCESS) { if (baseHeader->type == XR_TYPE_EVENT_DATA_EVENTS_LOST) { @@ -547,6 +590,22 @@ namespace MWVR return nullptr; } + void OpenXRManagerImpl::popEvent() + { + if (mEventQueue.size() > 0) + mEventQueue.pop(); + } + + void + OpenXRManagerImpl::xrQueueEvents() + { + XrEventDataBuffer eventBuffer; + while (xrNextEvent(eventBuffer)) + { + mEventQueue.push(eventBuffer); + } + } + MWVR::Pose fromXR(XrPosef pose) { return MWVR::Pose{ fromXR(pose.position), fromXR(pose.orientation) }; @@ -582,6 +641,26 @@ namespace MWVR return mEnabledExtensions.count(extensionName) != 0; } + bool OpenXRManagerImpl::frameShouldRender() + { + return xrSessionRunning() && !xrSessionStopRequested(); + } + + void OpenXRManagerImpl::xrResourceAcquired() + { + mAcquiredResources++; + } + + void OpenXRManagerImpl::xrResourceReleased() + { + mAcquiredResources--; + } + + bool OpenXRManagerImpl::xrSessionStopRequested() + { + return mSessionStopRequested; + } + void OpenXRManagerImpl::enablePredictions() { mPredictionsEnabled = true; diff --git a/apps/openmw/mwvr/openxrmanagerimpl.hpp b/apps/openmw/mwvr/openxrmanagerimpl.hpp index 8161450f2..ed9aeb596 100644 --- a/apps/openmw/mwvr/openxrmanagerimpl.hpp +++ b/apps/openmw/mwvr/openxrmanagerimpl.hpp @@ -21,6 +21,7 @@ #include #include #include +#include namespace MWVR { @@ -70,13 +71,22 @@ namespace MWVR XrSession xrSession() const { return mSession; }; XrInstance xrInstance() const { return mInstance; }; bool xrExtensionIsEnabled(const char* extensionName) const; + bool xrSessionStopRequested(); + bool frameShouldRender(); + void xrResourceAcquired(); + void xrResourceReleased(); protected: void LogLayersAndExtensions(); void LogInstanceInfo(); void LogReferenceSpaces(); + bool xrNextEvent(XrEventDataBuffer& eventBuffer); + void xrQueueEvents(); const XrEventDataBaseHeader* nextEvent(); - void HandleSessionStateChanged(const XrEventDataSessionStateChanged& stateChangedEvent); + bool processEvent(const XrEventDataBaseHeader* header); + void popEvent(); + bool handleSessionStateChanged(const XrEventDataSessionStateChanged& stateChangedEvent); + bool checkStopCondition(); private: @@ -94,13 +104,15 @@ namespace MWVR std::array mConfigViews{ { {XR_TYPE_VIEW_CONFIGURATION_VIEW}, {XR_TYPE_VIEW_CONFIGURATION_VIEW} } }; XrSpace mReferenceSpaceView = XR_NULL_HANDLE; XrSpace mReferenceSpaceStage = XR_NULL_HANDLE; - XrEventDataBuffer mEventDataBuffer{ XR_TYPE_EVENT_DATA_BUFFER }; XrFrameState mFrameState{}; XrSessionState mSessionState = XR_SESSION_STATE_UNKNOWN; + bool mSessionStopRequested = false; bool mSessionRunning = false; + uint32_t mAcquiredResources = 0; std::mutex mFrameStateMutex{}; std::mutex mEventMutex{}; std::set mEnabledExtensions; + std::queue mEventQueue; std::array mLayerDepth; }; diff --git a/apps/openmw/mwvr/openxrswapchainimpl.cpp b/apps/openmw/mwvr/openxrswapchainimpl.cpp index dff102bc3..4ffa21dce 100644 --- a/apps/openmw/mwvr/openxrswapchainimpl.cpp +++ b/apps/openmw/mwvr/openxrswapchainimpl.cpp @@ -165,6 +165,7 @@ namespace MWVR { void OpenXRSwapchainImpl::acquire(osg::GraphicsContext*) { + auto xr = Environment::get().getManager(); XrSwapchainImageAcquireInfo acquireInfo{ XR_TYPE_SWAPCHAIN_IMAGE_ACQUIRE_INFO }; // I am trusting that the openxr runtime won't diverge these indices so long as these are always called together. // If some dumb ass implementation decides to violate this we'll just have to work around that if it actually happens. @@ -177,18 +178,23 @@ namespace MWVR { XrSwapchainImageWaitInfo waitInfo{ XR_TYPE_SWAPCHAIN_IMAGE_WAIT_INFO }; waitInfo.timeout = XR_INFINITE_DURATION; CHECK_XRCMD(xrWaitSwapchainImage(mSwapchain, &waitInfo)); + xr->xrResourceAcquired(); CHECK_XRCMD(xrWaitSwapchainImage(mSwapchainDepth, &waitInfo)); + xr->xrResourceAcquired(); mIsAcquired = true; } void OpenXRSwapchainImpl::release(osg::GraphicsContext*) { + auto xr = Environment::get().getManager(); mIsAcquired = false; XrSwapchainImageReleaseInfo releaseInfo{ XR_TYPE_SWAPCHAIN_IMAGE_RELEASE_INFO }; CHECK_XRCMD(xrReleaseSwapchainImage(mSwapchain, &releaseInfo)); + xr->xrResourceReleased(); CHECK_XRCMD(xrReleaseSwapchainImage(mSwapchainDepth, &releaseInfo)); + xr->xrResourceReleased(); } void OpenXRSwapchainImpl::checkAcquired() const { diff --git a/apps/openmw/mwvr/vrsession.cpp b/apps/openmw/mwvr/vrsession.cpp index c37e1bcba..83f0fa66e 100644 --- a/apps/openmw/mwvr/vrsession.cpp +++ b/apps/openmw/mwvr/vrsession.cpp @@ -113,6 +113,7 @@ namespace MWVR } bool VRSession::isRunning() const { + return true; auto* xr = Environment::get().getManager(); return xr->xrSessionRunning(); } @@ -124,12 +125,11 @@ namespace MWVR beginPhase(FramePhase::Swap); auto* frameMeta = getFrame(FramePhase::Swap).get(); + auto leftView = viewer.getView("LeftEye"); + auto rightView = viewer.getView("RightEye"); - if (frameMeta->mShouldRender && isRunning()) + if (frameMeta->mShouldRender) { - auto leftView = viewer.getView("LeftEye"); - auto rightView = viewer.getView("RightEye"); - viewer.blitEyesToMirrorTexture(gc); gc->swapBuffersImplementation(); leftView->swapBuffers(gc); @@ -145,6 +145,7 @@ namespace MWVR Log(Debug::Debug) << frameMeta->mFrameNo << ": EndFrame " << std::this_thread::get_id(); xr->endFrame(frameMeta->mPredictedDisplayTime, 1, layerStack); + xr->xrResourceReleased(); } { @@ -167,7 +168,6 @@ namespace MWVR } getFrame(FramePhase::Swap) = nullptr; - mFramesInFlight--; } mCondition.notify_one(); } @@ -292,8 +292,9 @@ namespace MWVR frame->mPredictedDisplayTime = predictedDisplayTime; frame->mFrameNo = mFrames; frame->mPredictedPoses = predictedPoses; - frame->mShouldRender = isRunning(); - mFramesInFlight++; + frame->mShouldRender = xr->frameShouldRender(); + if (frame->mShouldRender) + xr->xrResourceAcquired(); } const PoseSet& VRSession::predictedPoses(FramePhase phase) diff --git a/apps/openmw/mwvr/vrsession.hpp b/apps/openmw/mwvr/vrsession.hpp index 46443437d..80e53ef0d 100644 --- a/apps/openmw/mwvr/vrsession.hpp +++ b/apps/openmw/mwvr/vrsession.hpp @@ -77,7 +77,6 @@ namespace MWVR osg::Matrix viewMatrix(FramePhase phase, Side side); osg::Matrix projectionMatrix(FramePhase phase, Side side); - int mFramesInFlight{ 0 }; std::array, (int)FramePhase::NumPhases> mFrame{ nullptr }; private: diff --git a/apps/openmw/mwvr/vrview.cpp b/apps/openmw/mwvr/vrview.cpp index 3071574d8..e3e0a5db8 100644 --- a/apps/openmw/mwvr/vrview.cpp +++ b/apps/openmw/mwvr/vrview.cpp @@ -57,12 +57,8 @@ namespace MWVR { void VRView::prerenderCallback(osg::RenderInfo& renderInfo) { - - if (Environment::get().getManager()->xrSessionRunning()) - { + if(Environment::get().getSession()->getFrame(VRSession::FramePhase::Draw)->mShouldRender) mSwapchain->beginFrame(renderInfo.getState()->getGraphicsContext()); - } - } void VRView::InitialDrawCallback::operator()(osg::RenderInfo& renderInfo) const @@ -86,18 +82,33 @@ namespace MWVR { auto* camera = slave._camera.get(); auto name = camera->getName(); - Side side = Side::RIGHT_SIDE; - if (name == "LeftEye") - side = Side::LEFT_SIDE; + // Update current cached cull mask of camera if it is active + auto mask = camera->getCullMask(); + if (mask == 0) + camera->setCullMask(mCullMask); + else + mCullMask = mask; - auto* session = Environment::get().getSession(); - auto viewMatrix = view.getCamera()->getViewMatrix(); + if (Environment::get().getSession()->getFrame(VRSession::FramePhase::Update)->mShouldRender) + { + Side side = Side::RIGHT_SIDE; + if (name == "LeftEye") + side = Side::LEFT_SIDE; - auto modifiedViewMatrix = viewMatrix * session->viewMatrix(VRSession::FramePhase::Update, side); - auto projectionMatrix = session->projectionMatrix(VRSession::FramePhase::Update, side); + auto* session = Environment::get().getSession(); + auto viewMatrix = view.getCamera()->getViewMatrix(); - camera->setViewMatrix(modifiedViewMatrix); - camera->setProjectionMatrix(projectionMatrix); + auto modifiedViewMatrix = viewMatrix * session->viewMatrix(VRSession::FramePhase::Update, side); + auto projectionMatrix = session->projectionMatrix(VRSession::FramePhase::Update, side); + + camera->setViewMatrix(modifiedViewMatrix); + camera->setProjectionMatrix(projectionMatrix); + } + else + { + // If the session is not active, we do not want to waste resources rendering frames. + camera->setCullMask(0); + } slave.updateSlaveImplementation(view); } diff --git a/apps/openmw/mwvr/vrview.hpp b/apps/openmw/mwvr/vrview.hpp index 4094afb7f..0d17b63a6 100644 --- a/apps/openmw/mwvr/vrview.hpp +++ b/apps/openmw/mwvr/vrview.hpp @@ -23,16 +23,11 @@ namespace MWVR class UpdateSlaveCallback : public osg::View::Slave::UpdateSlaveCallback { public: - UpdateSlaveCallback(osg::ref_ptr view, osg::GraphicsContext* gc) - : mView(view), mGC(gc) - {} - void updateSlave(osg::View& view, osg::View::Slave& slave) override; private: - osg::ref_ptr mXR; osg::ref_ptr mView; - osg::ref_ptr mGC; + osg::Node::NodeMask mCullMask; }; public: diff --git a/apps/openmw/mwvr/vrviewer.cpp b/apps/openmw/mwvr/vrviewer.cpp index a424d78c9..823e47aff 100644 --- a/apps/openmw/mwvr/vrviewer.cpp +++ b/apps/openmw/mwvr/vrviewer.cpp @@ -107,7 +107,7 @@ namespace MWVR mViewer->addSlave(camera, true); auto* slave = mViewer->findSlaveForCamera(camera); assert(slave); - slave->_updateSlaveCallback = new VRView::UpdateSlaveCallback(view, context); + slave->_updateSlaveCallback = new VRView::UpdateSlaveCallback(); if (mirror) mMsaaResolveMirrorTexture[i].reset(new VRFramebuffer(context->getState(),