Log Message
[WebXR] Move OpenXR calls off the main thread https://bugs.webkit.org/show_bug.cgi?id=217752
Reviewed by Youenn Fablet. The OpenXR API is synchronous. Many of the calls involve dealing with external hardware devices meaning that they have to potential to block the main thread. They should be moved to a different thread in order to avoid that. The PlatformXR::Instance creates a WorkQueue which is going to be used by the OpenXR devices to issue OpenXR calls and also serialize them to ensure that they are executed sequentially. The OpenXRDevice's are created in the main thread anyway because we need to get weak pointers from them in the main thread. * Modules/webxr/WebXRSystem.cpp: (WebCore::WebXRSystem::ensureImmersiveXRDeviceIsSelected): Use a scoped exit to call callback. Also the Vector of immersive devices is now a pointer which might be null, meaning no available devices. * platform/xr/PlatformXR.h: Added a "using" for the Vector of devices. * platform/xr/openxr/PlatformXROpenXR.cpp: (PlatformXR::Instance::Impl::queue const): New getter returning the WorkQueue. (PlatformXR::Instance::Impl::enumerateApiLayerProperties const): Added an ASSERT. (PlatformXR::Instance::Impl::checkInstanceExtensionProperties const): Ditto. (PlatformXR::Instance::Impl::Impl): Create the OpenXR WorkQueue and dispatch a task to perform the OpenXR system initialization in the queue. (PlatformXR::Instance::Impl::~Impl): Delete the instance in the WorkQueue (PlatformXR::Instance::enumerateImmersiveXRDevices): Moved the code to a task in the WorkQueue. (PlatformXR::OpenXRDevice::OpenXRDevice): Ditto. Also added a completion handler to notify the caller about the end of the device initialization process. (PlatformXR::OpenXRDevice::collectSupportedSessionModes): Added an ASSERT. (PlatformXR::OpenXRDevice::collectConfigurationViews): Ditto. * platform/xr/openxr/PlatformXROpenXR.h: Added WorkQueue attribute and parameter to device constructor.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (269031 => 269032)
--- trunk/Source/WebCore/ChangeLog 2020-10-27 11:40:56 UTC (rev 269031)
+++ trunk/Source/WebCore/ChangeLog 2020-10-27 12:30:53 UTC (rev 269032)
@@ -1,3 +1,37 @@
+2020-10-15 Sergio Villar Senin <[email protected]>
+
+ [WebXR] Move OpenXR calls off the main thread
+ https://bugs.webkit.org/show_bug.cgi?id=217752
+
+ Reviewed by Youenn Fablet.
+
+ The OpenXR API is synchronous. Many of the calls involve dealing with external hardware devices
+ meaning that they have to potential to block the main thread. They should be moved to a different
+ thread in order to avoid that.
+
+ The PlatformXR::Instance creates a WorkQueue which is going to be used by the OpenXR devices to
+ issue OpenXR calls and also serialize them to ensure that they are executed sequentially. The
+ OpenXRDevice's are created in the main thread anyway because we need to get weak pointers from
+ them in the main thread.
+
+ * Modules/webxr/WebXRSystem.cpp:
+ (WebCore::WebXRSystem::ensureImmersiveXRDeviceIsSelected): Use a scoped exit to call callback. Also
+ the Vector of immersive devices is now a pointer which might be null, meaning no available devices.
+ * platform/xr/PlatformXR.h: Added a "using" for the Vector of devices.
+ * platform/xr/openxr/PlatformXROpenXR.cpp:
+ (PlatformXR::Instance::Impl::queue const): New getter returning the WorkQueue.
+ (PlatformXR::Instance::Impl::enumerateApiLayerProperties const): Added an ASSERT.
+ (PlatformXR::Instance::Impl::checkInstanceExtensionProperties const): Ditto.
+ (PlatformXR::Instance::Impl::Impl): Create the OpenXR WorkQueue and dispatch a task to perform the
+ OpenXR system initialization in the queue.
+ (PlatformXR::Instance::Impl::~Impl): Delete the instance in the WorkQueue
+ (PlatformXR::Instance::enumerateImmersiveXRDevices): Moved the code to a task in the WorkQueue.
+ (PlatformXR::OpenXRDevice::OpenXRDevice): Ditto. Also added a completion handler to notify the caller
+ about the end of the device initialization process.
+ (PlatformXR::OpenXRDevice::collectSupportedSessionModes): Added an ASSERT.
+ (PlatformXR::OpenXRDevice::collectConfigurationViews): Ditto.
+ * platform/xr/openxr/PlatformXROpenXR.h: Added WorkQueue attribute and parameter to device constructor.
+
2020-10-27 Xabier Rodriguez Calvar <[email protected]>
[EME][GStreamer] Fix logging in utilities
Modified: trunk/Source/WebCore/Modules/webxr/WebXRSystem.cpp (269031 => 269032)
--- trunk/Source/WebCore/Modules/webxr/WebXRSystem.cpp 2020-10-27 11:40:56 UTC (rev 269031)
+++ trunk/Source/WebCore/Modules/webxr/WebXRSystem.cpp 2020-10-27 12:30:53 UTC (rev 269032)
@@ -88,20 +88,22 @@
platformXR.enumerateImmersiveXRDevices([this, protectedThis = makeRef(*this), isFirstXRDevicesEnumeration, callback = WTFMove(callback)](auto& immersiveXRDevices) mutable {
m_immersiveXRDevicesHaveBeenEnumerated = true;
+ auto callbackOnExit = makeScopeExit([&]() {
+ callOnMainThread(WTFMove(callback));
+ });
+
// https://immersive-web.github.io/webxr/#select-an-immersive-xr-device
auto* oldDevice = m_activeImmersiveDevice.get();
if (immersiveXRDevices.isEmpty()) {
m_activeImmersiveDevice = nullptr;
- callback();
return;
}
if (immersiveXRDevices.size() == 1) {
m_activeImmersiveDevice = makeWeakPtr(immersiveXRDevices.first().get());
- callback();
return;
}
- if (m_activeImmersiveSession && oldDevice && immersiveXRDevices.findMatching([&](auto& entry) { return entry.get() == oldDevice; }) != notFound)
+ if (m_activeImmersiveSession && oldDevice && immersiveXRDevices.findMatching([&](auto& entry) { return &entry == oldDevice; }) != notFound)
ASSERT(m_activeImmersiveDevice.get() == oldDevice);
else {
// FIXME: implement a better UA selection mechanism if required.
@@ -109,7 +111,6 @@
}
if (isFirstXRDevicesEnumeration || m_activeImmersiveDevice.get() == oldDevice) {
- callback();
return;
}
@@ -116,8 +117,6 @@
// FIXME: 7. Shut down any active XRSessions.
// FIXME: 8. Set the XR compatible boolean of all WebGLRenderingContextBase instances to false.
// FIXME: 9. Queue a task to fire an event named devicechange on the context object.
-
- callback();
});
}
Modified: trunk/Source/WebCore/platform/xr/PlatformXR.h (269031 => 269032)
--- trunk/Source/WebCore/platform/xr/PlatformXR.h 2020-10-27 11:40:56 UTC (rev 269031)
+++ trunk/Source/WebCore/platform/xr/PlatformXR.h 2020-10-27 12:30:53 UTC (rev 269032)
@@ -75,7 +75,8 @@
public:
static Instance& singleton();
- void enumerateImmersiveXRDevices(CompletionHandler<void(const Vector<std::unique_ptr<Device>>&)>&&);
+ using DeviceList = Vector<UniqueRef<Device>>;
+ void enumerateImmersiveXRDevices(CompletionHandler<void(const DeviceList&)>&&);
private:
friend LazyNeverDestroyed<Instance>;
@@ -85,7 +86,7 @@
struct Impl;
UniqueRef<Impl> m_impl;
- Vector<std::unique_ptr<Device>> m_immersiveXRDevices;
+ DeviceList m_immersiveXRDevices;
};
#endif // ENABLE(WEBXR)
Modified: trunk/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp (269031 => 269032)
--- trunk/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp 2020-10-27 11:40:56 UTC (rev 269031)
+++ trunk/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp 2020-10-27 12:30:53 UTC (rev 269032)
@@ -73,6 +73,7 @@
#if USE_OPENXR
XrInstance xrInstance() const { return m_instance; }
+ WorkQueue& queue() const { return m_workQueue; }
#endif
private:
@@ -81,6 +82,7 @@
bool checkInstanceExtensionProperties() const;
XrInstance m_instance { XR_NULL_HANDLE };
+ Ref<WorkQueue> m_workQueue;
#endif // USE_OPENXR
};
@@ -87,6 +89,7 @@
#if USE_OPENXR
void Instance::Impl::enumerateApiLayerProperties() const
{
+ ASSERT(&RunLoop::current() == &m_workQueue->runLoop());
uint32_t propertyCountOutput { 0 };
XrResult result = xrEnumerateApiLayerProperties(0, &propertyCountOutput, nullptr);
RETURN_IF_FAILED(result, "xrEnumerateApiLayerProperties()", m_instance);
@@ -119,6 +122,7 @@
bool Instance::Impl::checkInstanceExtensionProperties() const
{
+ ASSERT(&RunLoop::current() == &m_workQueue->runLoop());
uint32_t propertyCountOutput { 0 };
XrResult result = xrEnumerateInstanceExtensionProperties(nullptr, 0, &propertyCountOutput, nullptr);
RETURN_IF_FAILED(result, "xrEnumerateInstanceExtensionProperties", m_instance, false);
@@ -154,36 +158,41 @@
#endif // USE_OPENXR
Instance::Impl::Impl()
+#if USE_OPENXR
+ : m_workQueue(WorkQueue::create("OpenXR queue"))
+#endif
{
#if USE_OPENXR
- LOG(XR, "OpenXR: initializing\n");
+ m_workQueue->dispatch([this]() {
+ LOG(XR, "OpenXR: initializing\n");
- enumerateApiLayerProperties();
+ enumerateApiLayerProperties();
- if (!checkInstanceExtensionProperties())
- return;
+ if (!checkInstanceExtensionProperties())
+ return;
- static const char* s_applicationName = "WebXR (WebKit)";
- static const uint32_t s_applicationVersion = 1;
+ static const char* s_applicationName = "WebXR (WebKit)";
+ static const uint32_t s_applicationVersion = 1;
- const char* const enabledExtensions[] = {
- XR_MND_HEADLESS_EXTENSION_NAME,
- };
+ const char* const enabledExtensions[] = {
+ XR_MND_HEADLESS_EXTENSION_NAME,
+ };
- auto createInfo = createStructure<XrInstanceCreateInfo, XR_TYPE_INSTANCE_CREATE_INFO>();
- createInfo.createFlags = 0;
- std::memcpy(createInfo.applicationInfo.applicationName, s_applicationName, XR_MAX_APPLICATION_NAME_SIZE);
- createInfo.applicationInfo.apiVersion = XR_CURRENT_API_VERSION;
- createInfo.applicationInfo.applicationVersion = s_applicationVersion;
- createInfo.enabledApiLayerCount = 0;
- createInfo.enabledExtensionCount = WTF_ARRAY_LENGTH(enabledExtensions);
- createInfo.enabledExtensionNames = enabledExtensions;
+ auto createInfo = createStructure<XrInstanceCreateInfo, XR_TYPE_INSTANCE_CREATE_INFO>();
+ createInfo.createFlags = 0;
+ std::memcpy(createInfo.applicationInfo.applicationName, s_applicationName, XR_MAX_APPLICATION_NAME_SIZE);
+ createInfo.applicationInfo.apiVersion = XR_CURRENT_API_VERSION;
+ createInfo.applicationInfo.applicationVersion = s_applicationVersion;
+ createInfo.enabledApiLayerCount = 0;
+ createInfo.enabledExtensionCount = WTF_ARRAY_LENGTH(enabledExtensions);
+ createInfo.enabledExtensionNames = enabledExtensions;
- XrInstance instance;
- XrResult result = xrCreateInstance(&createInfo, &instance);
- RETURN_IF_FAILED(result, "xrCreateInstance()", m_instance);
- m_instance = instance;
- LOG(XR, "xrCreateInstance(): using instance %p\n", m_instance);
+ XrInstance instance;
+ XrResult result = xrCreateInstance(&createInfo, &instance);
+ RETURN_IF_FAILED(result, "xrCreateInstance()", m_instance);
+ m_instance = instance;
+ LOG(XR, "xrCreateInstance(): using instance %p\n", m_instance);
+ });
#endif // USE_OPENXR
}
@@ -190,8 +199,10 @@
Instance::Impl::~Impl()
{
#if USE_OPENXR
- if (m_instance != XR_NULL_HANDLE)
- xrDestroyInstance(m_instance);
+ m_workQueue->dispatch([this] {
+ if (m_instance != XR_NULL_HANDLE)
+ xrDestroyInstance(m_instance);
+ });
#endif
}
@@ -211,51 +222,64 @@
{
}
-void Instance::enumerateImmersiveXRDevices(CompletionHandler<void(const Vector<std::unique_ptr<Device>>& devices)>&& callback)
+void Instance::enumerateImmersiveXRDevices(CompletionHandler<void(const DeviceList& devices)>&& callback)
{
#if USE_OPENXR
- auto callbackOnExit = makeScopeExit([&]() {
- callback({ });
- });
- if (m_impl->xrInstance() == XR_NULL_HANDLE) {
- LOG(XR, "%s Unable to enumerate XR devices. No XrInstance present\n", __FUNCTION__);
- return;
- }
+ m_impl->queue().dispatch([this, callback = WTFMove(callback)]() mutable {
+ auto callbackOnExit = makeScopeExit([&]() {
+ callOnMainThread([callback = WTFMove(callback)]() mutable {
+ callback({ });
+ });
+ });
- auto systemGetInfo = createStructure<XrSystemGetInfo, XR_TYPE_SYSTEM_GET_INFO>();
- systemGetInfo.formFactor = XR_FORM_FACTOR_HEAD_MOUNTED_DISPLAY;
+ if (m_impl->xrInstance() == XR_NULL_HANDLE) {
+ LOG(XR, "%s Unable to enumerate XR devices. No XrInstance present\n", __FUNCTION__);
+ return;
+ }
- XrSystemId systemId;
- XrResult result = xrGetSystem(m_impl->xrInstance(), &systemGetInfo, &systemId);
- RETURN_IF_FAILED(result, "xrGetSystem", m_impl->xrInstance());
+ auto systemGetInfo = createStructure<XrSystemGetInfo, XR_TYPE_SYSTEM_GET_INFO>();
+ systemGetInfo.formFactor = XR_FORM_FACTOR_HEAD_MOUNTED_DISPLAY;
-#if !LOG_DISABLED
- auto systemProperties = createStructure<XrSystemProperties, XR_TYPE_SYSTEM_PROPERTIES>();
- result = xrGetSystemProperties(m_impl->xrInstance(), systemId, &systemProperties);
- if (result == XR_SUCCESS)
- LOG(XR, "Found XRSystem %lu: \"%s\", vendor ID %d\n", systemProperties.systemId, systemProperties.systemName, systemProperties.vendorId);
-#endif
+ XrSystemId systemId;
+ XrResult result = xrGetSystem(m_impl->xrInstance(), &systemGetInfo, &systemId);
+ RETURN_IF_FAILED(result, "xrGetSystem", m_impl->xrInstance());
- m_immersiveXRDevices.append(makeUnique<OpenXRDevice>(systemId, m_impl->xrInstance()));
- callback(m_immersiveXRDevices);
- callbackOnExit.release();
+ callbackOnExit.release();
+
+ callOnMainThread([this, callback = WTFMove(callback), systemId]() mutable {
+ m_immersiveXRDevices = DeviceList::from(makeUniqueRef<OpenXRDevice>(systemId, m_impl->xrInstance(), m_impl->queue(), [this, callback = WTFMove(callback)]() mutable {
+ ASSERT(isMainThread());
+ callback(m_immersiveXRDevices);
+ }));
+ });
+ });
+
#endif // USE_OPENXR
}
#if USE_OPENXR
-OpenXRDevice::OpenXRDevice(XrSystemId id, XrInstance instance)
+OpenXRDevice::OpenXRDevice(XrSystemId id, XrInstance instance, WorkQueue& queue, CompletionHandler<void()>&& callback)
: m_systemId(id)
, m_instance(instance)
+ , m_queue(queue)
{
- auto systemProperties = createStructure<XrSystemProperties, XR_TYPE_SYSTEM_PROPERTIES>();
- XrResult result = xrGetSystemProperties(instance, m_systemId, &systemProperties);
- if (result == XR_SUCCESS)
- m_supportsOrientationTracking = systemProperties.trackingProperties.orientationTracking == XR_TRUE;
- else
- LOG(XR, "xrGetSystemProperties(): error %s\n", resultToString(result, m_instance).utf8().data());
+ ASSERT(isMainThread());
+ m_queue.dispatch([this, callback = WTFMove(callback)]() mutable {
+ auto systemProperties = createStructure<XrSystemProperties, XR_TYPE_SYSTEM_PROPERTIES>();
+ auto result = xrGetSystemProperties(m_instance, m_systemId, &systemProperties);
+ if (XR_SUCCEEDED(result))
+ m_supportsOrientationTracking = systemProperties.trackingProperties.orientationTracking == XR_TRUE;
+#if !LOG_DISABLED
+ else
+ LOG(XR, "xrGetSystemProperties(): error %s\n", resultToString(result, m_instance).utf8().data());
+ LOG(XR, "Found XRSystem %lu: \"%s\", vendor ID %d\n", systemProperties.systemId, systemProperties.systemName, systemProperties.vendorId);
+#endif
- collectSupportedSessionModes();
- collectConfigurationViews();
+ collectSupportedSessionModes();
+ collectConfigurationViews();
+
+ callOnMainThread(WTFMove(callback));
+ });
}
Device::ListOfEnabledFeatures OpenXRDevice::enumerateReferenceSpaces(XrSession& session) const
@@ -299,6 +323,7 @@
void OpenXRDevice::collectSupportedSessionModes()
{
+ ASSERT(&RunLoop::current() == &m_queue.runLoop());
uint32_t viewConfigurationCount;
auto result = xrEnumerateViewConfigurations(m_instance, m_systemId, 0, &viewConfigurationCount, nullptr);
RETURN_IF_FAILED(result, "xrEnumerateViewConfigurations", m_instance);
@@ -341,6 +366,7 @@
void OpenXRDevice::collectConfigurationViews()
{
+ ASSERT(&RunLoop::current() == &m_queue.runLoop());
for (auto& config : m_viewConfigurationProperties.values()) {
uint32_t viewCount;
auto configType = config.viewConfigurationType;
Modified: trunk/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.h (269031 => 269032)
--- trunk/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.h 2020-10-27 11:40:56 UTC (rev 269031)
+++ trunk/Source/WebCore/platform/xr/openxr/PlatformXROpenXR.h 2020-10-27 12:30:53 UTC (rev 269032)
@@ -26,6 +26,7 @@
#if USE_OPENXR
#include <openxr/openxr.h>
+#include <wtf/WorkQueue.h>
namespace PlatformXR {
@@ -43,8 +44,9 @@
// the XRSystem is basically the entry point for the WebXR API available via the Navigator object.
class OpenXRDevice final : public Device {
public:
- OpenXRDevice(XrSystemId, XrInstance);
+ OpenXRDevice(XrSystemId, XrInstance, WorkQueue&, CompletionHandler<void()>&&);
XrSystemId xrSystemId() const { return m_systemId; }
+
private:
void collectSupportedSessionModes();
void collectConfigurationViews();
@@ -61,6 +63,8 @@
XrSystemId m_systemId;
XrInstance m_instance;
XrSession m_session;
+
+ WorkQueue& m_queue;
};
} // namespace PlatformXR
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
