[Idea] Never panic when calling extension methods
zmerp opened this issue · 4 comments
Currently when some extensions are disabled and you try to call some associated methods, for example request_display_refresh_rate, it would lead to a crash because there is one expect() call:
In such cases it seems OpenXR would return XR_ERROR_FUNCTION_UNSUPPORTED. We could also return that error code. In this way it would be simpler to just do .ok() for some calls like request_display_refresh_rate which don't require any special handling when it fails.
In such cases it seems OpenXR would return XR_ERROR_FUNCTION_UNSUPPORTED
What's the basis for this conclusion? In general, extension functions that are not implemented by a runtime cannot return any particular value because they do not exist at all.
All functions belonging to extensions have this XR_ERROR_FUNCTION_UNSUPPORTED return value. it's not well documented, except for xrGetInstanceProcAddr:
xrGetInstanceProcAddr must return XR_ERROR_FUNCTION_UNSUPPORTED if instance is a valid instance and the string specified in name is not the name of an OpenXR core or enabled extension function.
So it seems that extensions functions could return this value if the platform specific loader already have them defined statically but the associated extension is not enabled for a particular instance. So, even if in openxrs all extension functions are loaded dynamically, I think it would be semantically correct and equivalent to return XR_ERROR_FUNCTION_UNSUPPORTED when the associated extension was not enabled.
That's documenting the behavior of xrGetInstanceProcAddr, not the extension functions. An extension function which does not exist cannot have a specified return value.
I'm working on adding new high level wrappers for some extensions. Some existing wrappers (like Session::enumerate_color_spaces) panic when the relative extension was not loaded, while some (like HandTracker::create) return Err(sys::Result::ERROR_EXTENSION_NOT_PRESENT). I'm thinking that extension methods called from to Session or Instance + extension objects constructors should return ERROR_EXTENSION_NOT_PRESENT when the relative extension is not loaded; while it's ok for methods relative to extension objects (like HandTracker) to panic when the relative extension is not loaded.