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 <paul.lemire@kdab.com>
This commit is contained in:
Nicolas Guichard 2020-02-14 12:50:00 +01:00
parent 045a143c2c
commit 49dbe760e4
2 changed files with 109 additions and 3 deletions

View File

@ -42,6 +42,8 @@
#include "qshaderlanguage_p.h" #include "qshaderlanguage_p.h"
#include <QRegularExpression> #include <QRegularExpression>
#include <cctype>
QT_BEGIN_NAMESPACE QT_BEGIN_NAMESPACE
Q_LOGGING_CATEGORY(ShaderGenerator, "ShaderGenerator", QtWarningMsg) Q_LOGGING_CATEGORY(ShaderGenerator, "ShaderGenerator", QtWarningMsg)
@ -457,6 +459,13 @@ QByteArray QShaderGenerator::createShaderCode(const QStringList &enabledLayers)
QByteArray line = node.rule(format).substitution; QByteArray line = node.rule(format).substitution;
const QVector<QShaderNodePort> ports = node.ports(); const QVector<QShaderNodePort> ports = node.ports();
struct VariableReplacement {
QByteArray placeholder;
QByteArray variable;
};
QVector<VariableReplacement> variableReplacements;
// Generate temporary variable names vN // Generate temporary variable names vN
for (const QShaderNodePort &port : ports) { for (const QShaderNodePort &port : ports) {
const QString portName = port.name; const QString portName = port.name;
@ -472,10 +481,37 @@ QByteArray QShaderGenerator::createShaderCode(const QStringList &enabledLayers)
if (variableIndex < 0) if (variableIndex < 0)
continue; continue;
const auto placeholder = QByteArray(QByteArrayLiteral("$") + portName.toUtf8()); VariableReplacement replacement;
const auto variable = QByteArray(QByteArrayLiteral("v") + QByteArray::number(variableIndex)); 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 // Substitute variable names by generated vN variable names

View File

@ -198,6 +198,7 @@ private slots:
void shouldGenerateDifferentCodeDependingOnActiveLayers(); void shouldGenerateDifferentCodeDependingOnActiveLayers();
void shouldUseGlobalVariableRatherThanTemporaries(); void shouldUseGlobalVariableRatherThanTemporaries();
void shouldGenerateTemporariesWisely(); void shouldGenerateTemporariesWisely();
void shouldHandlePortNamesPrefixingOneAnother();
}; };
void tst_QShaderGenerator::shouldHaveDefaultState() 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) QTEST_MAIN(tst_QShaderGenerator)
#include "tst_qshadergenerator.moc" #include "tst_qshadergenerator.moc"