From 49dbe760e4e0d8a781b5336efdce4748a7d73a33 Mon Sep 17 00:00:00 2001 From: Nicolas Guichard Date: Fri, 14 Feb 2020 12:50:00 +0100 Subject: [PATCH] Fix QShaderGenerator crashing when a node port name prefixed another one QShaderGenerator didn't handle substitutions like `vec4 $color = mix($color1, $color2, $fac);` Note that `$color` is a prefix to `$color1` and `$color2`. For the substitution `QByteArray::replace` was used so if `$color` was handled first and replaced by `v1`, `$color1` and `$color2` were never correctly replaced and instead became `v11` and `v12` which caused a crash later on. Change-Id: Idaf800fdac468f33c323eb722701da5f8eb918d6 Reviewed-by: Paul Lemire --- src/gui/util/qshadergenerator.cpp | 42 ++++++++++- .../qshadergenerator/tst_qshadergenerator.cpp | 70 +++++++++++++++++++ 2 files changed, 109 insertions(+), 3 deletions(-) diff --git a/src/gui/util/qshadergenerator.cpp b/src/gui/util/qshadergenerator.cpp index bcb985de54..7ea42a7c9a 100644 --- a/src/gui/util/qshadergenerator.cpp +++ b/src/gui/util/qshadergenerator.cpp @@ -42,6 +42,8 @@ #include "qshaderlanguage_p.h" #include +#include + QT_BEGIN_NAMESPACE Q_LOGGING_CATEGORY(ShaderGenerator, "ShaderGenerator", QtWarningMsg) @@ -457,6 +459,13 @@ QByteArray QShaderGenerator::createShaderCode(const QStringList &enabledLayers) QByteArray line = node.rule(format).substitution; const QVector ports = node.ports(); + struct VariableReplacement { + QByteArray placeholder; + QByteArray variable; + }; + + QVector variableReplacements; + // Generate temporary variable names vN for (const QShaderNodePort &port : ports) { const QString portName = port.name; @@ -472,10 +481,37 @@ QByteArray QShaderGenerator::createShaderCode(const QStringList &enabledLayers) if (variableIndex < 0) continue; - const auto placeholder = QByteArray(QByteArrayLiteral("$") + portName.toUtf8()); - const auto variable = QByteArray(QByteArrayLiteral("v") + QByteArray::number(variableIndex)); + VariableReplacement replacement; + replacement.placeholder = QByteArrayLiteral("$") + portName.toUtf8(); + replacement.variable = QByteArrayLiteral("v") + QByteArray::number(variableIndex); - line.replace(placeholder, variable); + variableReplacements.append(std::move(replacement)); + } + + int begin = 0; + while ((begin = line.indexOf('$', begin)) != -1) { + int end = begin + 1; + char endChar = line.at(end); + const int size = line.size(); + while (end < size && (std::isalnum(endChar) || endChar == '_')) { + ++end; + endChar = line.at(end); + } + + const int placeholderLength = end - begin; + + const QByteArray variableName = line.mid(begin, placeholderLength); + const auto replacementIt = std::find_if(variableReplacements.cbegin(), variableReplacements.cend(), + [&variableName](const VariableReplacement &replacement) { + return variableName == replacement.placeholder; + }); + + if (replacementIt != variableReplacements.cend()) { + line.replace(begin, placeholderLength, replacementIt->variable); + begin += replacementIt->variable.length(); + } else { + begin = end; + } } // Substitute variable names by generated vN variable names diff --git a/tests/auto/gui/util/qshadergenerator/tst_qshadergenerator.cpp b/tests/auto/gui/util/qshadergenerator/tst_qshadergenerator.cpp index 59e93d127f..2b9b08a18a 100644 --- a/tests/auto/gui/util/qshadergenerator/tst_qshadergenerator.cpp +++ b/tests/auto/gui/util/qshadergenerator/tst_qshadergenerator.cpp @@ -198,6 +198,7 @@ private slots: void shouldGenerateDifferentCodeDependingOnActiveLayers(); void shouldUseGlobalVariableRatherThanTemporaries(); void shouldGenerateTemporariesWisely(); + void shouldHandlePortNamesPrefixingOneAnother(); }; void tst_QShaderGenerator::shouldHaveDefaultState() @@ -1229,6 +1230,75 @@ void tst_QShaderGenerator::shouldGenerateTemporariesWisely() } } +void tst_QShaderGenerator::shouldHandlePortNamesPrefixingOneAnother() +{ + // GIVEN + const auto gl4 = createFormat(QShaderFormat::OpenGLCoreProfile, 4, 0); + + auto color1 = createNode({ + createPort(QShaderNodePort::Output, "output") + }); + color1.addRule(gl4, QShaderNode::Rule("vec4 $output = color1;", + QByteArrayList() << "in vec4 color1;")); + + auto color2 = createNode({ + createPort(QShaderNodePort::Output, "output") + }); + color2.addRule(gl4, QShaderNode::Rule("vec4 $output = color2;", + QByteArrayList() << "in vec4 color2;")); + + auto addColor = createNode({ + createPort(QShaderNodePort::Output, "color"), + createPort(QShaderNodePort::Input, "color1"), + createPort(QShaderNodePort::Input, "color2"), + }); + addColor.addRule(gl4, QShaderNode::Rule("vec4 $color = $color1 + $color2;")); + + auto shaderOutput = createNode({ + createPort(QShaderNodePort::Input, "input") + }); + + shaderOutput.addRule(gl4, QShaderNode::Rule("shaderOutput = $input;", + QByteArrayList() << "out vec4 shaderOutput;")); + + // WHEN + const auto graph = [=] { + auto res = QShaderGraph(); + + res.addNode(color1); + res.addNode(color2); + res.addNode(addColor); + res.addNode(shaderOutput); + + res.addEdge(createEdge(color1.uuid(), "output", addColor.uuid(), "color1")); + res.addEdge(createEdge(color2.uuid(), "output", addColor.uuid(), "color2")); + res.addEdge(createEdge(addColor.uuid(), "color", shaderOutput.uuid(), "input")); + + return res; + }(); + + auto generator = QShaderGenerator(); + generator.graph = graph; + generator.format = gl4; + + const auto code = generator.createShaderCode(); + + // THEN + const auto expected = QByteArrayList() + << "#version 400 core" + << "" + << "in vec4 color1;" + << "in vec4 color2;" + << "out vec4 shaderOutput;" + << "" + << "void main()" + << "{" + << " shaderOutput = ((color1 + color2));" + << "}" + << ""; + QCOMPARE(code, expected.join("\n")); +} + QTEST_MAIN(tst_QShaderGenerator) #include "tst_qshadergenerator.moc"