Correct filtering of disabled entities

FilterLayerEntityJob would not check for the enabled-ness of
parent entities, if present.

We introduce another job that checks for each entity, whether
it is enabled or not. Jobs that need to access this flag must
take care to add a dependency on the UpdateTreeEnabledJob

Task-number: QTBUG-56235
Change-Id: Ic087fc8e9efdd4829cdb18ae3e8430344f6ecf43
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>

12 files changed, 203 insertions, 6 deletions

diff --git a/src/render/backend/entity.cpp b/src/render/backend/entity.cpp
index d8f396084..f4a57fa43 100644
--- a/src/render/backend/entity.cpp
+++ b/src/render/backend/entity.cpp
@@ -77,6 +77,7 @@ Entity::Entity()
     : BackendNode()
     , m_nodeManagers(nullptr)
     , m_boundingDirty(false)
+    , m_treeEnabled(true)
 {
 }
 
diff --git a/src/render/backend/entity_p.h b/src/render/backend/entity_p.h
index a2ed8a5b7..213dc041e 100644
--- a/src/render/backend/entity_p.h
+++ b/src/render/backend/entity_p.h
@@ -117,6 +117,9 @@ public:
     bool isBoundingVolumeDirty() const;
     void unsetBoundingVolumeDirty();
 
+    void setTreeEnabled(bool enabled) { m_treeEnabled = enabled; }
+    bool isTreeEnabled() const { return m_treeEnabled; }
+
     template<class Backend, uint INDEXBITS>
     Qt3DCore::QHandle<Backend, INDEXBITS> componentHandle() const
     {
@@ -174,6 +177,9 @@ private:
     HEntity m_parentHandle;
     QVector<HEntity > m_childrenHandles;
 
+    // true only if this and all parent nodes are enabled
+    bool m_treeEnabled;
+
     HMatrix m_worldTransform;
     QSharedPointer<Sphere> m_localBoundingVolume;
     QSharedPointer<Sphere> m_worldBoundingVolume;
diff --git a/src/render/backend/renderer.cpp b/src/render/backend/renderer.cpp
index 439327102..e166fa468 100644
--- a/src/render/backend/renderer.cpp
+++ b/src/render/backend/renderer.cpp
@@ -165,6 +165,7 @@ Renderer::Renderer(QRenderAspect::RenderType type)
     , m_expandBoundingVolumeJob(Render::ExpandBoundingVolumeJobPtr::create())
     , m_calculateBoundingVolumeJob(Render::CalculateBoundingVolumeJobPtr::create())
     , m_updateWorldBoundingVolumeJob(Render::UpdateWorldBoundingVolumeJobPtr::create())
+    , m_updateTreeEnabledJob(Render::UpdateTreeEnabledJobPtr::create())
     , m_sendRenderCaptureJob(Render::SendRenderCaptureJobPtr::create(this))
     , m_updateMeshTriangleListJob(Render::UpdateMeshTriangleListJobPtr::create())
     , m_filterCompatibleTechniqueJob(Render::FilterCompatibleTechniqueJobPtr::create())
@@ -436,6 +437,7 @@ void Renderer::setSceneRoot(QBackendNodeFactory *factory, Entity *sgRoot)
     m_calculateBoundingVolumeJob->setRoot(m_renderSceneRoot);
     m_cleanupJob->setRoot(m_renderSceneRoot);
     m_pickBoundingVolumeJob->setRoot(m_renderSceneRoot);
+    m_updateTreeEnabledJob->setRoot(m_renderSceneRoot);
 }
 
 void Renderer::registerEventFilter(QEventFilterService *service)
@@ -1228,6 +1230,7 @@ QVector<Qt3DCore::QAspectJobPtr> Renderer::renderBinJobs()
     // Add jobs
     renderBinJobs.push_back(m_updateShaderDataTransformJob);
     renderBinJobs.push_back(m_updateMeshTriangleListJob);
+    renderBinJobs.push_back(m_updateTreeEnabledJob);
     renderBinJobs.push_back(m_expandBoundingVolumeJob);
     renderBinJobs.push_back(m_updateWorldBoundingVolumeJob);
     renderBinJobs.push_back(m_calculateBoundingVolumeJob);
diff --git a/src/render/backend/renderer_p.h b/src/render/backend/renderer_p.h
index 501c158ef..cb78cbef3 100644
--- a/src/render/backend/renderer_p.h
+++ b/src/render/backend/renderer_p.h
@@ -68,6 +68,7 @@
 #include <Qt3DRender/private/updateshaderdatatransformjob_p.h>
 #include <Qt3DRender/private/framecleanupjob_p.h>
 #include <Qt3DRender/private/updateworldboundingvolumejob_p.h>
+#include <Qt3DRender/private/updatetreeenabledjob_p.h>
 #include <Qt3DRender/private/platformsurfacefilter_p.h>
 #include <Qt3DRender/private/sendrendercapturejob_p.h>
 #include <Qt3DRender/private/genericlambdajob_p.h>
@@ -191,6 +192,7 @@ public:
     inline ExpandBoundingVolumeJobPtr expandBoundingVolumeJob() const { return m_expandBoundingVolumeJob; }
     inline UpdateShaderDataTransformJobPtr updateShaderDataTransformJob() const { return m_updateShaderDataTransformJob; }
     inline CalculateBoundingVolumeJobPtr calculateBoundingVolumeJob() const { return m_calculateBoundingVolumeJob; }
+    inline UpdateTreeEnabledJobPtr updateTreeEnabledJob() const { return m_updateTreeEnabledJob; }
     inline UpdateWorldTransformJobPtr updateWorldTransformJob() const { return m_worldTransformJob; }
     inline UpdateWorldBoundingVolumeJobPtr updateWorldBoundingVolumeJob() const { return m_updateWorldBoundingVolumeJob; }
     inline UpdateMeshTriangleListJobPtr updateMeshTriangleListJob() const { return m_updateMeshTriangleListJob; }
@@ -306,6 +308,7 @@ private:
     ExpandBoundingVolumeJobPtr m_expandBoundingVolumeJob;
     CalculateBoundingVolumeJobPtr m_calculateBoundingVolumeJob;
     UpdateWorldBoundingVolumeJobPtr m_updateWorldBoundingVolumeJob;
+    UpdateTreeEnabledJobPtr m_updateTreeEnabledJob;
     SendRenderCaptureJobPtr m_sendRenderCaptureJob;
     UpdateMeshTriangleListJobPtr m_updateMeshTriangleListJob;
     FilterCompatibleTechniqueJobPtr m_filterCompatibleTechniqueJob;
diff --git a/src/render/backend/renderviewbuilder.cpp b/src/render/backend/renderviewbuilder.cpp
index 7210b76dd..55d8ba462 100644
--- a/src/render/backend/renderviewbuilder.cpp
+++ b/src/render/backend/renderviewbuilder.cpp
@@ -421,6 +421,7 @@ QVector<Qt3DCore::QAspectJobPtr> RenderViewBuilder::buildJobHierachy() const
     m_syncRenderViewInitializationJob->addDependency(m_renderViewJob);
 
     m_filterEntityByLayerJob->addDependency(m_syncRenderViewInitializationJob);
+    m_filterEntityByLayerJob->addDependency(m_renderer->updateTreeEnabledJob());
 
     m_syncRenderCommandBuildingJob->addDependency(m_syncRenderViewInitializationJob);
     for (const auto materialGatherer : qAsConst(m_materialGathererJobs)) {
diff --git a/src/render/jobs/filterlayerentityjob.cpp b/src/render/jobs/filterlayerentityjob.cpp
index 3f1a04785..5e1add275 100644
--- a/src/render/jobs/filterlayerentityjob.cpp
+++ b/src/render/jobs/filterlayerentityjob.cpp
@@ -91,7 +91,7 @@ void FilterLayerEntityJob::filterLayerAndEntity()
     for (const HEntity handle : handles) {
         Entity *entity = entityManager->data(handle);
 
-        if (!entity->isEnabled())
+        if (!entity->isTreeEnabled())
             continue;
 
         const Qt3DCore::QNodeIdVector entityLayers = entity->componentsUuid<Layer>();
@@ -117,7 +117,7 @@ void FilterLayerEntityJob::selectAllEntities()
     m_filteredEntities.reserve(handles.size());
     for (const HEntity handle : handles) {
         Entity *e = entityManager->data(handle);
-        if (e->isEnabled())
+        if (e->isTreeEnabled())
             m_filteredEntities.push_back(e);
     }
 }
diff --git a/src/render/jobs/job_common_p.h b/src/render/jobs/job_common_p.h
index e6eea36a8..f3be2e6b0 100644
--- a/src/render/jobs/job_common_p.h
+++ b/src/render/jobs/job_common_p.h
@@ -73,6 +73,7 @@ namespace JobTypes {
         PickBoundingVolume,
         RenderView,
         UpdateTransform,
+        UpdateTreeEnabled,
         ExpandBoundingVolume,
         FrameSubmissionPart1,
         LayerFiltering,
diff --git a/src/render/jobs/jobs.pri b/src/render/jobs/jobs.pri
index e8686a28e..2827d959d 100644
--- a/src/render/jobs/jobs.pri
+++ b/src/render/jobs/jobs.pri
@@ -26,7 +26,8 @@ HEADERS += \
     $$PWD/updateshaderdatatransformjob_p.h \
     $$PWD/updatemeshtrianglelistjob_p.h \
     $$PWD/pickboundingvolumeutils_p.h \
-    $$PWD/filtercompatibletechniquejob_p.h
+    $$PWD/filtercompatibletechniquejob_p.h \
+    $$PWD/updatetreeenabledjob_p.h
 
 SOURCES += \
     $$PWD/updateworldtransformjob.cpp \
@@ -51,4 +52,5 @@ SOURCES += \
     $$PWD/updateshaderdatatransformjob.cpp \
     $$PWD/updatemeshtrianglelistjob.cpp \
     $$PWD/pickboundingvolumeutils.cpp \
-    $$PWD/filtercompatibletechniquejob.cpp
+    $$PWD/filtercompatibletechniquejob.cpp \
+    $$PWD/updatetreeenabledjob.cpp
diff --git a/src/render/jobs/updatetreeenabledjob.cpp b/src/render/jobs/updatetreeenabledjob.cpp
new file mode 100644
index 000000000..6475ea78c
--- /dev/null
+++ b/src/render/jobs/updatetreeenabledjob.cpp
@@ -0,0 +1,87 @@
+/****************************************************************************
+**
+** Copyright (C) 2017 Klaralvdalens Datakonsult AB (KDAB).
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the Qt3D module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 3 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL3 included in the
+** packaging of this file. Please review the following information to
+** ensure the GNU Lesser General Public License version 3 requirements
+** will be met: https://www.gnu.org/licenses/lgpl-3.0.html.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 2.0 or (at your option) the GNU General
+** Public license version 3 or any later version approved by the KDE Free
+** Qt Foundation. The licenses are as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-2.0.html and
+** https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#include "updatetreeenabledjob_p.h"
+
+#include <Qt3DRender/private/entity_p.h>
+#include <Qt3DRender/private/job_common_p.h>
+
+#include <QThread>
+
+QT_BEGIN_NAMESPACE
+
+namespace Qt3DRender {
+namespace Render {
+
+namespace {
+
+void updateTreeEnabled(Entity *node, bool parentEnabled)
+{
+    const bool treeEnabled = node->isEnabled() && parentEnabled;
+    node->setTreeEnabled(treeEnabled);
+
+    const QVector<Entity*> children = node->children();
+    for (Entity *child : children)
+        updateTreeEnabled(child, treeEnabled);
+}
+
+}
+
+UpdateTreeEnabledJob::UpdateTreeEnabledJob()
+    : Qt3DCore::QAspectJob()
+    , m_node(nullptr)
+{
+    SET_JOB_RUN_STAT_TYPE(this, JobTypes::UpdateTreeEnabled, 0);
+}
+
+void UpdateTreeEnabledJob::setRoot(Entity *root)
+{
+    m_node = root;
+}
+
+void UpdateTreeEnabledJob::run()
+{
+    if (m_node)
+        updateTreeEnabled(m_node, true);
+}
+
+} // namespace Render
+} // namespace Qt3DRender
+
+QT_END_NAMESPACE
diff --git a/src/render/jobs/updatetreeenabledjob_p.h b/src/render/jobs/updatetreeenabledjob_p.h
new file mode 100644
index 000000000..19b71e229
--- /dev/null
+++ b/src/render/jobs/updatetreeenabledjob_p.h
@@ -0,0 +1,85 @@
+/****************************************************************************
+**
+** Copyright (C) 2017 Klaralvdalens Datakonsult AB (KDAB).
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the Qt3D module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 3 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL3 included in the
+** packaging of this file. Please review the following information to
+** ensure the GNU Lesser General Public License version 3 requirements
+** will be met: https://www.gnu.org/licenses/lgpl-3.0.html.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 2.0 or (at your option) the GNU General
+** Public license version 3 or any later version approved by the KDE Free
+** Qt Foundation. The licenses are as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-2.0.html and
+** https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#ifndef QT3DRENDER_RENDER_UPDATETREEENABLEDJOB_H
+#define QT3DRENDER_RENDER_UPDATETREEENABLEDJOB_H
+
+//
+//  W A R N I N G
+//  -------------
+//
+// This file is not part of the Qt API.  It exists for the convenience
+// of other Qt classes.  This header file may change from version to
+// version without notice, or even be removed.
+//
+// We mean it.
+//
+
+#include <Qt3DCore/qaspectjob.h>
+#include <Qt3DRender/private/qt3drender_global_p.h>
+
+#include <QSharedPointer>
+
+QT_BEGIN_NAMESPACE
+
+namespace Qt3DRender {
+namespace Render {
+
+class Entity;
+
+class QT3DRENDERSHARED_PRIVATE_EXPORT UpdateTreeEnabledJob : public Qt3DCore::QAspectJob
+{
+public:
+    UpdateTreeEnabledJob();
+
+    void setRoot(Entity *root);
+    void run() Q_DECL_OVERRIDE;
+
+private:
+    Entity *m_node;
+};
+
+typedef QSharedPointer<UpdateTreeEnabledJob> UpdateTreeEnabledJobPtr;
+
+} // namespace Render
+} // namespace Qt3DRender
+
+QT_END_NAMESPACE
+
+#endif // QT3DRENDER_RENDER_UPDATETREEENABLEDJOB_H
diff --git a/tests/auto/render/layerfiltering/tst_layerfiltering.cpp b/tests/auto/render/layerfiltering/tst_layerfiltering.cpp
index ad4ef2b90..e96b84e3e 100644
--- a/tests/auto/render/layerfiltering/tst_layerfiltering.cpp
+++ b/tests/auto/render/layerfiltering/tst_layerfiltering.cpp
@@ -37,6 +37,7 @@
 #include <Qt3DRender/qrenderaspect.h>
 #include <Qt3DRender/private/qrenderaspect_p.h>
 #include <Qt3DRender/private/filterlayerentityjob_p.h>
+#include <Qt3DRender/private/updatetreeenabledjob_p.h>
 #include <Qt3DRender/qlayer.h>
 
 QT_BEGIN_NAMESPACE
@@ -251,6 +252,12 @@ private Q_SLOTS:
         QScopedPointer<Qt3DRender::TestAspect> aspect(new Qt3DRender::TestAspect(entitySubtree));
 
         // WHEN
+        Qt3DRender::Render::Entity *backendRoot = aspect->nodeManagers()->renderNodesManager()->getOrCreateResource(entitySubtree->id());
+        Qt3DRender::Render::UpdateTreeEnabledJob updateTreeEnabledJob;
+        updateTreeEnabledJob.setRoot(backendRoot);
+        updateTreeEnabledJob.run();
+
+        // WHEN
         Qt3DRender::Render::FilterLayerEntityJob filterJob;
         filterJob.setHasLayerFilter(hasLayerFilter);
         filterJob.setLayers(layerIds);
diff --git a/tests/auto/render/renderviewbuilder/tst_renderviewbuilder.cpp b/tests/auto/render/renderviewbuilder/tst_renderviewbuilder.cpp
index f33d6dd92..1ab687b34 100644
--- a/tests/auto/render/renderviewbuilder/tst_renderviewbuilder.cpp
+++ b/tests/auto/render/renderviewbuilder/tst_renderviewbuilder.cpp
@@ -229,8 +229,9 @@ private Q_SLOTS:
         QCOMPARE(renderViewBuilder.syncRenderViewInitializationJob()->dependencies().first().data(), renderViewBuilder.renderViewJob().data());
 
         // Step 3
-        QCOMPARE(renderViewBuilder.filterEntityByLayerJob()->dependencies().size(), 1);
-        QCOMPARE(renderViewBuilder.filterEntityByLayerJob()->dependencies().first().data(), renderViewBuilder.syncRenderViewInitializationJob().data());
+        QCOMPARE(renderViewBuilder.filterEntityByLayerJob()->dependencies().size(), 2);
+        QVERIFY(renderViewBuilder.filterEntityByLayerJob()->dependencies().contains(renderViewBuilder.syncRenderViewInitializationJob()));
+        QVERIFY(renderViewBuilder.filterEntityByLayerJob()->dependencies().contains(testAspect.renderer()->updateTreeEnabledJob()));
 
         QCOMPARE(renderViewBuilder.setClearDrawBufferIndexJob()->dependencies().size(), 1);
         QCOMPARE(renderViewBuilder.setClearDrawBufferIndexJob()->dependencies().first().data(), renderViewBuilder.syncRenderViewInitializationJob().data());