[Bf-blender-cvs] [330f062099e] soc-2019-openxr: Safe and informative GHOST_Xr error handling
Julian Eisel
noreply at git.blender.org
Fri Jul 12 14:43:04 CEST 2019
Commit: 330f062099e3c332d902f7c1667ce71b3e043af1
Author: Julian Eisel
Date: Fri Jul 12 12:55:16 2019 +0200
Branches: soc-2019-openxr
https://developer.blender.org/rB330f062099e3c332d902f7c1667ce71b3e043af1
Safe and informative GHOST_Xr error handling
This wraps all functions that could fail into some proper (although
unfinished) error handling.
These were the requirements for the error handling strategy:
* If an error occurs, cleanly exit the VR session (or the context if the
error happend during its set up), causing no resource leaks or side
effects to the rest of Blender.
* Show a *useful* error message to the user.
* Don't impair readability of code too much with error handling.
After some back and forth I decided Ghost internal exceptions are the
best way to go about this. I get exceptions are a controversial feature,
IMHO that's because most people use them 'wrong' however. Here's the
rationale:
* Most alternatives require early exiting functions. This early exiting
has to be 'bubbled up' the call stack to the point that performs error
handling. So the code gets really impaired by error checking. Tried
this first and wasn't happy with it at all. Even if error handling is
wrapped into macros.
* All GHOST_Xr resources are managed via RAII. So stack unwinding will
cause them to be released cleanly whenever an exception is thrown.
* GHOST_Xr has a clear boundary (the Ghost C-API) with only a handful of
public functions. That is the only place we need to have try-catch
blocks at.
(Generally, try-catch blocks at kinda random places are a bad code
smell IMHO. Module boundaries are a valid place to put them.)
* Exceptions allow us to pass multiple bits of error information through
mulitple layers of the call stack. This information can also be made
specific with a useful error message.
As of now, they conain a user error message, the OpenXR error code (if
any), as well as the exact source code location the error was caught
at.
So if an error is caught inside GHOST_Xr code, an exception is thrown
with specific and hopefully useful information in it. In the Ghost C-API,
these exceptions are caught and passed on to a custom error handling
callback. This callback will be defined by the Blender window-manager
and output the error information via a usual user report popup (not done
yet). It can also ensure the entire session is shut down.
Note that the majority of errors OpenXR can return are for invalid API
usage. Assuming the API usage is valid though, most error messages
should never reach users and can be left a bit more vague. Maybe we can
add something like "This is probably a bug and should be reported" to
those.
===================================================================
M intern/ghost/GHOST_IXrContext.h
M intern/ghost/intern/GHOST_C-api.cpp
M intern/ghost/intern/GHOST_Xr.cpp
M intern/ghost/intern/GHOST_XrContext.cpp
M intern/ghost/intern/GHOST_XrContext.h
A intern/ghost/intern/GHOST_XrException.h
M intern/ghost/intern/GHOST_XrSession.cpp
M intern/ghost/intern/GHOST_Xr_intern.h
===================================================================
diff --git a/intern/ghost/GHOST_IXrContext.h b/intern/ghost/GHOST_IXrContext.h
index 796805121b5..362bc923ee8 100644
--- a/intern/ghost/GHOST_IXrContext.h
+++ b/intern/ghost/GHOST_IXrContext.h
@@ -32,6 +32,8 @@ class GHOST_IXrContext {
virtual bool isSessionRunning() const = 0;
virtual void drawSessionViews(void *draw_customdata) = 0;
+ virtual void dispatchErrorMessage(const class GHOST_XrException *) const = 0;
+
virtual void setGraphicsContextBindFuncs(GHOST_XrGraphicsContextBindFn bind_fn,
GHOST_XrGraphicsContextUnbindFn unbind_fn) = 0;
virtual void setDrawViewFunc(GHOST_XrDrawViewFn draw_view_fn) = 0;
diff --git a/intern/ghost/intern/GHOST_C-api.cpp b/intern/ghost/intern/GHOST_C-api.cpp
index 5d7a774336c..caa88f46ff9 100644
--- a/intern/ghost/intern/GHOST_C-api.cpp
+++ b/intern/ghost/intern/GHOST_C-api.cpp
@@ -32,6 +32,7 @@
#include "GHOST_IEventConsumer.h"
#include "GHOST_IXrContext.h"
#include "intern/GHOST_CallbackEventConsumer.h"
+#include "intern/GHOST_XrException.h"
GHOST_SystemHandle GHOST_CreateSystem(void)
{
@@ -907,29 +908,46 @@ void GHOST_EndIME(GHOST_WindowHandle windowhandle)
#ifdef WITH_OPENXR
+# define GHOST_XR_CAPI_CALL(call, ctx) \
+ try { \
+ call; \
+ } \
+ catch (GHOST_XrException & e) { \
+ (ctx)->dispatchErrorMessage(&e); \
+ }
+
+# define GHOST_XR_CAPI_CALL_RET(call, ctx) \
+ try { \
+ return call; \
+ } \
+ catch (GHOST_XrException & e) { \
+ (ctx)->dispatchErrorMessage(&e); \
+ }
+
void GHOST_XrSessionStart(GHOST_XrContextHandle xr_contexthandle,
const GHOST_XrSessionBeginInfo *begin_info)
{
GHOST_IXrContext *xr_context = (GHOST_IXrContext *)xr_contexthandle;
- xr_context->startSession(begin_info);
+ GHOST_XR_CAPI_CALL(xr_context->startSession(begin_info), xr_context);
}
void GHOST_XrSessionEnd(GHOST_XrContextHandle xr_contexthandle)
{
GHOST_IXrContext *xr_context = (GHOST_IXrContext *)xr_contexthandle;
- xr_context->endSession();
+ GHOST_XR_CAPI_CALL(xr_context->endSession(), xr_context);
}
int GHOST_XrSessionIsRunning(const GHOST_XrContextHandle xr_contexthandle)
{
const GHOST_IXrContext *xr_context = (GHOST_IXrContext *)xr_contexthandle;
- return xr_context->isSessionRunning();
+ GHOST_XR_CAPI_CALL_RET(xr_context->isSessionRunning(), xr_context);
+ return 0; // Only reached if exception is thrown.
}
void GHOST_XrSessionDrawViews(GHOST_XrContextHandle xr_contexthandle, void *draw_customdata)
{
GHOST_IXrContext *xr_context = (GHOST_IXrContext *)xr_contexthandle;
- xr_context->drawSessionViews(draw_customdata);
+ GHOST_XR_CAPI_CALL(xr_context->drawSessionViews(draw_customdata), xr_context);
}
void GHOST_XrGraphicsContextBindFuncs(GHOST_XrContextHandle xr_contexthandle,
@@ -937,13 +955,13 @@ void GHOST_XrGraphicsContextBindFuncs(GHOST_XrContextHandle xr_contexthandle,
GHOST_XrGraphicsContextUnbindFn unbind_fn)
{
GHOST_IXrContext *xr_context = (GHOST_IXrContext *)xr_contexthandle;
- xr_context->setGraphicsContextBindFuncs(bind_fn, unbind_fn);
+ GHOST_XR_CAPI_CALL(xr_context->setGraphicsContextBindFuncs(bind_fn, unbind_fn), xr_context);
}
void GHOST_XrDrawViewFunc(GHOST_XrContextHandle xr_contexthandle, GHOST_XrDrawViewFn draw_view_fn)
{
GHOST_IXrContext *xr_context = (GHOST_IXrContext *)xr_contexthandle;
- xr_context->setDrawViewFunc(draw_view_fn);
+ GHOST_XR_CAPI_CALL(xr_context->setDrawViewFunc(draw_view_fn), xr_context);
}
#endif
diff --git a/intern/ghost/intern/GHOST_Xr.cpp b/intern/ghost/intern/GHOST_Xr.cpp
index dfdd0d8c0cf..f23b156248f 100644
--- a/intern/ghost/intern/GHOST_Xr.cpp
+++ b/intern/ghost/intern/GHOST_Xr.cpp
@@ -27,6 +27,7 @@
#include "GHOST_Xr_intern.h"
#include "GHOST_XrContext.h"
+#include "GHOST_XrException.h"
/**
* \brief Initialize the window manager XR-Context.
@@ -37,8 +38,13 @@ GHOST_XrContextHandle GHOST_XrContextCreate(const GHOST_XrContextCreateInfo *cre
{
GHOST_XrContext *xr_context = new GHOST_XrContext(create_info);
- if (xr_context->initialize(create_info) == GHOST_kFailure) {
- return nullptr;
+ // TODO GHOST_XrContext's should probably be owned by the GHOST_System, which will handle context
+ // creation and destruction. Try-catch logic can be moved to C-API then.
+ try {
+ xr_context->initialize(create_info);
+ }
+ catch (GHOST_XrException &e) {
+ xr_context->dispatchErrorMessage(&e);
}
return (GHOST_XrContextHandle)xr_context;
diff --git a/intern/ghost/intern/GHOST_XrContext.cpp b/intern/ghost/intern/GHOST_XrContext.cpp
index 0db55f6556f..c94b911fc8b 100644
--- a/intern/ghost/intern/GHOST_XrContext.cpp
+++ b/intern/ghost/intern/GHOST_XrContext.cpp
@@ -25,6 +25,7 @@
#include "GHOST_Types.h"
#include "GHOST_Xr_intern.h"
+#include "GHOST_XrException.h"
#include "GHOST_XrSession.h"
#include "GHOST_XrContext.h"
@@ -57,6 +58,8 @@ GHOST_XrContext::GHOST_XrContext(const GHOST_XrContextCreateInfo *create_info)
}
GHOST_XrContext::~GHOST_XrContext()
{
+ // TODO OpenXR calls here can fail, but we should not throw an exception in the destructor.
+
if (m_oxr->debug_messenger != XR_NULL_HANDLE) {
assert(m_oxr->s_xrDestroyDebugUtilsMessengerEXT_fn != nullptr);
m_oxr->s_xrDestroyDebugUtilsMessengerEXT_fn(m_oxr->debug_messenger);
@@ -66,11 +69,10 @@ GHOST_XrContext::~GHOST_XrContext()
}
}
-GHOST_TSuccess GHOST_XrContext::initialize(const GHOST_XrContextCreateInfo *create_info)
+void GHOST_XrContext::initialize(const GHOST_XrContextCreateInfo *create_info)
{
- if (!enumerateApiLayers() || !enumerateExtensions()) {
- return GHOST_kFailure;
- }
+ enumerateApiLayers();
+ enumerateExtensions();
XR_DEBUG_ONLY_CALL(this, printAvailableAPILayersAndExtensionsInfo());
m_gpu_binding_type = determineGraphicsBindingTypeToEnable(create_info);
@@ -79,8 +81,6 @@ GHOST_TSuccess GHOST_XrContext::initialize(const GHOST_XrContextCreateInfo *crea
createOpenXRInstance();
printInstanceInfo();
XR_DEBUG_ONLY_CALL(this, initDebugMessenger());
-
- return GHOST_kSuccess;
}
void GHOST_XrContext::createOpenXRInstance()
@@ -99,7 +99,8 @@ void GHOST_XrContext::createOpenXRInstance()
create_info.enabledExtensionNames = m_enabled_extensions.data();
XR_DEBUG_ONLY_CALL(this, printExtensionsAndAPILayersToEnable());
- xrCreateInstance(&create_info, &m_oxr->instance);
+ CHECK_XR(xrCreateInstance(&create_info, &m_oxr->instance),
+ "Failed to connect to an OpenXR runtime.");
}
/** \} */ /* Create, Initialize and Destruct */
@@ -114,7 +115,8 @@ void GHOST_XrContext::printInstanceInfo()
assert(m_oxr->instance != XR_NULL_HANDLE);
XrInstanceProperties instance_properties{XR_TYPE_INSTANCE_PROPERTIES};
- xrGetInstanceProperties(m_oxr->instance, &instance_properties);
+ CHECK_XR(xrGetInstanceProperties(m_oxr->instance, &instance_properties),
+ "Failed to get OpenXR runtime information. Do you have an active runtime set up?");
printf("Connected to OpenXR runtime: %s (Version %u.%u.%u)\n",
instance_properties.runtimeName,
@@ -167,9 +169,12 @@ void GHOST_XrContext::initDebugMessenger()
m_oxr->instance,
"xrDestroyDebugUtilsMessengerEXT",
(PFN_xrVoidFunction *)&m_oxr->s_xrDestroyDebugUtilsMessengerEXT_fn))) {
- fprintf(stderr, "Could not use XR_EXT_debug_utils to enable debug prints.\n");
m_oxr->s_xrCreateDebugUtilsMessengerEXT_fn = nullptr;
m_oxr->s_xrDestroyDebugUtilsMessengerEXT_fn = nullptr;
+
+ fprintf(stderr,
+ "Could not use XR_EXT_debug_utils to enable debug prints. Not a fatal error, "
+ "continuing without the messenger.\n");
return;
}
@@ -182,8 +187,23 @@ void GHOST_XrContext::initDebugMessenger()
XR_DEBUG_UTILS_MESSAGE_TYPE_PERFORMANCE_BIT_EXT;
create_info.userCallback = debug_messenger_func;
- m_oxr->s_xrCreateDebugUtilsMessengerEXT_fn(
- m_oxr->instance, &create_info, &m_oxr->debug_messenger);
+ if (XR_FAILED(m_oxr->s_xrCreateDebugUtilsMessengerEXT_fn(
+ m_oxr->instance, &create_info, &m_oxr->debug_messenger))) {
+ fprintf(stderr,
+ "Failed to create OpenXR debug messenger. Not a fatal error, continuing without the "
+ "messenger.\n");
+ return;
+ }
+}
+
+void GHOST_XrContext::dispatchErrorMessage(const GHOST_XrException *exception) const
+{
+ // TODO dummy
+ printf("Error: %s %i (%s:%i)\n",
+ exception->m_msg,
+ exception->m_res,
+ exception->m_file,
+ exception->m_line);
}
/** \} */ /* Debug Printing */
@@ -196,20 +216,18 @@ void GHOST_XrContext::initDebugMessenger()
/**
* \param layer_name May be NULL for extensions not belonging to a specific layer.
*/
-GHOST_TSuccess GHOST_XrContext::enumerateExtensionsEx(
- std::vector<XrExtensionProperties> &extensions, const char *layer_name)
+void GHOST_XrContext::enumerateExtensionsEx(std::vector<XrExtensionProperties> &extensions,
+ const char *layer_name)
{
uint32_t extension_count = 0;
/* Get count for array creation/init first. */
- if (XR_FAILED(
- xrEnumerateInstanceExtensionProperties(layer_name, 0, &extension_count, nullptr))) {
- return GHOST_kFailure;
- }
+ CHECK_XR(xrEnumerateInstanceExtensionProperties(layer_name, 0, &extension_count, nullptr),
+ "Failed to query OpenXR runtime information. Do you have an active runtime set up?");
if (extension_count == 0) {
/* Extensions are optional, can successfully exit. */
- return GHOST_kSuccess;
+ return;
}
for (uint32_t i = 0; i < extension_count; i++) {
@@ -218,28 +236,26 @@ GHOST_TSuccess GHOST_XrContext::enumerateExtensionsEx(
}
/* Actually get the extensions. */
- xrEnumerateInstanceExtensionProperties(
- layer_name, extension_count, &extension_count, extensions.data());
-
- return GHOST_kSuccess;
+ CHECK_XR(xrEnumerateInstanceExtensionProperties(
+ layer_name, extension_count, &extension_count, extensions.data()),
+ "Failed to query OpenXR runtime information. Do you have an active runtime set up?
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list