QRenderSurfaceSelector cleanup

Change-Id: Ic38c88d5e940f04667278015dc4f6c45f162e936
Task-number: QTBUG-51508
Reviewed-by: Sean Harmer <sean.harmer@kdab.com>

9 files changed, 81 insertions, 86 deletions

diff --git a/examples/qt3d/anaglyph-rendering/StereoFrameGraph.qml b/examples/qt3d/anaglyph-rendering/StereoFrameGraph.qml
index c39afc9d6..f48a066b9 100644
--- a/examples/qt3d/anaglyph-rendering/StereoFrameGraph.qml
+++ b/examples/qt3d/anaglyph-rendering/StereoFrameGraph.qml
@@ -55,7 +55,7 @@ Viewport {
 
     property alias leftCamera: leftCameraSelector.camera
     property alias rightCamera: rightCameraSelector.camera
-    property alias window: surfaceSelector.window
+    property alias window: surfaceSelector.surface
 
     RenderSurfaceSelector {
         id: surfaceSelector
diff --git a/examples/qt3d/examples-common/qt3dquickwindow.cpp b/examples/qt3d/examples-common/qt3dquickwindow.cpp
index 72e10c34e..e3e40912b 100644
--- a/examples/qt3d/examples-common/qt3dquickwindow.cpp
+++ b/examples/qt3d/examples-common/qt3dquickwindow.cpp
@@ -213,7 +213,7 @@ void Qt3DQuickWindow::setWindowSurface(QObject *rootObject)
         return;
     }
 
-    surfaceSelector->setWindow(this);
+    surfaceSelector->setSurface(this);
 }
 
 void Qt3DQuickWindow::setCameraAspectModeHelper()
diff --git a/examples/qt3d/materials/SortedForwardRenderer.qml b/examples/qt3d/materials/SortedForwardRenderer.qml
index 0c477d2fa..ba2922e90 100644
--- a/examples/qt3d/materials/SortedForwardRenderer.qml
+++ b/examples/qt3d/materials/SortedForwardRenderer.qml
@@ -57,7 +57,7 @@ TechniqueFilter {
 
     // Expose camera to allow user to choose which camera to use for rendering
     property alias camera: cameraSelector.camera
-    property alias window: surfaceSelector.window
+    property alias window: surfaceSelector.surface
 
     // Select the forward rendering Technique of any used Effect
     matchAll: [ FilterKey { name: "renderingStyle"; value: "forward" } ]
diff --git a/src/quick3d/imports/scene3d/scene3ditem.cpp b/src/quick3d/imports/scene3d/scene3ditem.cpp
index 1364f3c05..c29a4f4d6 100644
--- a/src/quick3d/imports/scene3d/scene3ditem.cpp
+++ b/src/quick3d/imports/scene3d/scene3ditem.cpp
@@ -214,8 +214,8 @@ void Scene3DItem::setWindowSurface(QObject *rootObject)
 
     // Set the item's window surface if it appears
     // the surface wasn't set on the surfaceSelector
-    if (!surfaceSelector->window())
-        surfaceSelector->setWindow(this->window());
+    if (!surfaceSelector->surface())
+        surfaceSelector->setSurface(this->window());
 }
 
 void Scene3DItem::setItemArea(const QSize &area)
diff --git a/src/render/defaults/qforwardrenderer.cpp b/src/render/defaults/qforwardrenderer.cpp
index 96e2478e0..cfd68f3dc 100644
--- a/src/render/defaults/qforwardrenderer.cpp
+++ b/src/render/defaults/qforwardrenderer.cpp
@@ -109,7 +109,7 @@ QForwardRenderer::QForwardRenderer(QNode *parent)
     QObject::connect(d->m_viewport, SIGNAL(clearColorChanged(const QColor &)), this, SIGNAL(clearColorChanged(const QColor &)));
     QObject::connect(d->m_viewport, SIGNAL(normalizedRectChanged(const QRectF &)), this, SIGNAL(viewportRectChanged(const QRectF &)));
     QObject::connect(d->m_cameraSelector, SIGNAL(cameraChanged(Qt3DCore::QEntity *)), this, SIGNAL(cameraChanged(Qt3DCore::QEntity *)));
-    QObject::connect(d->m_surfaceSelector, SIGNAL(surfaceChanged(QSurface *)), this, SIGNAL(surfaceChanged(QSurface *)));
+    QObject::connect(d->m_surfaceSelector, SIGNAL(surfaceChanged(QObject *)), this, SIGNAL(surfaceChanged(QObject *)));
     d->init();
 }
 
@@ -143,7 +143,7 @@ void QForwardRenderer::setCamera(Qt3DCore::QEntity *camera)
     d->m_cameraSelector->setCamera(camera);
 }
 
-void QForwardRenderer::setSurface(QSurface *surface)
+void QForwardRenderer::setSurface(QObject *surface)
 {
     Q_D(QForwardRenderer);
     d->m_surfaceSelector->setSurface(surface);
@@ -184,7 +184,7 @@ Qt3DCore::QEntity *QForwardRenderer::camera() const
     return d->m_cameraSelector->camera();
 }
 
-QSurface *QForwardRenderer::surface() const
+QObject *QForwardRenderer::surface() const
 {
     Q_D(const QForwardRenderer);
     return d->m_surfaceSelector->surface();
diff --git a/src/render/defaults/qforwardrenderer.h b/src/render/defaults/qforwardrenderer.h
index 6e1618b2b..4ab36a9ab 100644
--- a/src/render/defaults/qforwardrenderer.h
+++ b/src/render/defaults/qforwardrenderer.h
@@ -55,7 +55,7 @@ class QForwardRendererPrivate;
 class QT3DRENDERSHARED_EXPORT QForwardRenderer : public QTechniqueFilter
 {
     Q_OBJECT
-    Q_PROPERTY(QSurface *surface READ surface WRITE setSurface NOTIFY surfaceChanged)
+    Q_PROPERTY(QObject *surface READ surface WRITE setSurface NOTIFY surfaceChanged)
     Q_PROPERTY(QRectF viewportRect READ viewportRect WRITE setViewportRect NOTIFY viewportRectChanged)
     Q_PROPERTY(QColor clearColor READ clearColor WRITE setClearColor NOTIFY clearColorChanged)
     Q_PROPERTY(Qt3DCore::QEntity *camera READ camera WRITE setCamera NOTIFY cameraChanged)
@@ -66,19 +66,19 @@ public:
     QRectF viewportRect() const;
     QColor clearColor() const;
     Qt3DCore::QEntity *camera() const;
-    QSurface *surface() const;
+    QObject *surface() const;
 
 public Q_SLOTS:
     void setViewportRect(const QRectF &viewportRect);
     void setClearColor(const QColor &clearColor);
     void setCamera(Qt3DCore::QEntity *camera);
-    void setSurface(QSurface * surface);
+    void setSurface(QObject * surface);
 
 Q_SIGNALS:
     void viewportRectChanged(const QRectF &viewportRect);
     void clearColorChanged(const QColor &clearColor);
     void cameraChanged(Qt3DCore::QEntity *camera);
-    void surfaceChanged(QSurface *surface);
+    void surfaceChanged(QObject *surface);
 
 private:
     Q_DECLARE_PRIVATE(QForwardRenderer)
diff --git a/src/render/framegraph/qrendersurfaceselector.cpp b/src/render/framegraph/qrendersurfaceselector.cpp
index 8bd31ac12..8b8429430 100644
--- a/src/render/framegraph/qrendersurfaceselector.cpp
+++ b/src/render/framegraph/qrendersurfaceselector.cpp
@@ -83,6 +83,18 @@ namespace Qt3DRender {
  * Holds the size of the external render target.
  */
 
+namespace {
+
+QSurface *surface_cast(QObject *obj)
+{
+    // QSurface can only be a QWindow or a QOffscreenSurface
+    if (qobject_cast<QWindow *>(obj) != Q_NULLPTR)
+        return static_cast<QWindow *>(obj);
+     return qobject_cast<QOffscreenSurface *>(obj);
+}
+
+} // anonymous
+
 QRenderSurfaceSelectorPrivate::QRenderSurfaceSelectorPrivate()
     : Qt3DRender::QFrameGraphNodePrivate()
     , m_surface(Q_NULLPTR)
@@ -119,42 +131,78 @@ QRenderSurfaceSelector::~QRenderSurfaceSelector()
     QNode::cleanup();
 }
 
-QSurface *QRenderSurfaceSelector::surface() const
+QObject *QRenderSurfaceSelector::surface() const
 {
     Q_D(const QRenderSurfaceSelector);
-    return d->m_surface;
-}
-
-QWindow *QRenderSurfaceSelector::window() const
-{
-    Q_D(const QRenderSurfaceSelector);
-    if (d->m_surface && d->m_surface->surfaceClass() == QSurface::Window)
-        return static_cast<QWindow *>(d->m_surface);
-    return Q_NULLPTR;
+    // All implementations of QSurface are also subclasses QObject
+    // so there's no risky business involved when doing this
+    return reinterpret_cast<QObject *>(d->m_surface);
 }
 
 /*! \property QRenderSurfaceSelector::surface
  *
  * Sets \a surface.
  */
-void QRenderSurfaceSelector::setSurface(QSurface *surface)
+void QRenderSurfaceSelector::setSurface(QObject *surface)
 {
     Q_D(QRenderSurfaceSelector);
-    if (d->m_surface == surface)
+    QSurface *surfaceObj = Q_NULLPTR;
+
+    if (surface) {
+        surfaceObj = surface_cast(surface);
+        Q_ASSERT_X(surfaceObj, Q_FUNC_INFO, "surface is not a valid QSurface * object");
+    }
+
+    if (d->m_surface == surfaceObj)
         return;
 
-    d->m_surface = surface;
+    d->m_surface = surfaceObj;
     // The platform surface filter only deals with QObject
     // We assume therefore that our surface is actually a QObject underneath
     if (d->m_surface) {
         switch (d->m_surface->surfaceClass()) {
-        case QSurface::Window:
-            d->m_surfaceEventFilter->setSurface(static_cast<QWindow *>(d->m_surface));
+        case QSurface::Window: {
+            QWindow *window = static_cast<QWindow *>(d->m_surface);
+            d->m_surfaceEventFilter->setSurface(window);
+
+            if (window) {
+                QObject::connect(window, &QWindow::widthChanged, [=] (int width) {
+                    if (d->m_changeArbiter != Q_NULLPTR) {
+                        Qt3DCore::QScenePropertyChangePtr change(
+                                    new Qt3DCore::QScenePropertyChange(
+                                        Qt3DCore::NodeUpdated,
+                                        Qt3DCore::QSceneChange::Node,
+                                        id()));
+
+                        change->setPropertyName("width");
+                        change->setValue(QVariant::fromValue(width));
+                        d->notifyObservers(change);
+                    }
+                });
+                QObject::connect(window, &QWindow::heightChanged, [=] (int height) {
+                    if (d->m_changeArbiter != Q_NULLPTR) {
+                        Qt3DCore::QScenePropertyChangePtr change(
+                                    new Qt3DCore::QScenePropertyChange(
+                                        Qt3DCore::NodeUpdated,
+                                        Qt3DCore::QSceneChange::Node,
+                                        id()));
+
+                        change->setPropertyName("height");
+                        change->setValue(QVariant::fromValue(height));
+                        d->notifyObservers(change);
+                    }
+                });
+            }
+
             break;
-        case QSurface::Offscreen:
+        }
+        case QSurface::Offscreen: {
             d->m_surfaceEventFilter->setSurface(static_cast<QOffscreenSurface *>(d->m_surface));
             break;
+        }
+
         default:
+            Q_UNREACHABLE();
             break;
         }
     } else {
@@ -181,55 +229,6 @@ void QRenderSurfaceSelector::setExternalRenderTargetSize(const QSize &size)
     emit externalRenderTargetSizeChanged(size);
 }
 
-/*! \property QRenderSurfaceSelector::window
- *
- * Sets \a window.
- */
-void QRenderSurfaceSelector::setWindow(QWindow *window)
-{
-    Q_D(QRenderSurfaceSelector);
-    QWindow *currentWindow = Q_NULLPTR;
-    if (d->m_surface && d->m_surface->surfaceClass() == QSurface::Window)
-        currentWindow = static_cast<QWindow *>(d->m_surface);
-    if (currentWindow == window)
-        return;
-
-    d->m_surface = window;
-    d->m_surfaceEventFilter->setSurface(window);
-
-    if (window) {
-        QObject::connect(window, &QWindow::widthChanged, [=] (int width) {
-            if (d->m_changeArbiter != Q_NULLPTR) {
-                Qt3DCore::QScenePropertyChangePtr change(
-                            new Qt3DCore::QScenePropertyChange(
-                                Qt3DCore::NodeUpdated,
-                                Qt3DCore::QSceneChange::Node,
-                                id()));
-
-                change->setPropertyName("width");
-                change->setValue(QVariant::fromValue(width));
-                d->notifyObservers(change);
-            }
-        });
-        QObject::connect(window, &QWindow::heightChanged, [=] (int height) {
-            if (d->m_changeArbiter != Q_NULLPTR) {
-                Qt3DCore::QScenePropertyChangePtr change(
-                            new Qt3DCore::QScenePropertyChange(
-                                Qt3DCore::NodeUpdated,
-                                Qt3DCore::QSceneChange::Node,
-                                id()));
-
-                change->setPropertyName("height");
-                change->setValue(QVariant::fromValue(height));
-                d->notifyObservers(change);
-            }
-        });
-    }
-
-    emit windowChanged(window);
-    emit surfaceChanged(d->m_surface);
-}
-
 /*!
  * \internal
  */
diff --git a/src/render/framegraph/qrendersurfaceselector.h b/src/render/framegraph/qrendersurfaceselector.h
index 83d10d180..954b8f37f 100644
--- a/src/render/framegraph/qrendersurfaceselector.h
+++ b/src/render/framegraph/qrendersurfaceselector.h
@@ -56,26 +56,22 @@ class QRenderSurfaceSelectorPrivate;
 class QT3DRENDERSHARED_EXPORT QRenderSurfaceSelector : public Qt3DRender::QFrameGraphNode
 {
     Q_OBJECT
-    Q_PROPERTY(QSurface *surface READ surface WRITE setSurface NOTIFY surfaceChanged)
-    Q_PROPERTY(QWindow *window READ window WRITE setWindow NOTIFY windowChanged)
+    Q_PROPERTY(QObject *surface READ surface WRITE setSurface NOTIFY surfaceChanged)
     Q_PROPERTY(QSize externalRenderTargetSize READ externalRenderTargetSize NOTIFY externalRenderTargetSizeChanged)
 
 public:
     explicit QRenderSurfaceSelector(Qt3DCore::QNode *parent = nullptr);
     ~QRenderSurfaceSelector();
 
-    QSurface *surface() const;
-    QWindow *window() const;
+    QObject *surface() const;
     QSize externalRenderTargetSize() const;
     void setExternalRenderTargetSize(const QSize &size);
 
 public Q_SLOTS:
-    void setSurface(QSurface *surface);
-    void setWindow(QWindow *window);
+    void setSurface(QObject *surface);
 
 Q_SIGNALS:
-    void surfaceChanged(QSurface *surface);
-    void windowChanged(QWindow *window);
+    void surfaceChanged(QObject *surface);
     void externalRenderTargetSizeChanged(const QSize &size);
 
 protected:
diff --git a/src/render/framegraph/rendersurfaceselector.cpp b/src/render/framegraph/rendersurfaceselector.cpp
index 838b03c57..a6a783a97 100644
--- a/src/render/framegraph/rendersurfaceselector.cpp
+++ b/src/render/framegraph/rendersurfaceselector.cpp
@@ -65,7 +65,7 @@ RenderSurfaceSelector::RenderSurfaceSelector()
 void RenderSurfaceSelector::updateFromPeer(Qt3DCore::QNode *peer)
 {
     QRenderSurfaceSelector *selector = static_cast<QRenderSurfaceSelector *>(peer);
-    m_surface = selector->surface();
+    m_surface = reinterpret_cast<QSurface *>(selector->surface());
     setEnabled(selector->isEnabled());
     setRenderTargetSize(selector->externalRenderTargetSize());
     if (m_surface) {