From 27d35a3ed0d71fd2ad5f213591172c85a767f5ca Mon Sep 17 00:00:00 2001
From: Nicolas Guichard <nicolas.guichard@kdab.com>
Date: Mon, 17 Feb 2020 13:19:45 +0100
Subject: [PATCH] QShaderGraph: Fix statement creation for graphs with dangling
 branches

For graphs like this one:

Input ----> Function1 ----> Output
      \
       ---> Function2
         (unbound output)

We would have generated only 2 statements, for Function1 and Output.

This change fixes this by treating Function2 like an output.
Therefore it generates 4 statements: Input, Function1, Output and
Function2.

Change-Id: Iaada40b9b949d771806dd47efad4f7ef2a775b48
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>
---
 src/gui/util/qshadergraph.cpp                 | 17 ++++---
 .../util/qshadergraph/tst_qshadergraph.cpp    | 45 +++++++++++++++++++
 2 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/src/gui/util/qshadergraph.cpp b/src/gui/util/qshadergraph.cpp
index b05b710713..46fe6ac6d6 100644
--- a/src/gui/util/qshadergraph.cpp
+++ b/src/gui/util/qshadergraph.cpp
@@ -44,13 +44,20 @@ QT_BEGIN_NAMESPACE
 
 namespace
 {
-    QVector<QShaderNode> copyOutputNodes(const QVector<QShaderNode> &nodes)
+    QVector<QShaderNode> copyOutputNodes(const QVector<QShaderNode> &nodes, const QVector<QShaderGraph::Edge> &edges)
     {
         auto res = QVector<QShaderNode>();
         std::copy_if(nodes.cbegin(), nodes.cend(),
                      std::back_inserter(res),
-                     [] (const QShaderNode &node) {
-                         return node.type() == QShaderNode::Output;
+                     [&edges] (const QShaderNode &node) {
+                         return node.type() == QShaderNode::Output ||
+                                (node.type() == QShaderNode::Function &&
+                                 !std::any_of(edges.cbegin(),
+                                              edges.cend(),
+                                              [&node] (const QShaderGraph::Edge &edge) {
+                                                  return edge.sourceNodeUuid ==
+                                                         node.uuid();
+                                              }));
                      });
         return res;
     }
@@ -210,8 +217,8 @@ QVector<QShaderGraph::Statement> QShaderGraph::createStatements(const QStringLis
 
     auto result = QVector<Statement>();
     QVector<Edge> currentEdges = enabledEdges;
-    QVector<QUuid> currentUuids = [enabledNodes] {
-        const QVector<QShaderNode> inputs = copyOutputNodes(enabledNodes);
+    QVector<QUuid> currentUuids = [enabledNodes, enabledEdges] {
+        const QVector<QShaderNode> inputs = copyOutputNodes(enabledNodes, enabledEdges);
         auto res = QVector<QUuid>();
         std::transform(inputs.cbegin(), inputs.cend(),
                        std::back_inserter(res),
diff --git a/tests/auto/gui/util/qshadergraph/tst_qshadergraph.cpp b/tests/auto/gui/util/qshadergraph/tst_qshadergraph.cpp
index ce2d38c24f..50c925a111 100644
--- a/tests/auto/gui/util/qshadergraph/tst_qshadergraph.cpp
+++ b/tests/auto/gui/util/qshadergraph/tst_qshadergraph.cpp
@@ -114,6 +114,7 @@ private slots:
     void shouldSurviveCyclesDuringGraphSerialization();
     void shouldDealWithEdgesJumpingOverLayers();
     void shouldGenerateDifferentStatementsDependingOnActiveLayers();
+    void shouldDealWithBranchesWithoutOutput();
 };
 
 void tst_QShaderGraph::shouldHaveEdgeDefaultState()
@@ -774,6 +775,50 @@ void tst_QShaderGraph::shouldGenerateDifferentStatementsDependingOnActiveLayers(
     }
 }
 
+void tst_QShaderGraph::shouldDealWithBranchesWithoutOutput()
+{
+    // GIVEN
+    const auto input = createNode({
+        createPort(QShaderNodePort::Output, "input")
+    });
+    const auto output = createNode({
+        createPort(QShaderNodePort::Input, "output")
+    });
+    const auto danglingFunction = createNode({
+        createPort(QShaderNodePort::Input, "functionInput"),
+        createPort(QShaderNodePort::Output, "unbound")
+    });
+    const auto function = createNode({
+        createPort(QShaderNodePort::Input, "functionInput"),
+        createPort(QShaderNodePort::Output, "functionOutput")
+    });
+
+    const auto graph = [=] {
+        auto res = QShaderGraph();
+        res.addNode(input);
+        res.addNode(function);
+        res.addNode(danglingFunction);
+        res.addNode(output);
+        res.addEdge(createEdge(input.uuid(), "input", function.uuid(), "functionInput"));
+        res.addEdge(createEdge(input.uuid(), "input", danglingFunction.uuid(), "functionInput"));
+        res.addEdge(createEdge(function.uuid(), "functionOutput", output.uuid(), "output"));
+        return res;
+    }();
+
+    // WHEN
+    const auto statements = graph.createStatements();
+
+    // THEN
+    // Note that no edge leads to the unbound input
+    const auto expected = QVector<QShaderGraph::Statement>()
+            << createStatement(input, {}, {0})
+            << createStatement(function, {0}, {1})
+            << createStatement(output, {1}, {})
+            << createStatement(danglingFunction, {0}, {2});
+    dumpStatementsIfNeeded(statements, expected);
+    QCOMPARE(statements, expected);
+}
+
 QTEST_MAIN(tst_QShaderGraph)
 
 #include "tst_qshadergraph.moc"