Do not create OpenGL context until render aspect is registered

This avoids a race where the renderer creates a context based on the
default format when the render aspect is created. The alternative
fix would have been to simply create the QRenderAspect after
setting the default surface format. However, that would mean being
careful to do that everywhere in the future too. This fixes it
more neatly and completely by creating the renderer in the
render aspect's onRegistered() function.

Whilst in there, this also solves the previous asymmetry in the
onRegistered() and onUnregistred() functions. We now also
unregister backend types and re-register them in onRegistered.
This is necessary because the mappers depend upon the renderer.

Also added a todo to make the unregisterBackendType() functions
public for 5.8.

Task-number: QTBUG-54370
Task-number: QTBUG-53880
Change-Id: I09a774739df069d3210ed0efe6dd504d968b05f3
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>

2 files changed, 70 insertions, 21 deletions

diff --git a/src/render/frontend/qrenderaspect.cpp b/src/render/frontend/qrenderaspect.cpp
index d6c69855c..2f529ed00 100644
--- a/src/render/frontend/qrenderaspect.cpp
+++ b/src/render/frontend/qrenderaspect.cpp
@@ -162,6 +162,7 @@ QRenderAspectPrivate::QRenderAspectPrivate(QRenderAspect::RenderType type)
     , m_worldTransformJob(Render::UpdateWorldTransformJobPtr::create())
     , m_updateBoundingVolumeJob(Render::UpdateBoundingVolumeJobPtr::create())
     , m_calculateBoundingVolumeJob(Render::CalculateBoundingVolumeJobPtr::create(m_nodeManagers))
+    , m_renderType(type)
 {
     initResources();
 
@@ -175,11 +176,6 @@ QRenderAspectPrivate::QRenderAspectPrivate(QRenderAspect::RenderType type)
 
     // All world stuff depends on the RenderEntity's localBoundingVolume
     m_worldTransformJob->addDependency(m_calculateBoundingVolumeJob);
-
-    // Create a renderer implementation given
-    // a specific rendering API -> only OpenGL for now
-    m_renderer = new Render::Renderer(type);
-    m_renderer->setNodeManagers(m_nodeManagers);
 }
 
 /*! \internal */
@@ -250,6 +246,62 @@ void QRenderAspectPrivate::registerBackendTypes()
     q->registerBackendType<QObjectPicker>(QSharedPointer<Render::NodeFunctor<Render::ObjectPicker, Render::ObjectPickerManager> >::create(m_renderer, m_nodeManagers->objectPickerManager()));
 }
 
+/*! \internal */
+void QRenderAspectPrivate::unregisterBackendTypes()
+{
+    unregisterBackendType<Qt3DCore::QEntity>();
+    unregisterBackendType<Qt3DCore::QTransform>();
+
+    unregisterBackendType<Qt3DRender::QCameraLens>();
+    unregisterBackendType<QLayer>();
+    unregisterBackendType<QSceneLoader>();
+    unregisterBackendType<QRenderTarget>();
+    unregisterBackendType<QRenderTargetOutput>();
+    unregisterBackendType<QRenderSettings>();
+    unregisterBackendType<QRenderState>();
+
+    // Geometry + Compute
+    unregisterBackendType<QAttribute>();
+    unregisterBackendType<QBuffer>();
+    unregisterBackendType<QComputeCommand>();
+    unregisterBackendType<QGeometry>();
+    unregisterBackendType<QGeometryRenderer>();
+
+    // Textures
+    unregisterBackendType<QAbstractTexture>();
+    unregisterBackendType<QAbstractTextureImage>();
+
+    // Material system
+    unregisterBackendType<QEffect>();
+    unregisterBackendType<QFilterKey>();
+    unregisterBackendType<QAbstractLight>();
+    unregisterBackendType<QMaterial>();
+    unregisterBackendType<QParameter>();
+    unregisterBackendType<QRenderPass>();
+    unregisterBackendType<QShaderData>();
+    unregisterBackendType<QShaderProgram>();
+    unregisterBackendType<QTechnique>();
+
+    // Framegraph
+    unregisterBackendType<QCameraSelector>();
+    unregisterBackendType<QClearBuffers>();
+    unregisterBackendType<QDispatchCompute>();
+    unregisterBackendType<QFrustumCulling>();
+    unregisterBackendType<QLayerFilter>();
+    unregisterBackendType<QNoDraw>();
+    unregisterBackendType<QRenderPassFilter>();
+    unregisterBackendType<QRenderStateSet>();
+    unregisterBackendType<QRenderSurfaceSelector>();
+    unregisterBackendType<QRenderTargetSelector>();
+    unregisterBackendType<QSortPolicy>();
+    unregisterBackendType<QTechniqueFilter>();
+    unregisterBackendType<QViewport>();
+
+    // Picking
+    // unregisterBackendType<QBoundingVolumeDebug>();
+    unregisterBackendType<QObjectPicker>();
+}
+
 /*!
  * The constructor creates a new QRenderAspect::QRenderAspect instance with the
  * specified \a parent.
@@ -271,12 +323,7 @@ QRenderAspect::QRenderAspect(QRenderAspect::RenderType type, QObject *parent)
 QRenderAspect::QRenderAspect(QRenderAspectPrivate &dd, QObject *parent)
     : QAbstractAspect(dd, parent)
 {
-    // Won't return until the private RenderThread in Renderer has been created
-    // The Renderer is set to wait the surface with a wait condition
-    // Threads modifying the Renderer should be synchronized using the Renderer's mutex
     setObjectName(QStringLiteral("Render Aspect"));
-    Q_D(QRenderAspect);
-    d->registerBackendTypes();
 }
 
 /*! \internal */
@@ -415,9 +462,16 @@ void QRenderAspect::onEngineStartup()
 
 void QRenderAspect::onRegistered()
 {
-    // TODO: Remove the m_initialized variable and split out onInitialize()
-    // and setting a resource (the QSurface) on the aspects.
+    // Create a renderer each time as this is destroyed in onUnregistered below. If
+    // using a threaded renderer, this blocks until the render thread has been created
+    // and started.
     Q_D(QRenderAspect);
+    d->m_renderer = new Render::Renderer(d->m_renderType);
+    d->m_renderer->setNodeManagers(d->m_nodeManagers);
+
+    // Register backend types now that we have a renderer
+    d->registerBackendTypes();
+
     if (!d->m_initialized) {
 
         // Register the VSyncFrameAdvanceService to drive the aspect manager loop
@@ -434,14 +488,6 @@ void QRenderAspect::onRegistered()
         d->m_initialized = true;
     }
 
-    //    QSurface *surface = nullptr;
-    //    const QVariant &v = data.value(QStringLiteral("surface"));
-    //    if (v.isValid())
-    //        surface = v.value<QSurface *>();
-
-    //    if (surface)
-    //        d->setSurface(surface);
-
     if (d->m_aspectManager)
         d->m_renderer->registerEventFilter(d->services()->eventFilterService());
 }
@@ -459,6 +505,8 @@ void QRenderAspect::onUnregistered()
         d->m_renderer->destroyAllocators(d->jobManager());
     }
 
+    d->unregisterBackendTypes();
+
     // Waits for the render thread to join (if using threaded renderer)
     delete d->m_renderer;
     d->m_renderer = nullptr;
diff --git a/src/render/frontend/qrenderaspect_p.h b/src/render/frontend/qrenderaspect_p.h
index e0ea0cc00..a71b982c8 100644
--- a/src/render/frontend/qrenderaspect_p.h
+++ b/src/render/frontend/qrenderaspect_p.h
@@ -81,6 +81,7 @@ public:
     Q_DECLARE_PUBLIC(QRenderAspect)
 
     void registerBackendTypes();
+    void unregisterBackendTypes();
     void loadSceneParsers();
     void renderInitialize(QOpenGLContext *context);
     void renderSynchronous();
@@ -98,7 +99,7 @@ public:
     Render::UpdateBoundingVolumeJobPtr m_updateBoundingVolumeJob;
     Render::CalculateBoundingVolumeJobPtr m_calculateBoundingVolumeJob;
     QList<QSceneIOHandler *> m_sceneIOHandler;
-
+    QRenderAspect::RenderType m_renderType;
 };
 
 }