From a9f4b0561ba5cf372fb49076ef8013f0de13d30b Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Sat, 1 Jul 2017 19:35:53 -0700 Subject: Fix warning about unused variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit assimpimporter.cpp:616:75: error: unused parameter ‘basePath’ [-Werror=unused-parameter] Change-Id: I8d96dea9955d4c749b99fffd14cd62dd3d0fa45f Reviewed-by: Paul Lemire --- src/plugins/sceneparsers/assimp/assimpimporter.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/plugins/sceneparsers/assimp/assimpimporter.cpp b/src/plugins/sceneparsers/assimp/assimpimporter.cpp index a2264964c..6611a2529 100644 --- a/src/plugins/sceneparsers/assimp/assimpimporter.cpp +++ b/src/plugins/sceneparsers/assimp/assimpimporter.cpp @@ -616,6 +616,7 @@ void AssimpImporter::readSceneFile(const QString &path) */ void AssimpImporter::readSceneData(const QByteArray& data, const QString &basePath) { + Q_UNUSED(basePath); cleanup(); m_scene = new SceneImporter(); -- cgit v1.2.3 From d22d786d94540d50fde776c5f44ccdb041824343 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Sun, 28 Jan 2018 21:59:10 -0800 Subject: Initialize the unused member MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GCC 7 complains that it is used uninitialized. The line number points to the opening brace of the class, so I suppose it's the implicit copy constructor. Loading undefined floating point values is probably a bad idea. Could generate an FP exception. animationutils_p.h:126:8: error: ‘.Qt3DAnimation::Animation::ChannelNameAndType::pad’ may be used uninitialized in this function [-Werror=maybe-uninitialized] Change-Id: I11d489f37e7e424b971dfffd150e3268d4be9294 Reviewed-by: Paul Lemire --- src/animation/backend/animationutils_p.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/animation/backend/animationutils_p.h b/src/animation/backend/animationutils_p.h index a0ab218d4..9778745c4 100644 --- a/src/animation/backend/animationutils_p.h +++ b/src/animation/backend/animationutils_p.h @@ -142,6 +142,7 @@ struct ChannelNameAndType , jointIndex(-1) , mappingId() , jointTransformComponent(NoTransformComponent) + , pad(0) {} ChannelNameAndType(const QString &_name, @@ -154,6 +155,7 @@ struct ChannelNameAndType , jointIndex(_jointIndex) , mappingId(_mappingId) , jointTransformComponent(NoTransformComponent) + , pad(0) {} ChannelNameAndType(const QString &_name, @@ -165,6 +167,7 @@ struct ChannelNameAndType , jointIndex(invalidIndex) , mappingId() , jointTransformComponent(_jointTransformComponent) + , pad(0) {} bool operator==(const ChannelNameAndType &rhs) const -- cgit v1.2.3 From 929e72e36cc10556bdf33a50911f2cd651f0b8dc Mon Sep 17 00:00:00 2001 From: Svenn-Arne Dragly Date: Mon, 22 Jan 2018 10:44:55 +0100 Subject: Organize dirty bits in categories To get a better overview of which dirty bits changed at what time, this change organizes them in the following categories: - current: dirty bits at job building - marked: dirty bits marked since last job building - remaining: dirty bits set but not cleared in the previous frame Further, within renderBinJobs, we add a variable "notCleared" to keep track of which bits in "current" should be set in "remaining". Change-Id: I43a42a4fd495a6d9f794721d1f09381718dfa647 Reviewed-by: Paul Lemire --- src/render/backend/abstractrenderer_p.h | 2 ++ src/render/backend/renderer.cpp | 52 ++++++++++++++++++--------------- src/render/backend/renderer_p.h | 12 ++++++-- 3 files changed, 41 insertions(+), 25 deletions(-) diff --git a/src/render/backend/abstractrenderer_p.h b/src/render/backend/abstractrenderer_p.h index c934dbfae..bc8a9812f 100644 --- a/src/render/backend/abstractrenderer_p.h +++ b/src/render/backend/abstractrenderer_p.h @@ -141,7 +141,9 @@ public: virtual void markDirty(BackendNodeDirtySet changes, BackendNode *node) = 0; virtual BackendNodeDirtySet dirtyBits() = 0; +#if defined(QT_BUILD_INTERNAL) virtual void clearDirtyBits(BackendNodeDirtySet changes) = 0; +#endif virtual bool shouldRender() = 0; virtual void skipNextFrame() = 0; diff --git a/src/render/backend/renderer.cpp b/src/render/backend/renderer.cpp index 20aeaf2df..c1b5760c7 100644 --- a/src/render/backend/renderer.cpp +++ b/src/render/backend/renderer.cpp @@ -162,7 +162,6 @@ Renderer::Renderer(QRenderAspect::RenderType type) , m_waitForInitializationToBeCompleted(0) , m_pickEventFilter(new PickEventFilter()) , m_exposed(0) - , m_changeSet(0) , m_lastFrameCorrect(0) , m_glContext(nullptr) , m_shareContext(nullptr) @@ -514,7 +513,7 @@ void Renderer::setSceneRoot(QBackendNodeFactory *factory, Entity *sgRoot) m_updateTreeEnabledJob->setRoot(m_renderSceneRoot); // Set all flags to dirty - m_changeSet |= AbstractRenderer::AllDirty; + m_dirtyBits.marked |= AbstractRenderer::AllDirty; } void Renderer::registerEventFilter(QEventFilterService *service) @@ -1445,25 +1444,29 @@ Renderer::ViewSubmissionResultData Renderer::submitRenderViews(const QVectorrenderPolicy() == QRenderSettings::Always - || m_changeSet != 0 + || m_dirtyBits.marked != 0 + || m_dirtyBits.remaining != 0 || !m_lastFrameCorrect.load()); } @@ -1497,28 +1500,31 @@ QVector Renderer::renderBinJobs() m_updateLevelOfDetailJob->setFrameGraphRoot(frameGraphRoot()); - BackendNodeDirtySet changesToUnset = dirtyBits(); + const BackendNodeDirtySet dirtyBitsForFrame = m_dirtyBits.marked | m_dirtyBits.remaining; + m_dirtyBits.marked = 0; + m_dirtyBits.remaining = 0; + BackendNodeDirtySet notCleared = 0; // Add jobs - const bool entitiesEnabledDirty = changesToUnset & AbstractRenderer::EntityEnabledDirty; + const bool entitiesEnabledDirty = dirtyBitsForFrame & AbstractRenderer::EntityEnabledDirty; if (entitiesEnabledDirty) { renderBinJobs.push_back(m_updateTreeEnabledJob); m_calculateBoundingVolumeJob->addDependency(m_updateTreeEnabledJob); } - if (changesToUnset & AbstractRenderer::TransformDirty) { + if (dirtyBitsForFrame & AbstractRenderer::TransformDirty) { renderBinJobs.push_back(m_worldTransformJob); renderBinJobs.push_back(m_updateWorldBoundingVolumeJob); renderBinJobs.push_back(m_updateShaderDataTransformJob); } - if (changesToUnset & AbstractRenderer::GeometryDirty) { + if (dirtyBitsForFrame & AbstractRenderer::GeometryDirty) { renderBinJobs.push_back(m_calculateBoundingVolumeJob); renderBinJobs.push_back(m_updateMeshTriangleListJob); } - if (changesToUnset & AbstractRenderer::GeometryDirty || - changesToUnset & AbstractRenderer::TransformDirty) { + if (dirtyBitsForFrame & AbstractRenderer::GeometryDirty || + dirtyBitsForFrame & AbstractRenderer::TransformDirty) { renderBinJobs.push_back(m_expandBoundingVolumeJob); } @@ -1534,13 +1540,13 @@ QVector Renderer::renderBinJobs() // Jobs to prepare GL Resource upload renderBinJobs.push_back(m_vaoGathererJob); - if (changesToUnset & AbstractRenderer::BuffersDirty) + if (dirtyBitsForFrame & AbstractRenderer::BuffersDirty) renderBinJobs.push_back(m_bufferGathererJob); - if (changesToUnset & AbstractRenderer::ShadersDirty) + if (dirtyBitsForFrame & AbstractRenderer::ShadersDirty) renderBinJobs.push_back(m_shaderGathererJob); - if (changesToUnset & AbstractRenderer::TexturesDirty) { + if (dirtyBitsForFrame & AbstractRenderer::TexturesDirty) { renderBinJobs.push_back(m_syncTextureLoadingJob); renderBinJobs.push_back(m_textureGathererJob); } @@ -1548,7 +1554,7 @@ QVector Renderer::renderBinJobs() // Layer cache is dependent on layers, layer filters and the enabled flag // on entities - const bool layersDirty = changesToUnset & AbstractRenderer::LayersDirty; + const bool layersDirty = dirtyBitsForFrame & AbstractRenderer::LayersDirty; const bool layersCacheNeedsToBeRebuilt = layersDirty || entitiesEnabledDirty; bool layersCacheRebuilt = false; @@ -1584,19 +1590,19 @@ QVector Renderer::renderBinJobs() // Only reset LayersDirty flag once we have really rebuilt the caches if (layersDirty && !layersCacheRebuilt) - changesToUnset.setFlag(AbstractRenderer::LayersDirty, false); + notCleared |= AbstractRenderer::LayersDirty; if (entitiesEnabledDirty && !layersCacheRebuilt) - changesToUnset.setFlag(AbstractRenderer::EntityEnabledDirty, false); + notCleared |= AbstractRenderer::EntityEnabledDirty; // Clear dirty bits // TO DO: When secondary GL thread is integrated, the following line can be removed - changesToUnset.setFlag(AbstractRenderer::ShadersDirty, false); + notCleared |= AbstractRenderer::ShadersDirty; // Clear all dirty flags but Compute so that // we still render every frame when a compute shader is used in a scene - if (changesToUnset.testFlag(Renderer::ComputeDirty)) - changesToUnset.setFlag(Renderer::ComputeDirty, false); - clearDirtyBits(changesToUnset); + notCleared |= Renderer::ComputeDirty; + + m_dirtyBits.remaining = dirtyBitsForFrame & notCleared; return renderBinJobs; } @@ -1722,7 +1728,7 @@ void Renderer::performCompute(const RenderView *, RenderCommand *command) command->m_workGroups[2]); } // HACK: Reset the compute flag to dirty - m_changeSet |= AbstractRenderer::ComputeDirty; + m_dirtyBits.marked |= AbstractRenderer::ComputeDirty; #if defined(QT3D_RENDER_ASPECT_OPENGL_DEBUG) int err = m_graphicsContext->openGLContext()->functions()->glGetError(); diff --git a/src/render/backend/renderer_p.h b/src/render/backend/renderer_p.h index d4917e28c..987576059 100644 --- a/src/render/backend/renderer_p.h +++ b/src/render/backend/renderer_p.h @@ -182,8 +182,10 @@ public: void markDirty(BackendNodeDirtySet changes, BackendNode *node) Q_DECL_OVERRIDE; BackendNodeDirtySet dirtyBits() Q_DECL_OVERRIDE; - void clearDirtyBits(BackendNodeDirtySet changes) Q_DECL_OVERRIDE; +#if defined(QT_BUILD_INTERNAL) + void clearDirtyBits(BackendNodeDirtySet changes) Q_DECL_OVERRIDE; +#endif bool shouldRender() Q_DECL_OVERRIDE; void skipNextFrame() Q_DECL_OVERRIDE; @@ -307,7 +309,13 @@ private: QVector m_dirtyAttributes; QVector m_dirtyGeometry; QAtomicInt m_exposed; - BackendNodeDirtySet m_changeSet; + + struct DirtyBits { + BackendNodeDirtySet marked = 0; // marked dirty since last job build + BackendNodeDirtySet remaining = 0; // remaining dirty after jobs have finished + }; + DirtyBits m_dirtyBits; + QAtomicInt m_lastFrameCorrect; QOpenGLContext *m_glContext; QOpenGLContext *m_shareContext; -- cgit v1.2.3 From a819190eb71ceb6bb9202f0b3497cc117d8331e7 Mon Sep 17 00:00:00 2001 From: Svenn-Arne Dragly Date: Tue, 30 Jan 2018 09:21:38 +0100 Subject: Make ShaderGathererJob depend on FilterCompatibleTechniques The ShaderGathererJob will use Technique::isCompatibleWithRenderer() while looking for dirty shaders. This is set in the FilterCompatibleTechniquesJob, but it might run after (or, even worse, at the same time as) ShaderGathererJob. Task-number: QTBUG-66024 Change-Id: I929e87b9c67068b51f7d64c637b1741d743b1839 Reviewed-by: Paul Lemire --- src/render/backend/renderer.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/render/backend/renderer.cpp b/src/render/backend/renderer.cpp index c1b5760c7..7ed62a98f 100644 --- a/src/render/backend/renderer.cpp +++ b/src/render/backend/renderer.cpp @@ -217,6 +217,8 @@ Renderer::Renderer(QRenderAspect::RenderType type) m_updateLevelOfDetailJob->addDependency(m_updateMeshTriangleListJob); m_pickBoundingVolumeJob->addDependency(m_updateMeshTriangleListJob); + m_shaderGathererJob->addDependency(m_filterCompatibleTechniqueJob); + m_filterCompatibleTechniqueJob->setRenderer(this); m_defaultRenderStateSet = new RenderStateSet; -- cgit v1.2.3 From d02c54e3349e04fd21f22e67bf88c4a1631faf45 Mon Sep 17 00:00:00 2001 From: Svenn-Arne Dragly Date: Mon, 22 Jan 2018 10:42:36 +0100 Subject: Fix OnDemand render policy The OnDemand render policy has no effect because the ShadersDirty and ComputeDirty flags are always set. This commit resets ShadersDirty so that ShaderGathererJob is only run when needed. It also introduces the TechniquesDirty flag so we know when to run FilterCompatibleTechniqueJob and makes sure the job is only run if the context has been initialized and the renderer is running. Task-number: QTBUG-65965 Change-Id: Icbcc03ed2dc6b14c6580cc794267b5a88e5f4ca2 Reviewed-by: Paul Lemire --- src/render/backend/abstractrenderer_p.h | 1 + src/render/backend/renderer.cpp | 154 ++++++++++----------- src/render/jobs/filtercompatibletechniquejob.cpp | 13 +- src/render/materialsystem/technique.cpp | 26 ++-- .../tst_filtercompatibletechniquejob.cpp | 29 ---- tests/auto/render/renderer/tst_renderer.cpp | 28 +--- tests/auto/render/technique/tst_technique.cpp | 16 +++ 7 files changed, 118 insertions(+), 149 deletions(-) diff --git a/src/render/backend/abstractrenderer_p.h b/src/render/backend/abstractrenderer_p.h index bc8a9812f..5603a9195 100644 --- a/src/render/backend/abstractrenderer_p.h +++ b/src/render/backend/abstractrenderer_p.h @@ -108,6 +108,7 @@ public: SkeletonDataDirty = 1 << 10, JointDirty = 1 << 11, LayersDirty = 1 << 12, + TechniquesDirty = 1 << 13, AllDirty = 0xffffff }; Q_DECLARE_FLAGS(BackendNodeDirtySet, BackendNodeDirtyFlag) diff --git a/src/render/backend/renderer.cpp b/src/render/backend/renderer.cpp index 7ed62a98f..cfe0cd98a 100644 --- a/src/render/backend/renderer.cpp +++ b/src/render/backend/renderer.cpp @@ -1013,72 +1013,71 @@ void Renderer::lookForDirtyTextures() // Executed in a job void Renderer::lookForDirtyShaders() { - if (isRunning()) { - const QVector activeTechniques = m_nodesManager->techniqueManager()->activeHandles(); - const QVector activeBuilders = m_nodesManager->shaderBuilderManager()->activeHandles(); - for (const HTechnique &techniqueHandle : activeTechniques) { - Technique *technique = m_nodesManager->techniqueManager()->data(techniqueHandle); - // If api of the renderer matches the one from the technique - if (technique->isCompatibleWithRenderer()) { - const auto passIds = technique->renderPasses(); - for (const QNodeId passId : passIds) { - RenderPass *renderPass = m_nodesManager->renderPassManager()->lookupResource(passId); - HShader shaderHandle = m_nodesManager->shaderManager()->lookupHandle(renderPass->shaderProgram()); - Shader *shader = m_nodesManager->shaderManager()->data(shaderHandle); - - ShaderBuilder *shaderBuilder = nullptr; - for (const HShaderBuilder &builderHandle : activeBuilders) { - ShaderBuilder *builder = m_nodesManager->shaderBuilderManager()->data(builderHandle); - if (builder->shaderProgramId() == shader->peerId()) { - shaderBuilder = builder; - break; - } + Q_ASSERT(isRunning()); + const QVector activeTechniques = m_nodesManager->techniqueManager()->activeHandles(); + const QVector activeBuilders = m_nodesManager->shaderBuilderManager()->activeHandles(); + for (const HTechnique &techniqueHandle : activeTechniques) { + Technique *technique = m_nodesManager->techniqueManager()->data(techniqueHandle); + // If api of the renderer matches the one from the technique + if (technique->isCompatibleWithRenderer()) { + const auto passIds = technique->renderPasses(); + for (const QNodeId passId : passIds) { + RenderPass *renderPass = m_nodesManager->renderPassManager()->lookupResource(passId); + HShader shaderHandle = m_nodesManager->shaderManager()->lookupHandle(renderPass->shaderProgram()); + Shader *shader = m_nodesManager->shaderManager()->data(shaderHandle); + + ShaderBuilder *shaderBuilder = nullptr; + for (const HShaderBuilder &builderHandle : activeBuilders) { + ShaderBuilder *builder = m_nodesManager->shaderBuilderManager()->data(builderHandle); + if (builder->shaderProgramId() == shader->peerId()) { + shaderBuilder = builder; + break; } + } - if (shaderBuilder) { - shaderBuilder->setGraphicsApi(*technique->graphicsApiFilter()); - - for (int i = 0; i <= ShaderBuilder::Compute; i++) { - const auto builderType = static_cast(i); - if (!shaderBuilder->shaderGraph(builderType).isValid()) - continue; - - if (shaderBuilder->isShaderCodeDirty(builderType)) { - shaderBuilder->generateCode(builderType); - } - - QShaderProgram::ShaderType shaderType = QShaderProgram::Vertex; - switch (builderType) { - case ShaderBuilder::Vertex: - shaderType = QShaderProgram::Vertex; - break; - case ShaderBuilder::TessellationControl: - shaderType = QShaderProgram::TessellationControl; - break; - case ShaderBuilder::TessellationEvaluation: - shaderType = QShaderProgram::TessellationEvaluation; - break; - case ShaderBuilder::Geometry: - shaderType = QShaderProgram::Geometry; - break; - case ShaderBuilder::Fragment: - shaderType = QShaderProgram::Fragment; - break; - case ShaderBuilder::Compute: - shaderType = QShaderProgram::Compute; - break; - } - - const auto code = shaderBuilder->shaderCode(builderType); - shader->setShaderCode(shaderType, code); + if (shaderBuilder) { + shaderBuilder->setGraphicsApi(*technique->graphicsApiFilter()); + + for (int i = 0; i <= ShaderBuilder::Compute; i++) { + const auto builderType = static_cast(i); + if (!shaderBuilder->shaderGraph(builderType).isValid()) + continue; + + if (shaderBuilder->isShaderCodeDirty(builderType)) { + shaderBuilder->generateCode(builderType); } - } - if (Q_UNLIKELY(shader->hasPendingNotifications())) - shader->submitPendingNotifications(); - if (shader != nullptr && !shader->isLoaded()) - m_dirtyShaders.push_back(shaderHandle); + QShaderProgram::ShaderType shaderType = QShaderProgram::Vertex; + switch (builderType) { + case ShaderBuilder::Vertex: + shaderType = QShaderProgram::Vertex; + break; + case ShaderBuilder::TessellationControl: + shaderType = QShaderProgram::TessellationControl; + break; + case ShaderBuilder::TessellationEvaluation: + shaderType = QShaderProgram::TessellationEvaluation; + break; + case ShaderBuilder::Geometry: + shaderType = QShaderProgram::Geometry; + break; + case ShaderBuilder::Fragment: + shaderType = QShaderProgram::Fragment; + break; + case ShaderBuilder::Compute: + shaderType = QShaderProgram::Compute; + break; + } + + const auto code = shaderBuilder->shaderCode(builderType); + shader->setShaderCode(shaderType, code); + } } + + if (Q_UNLIKELY(shader->hasPendingNotifications())) + shader->submitPendingNotifications(); + if (shader != nullptr && !shader->isLoaded()) + m_dirtyShaders.push_back(shaderHandle); } } } @@ -1536,7 +1535,6 @@ QVector Renderer::renderBinJobs() renderBinJobs.push_back(m_cleanupJob); renderBinJobs.push_back(m_sendRenderCaptureJob); renderBinJobs.push_back(m_sendBufferCaptureJob); - renderBinJobs.push_back(m_filterCompatibleTechniqueJob); renderBinJobs.append(bufferJobs); // Jobs to prepare GL Resource upload @@ -1545,9 +1543,6 @@ QVector Renderer::renderBinJobs() if (dirtyBitsForFrame & AbstractRenderer::BuffersDirty) renderBinJobs.push_back(m_bufferGathererJob); - if (dirtyBitsForFrame & AbstractRenderer::ShadersDirty) - renderBinJobs.push_back(m_shaderGathererJob); - if (dirtyBitsForFrame & AbstractRenderer::TexturesDirty) { renderBinJobs.push_back(m_syncTextureLoadingJob); renderBinJobs.push_back(m_textureGathererJob); @@ -1558,7 +1553,6 @@ QVector Renderer::renderBinJobs() // on entities const bool layersDirty = dirtyBitsForFrame & AbstractRenderer::LayersDirty; const bool layersCacheNeedsToBeRebuilt = layersDirty || entitiesEnabledDirty; - bool layersCacheRebuilt = false; QMutexLocker lock(m_renderQueue->mutex()); if (m_renderQueue->wasReset()) { // Have we rendered yet? (Scene3D case) @@ -1584,25 +1578,25 @@ QVector Renderer::renderBinJobs() builder.prepareJobs(); renderBinJobs.append(builder.buildJobHierachy()); } - layersCacheRebuilt = true; // Set target number of RenderViews m_renderQueue->setTargetRenderViewCount(fgBranchCount); - } - - // Only reset LayersDirty flag once we have really rebuilt the caches - if (layersDirty && !layersCacheRebuilt) - notCleared |= AbstractRenderer::LayersDirty; - if (entitiesEnabledDirty && !layersCacheRebuilt) + } else { + // FilterLayerEntityJob is part of the RenderViewBuilder jobs and must be run later + // if none of those jobs are started this frame notCleared |= AbstractRenderer::EntityEnabledDirty; + notCleared |= AbstractRenderer::LayersDirty; + } - // Clear dirty bits - // TO DO: When secondary GL thread is integrated, the following line can be removed - notCleared |= AbstractRenderer::ShadersDirty; - - // Clear all dirty flags but Compute so that - // we still render every frame when a compute shader is used in a scene - notCleared |= Renderer::ComputeDirty; + if (isRunning() && m_graphicsContext->isInitialized()) { + if (dirtyBitsForFrame & AbstractRenderer::TechniquesDirty ) + renderBinJobs.push_back(m_filterCompatibleTechniqueJob); + if (dirtyBitsForFrame & AbstractRenderer::ShadersDirty) + renderBinJobs.push_back(m_shaderGathererJob); + } else { + notCleared |= AbstractRenderer::TechniquesDirty; + notCleared |= AbstractRenderer::ShadersDirty; + } m_dirtyBits.remaining = dirtyBitsForFrame & notCleared; diff --git a/src/render/jobs/filtercompatibletechniquejob.cpp b/src/render/jobs/filtercompatibletechniquejob.cpp index 362e4088f..080ccd306 100644 --- a/src/render/jobs/filtercompatibletechniquejob.cpp +++ b/src/render/jobs/filtercompatibletechniquejob.cpp @@ -79,14 +79,13 @@ Renderer *FilterCompatibleTechniqueJob::renderer() const void FilterCompatibleTechniqueJob::run() { Q_ASSERT(m_manager != nullptr && m_renderer != nullptr); + Q_ASSERT(m_renderer->isRunning() && m_renderer->graphicsContext()->isInitialized()); - if (m_renderer->isRunning() && m_renderer->graphicsContext()->isInitialized()) { - const QVector dirtyTechniqueIds = m_manager->takeDirtyTechniques(); - for (const Qt3DCore::QNodeId techniqueId : dirtyTechniqueIds) { - Technique *technique = m_manager->lookupResource(techniqueId); - if (Q_LIKELY(technique != nullptr)) - technique->setCompatibleWithRenderer((*m_renderer->contextInfo() == *technique->graphicsApiFilter())); - } + const QVector dirtyTechniqueIds = m_manager->takeDirtyTechniques(); + for (const Qt3DCore::QNodeId techniqueId : dirtyTechniqueIds) { + Technique *technique = m_manager->lookupResource(techniqueId); + if (Q_LIKELY(technique != nullptr)) + technique->setCompatibleWithRenderer((*m_renderer->contextInfo() == *technique->graphicsApiFilter())); } } diff --git a/src/render/materialsystem/technique.cpp b/src/render/materialsystem/technique.cpp index 4fd1555e1..5438fa9c8 100644 --- a/src/render/materialsystem/technique.cpp +++ b/src/render/materialsystem/technique.cpp @@ -102,43 +102,53 @@ void Technique::sceneChangeEvent(const Qt3DCore::QSceneChangePtr &e) switch (e->type()) { case PropertyUpdated: { const auto change = qSharedPointerCast(e); - if (change->propertyName() == QByteArrayLiteral("graphicsApiFilterData")) { + if (change->propertyName() == QByteArrayLiteral("enabled")) { + markDirty(AbstractRenderer::TechniquesDirty); + } else if (change->propertyName() == QByteArrayLiteral("graphicsApiFilterData")) { GraphicsApiFilterData filterData = change->value().value(); m_graphicsApiFilterData = filterData; // Notify the manager that our graphicsApiFilterData has changed // and that we therefore need to be check for compatibility again m_isCompatibleWithRenderer = false; m_nodeManager->techniqueManager()->addDirtyTechnique(peerId()); + markDirty(AbstractRenderer::TechniquesDirty); } break; } case PropertyValueAdded: { const auto change = qSharedPointerCast(e); - if (change->propertyName() == QByteArrayLiteral("pass")) + if (change->propertyName() == QByteArrayLiteral("pass")) { appendRenderPass(change->addedNodeId()); - else if (change->propertyName() == QByteArrayLiteral("parameter")) + markDirty(AbstractRenderer::TechniquesDirty); + } else if (change->propertyName() == QByteArrayLiteral("parameter")) { m_parameterPack.appendParameter(change->addedNodeId()); - else if (change->propertyName() == QByteArrayLiteral("filterKeys")) + markDirty(AbstractRenderer::TechniquesDirty); + } else if (change->propertyName() == QByteArrayLiteral("filterKeys")) { appendFilterKey(change->addedNodeId()); + markDirty(AbstractRenderer::TechniquesDirty); + } break; } case PropertyValueRemoved: { const auto change = qSharedPointerCast(e); - if (change->propertyName() == QByteArrayLiteral("pass")) + if (change->propertyName() == QByteArrayLiteral("pass")) { removeRenderPass(change->removedNodeId()); - else if (change->propertyName() == QByteArrayLiteral("parameter")) + markDirty(AbstractRenderer::TechniquesDirty); + } else if (change->propertyName() == QByteArrayLiteral("parameter")) { m_parameterPack.removeParameter(change->removedNodeId()); - else if (change->propertyName() == QByteArrayLiteral("filterKeys")) + markDirty(AbstractRenderer::TechniquesDirty); + } else if (change->propertyName() == QByteArrayLiteral("filterKeys")) { removeFilterKey(change->removedNodeId()); + markDirty(AbstractRenderer::TechniquesDirty); + } break; } default: break; } - markDirty(AbstractRenderer::AllDirty); BackendNode::sceneChangeEvent(e); } diff --git a/tests/auto/render/filtercompatibletechniquejob/tst_filtercompatibletechniquejob.cpp b/tests/auto/render/filtercompatibletechniquejob/tst_filtercompatibletechniquejob.cpp index 83e816861..4d4a08a34 100644 --- a/tests/auto/render/filtercompatibletechniquejob/tst_filtercompatibletechniquejob.cpp +++ b/tests/auto/render/filtercompatibletechniquejob/tst_filtercompatibletechniquejob.cpp @@ -192,35 +192,6 @@ private Q_SLOTS: QCOMPARE(backendFilterCompatibleTechniqueJob.renderer(), &renderer); } - void checkRunRendererNotRunning() - { - // GIVEN - Qt3DRender::Render::FilterCompatibleTechniqueJob backendFilterCompatibleTechniqueJob; - Qt3DRender::TestAspect testAspect(buildTestScene()); - - // WHEN - Qt3DRender::Render::NodeManagers *nodeManagers = testAspect.nodeManagers(); - QVERIFY(nodeManagers); - Qt3DRender::Render::TechniqueManager *techniqueManager = nodeManagers->techniqueManager(); - QVERIFY(techniqueManager); - backendFilterCompatibleTechniqueJob.setManager(techniqueManager); - backendFilterCompatibleTechniqueJob.setRenderer(testAspect.renderer()); - testAspect.initializeRenderer(); - testAspect.renderer()->shutdown(); - - // THEN - QCOMPARE(testAspect.renderer()->isRunning(), false); - QVector handles = testAspect.nodeManagers()->techniqueManager()->activeHandles(); - QCOMPARE(handles.size(), 3); - - // WHEN - backendFilterCompatibleTechniqueJob.run(); - - // THEN -> untouched since not running - const QVector dirtyTechniquesId = testAspect.nodeManagers()->techniqueManager()->takeDirtyTechniques(); - QCOMPARE(dirtyTechniquesId.size(), 3); - } - void checkRunRendererRunning() { // GIVEN diff --git a/tests/auto/render/renderer/tst_renderer.cpp b/tests/auto/render/renderer/tst_renderer.cpp index 85d978926..b53c26f83 100644 --- a/tests/auto/render/renderer/tst_renderer.cpp +++ b/tests/auto/render/renderer/tst_renderer.cpp @@ -60,6 +60,9 @@ private Q_SLOTS: renderer.setSettings(&settings); renderer.initialize(); + // NOTE: FilterCompatibleTechniqueJob and ShaderGathererJob cannot run because the context + // is not initialized in this test + const int singleRenderViewJobCount = 11 + 2 * Qt3DRender::Render::RenderViewBuilder::optimalJobCount(); // RenderViewBuilder renderViewJob, // renderableEntityFilterJob, @@ -84,7 +87,6 @@ private Q_SLOTS: 1 + // cleanupJob 1 + // sendRenderCaptureJob 1 + // sendBufferCaptureJob - 1 + // filterCompatibleTechniquesJob 1 + // VAOGatherer 1 + // updateSkinningPaletteJob singleRenderViewJobCount); // Only valid for the first call to renderBinJobs(), since subsequent calls won't have the renderqueue reset @@ -100,7 +102,6 @@ private Q_SLOTS: 1 + // cleanupJob 1 + // sendRenderCaptureJob 1 + // sendBufferCaptureJob - 1 + // filterCompatibleTechniquesJob 1 + // VAOGatherer 1 + // updateSkinningPaletteJob 1); // EntityEnabledDirty @@ -117,7 +118,6 @@ private Q_SLOTS: 1 + // cleanupJob 1 + // sendRenderCaptureJob 1 + // sendBufferCaptureJob - 1 + // filterCompatibleTechniquesJob 1 + // VAOGatherer 1 + // WorldTransformJob 1 + // UpdateWorldBoundingVolume @@ -137,7 +137,6 @@ private Q_SLOTS: 1 + // cleanupJob 1 + // sendRenderCaptureJob 1 + // sendBufferCaptureJob - 1 + // filterCompatibleTechniquesJob 1 + // VAOGatherer 1 + // CalculateBoundingVolumeJob 1 + // UpdateMeshTriangleListJob @@ -156,30 +155,12 @@ private Q_SLOTS: 1 + // cleanupJob 1 + // sendRenderCaptureJob 1 + // sendBufferCaptureJob - 1 + // filterCompatibleTechniquesJob 1 + // VAOGatherer 1 + // updateSkinningPaletteJob 1); // BufferGathererJob renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty); - // WHEN - renderer.markDirty(Qt3DRender::Render::AbstractRenderer::ShadersDirty, nullptr); - jobs = renderer.renderBinJobs(); - - // THEN (level - QCOMPARE(jobs.size(), - 1 + // updateLevelOfDetailJob - 1 + // cleanupJob - 1 + // sendRenderCaptureJob - 1 + // sendBufferCaptureJob - 1 + // filterCompatibleTechniquesJob - 1 + // VAOGatherer - 1 + // updateSkinningPaletteJob - 1); // ShaderGathererJob - - renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty); - // WHEN renderer.markDirty(Qt3DRender::Render::AbstractRenderer::TexturesDirty, nullptr); jobs = renderer.renderBinJobs(); @@ -190,7 +171,6 @@ private Q_SLOTS: 1 + // cleanupJob 1 + // sendRenderCaptureJob 1 + // sendBufferCaptureJob - 1 + // filterCompatibleTechniquesJob 1 + // VAOGatherer 1 + // TexturesGathererJob 1 + // updateSkinningPaletteJob @@ -208,7 +188,6 @@ private Q_SLOTS: 1 + // cleanupJob 1 + // sendRenderCaptureJob 1 + // sendBufferCaptureJob - 1 + // filterCompatibleTechniquesJob 1 + // VAOGatherer 1 + // EntityEnabledDirty 1 + // WorldTransformJob @@ -218,7 +197,6 @@ private Q_SLOTS: 1 + // CalculateBoundingVolumeJob 1 + // UpdateMeshTriangleListJob 1 + // BufferGathererJob - 1 + // ShaderGathererJob 1 + // TexturesGathererJob 1 + // updateSkinningPaletteJob 1); // SyncTexturesGathererJob diff --git a/tests/auto/render/technique/tst_technique.cpp b/tests/auto/render/technique/tst_technique.cpp index a712434d1..7dfd5b85c 100644 --- a/tests/auto/render/technique/tst_technique.cpp +++ b/tests/auto/render/technique/tst_technique.cpp @@ -193,6 +193,8 @@ private Q_SLOTS: // THEN QCOMPARE(backendTechnique.isEnabled(), newValue); + QVERIFY(renderer.dirtyBits() & Qt3DRender::Render::AbstractRenderer::TechniquesDirty); + renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty); } { // WHEN @@ -217,6 +219,8 @@ private Q_SLOTS: QCOMPARE(dirtyTechniques.size(), 1); QCOMPARE(dirtyTechniques.first(), backendTechnique.peerId()); + QVERIFY(renderer.dirtyBits() & Qt3DRender::Render::AbstractRenderer::TechniquesDirty); + renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty); } { Qt3DRender::QParameter parameter; @@ -230,6 +234,8 @@ private Q_SLOTS: // THEN QCOMPARE(backendTechnique.parameters().size(), 1); QCOMPARE(backendTechnique.parameters().first(), parameter.id()); + QVERIFY(renderer.dirtyBits() & Qt3DRender::Render::AbstractRenderer::TechniquesDirty); + renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty); } { // WHEN @@ -239,6 +245,8 @@ private Q_SLOTS: // THEN QCOMPARE(backendTechnique.parameters().size(), 0); + QVERIFY(renderer.dirtyBits() & Qt3DRender::Render::AbstractRenderer::TechniquesDirty); + renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty); } } { @@ -253,6 +261,8 @@ private Q_SLOTS: // THEN QCOMPARE(backendTechnique.filterKeys().size(), 1); QCOMPARE(backendTechnique.filterKeys().first(), filterKey.id()); + QVERIFY(renderer.dirtyBits() & Qt3DRender::Render::AbstractRenderer::TechniquesDirty); + renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty); } { // WHEN @@ -262,6 +272,8 @@ private Q_SLOTS: // THEN QCOMPARE(backendTechnique.filterKeys().size(), 0); + QVERIFY(renderer.dirtyBits() & Qt3DRender::Render::AbstractRenderer::TechniquesDirty); + renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty); } } { @@ -276,6 +288,8 @@ private Q_SLOTS: // THEN QCOMPARE(backendTechnique.renderPasses().size(), 1); QCOMPARE(backendTechnique.renderPasses().first(), pass.id()); + QVERIFY(renderer.dirtyBits() & Qt3DRender::Render::AbstractRenderer::TechniquesDirty); + renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty); } { // WHEN @@ -285,6 +299,8 @@ private Q_SLOTS: // THEN QCOMPARE(backendTechnique.renderPasses().size(), 0); + QVERIFY(renderer.dirtyBits() & Qt3DRender::Render::AbstractRenderer::TechniquesDirty); + renderer.clearDirtyBits(Qt3DRender::Render::AbstractRenderer::AllDirty); } } } -- cgit v1.2.3 From 959db7d038544803fe4cf2601523ea3af224652e Mon Sep 17 00:00:00 2001 From: Svenn-Arne Dragly Date: Mon, 29 Jan 2018 18:25:04 +0100 Subject: Make PickBoundingVolumeJob depend on ExpandBoundingVolumeJob PickBoundingVolumeJob uses QEntity::worldBoundingVolumeWithChildren(), which is set by ExpandBoundingVolumeJob. Change-Id: Ic03360a694254e45c9abfd6863a7b101910bb7fc Reviewed-by: Paul Lemire --- src/render/backend/renderer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/render/backend/renderer.cpp b/src/render/backend/renderer.cpp index cfe0cd98a..b5b387cc5 100644 --- a/src/render/backend/renderer.cpp +++ b/src/render/backend/renderer.cpp @@ -205,6 +205,7 @@ Renderer::Renderer(QRenderAspect::RenderType type) m_updateWorldBoundingVolumeJob->addDependency(m_calculateBoundingVolumeJob); m_expandBoundingVolumeJob->addDependency(m_updateWorldBoundingVolumeJob); m_updateShaderDataTransformJob->addDependency(m_worldTransformJob); + m_pickBoundingVolumeJob->addDependency(m_expandBoundingVolumeJob); // Dirty texture gathering depends on m_syncTextureLoadingJob // m_syncTextureLoadingJob will depend on the texture loading jobs -- cgit v1.2.3 From 30abe028f9a95fa32fbb77cb60063d062e13f7ff Mon Sep 17 00:00:00 2001 From: Paul Lemire Date: Thu, 25 Jan 2018 10:10:22 +0100 Subject: LoadSceneJob: refactored to minimize code duplication Also improve error output Change-Id: Ibf4a4340c83dadd507f4f356682cc861930fe2fd Reviewed-by: Sean Harmer --- src/render/jobs/loadscenejob.cpp | 103 +++++++++++++++++++++++---------------- src/render/jobs/loadscenejob_p.h | 8 +++ src/render/renderlogging.cpp | 1 + src/render/renderlogging_p.h | 1 + 4 files changed, 72 insertions(+), 41 deletions(-) diff --git a/src/render/jobs/loadscenejob.cpp b/src/render/jobs/loadscenejob.cpp index 5be733a4d..f767fe720 100644 --- a/src/render/jobs/loadscenejob.cpp +++ b/src/render/jobs/loadscenejob.cpp @@ -46,7 +46,7 @@ #include #include #include - +#include #include #include @@ -93,7 +93,6 @@ void LoadSceneJob::run() { // Iterate scene IO handlers until we find one that can handle this file type Qt3DCore::QEntity *sceneSubTree = nullptr; - Scene *scene = m_managers->sceneManager()->lookupResource(m_sceneComponent); Q_ASSERT(scene); @@ -107,52 +106,37 @@ void LoadSceneJob::run() if (m_data.isEmpty()) { const QString path = QUrlHelper::urlToLocalFileOrQrc(m_source); - QFileInfo finfo(path); + const QFileInfo finfo(path); + qCDebug(SceneLoaders) << Q_FUNC_INFO << "Attempting to load" << finfo.filePath(); if (finfo.exists()) { - QStringList extensions(finfo.suffix()); - - for (QSceneImporter *sceneImporter : qAsConst(m_sceneImporters)) { - if (!sceneImporter->areFileTypesSupported(extensions)) - continue; - - // If the file type is supported -> enter Loading status - scene->setStatus(QSceneLoader::Loading); - - // File type is supported, try to load it - sceneImporter->setSource(m_source); - sceneSubTree = sceneImporter->scene(); - if (sceneSubTree != nullptr) { - // Successfully built a subtree - finalStatus = QSceneLoader::Ready; - break; - } - } + const QStringList extensions(finfo.suffix()); + sceneSubTree = tryLoadScene(scene, + finalStatus, + extensions, + [this] (QSceneImporter *importer) { + importer->setSource(m_source); + }); + } else { + qCWarning(SceneLoaders) << Q_FUNC_INFO << finfo.filePath() << "doesn't exist"; } } else { QStringList extensions; QMimeDatabase db; - QMimeType mtype = db.mimeTypeForData(m_data); - if (mtype.isValid()) { + const QMimeType mtype = db.mimeTypeForData(m_data); + + if (mtype.isValid()) extensions = mtype.suffixes(); - } + else + qCWarning(SceneLoaders) << Q_FUNC_INFO << "Invalid mime type" << mtype; - QString basePath = m_source.adjusted(QUrl::RemoveFilename).toString(); - for (QSceneImporter *sceneImporter : qAsConst(m_sceneImporters)) { - if (!sceneImporter->areFileTypesSupported(extensions)) - continue; - - // If the file type is supported -> enter Loading status - scene->setStatus(QSceneLoader::Loading); - - // File type is supported, try to load it - sceneImporter->setData(m_data, basePath); - sceneSubTree = sceneImporter->scene(); - if (sceneSubTree != nullptr) { - // Successfully built a subtree - finalStatus = QSceneLoader::Ready; - break; - } - } + const QString basePath = m_source.adjusted(QUrl::RemoveFilename).toString(); + + sceneSubTree = tryLoadScene(scene, + finalStatus, + extensions, + [this, basePath] (QSceneImporter *importer) { + importer->setData(m_data, basePath); + }); } } @@ -167,6 +151,43 @@ void LoadSceneJob::run() scene->setStatus(finalStatus); } +Qt3DCore::QEntity *LoadSceneJob::tryLoadScene(Scene *scene, + QSceneLoader::Status &finalStatus, + const QStringList &extensions, + const std::function &importerSetupFunc) +{ + Qt3DCore::QEntity *sceneSubTree = nullptr; + bool foundSuitableLoggerPlugin = false; + + for (QSceneImporter *sceneImporter : qAsConst(m_sceneImporters)) { + if (!sceneImporter->areFileTypesSupported(extensions)) + continue; + + foundSuitableLoggerPlugin = true; + + // If the file type is supported -> enter Loading status + scene->setStatus(QSceneLoader::Loading); + + // Set source file or data on importer + importerSetupFunc(sceneImporter); + + // File type is supported, try to load it + sceneSubTree = sceneImporter->scene(); + if (sceneSubTree != nullptr) { + // Successfully built a subtree + finalStatus = QSceneLoader::Ready; + break; + } + + qCWarning(SceneLoaders) << Q_FUNC_INFO << "Failed to import" << m_source << "with errors" << sceneImporter->errors(); + } + + if (!foundSuitableLoggerPlugin) + qCWarning(SceneLoaders) << Q_FUNC_INFO << "Found not suitable importer plugin for" << m_source; + + return sceneSubTree; +} + } // namespace Render } // namespace Qt3DRender diff --git a/src/render/jobs/loadscenejob_p.h b/src/render/jobs/loadscenejob_p.h index 5217c6f43..e3e947700 100644 --- a/src/render/jobs/loadscenejob_p.h +++ b/src/render/jobs/loadscenejob_p.h @@ -53,8 +53,10 @@ #include #include +#include #include #include +#include QT_BEGIN_NAMESPACE @@ -64,6 +66,7 @@ class QSceneImporter; namespace Render { +class Scene; class NodeManagers; class Q_AUTOTEST_EXPORT LoadSceneJob : public Qt3DCore::QAspectJob @@ -87,6 +90,11 @@ private: Qt3DCore::QNodeId m_sceneComponent; NodeManagers *m_managers; QList m_sceneImporters; + + Qt3DCore::QEntity *tryLoadScene(Scene *scene, + QSceneLoader::Status &finalStatus, + const QStringList &extensions, + const std::function &importerSetupFunc); }; typedef QSharedPointer LoadSceneJobPtr; diff --git a/src/render/renderlogging.cpp b/src/render/renderlogging.cpp index b9d423163..2eb1835e6 100644 --- a/src/render/renderlogging.cpp +++ b/src/render/renderlogging.cpp @@ -49,6 +49,7 @@ Q_LOGGING_CATEGORY(Backend, "Qt3D.Renderer.Backend", QtWarningMsg) Q_LOGGING_CATEGORY(Frontend, "Qt3D.Renderer.Frontend", QtWarningMsg) Q_LOGGING_CATEGORY(Io, "Qt3D.Renderer.IO", QtWarningMsg) Q_LOGGING_CATEGORY(Jobs, "Qt3D.Renderer.Jobs", QtWarningMsg) +Q_LOGGING_CATEGORY(SceneLoaders, "Qt3D.Renderer.SceneLoaders", QtWarningMsg) Q_LOGGING_CATEGORY(Framegraph, "Qt3D.Renderer.Framegraph", QtWarningMsg) Q_LOGGING_CATEGORY(RenderNodes, "Qt3D.Renderer.RenderNodes", QtWarningMsg) Q_LOGGING_CATEGORY(Rendering, "Qt3D.Renderer.Rendering", QtWarningMsg) diff --git a/src/render/renderlogging_p.h b/src/render/renderlogging_p.h index dfa761e46..00ae572f4 100644 --- a/src/render/renderlogging_p.h +++ b/src/render/renderlogging_p.h @@ -63,6 +63,7 @@ Q_DECLARE_LOGGING_CATEGORY(Backend) Q_DECLARE_LOGGING_CATEGORY(Frontend) Q_DECLARE_LOGGING_CATEGORY(Io) Q_DECLARE_LOGGING_CATEGORY(Jobs) +Q_DECLARE_LOGGING_CATEGORY(SceneLoaders) Q_DECLARE_LOGGING_CATEGORY(Framegraph) Q_DECLARE_LOGGING_CATEGORY(RenderNodes) Q_DECLARE_LOGGING_CATEGORY(Rendering) -- cgit v1.2.3 From 89b7f69886da5de98a7ff27d44ca34cd2c1e45e1 Mon Sep 17 00:00:00 2001 From: Antti Kokko Date: Tue, 30 Jan 2018 15:35:17 +0200 Subject: Add changes file for Qt 5.10.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: Ibbcf306b33c9a6b5d67c71b5c76d4e45055b9452 Reviewed-by: Tomi Korpipää --- dist/changes-5.10.1 | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 dist/changes-5.10.1 diff --git a/dist/changes-5.10.1 b/dist/changes-5.10.1 new file mode 100644 index 000000000..62ea7ddd2 --- /dev/null +++ b/dist/changes-5.10.1 @@ -0,0 +1,32 @@ +Qt 5.10.1 is a bug-fix release. It maintains both forward and backward +compatibility (source and binary) with Qt 5.10.0. + +For more details, refer to the online documentation included in this +distribution. The documentation is also available online: + +http://doc.qt.io/qt-5/index.html + +The Qt version 5.10 series is binary compatible with the 5.9.x series. +Applications compiled for 5.9 will continue to run with 5.10. + +Some of the changes listed in this file include issue tracking numbers +corresponding to tasks in the Qt Bug Tracker: + +https://bugreports.qt.io/ + +Each of these identifiers can be entered in the bug tracker to obtain more +information about a particular change. + +This release contains all fixes included in the Qt 5.9.4 release. + +**************************************************************************** +* Qt 5.10.1 Changes * +**************************************************************************** + +UNSPECIFIED +----------- + + - [QTBUG-65123] (Q)BlitFramebuffer has been corrected to treat the source + and destination rectangles to be in the usual Qt coordinate system with + Y + at top. -- cgit v1.2.3 From b923dae74e9baf75aae99f87b19568ec1dc39f82 Mon Sep 17 00:00:00 2001 From: Sean Harmer Date: Mon, 29 Jan 2018 11:04:45 +0000 Subject: Ensure node creation changes are sent before using in list properties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This completes the fix for out of order event delivery related to creation changes. We now ensure that QNodes used as values in singular and list properties are fully constructed on the backend before they are referenced in properties of other nodes. Also added a check to not recurse into sending too many changes when adding a child node. Written with Svenn-Arne Dragly. Task-number: Task-number: QTBUG-65956 Change-Id: I1470e0f685c81d1277ac04ad985ec1b76f1c27c0 Reviewed-by: Paul Lemire (cherry picked from commit 8fa23602cff47de6d19d05a8428a8e753bf73d61) Reviewed-by: Antti Määttä --- src/core/changes/qpropertynodeaddedchange.cpp | 10 ++++ src/core/nodes/qnode.cpp | 17 +++++- src/core/nodes/qnode_p.h | 4 +- tests/auto/core/nodes/tst_nodes.cpp | 83 +++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 2 deletions(-) diff --git a/src/core/changes/qpropertynodeaddedchange.cpp b/src/core/changes/qpropertynodeaddedchange.cpp index 390a170b6..9f8e50872 100644 --- a/src/core/changes/qpropertynodeaddedchange.cpp +++ b/src/core/changes/qpropertynodeaddedchange.cpp @@ -76,6 +76,16 @@ QPropertyNodeAddedChange::QPropertyNodeAddedChange(QNodeId subjectId, QNode *nod { Q_D(QPropertyNodeAddedChange); d->m_addedNodeIdTypePair = QNodeIdTypePair(node->id(), QNodePrivate::findStaticMetaObject(node->metaObject())); + + // Ensure the node has issued a node creation change. We can end + // up here if a newly created node with a parent is immediately set + // as a property on another node. In this case the deferred call to + // _q_postConstructorInit() will not have happened yet as the event + // loop will still be blocked. So force it here and we catch this + // eventuality in the _q_postConstructorInit() function so that we + // do not repeat the creation and new child scene change events. + if (node) + QNodePrivate::get(node)->_q_postConstructorInit(); } /*! \internal */ diff --git a/src/core/nodes/qnode.cpp b/src/core/nodes/qnode.cpp index dbe3fd102..e5a3aca71 100644 --- a/src/core/nodes/qnode.cpp +++ b/src/core/nodes/qnode.cpp @@ -74,6 +74,7 @@ QNodePrivate::QNodePrivate() , m_blockNotifications(false) , m_hasBackendNode(false) , m_enabled(true) + , m_notifiedParent(false) , m_defaultPropertyTrackMode(QNode::TrackFinalValues) , m_propertyChangesSetup(false) , m_signals(this) @@ -210,13 +211,19 @@ void QNodePrivate::_q_addChild(QNode *childNode) Q_ASSERT(childNode); Q_ASSERT_X(childNode->parent() == q_func(), Q_FUNC_INFO, "not a child of this node"); + // Have we already notified the parent about its new child? If so, bail out + // early so that we do not send more than one new child event to the backend + QNodePrivate *childD = QNodePrivate::get(childNode); + if (childD->m_notifiedParent == true) + return; + // Store our id as the parentId in the child so that even if the child gets // removed from the scene as part of the destruction of the parent, when the // parent's children are deleted in the QObject dtor, we still have access to // the parentId. If we didn't store this, we wouldn't have access at that time // because the parent would then only be a QObject, the QNode part would have // been destroyed already. - QNodePrivate::get(childNode)->m_parentId = m_id; + childD->m_parentId = m_id; if (!m_scene) return; @@ -224,6 +231,11 @@ void QNodePrivate::_q_addChild(QNode *childNode) // We need to send a QPropertyNodeAddedChange to the backend // to notify the backend that we have a new child if (m_changeArbiter != nullptr) { + // Flag that we have notified the parent. We do this immediately before + // creating the change because that recurses back into this function and + // we need to catch that to avoid sending more than one new child event + // to the backend. + childD->m_notifiedParent = true; const auto change = QPropertyNodeAddedChangePtr::create(m_id, childNode); change->setPropertyName("children"); notifyObservers(change); @@ -299,6 +311,9 @@ void QNodePrivate::_q_setParentHelper(QNode *parent) notifyDestructionChangesAndRemoveFromScene(); } + // Flag that we need to notify any new parent + m_notifiedParent = false; + // Basically QObject::setParent but for QObjectPrivate QObjectPrivate::setParent_helper(parent); QNode *newParentNode = q->parentNode(); diff --git a/src/core/nodes/qnode_p.h b/src/core/nodes/qnode_p.h index ad9d2376e..87a0226f1 100644 --- a/src/core/nodes/qnode_p.h +++ b/src/core/nodes/qnode_p.h @@ -100,6 +100,7 @@ public: bool m_blockNotifications; bool m_hasBackendNode; bool m_enabled; + bool m_notifiedParent; QNode::PropertyTrackingMode m_defaultPropertyTrackMode; QHash m_trackedPropertiesOverrides; @@ -137,10 +138,11 @@ public: static const QMetaObject *findStaticMetaObject(const QMetaObject *metaObject); + void _q_postConstructorInit(); + private: void notifyCreationChange(); void notifyDestructionChangesAndRemoveFromScene(); - void _q_postConstructorInit(); void _q_addChild(QNode *childNode); void _q_removeChild(QNode *childNode); void _q_setParentHelper(QNode *parent); diff --git a/tests/auto/core/nodes/tst_nodes.cpp b/tests/auto/core/nodes/tst_nodes.cpp index 49618821c..81af0cd63 100644 --- a/tests/auto/core/nodes/tst_nodes.cpp +++ b/tests/auto/core/nodes/tst_nodes.cpp @@ -81,6 +81,7 @@ private slots: void checkConstructionSetParentMix(); // QTBUG-60612 void checkConstructionWithParent(); + void checkConstructionAsListElement(); void appendingComponentToEntity(); void appendingParentlessComponentToEntity(); @@ -240,6 +241,43 @@ public slots: emit nodePropertyChanged(node); } + void addAttribute(MyQNode *attribute) + { + Qt3DCore::QNodePrivate *d = Qt3DCore::QNodePrivate::get(this); + if (!m_attributes.contains(attribute)) { + m_attributes.append(attribute); + + // Ensures proper bookkeeping + d->registerDestructionHelper(attribute, &MyQNode::removeAttribute, m_attributes); + + // We need to add it as a child of the current node if it has been declared inline + // Or not previously added as a child of the current node so that + // 1) The backend gets notified about it's creation + // 2) When the current node is destroyed, it gets destroyed as well + if (!attribute->parent()) + attribute->setParent(this); + + if (d->m_changeArbiter != nullptr) { + const auto change = Qt3DCore::QPropertyNodeAddedChangePtr::create(id(), attribute); + change->setPropertyName("attribute"); + d->notifyObservers(change); + } + } + } + + void removeAttribute(MyQNode *attribute) + { + Qt3DCore::QNodePrivate *d = Qt3DCore::QNodePrivate::get(this); + if (d->m_changeArbiter != nullptr) { + const auto change = Qt3DCore::QPropertyNodeRemovedChangePtr::create(id(), attribute); + change->setPropertyName("attribute"); + d->notifyObservers(change); + } + m_attributes.removeOne(attribute); + // Remove bookkeeping connection + d->unregisterDestructionHelper(attribute); + } + signals: void customPropertyChanged(); void nodePropertyChanged(MyQNode *node); @@ -247,6 +285,7 @@ signals: protected: QString m_customProperty; MyQNode *m_nodeProperty; + QVector m_attributes; }; class MyQEntity : public Qt3DCore::QEntity @@ -921,6 +960,50 @@ void tst_Nodes::checkConstructionWithParent() QCOMPARE(propertyEvent->value().value(), node->id()); } +void tst_Nodes::checkConstructionAsListElement() +{ + // GIVEN + ObserverSpy spy; + Qt3DCore::QScene scene; + QScopedPointer root(new MyQNode()); + + // WHEN + root->setArbiterAndScene(&spy, &scene); + root->setSimulateBackendCreated(true); + + // THEN + QVERIFY(Qt3DCore::QNodePrivate::get(root.data())->scene() != nullptr); + + // WHEN we create a child and then set it as a Node* property + auto *node = new MyQNode(root.data()); + root->addAttribute(node); + + // THEN we should get one creation change, one child added change + // and one property change event, in that order. + QCoreApplication::processEvents(); + + QCOMPARE(root->children().count(), 1); + QCOMPARE(spy.events.size(), 3); // 1 creation change, 1 child added change, 1 property change + + // Ensure first event is child node's creation change + const auto creationEvent = spy.events.takeFirst().change().dynamicCast(); + QVERIFY(!creationEvent.isNull()); + QCOMPARE(creationEvent->subjectId(), node->id()); + + const auto newChildEvent = spy.events.takeFirst().change().dynamicCast(); + QVERIFY(!newChildEvent.isNull()); + QCOMPARE(newChildEvent->subjectId(), root->id()); + QCOMPARE(newChildEvent->propertyName(), "children"); + QCOMPARE(newChildEvent->addedNodeId(), node->id()); + + // Ensure second and last event is property set change + const auto propertyEvent = spy.events.takeFirst().change().dynamicCast(); + QVERIFY(!propertyEvent.isNull()); + QCOMPARE(propertyEvent->subjectId(), root->id()); + QCOMPARE(propertyEvent->propertyName(), "attribute"); + QCOMPARE(newChildEvent->addedNodeId(), node->id()); +} + void tst_Nodes::appendingParentlessComponentToEntity() { // GIVEN -- cgit v1.2.3 From 156afc0f9dad1c16ee9a0ed31307121941443491 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A4=C3=A4tt=C3=A4=20Antti?= Date: Mon, 5 Feb 2018 16:36:44 +0200 Subject: Fix crash if sharecontext is requested before initailization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scene2D can sometimes receive the render initialization event before the qt3d renderer has been initialized. This causes crash because the sharecontext hasn't been set yet. Add safeguard against this. Task-number: QT3DS-904 Change-Id: Ib50a60ed89c12ac54c9165266466d9804affe77c Reviewed-by: Laszlo Agocs Reviewed-by: Paul Lemire Reviewed-by: Sean Harmer (cherry picked from commit 13f340c92bdf725d214ab4840fc2e071d12d6e00) Reviewed-by: Antti Määttä --- src/render/backend/renderer.cpp | 75 ++++++++++++++++++++++------------------- src/render/backend/renderer_p.h | 1 + 2 files changed, 42 insertions(+), 34 deletions(-) diff --git a/src/render/backend/renderer.cpp b/src/render/backend/renderer.cpp index b5b387cc5..6217338eb 100644 --- a/src/render/backend/renderer.cpp +++ b/src/render/backend/renderer.cpp @@ -307,7 +307,11 @@ NodeManagers *Renderer::nodeManagers() const */ QOpenGLContext *Renderer::shareContext() const { - return m_shareContext ? m_shareContext : m_graphicsContext->openGLContext()->shareContext(); + QMutexLocker lock(&m_shareContextMutex); + return m_shareContext ? m_shareContext + : (m_graphicsContext->openGLContext() + ? m_graphicsContext->openGLContext()->shareContext() + : nullptr); } void Renderer::setOpenGLContext(QOpenGLContext *context) @@ -325,45 +329,48 @@ void Renderer::initialize() QOpenGLContext* ctx = m_glContext; - // If we are using our own context (not provided by QtQuick), - // we need to create it - if (!m_glContext) { - ctx = new QOpenGLContext; - ctx->setShareContext(qt_gl_global_share_context()); - - // TO DO: Shouldn't we use the highest context available and trust - // QOpenGLContext to fall back on the best lowest supported ? - const QByteArray debugLoggingMode = qgetenv("QT3DRENDER_DEBUG_LOGGING"); + { + QMutexLocker lock(&m_shareContextMutex); + // If we are using our own context (not provided by QtQuick), + // we need to create it + if (!m_glContext) { + ctx = new QOpenGLContext; + ctx->setShareContext(qt_gl_global_share_context()); + + // TO DO: Shouldn't we use the highest context available and trust + // QOpenGLContext to fall back on the best lowest supported ? + const QByteArray debugLoggingMode = qgetenv("QT3DRENDER_DEBUG_LOGGING"); + + if (!debugLoggingMode.isEmpty()) { + QSurfaceFormat sf = ctx->format(); + sf.setOption(QSurfaceFormat::DebugContext); + ctx->setFormat(sf); + } - if (!debugLoggingMode.isEmpty()) { - QSurfaceFormat sf = ctx->format(); - sf.setOption(QSurfaceFormat::DebugContext); - ctx->setFormat(sf); + // Create OpenGL context + if (ctx->create()) + qCDebug(Backend) << "OpenGL context created with actual format" << ctx->format(); + else + qCWarning(Backend) << Q_FUNC_INFO << "OpenGL context creation failed"; + m_ownedContext = true; + } else { + // Context is not owned by us, so we need to know if it gets destroyed + m_contextConnection = QObject::connect(m_glContext, &QOpenGLContext::aboutToBeDestroyed, + [this] { releaseGraphicsResources(); }); } - // Create OpenGL context - if (ctx->create()) - qCDebug(Backend) << "OpenGL context created with actual format" << ctx->format(); - else - qCWarning(Backend) << Q_FUNC_INFO << "OpenGL context creation failed"; - m_ownedContext = true; - } else { - // Context is not owned by us, so we need to know if it gets destroyed - m_contextConnection = QObject::connect(m_glContext, &QOpenGLContext::aboutToBeDestroyed, - [this] { releaseGraphicsResources(); }); - } + if (!ctx->shareContext()) { + m_shareContext = new QOpenGLContext; + m_shareContext->setFormat(ctx->format()); + m_shareContext->setShareContext(ctx); + m_shareContext->create(); + } - if (!ctx->shareContext()) { - m_shareContext = new QOpenGLContext; - m_shareContext->setFormat(ctx->format()); - m_shareContext->setShareContext(ctx); - m_shareContext->create(); + // Note: we don't have a surface at this point + // The context will be made current later on (at render time) + m_graphicsContext->setOpenGLContext(ctx); } - // Note: we don't have a surface at this point - // The context will be made current later on (at render time) - m_graphicsContext->setOpenGLContext(ctx); - // Store the format used by the context and queue up creating an // offscreen surface in the main thread so that it is available // for use when we want to shutdown the renderer. We need to create diff --git a/src/render/backend/renderer_p.h b/src/render/backend/renderer_p.h index 987576059..cbc686ab1 100644 --- a/src/render/backend/renderer_p.h +++ b/src/render/backend/renderer_p.h @@ -319,6 +319,7 @@ private: QAtomicInt m_lastFrameCorrect; QOpenGLContext *m_glContext; QOpenGLContext *m_shareContext; + mutable QMutex m_shareContextMutex; PickBoundingVolumeJobPtr m_pickBoundingVolumeJob; qint64 m_time; -- cgit v1.2.3 From 31f424bb81cd2583920d3d521e1e01f01c2d28e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A4=C3=A4tt=C3=A4=20Antti?= Date: Mon, 5 Feb 2018 13:42:52 +0200 Subject: Fix crash in scene2d at shutdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The resource manager policy for scene2d nodes has been changed so the scene2d constructor gets called multiple times at startup. That in turn increments render thread user counter every time. The cleanup code gets called for each instanciated QScene2D node so at shutdown the counter never reaches zero and the render thread is not closed properly. Change the implementation so that the counter gets incremented only when the render thread has been properly initialized. Task-number: QTBUG-66003 Change-Id: I33a5b1f407e65329776bcabe0b66ff049581a435 Reviewed-by: Svenn-Arne Dragly Reviewed-by: Miikka Heikkinen Reviewed-by: Paul Lemire Reviewed-by: Sean Harmer (cherry picked from commit 564dfd87c5b1317dcf9fbc4d1c8d858c72513421) Reviewed-by: Antti Määttä --- src/quick3d/quick3dscene2d/items/scene2d.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/quick3d/quick3dscene2d/items/scene2d.cpp b/src/quick3d/quick3dscene2d/items/scene2d.cpp index 4abc7cf42..1147abf68 100644 --- a/src/quick3d/quick3dscene2d/items/scene2d.cpp +++ b/src/quick3d/quick3dscene2d/items/scene2d.cpp @@ -123,7 +123,7 @@ Scene2D::Scene2D() , m_mouseEnabled(true) , m_renderPolicy(Qt3DRender::Quick::QScene2D::Continuous) { - renderThreadClientCount->fetchAndAddAcquire(1); + } Scene2D::~Scene2D() @@ -146,6 +146,8 @@ void Scene2D::initializeSharedObject() return; } + renderThreadClientCount->fetchAndAddAcquire(1); + renderThread->setObjectName(QStringLiteral("Scene2D::renderThread")); m_renderThread = renderThread; m_sharedObject->m_renderThread = m_renderThread; @@ -413,10 +415,11 @@ void Scene2D::cleanup() m_sharedObject->wake(); m_sharedObject = nullptr; } - - renderThreadClientCount->fetchAndSubAcquire(1); - if (renderThreadClientCount->load() == 0) - renderThread->quit(); + if (m_renderThread) { + renderThreadClientCount->fetchAndSubAcquire(1); + if (renderThreadClientCount->load() == 0) + renderThread->quit(); + } } -- cgit v1.2.3