Skip to content

Commit b27b72c

Browse files
committed
Refactor shader program cache
When multiple targets stamp, one of them might use the wrong shader program (with uniforms set by another target).
1 parent db269be commit b27b72c

File tree

6 files changed

+58
-23
lines changed

6 files changed

+58
-23
lines changed

src/renderedtarget.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,7 @@ void RenderedTarget::render(double scale) const
546546
ShaderManager *shaderManager = ShaderManager::instance();
547547

548548
if (!m_shaderProgram) {
549-
m_shaderProgram = shaderManager->getShaderProgram(m_graphicEffects);
549+
m_shaderProgram = shaderManager->getShaderProgram(this, m_graphicEffects);
550550
Q_ASSERT(m_shaderProgram);
551551
Q_ASSERT(m_shaderProgram->isLinked());
552552

src/shadermanager.cpp

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ ShaderManager *ShaderManager::instance()
9797
return globalInstance;
9898
}
9999

100-
QOpenGLShaderProgram *ShaderManager::getShaderProgram(const std::unordered_map<Effect, double> &effectValues)
100+
QOpenGLShaderProgram *ShaderManager::getShaderProgram(const IRenderedTarget *target, const std::unordered_map<Effect, double> &effectValues)
101101
{
102102
int effectBits = 0;
103103
bool firstSet = false;
@@ -115,11 +115,24 @@ QOpenGLShaderProgram *ShaderManager::getShaderProgram(const std::unordered_map<E
115115
QOpenGLShaderProgram *program = createShaderProgram(effectValues);
116116

117117
if (program)
118-
m_shaderPrograms[effectBits] = program;
118+
m_shaderPrograms[effectBits] = { { target, program } };
119119

120120
return program;
121-
} else
122-
return it->second;
121+
} else {
122+
const auto &map = it->second;
123+
auto it = map.find(target);
124+
125+
if (it == map.cend()) {
126+
// Create a new shader program if this combination doesn't exist for the given target
127+
QOpenGLShaderProgram *program = createShaderProgram(effectValues);
128+
129+
if (program)
130+
m_shaderPrograms[effectBits][target] = program;
131+
132+
return program;
133+
} else
134+
return it->second;
135+
}
123136
}
124137

125138
void ShaderManager::getUniformValuesForEffects(const std::unordered_map<Effect, double> &effectValues, std::unordered_map<Effect, float> &dst)

src/shadermanager.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ class QOpenGLShader;
1212
namespace scratchcpprender
1313
{
1414

15+
class IRenderedTarget;
16+
1517
class ShaderManager : public QObject
1618
{
1719
public:
@@ -31,7 +33,7 @@ class ShaderManager : public QObject
3133

3234
static ShaderManager *instance();
3335

34-
QOpenGLShaderProgram *getShaderProgram(const std::unordered_map<Effect, double> &effectValues);
36+
QOpenGLShaderProgram *getShaderProgram(const IRenderedTarget *target, const std::unordered_map<Effect, double> &effectValues);
3537
static void getUniformValuesForEffects(const std::unordered_map<Effect, double> &effectValues, std::unordered_map<Effect, float> &dst);
3638
static void setUniforms(QOpenGLShaderProgram *program, int textureUnit, const QSize skinSize, const std::unordered_map<Effect, double> &effectValues);
3739

@@ -52,7 +54,7 @@ class ShaderManager : public QObject
5254
static std::unordered_set<Effect> m_effects;
5355

5456
QOpenGLShader *m_vertexShader = nullptr;
55-
std::unordered_map<int, QOpenGLShaderProgram *> m_shaderPrograms;
57+
std::unordered_map<int, std::unordered_map<const IRenderedTarget *, QOpenGLShaderProgram *>> m_shaderPrograms;
5658
QByteArray m_fragmentShaderSource;
5759
};
5860

src/targetpainter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ void TargetPainter::paint(QNanoPainter *painter)
6262
ShaderManager *shaderManager = ShaderManager::instance();
6363

6464
const auto &effects = m_target->graphicEffects();
65-
QOpenGLShaderProgram *shaderProgram = shaderManager->getShaderProgram(effects);
65+
QOpenGLShaderProgram *shaderProgram = shaderManager->getShaderProgram(m_target, effects);
6666
Q_ASSERT(shaderProgram);
6767
Q_ASSERT(shaderProgram->isLinked());
6868

test/shadermanager/CMakeLists.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,11 @@ add_executable(
66
target_link_libraries(
77
shadermanager_test
88
GTest::gtest_main
9+
GTest::gmock_main
910
scratchcpp
1011
scratchcpp-render
12+
scratchcpprender_mocks
13+
qnanopainter
1114
${QT_LIBS}
1215
)
1316

test/shadermanager/shadermanager_test.cpp

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <scratchcpp/scratchconfiguration.h>
77
#include <shadermanager.h>
88
#include <graphicseffect.h>
9+
#include <renderedtargetmock.h>
910

1011
#include "../common.h"
1112

@@ -34,6 +35,7 @@ class ShaderManagerTest : public testing::Test
3435

3536
QOpenGLContext m_context;
3637
QOffscreenSurface m_surface;
38+
RenderedTargetMock m_target;
3739
};
3840

3941
TEST_F(ShaderManagerTest, RegisteredEffects)
@@ -71,7 +73,7 @@ TEST_F(ShaderManagerTest, GetShaderProgram)
7173
ShaderManager manager;
7274
const std::unordered_map<ShaderManager::Effect, double> effects = { { ShaderManager::Effect::Color, 64.9 }, { ShaderManager::Effect::Ghost, 12.5 } };
7375

74-
QOpenGLShaderProgram *program = manager.getShaderProgram(effects);
76+
QOpenGLShaderProgram *program = manager.getShaderProgram(&m_target, effects);
7577
ASSERT_EQ(program->parent(), &manager);
7678
ASSERT_TRUE(program->isLinked());
7779

@@ -83,21 +85,36 @@ TEST_F(ShaderManagerTest, GetShaderProgram)
8385
ASSERT_EQ(frag->shaderType(), QOpenGLShader::Fragment);
8486

8587
// Test shader program cache
86-
program = manager.getShaderProgram(effects);
88+
program = manager.getShaderProgram(&m_target, effects);
8789
ASSERT_EQ(program, program);
8890

89-
program = manager.getShaderProgram(effects);
91+
program = manager.getShaderProgram(&m_target, effects);
9092
ASSERT_EQ(program, program);
9193
}
9294

95+
TEST_F(ShaderManagerTest, GetShaderProgram_AnotherTarget)
96+
{
97+
ShaderManager manager;
98+
const std::unordered_map<ShaderManager::Effect, double> effects = { { ShaderManager::Effect::Color, 64.9 }, { ShaderManager::Effect::Ghost, 12.5 } };
99+
100+
QOpenGLShaderProgram *program = manager.getShaderProgram(&m_target, effects);
101+
102+
RenderedTargetMock anotherTarget;
103+
QOpenGLShaderProgram *anotherProgram = manager.getShaderProgram(&anotherTarget, effects);
104+
ASSERT_NE(anotherProgram, nullptr);
105+
ASSERT_NE(anotherProgram, program);
106+
107+
ASSERT_EQ(manager.getShaderProgram(&anotherTarget, effects), anotherProgram);
108+
}
109+
93110
TEST_F(ShaderManagerTest, SetUniforms)
94111
{
95112
QOpenGLFunctions glF(&m_context);
96113
glF.initializeOpenGLFunctions();
97114
ShaderManager manager;
98115

99116
std::unordered_map<ShaderManager::Effect, double> effects = { { ShaderManager::Effect::Color, 64.9 }, { ShaderManager::Effect::Ghost, 12.5 } };
100-
QOpenGLShaderProgram *program = manager.getShaderProgram(effects);
117+
QOpenGLShaderProgram *program = manager.getShaderProgram(&m_target, effects);
101118
program->bind();
102119
manager.setUniforms(program, 4, QSize(), effects);
103120

@@ -130,7 +147,7 @@ TEST_F(ShaderManagerTest, ColorEffectValue)
130147

131148
// In range
132149
std::unordered_map<ShaderManager::Effect, double> effects = { { effect, 64.9 } };
133-
QOpenGLShaderProgram *program = manager.getShaderProgram(effects);
150+
QOpenGLShaderProgram *program = manager.getShaderProgram(&m_target, effects);
134151
program->bind();
135152
manager.setUniforms(program, 0, QSize(), effects);
136153
manager.getUniformValuesForEffects(effects, values);
@@ -142,7 +159,7 @@ TEST_F(ShaderManagerTest, ColorEffectValue)
142159

143160
// Below the minimum
144161
effects[effect] = -395.7;
145-
program = manager.getShaderProgram(effects);
162+
program = manager.getShaderProgram(&m_target, effects);
146163
program->bind();
147164
manager.setUniforms(program, 0, QSize(), effects);
148165
manager.getUniformValuesForEffects(effects, values);
@@ -154,7 +171,7 @@ TEST_F(ShaderManagerTest, ColorEffectValue)
154171

155172
// Above the maximum
156173
effects[effect] = 579.05;
157-
program = manager.getShaderProgram(effects);
174+
program = manager.getShaderProgram(&m_target, effects);
158175
program->bind();
159176
manager.setUniforms(program, 0, QSize(), effects);
160177
manager.getUniformValuesForEffects(effects, values);
@@ -181,7 +198,7 @@ TEST_F(ShaderManagerTest, BrightnessEffectValue)
181198

182199
// In range
183200
std::unordered_map<ShaderManager::Effect, double> effects = { { effect, 4.6 } };
184-
QOpenGLShaderProgram *program = manager.getShaderProgram(effects);
201+
QOpenGLShaderProgram *program = manager.getShaderProgram(&m_target, effects);
185202
program->bind();
186203
manager.setUniforms(program, 0, QSize(), effects);
187204
manager.getUniformValuesForEffects(effects, values);
@@ -193,7 +210,7 @@ TEST_F(ShaderManagerTest, BrightnessEffectValue)
193210

194211
// Below the minimum
195212
effects[effect] = -102.9;
196-
program = manager.getShaderProgram(effects);
213+
program = manager.getShaderProgram(&m_target, effects);
197214
program->bind();
198215
manager.setUniforms(program, 0, QSize(), effects);
199216
manager.getUniformValuesForEffects(effects, values);
@@ -205,7 +222,7 @@ TEST_F(ShaderManagerTest, BrightnessEffectValue)
205222

206223
// Above the maximum
207224
effects[effect] = 353.2;
208-
program = manager.getShaderProgram(effects);
225+
program = manager.getShaderProgram(&m_target, effects);
209226
program->bind();
210227
manager.setUniforms(program, 0, QSize(), effects);
211228
manager.getUniformValuesForEffects(effects, values);
@@ -232,7 +249,7 @@ TEST_F(ShaderManagerTest, GhostEffectValue)
232249

233250
// In range
234251
std::unordered_map<ShaderManager::Effect, double> effects = { { effect, 58.5 } };
235-
QOpenGLShaderProgram *program = manager.getShaderProgram(effects);
252+
QOpenGLShaderProgram *program = manager.getShaderProgram(&m_target, effects);
236253
program->bind();
237254
manager.setUniforms(program, 0, QSize(), effects);
238255
manager.getUniformValuesForEffects(effects, values);
@@ -281,7 +298,7 @@ TEST_F(ShaderManagerTest, FisheyeEffectValue)
281298

282299
// In range
283300
std::unordered_map<ShaderManager::Effect, double> effects = { { effect, 58.5 } };
284-
QOpenGLShaderProgram *program = manager.getShaderProgram(effects);
301+
QOpenGLShaderProgram *program = manager.getShaderProgram(&m_target, effects);
285302
program->bind();
286303
manager.setUniforms(program, 0, QSize(), effects);
287304
manager.getUniformValuesForEffects(effects, values);
@@ -329,7 +346,7 @@ TEST_F(ShaderManagerTest, WhirlEffectValue)
329346

330347
// In range
331348
std::unordered_map<ShaderManager::Effect, double> effects = { { effect, 58.5 } };
332-
QOpenGLShaderProgram *program = manager.getShaderProgram(effects);
349+
QOpenGLShaderProgram *program = manager.getShaderProgram(&m_target, effects);
333350
program->bind();
334351
manager.setUniforms(program, 0, QSize(), effects);
335352
manager.getUniformValuesForEffects(effects, values);
@@ -366,7 +383,7 @@ TEST_F(ShaderManagerTest, PixelateEffectValue)
366383

367384
// In range
368385
std::unordered_map<ShaderManager::Effect, double> effects = { { effect, 58.5 } };
369-
QOpenGLShaderProgram *program = manager.getShaderProgram(effects);
386+
QOpenGLShaderProgram *program = manager.getShaderProgram(&m_target, effects);
370387
program->bind();
371388
manager.setUniforms(program, 0, QSize(), effects);
372389
manager.getUniformValuesForEffects(effects, values);
@@ -403,7 +420,7 @@ TEST_F(ShaderManagerTest, MosaicEffectValue)
403420

404421
// In range
405422
std::unordered_map<ShaderManager::Effect, double> effects = { { effect, 58.5 } };
406-
QOpenGLShaderProgram *program = manager.getShaderProgram(effects);
423+
QOpenGLShaderProgram *program = manager.getShaderProgram(&m_target, effects);
407424
program->bind();
408425
manager.setUniforms(program, 0, QSize(), effects);
409426
manager.getUniformValuesForEffects(effects, values);

0 commit comments

Comments
 (0)