Skip to content

Commit 97cae93

Browse files
committed
Refactor runRenderer to use structured types
Replace the 10-parameter runRenderer signature with RenderSession, RenderItem, and RenderProfileResult value types. Each call now returns isolated profiling data that the caller accumulates. Test run is now decoupled, so in future, we would be able to introduce some parallelism in these tests if needed. Minor fixes: - correct ScopedTimer scoping in validate(), - fix a copy-paste timer name in the Slang renderer.
1 parent c1ea70b commit 97cae93

7 files changed

Lines changed: 239 additions & 209 deletions

File tree

source/MaterialXTest/MaterialXRender/RenderUtil.cpp

Lines changed: 16 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -57,23 +57,23 @@ bool ShaderRenderTester::validate(const mx::FilePath optionsFilePath)
5757
// Data search path
5858
runState.searchPath = mx::getDefaultDataSearchPath();
5959

60-
mx::ScopedTimer ioTimer(&profiler.times().ioTime);
61-
mx::FilePathVec files = collectTestFiles(runState);
62-
ioTimer.endTimer();
60+
mx::FilePathVec files;
61+
{
62+
mx::ScopedTimer t(&profiler.times().ioTime);
63+
files = collectTestFiles(runState);
64+
}
6365

6466
// Load in the library dependencies once.
6567
// This will be imported in each test document below.
66-
ioTimer.startTimer();
67-
loadDependentLibraries(runState);
68-
ioTimer.endTimer();
68+
{
69+
mx::ScopedTimer t(&profiler.times().ioTime);
70+
loadDependentLibraries(runState);
71+
}
6972

7073
// Create renderers and generators
7174
initializeGeneratorContext(runState, logger, profiler);
7275

73-
// Map to replace "/" in Element path and ":" in namespaced names with "_".
74-
mx::StringMap pathMap;
75-
pathMap["/"] = "_";
76-
pathMap[":"] = "_";
76+
RenderSession session { runState.options, logger.renderLog() };
7777

7878
for (const mx::FilePath& filename : files)
7979
{
@@ -84,9 +84,10 @@ bool ShaderRenderTester::validate(const mx::FilePath optionsFilePath)
8484

8585
for (const auto& element : docInfo.elements)
8686
{
87-
mx::string elementName = mx::createValidName(mx::replaceSubstrings(element->getNamePath(), pathMap));
88-
runRenderer(elementName, element, *runState.context, docInfo.doc, logger.renderLog(), runState.options,
89-
profiler.times(), docInfo.imageSearchPath, docInfo.outputPath, nullptr);
87+
RenderItem item { element, docInfo.imageSearchPath, docInfo.outputPath, nullptr };
88+
auto result = runRenderer(session, item, *runState.context);
89+
profiler.times().languageTimes.accumulate(result.languageTimes);
90+
profiler.times().elementsTested += result.elementsTested;
9091
}
9192
}
9293

@@ -486,11 +487,10 @@ DocumentInfo ShaderRenderTester::loadAndValidateDocument(const mx::FilePath& fil
486487
{
487488
DocumentInfo info;
488489

489-
mx::ScopedTimer ioTimer(&profiler.times().ioTime);
490-
491490
if (_skipFiles.count(filename.getBaseName()) > 0)
492491
return info;
493492

493+
mx::ScopedTimer ioTimer(&profiler.times().ioTime);
494494
info.doc = mx::createDocument();
495495
try
496496
{
@@ -589,13 +589,6 @@ void TestRunLogger::start(const std::string& target, const GenShaderUtil::TestSu
589589
#endif
590590
}
591591

592-
TestRunLogger::~TestRunLogger()
593-
{
594-
_renderLog.reset();
595-
_validationLog.reset();
596-
_profilingLog.reset();
597-
}
598-
599592
// ---------------------------------------------------------------------------
600593
// TestRunProfiler
601594
// ---------------------------------------------------------------------------
@@ -646,10 +639,7 @@ void TestRunTracer::start(const std::string& target, const GenShaderUtil::TestSu
646639
}
647640
}
648641

649-
TestRunTracer::~TestRunTracer()
650-
{
651-
_state.reset();
652-
}
642+
TestRunTracer::~TestRunTracer() = default;
653643

654644
#endif // MATERIALX_BUILD_PERFETTO_TRACING
655645

source/MaterialXTest/MaterialXRender/RenderUtil.h

Lines changed: 74 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,19 @@ class LanguageProfileTimes
5353
output << "\tI/O: " << ioTime << " seconds" << std::endl;
5454
output << "\tImage save: " << imageSaveTime << " seconds" << std::endl;
5555
}
56+
57+
void accumulate(const LanguageProfileTimes& other)
58+
{
59+
totalTime += other.totalTime;
60+
setupTime += other.setupTime;
61+
transparencyTime += other.transparencyTime;
62+
generationTime += other.generationTime;
63+
compileTime += other.compileTime;
64+
renderTime += other.renderTime;
65+
ioTime += other.ioTime;
66+
imageSaveTime += other.imageSaveTime;
67+
}
68+
5669
double totalTime = 0.0;
5770
double setupTime = 0.0;
5871
double transparencyTime = 0.0;
@@ -104,8 +117,6 @@ struct DocumentInfo
104117
class TestRunLogger
105118
{
106119
public:
107-
~TestRunLogger();
108-
109120
void start(const std::string& target, const GenShaderUtil::TestSuiteOptions& options);
110121

111122
std::ostream& renderLog() { return _renderLog ? *_renderLog : std::cout; }
@@ -124,7 +135,7 @@ class TestRunLogger
124135
class TestRunProfiler
125136
{
126137
public:
127-
~TestRunProfiler() { _totalTimer.reset(); }
138+
~TestRunProfiler() = default;
128139

129140
void start();
130141
void printSummary(const GenShaderUtil::TestSuiteOptions& options,
@@ -161,6 +172,59 @@ struct TestRunState
161172
std::unique_ptr<mx::GenContext> context;
162173
};
163174

175+
// Read-only test configuration for the entire validate() run.
176+
// Note: the log stream requires synchronization for concurrent use.
177+
struct RenderSession
178+
{
179+
const GenShaderUtil::TestSuiteOptions& testOptions;
180+
std::ostream& log;
181+
};
182+
183+
// Per-element data passed to each runRenderer call.
184+
struct RenderItem
185+
{
186+
RenderItem(mx::TypedElementPtr elem,
187+
mx::FileSearchPath searchPath,
188+
mx::FilePath outPath,
189+
mx::ImageVec* images = nullptr)
190+
: element(std::move(elem)),
191+
imageSearchPath(std::move(searchPath)),
192+
outputPath(std::move(outPath)),
193+
imageVec(images) {}
194+
195+
mx::TypedElementPtr element;
196+
mx::FileSearchPath imageSearchPath;
197+
mx::FilePath outputPath;
198+
mx::ImageVec* imageVec = nullptr;
199+
200+
const std::string& shaderName() const
201+
{
202+
if (_cachedShaderName.empty())
203+
{
204+
mx::StringMap pathMap;
205+
pathMap["/"] = "_";
206+
pathMap[":"] = "_";
207+
_cachedShaderName = mx::createValidName(
208+
mx::replaceSubstrings(element->getNamePath(), pathMap));
209+
}
210+
return _cachedShaderName;
211+
}
212+
213+
mx::DocumentPtr doc() const { return element->getDocument(); }
214+
215+
private:
216+
mutable std::string _cachedShaderName;
217+
};
218+
219+
// Returned by runRenderer — each call produces its own isolated profiling data.
220+
// The caller accumulates results, making future parallelism straightforward.
221+
struct RenderProfileResult
222+
{
223+
LanguageProfileTimes languageTimes;
224+
unsigned int elementsTested = 0;
225+
bool success = true;
226+
};
227+
164228
// Base class used for performing compilation and render tests for a given
165229
// shading language and target.
166230
//
@@ -209,17 +273,13 @@ class ShaderRenderTester
209273
// Create a renderer for the generated code
210274
virtual void createRenderer(std::ostream& log) = 0;
211275

212-
// Run the renderer
213-
virtual bool runRenderer(const std::string& shaderName,
214-
mx::TypedElementPtr element,
215-
mx::GenContext& context,
216-
mx::DocumentPtr doc,
217-
std::ostream& log,
218-
const GenShaderUtil::TestSuiteOptions& testOptions,
219-
RenderProfileTimes& profileTimes,
220-
const mx::FileSearchPath& imageSearchPath,
221-
const std::string& outputPath = ".",
222-
mx::ImageVec* imageVec = nullptr) = 0;
276+
// Run the renderer.
277+
// GenContext is a separate argument because it is mutable (written per-element)
278+
// and must be cloned per-thread for future parallel execution.
279+
virtual RenderProfileResult runRenderer(
280+
const RenderSession& session,
281+
const RenderItem& item,
282+
mx::GenContext& context) = 0;
223283

224284
// Save an image
225285
virtual bool saveImage(const mx::FilePath&, mx::ConstImagePtr, bool) const { return false; };

source/MaterialXTest/MaterialXRenderGlsl/RenderGlsl.cpp

Lines changed: 32 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,10 @@ class GlslShaderRenderTester : public RenderUtil::ShaderRenderTester
4141

4242
void createRenderer(std::ostream& log) override;
4343

44-
bool runRenderer(const std::string& shaderName,
45-
mx::TypedElementPtr element,
46-
mx::GenContext& context,
47-
mx::DocumentPtr doc,
48-
std::ostream& log,
49-
const GenShaderUtil::TestSuiteOptions& testOptions,
50-
RenderUtil::RenderProfileTimes& profileTimes,
51-
const mx::FileSearchPath& imageSearchPath,
52-
const std::string& outputPath = ".",
53-
mx::ImageVec* imageVec = nullptr) override;
44+
RenderUtil::RenderProfileResult runRenderer(
45+
const RenderUtil::RenderSession& session,
46+
const RenderUtil::RenderItem& item,
47+
mx::GenContext& context) override;
5448

5549
bool saveImage(const mx::FilePath& filePath, mx::ConstImagePtr image, bool verticalFlip) const override;
5650

@@ -151,22 +145,23 @@ bool GlslShaderRenderTester::saveImage(const mx::FilePath& filePath, mx::ConstIm
151145
return _renderer->getImageHandler()->saveImage(filePath, image, verticalFlip);
152146
}
153147

154-
bool GlslShaderRenderTester::runRenderer(const std::string& shaderName,
155-
mx::TypedElementPtr element,
156-
mx::GenContext& context,
157-
mx::DocumentPtr doc,
158-
std::ostream& log,
159-
const GenShaderUtil::TestSuiteOptions& testOptions,
160-
RenderUtil::RenderProfileTimes& profileTimes,
161-
const mx::FileSearchPath& imageSearchPath,
162-
const std::string& outputPath,
163-
mx::ImageVec* imageVec)
148+
RenderUtil::RenderProfileResult GlslShaderRenderTester::runRenderer(
149+
const RenderUtil::RenderSession& session,
150+
const RenderUtil::RenderItem& item,
151+
mx::GenContext& context)
164152
{
153+
RenderUtil::RenderProfileResult result;
154+
const std::string& shaderName = item.shaderName();
155+
mx::DocumentPtr doc = item.doc();
156+
mx::TypedElementPtr element = item.element;
157+
const GenShaderUtil::TestSuiteOptions& testOptions = session.testOptions;
158+
std::ostream& log = session.log;
159+
165160
MX_TRACE_FUNCTION(mx::Tracing::Category::Render);
166161
MX_TRACE_SCOPE(mx::Tracing::Category::Material, shaderName.c_str());
167162
std::cout << "Validating GLSL rendering for: " << doc->getSourceUri() << std::endl;
168163

169-
mx::ScopedTimer totalGLSLTime(&profileTimes.languageTimes.totalTime);
164+
mx::ScopedTimer totalGLSLTime(&result.languageTimes.totalTime);
170165

171166
const mx::ShaderGenerator& shadergen = context.getShaderGenerator();
172167
mx::FileSearchPath searchPath = mx::getDefaultDataSearchPath();
@@ -180,13 +175,13 @@ bool GlslShaderRenderTester::runRenderer(const std::string& shaderName,
180175

181176
for (auto options : optionsList)
182177
{
183-
profileTimes.elementsTested++;
178+
result.elementsTested++;
184179

185-
mx::FilePath outputFilePath = outputPath;
180+
mx::FilePath outputFilePath = item.outputPath;
186181

187182
// Note: mkdir will fail if the directory already exists which is ok.
188183
{
189-
mx::ScopedTimer ioDir(&profileTimes.languageTimes.ioTime);
184+
mx::ScopedTimer ioDir(&result.languageTimes.ioTime);
190185
outputFilePath.createDirectory(true);
191186

192187
// Use separate directory for reduced output
@@ -201,11 +196,11 @@ bool GlslShaderRenderTester::runRenderer(const std::string& shaderName,
201196
mx::ShaderPtr shader;
202197
try
203198
{
204-
mx::ScopedTimer transpTimer(&profileTimes.languageTimes.transparencyTime);
199+
mx::ScopedTimer transpTimer(&result.languageTimes.transparencyTime);
205200
options.hwTransparency = mx::isTransparentSurface(element, shadergen.getTarget());
206201
transpTimer.endTimer();
207202

208-
mx::ScopedTimer generationTimer(&profileTimes.languageTimes.generationTime);
203+
mx::ScopedTimer generationTimer(&result.languageTimes.generationTime);
209204
MX_TRACE_SCOPE(mx::Tracing::Category::ShaderGen, "GenerateShader");
210205
mx::GenOptions& contextOptions = context.getOptions();
211206
contextOptions = options;
@@ -223,7 +218,8 @@ bool GlslShaderRenderTester::runRenderer(const std::string& shaderName,
223218
if (shader == nullptr)
224219
{
225220
log << ">> Failed to generate shader\n";
226-
return false;
221+
result.success = false;
222+
return result;
227223
}
228224
const std::string& vertexSourceCode = shader->getSourceCode(mx::Stage::VERTEX);
229225
const std::string& pixelSourceCode = shader->getSourceCode(mx::Stage::PIXEL);
@@ -233,7 +229,7 @@ bool GlslShaderRenderTester::runRenderer(const std::string& shaderName,
233229
if (testOptions.dumpGeneratedCode)
234230
{
235231
MX_TRACE_SCOPE(mx::Tracing::Category::Render, "DumpGeneratedCode");
236-
mx::ScopedTimer dumpTimer(&profileTimes.languageTimes.ioTime);
232+
mx::ScopedTimer dumpTimer(&result.languageTimes.ioTime);
237233
std::ofstream file;
238234
file.open(shaderPath + "_vs.glsl");
239235
file << vertexSourceCode;
@@ -285,15 +281,15 @@ bool GlslShaderRenderTester::runRenderer(const std::string& shaderName,
285281

286282
{
287283
MX_TRACE_SCOPE(mx::Tracing::Category::Render, "CompileShader");
288-
mx::ScopedTimer compileTimer(&profileTimes.languageTimes.compileTime);
284+
mx::ScopedTimer compileTimer(&result.languageTimes.compileTime);
289285
_renderer->createProgram(shader);
290286
_renderer->validateInputs();
291287
}
292288

293289
if (testOptions.dumpUniformsAndAttributes)
294290
{
295291
MX_TRACE_SCOPE(mx::Tracing::Category::Render, "DumpUniformsAndAttributes");
296-
mx::ScopedTimer printTimer(&profileTimes.languageTimes.ioTime);
292+
mx::ScopedTimer printTimer(&result.languageTimes.ioTime);
297293
log << "* Uniform:" << std::endl;
298294
program->printUniforms(log);
299295
log << "* Attributes:" << std::endl;
@@ -353,8 +349,8 @@ bool GlslShaderRenderTester::runRenderer(const std::string& shaderName,
353349

354350
{
355351
MX_TRACE_SCOPE(mx::Tracing::Category::Render, "RenderMaterial");
356-
mx::ScopedTimer renderTimer(&profileTimes.languageTimes.renderTime);
357-
_renderer->getImageHandler()->setSearchPath(imageSearchPath);
352+
mx::ScopedTimer renderTimer(&result.languageTimes.renderTime);
353+
_renderer->getImageHandler()->setSearchPath(item.imageSearchPath);
358354
unsigned int width = (unsigned int) testOptions.renderSize[0] * supersampleFactor;
359355
unsigned int height = (unsigned int) testOptions.renderSize[1] * supersampleFactor;
360356
_renderer->setSize(width, height);
@@ -364,7 +360,7 @@ bool GlslShaderRenderTester::runRenderer(const std::string& shaderName,
364360

365361
{
366362
MX_TRACE_SCOPE(mx::Tracing::Category::Render, "CaptureAndSaveImage");
367-
mx::ScopedTimer ioTimer(&profileTimes.languageTimes.imageSaveTime);
363+
mx::ScopedTimer ioTimer(&result.languageTimes.imageSaveTime);
368364
std::string fileName = shaderPath + "_glsl.png";
369365
mx::ImagePtr image = _renderer->captureImage();
370366
if (image)
@@ -374,9 +370,9 @@ bool GlslShaderRenderTester::runRenderer(const std::string& shaderName,
374370
image = image->applyBoxDownsample(supersampleFactor);
375371
}
376372
_renderer->getImageHandler()->saveImage(fileName, image, true);
377-
if (imageVec)
373+
if (item.imageVec)
378374
{
379-
imageVec->push_back(image);
375+
item.imageVec->push_back(image);
380376
}
381377
}
382378
}
@@ -409,7 +405,7 @@ bool GlslShaderRenderTester::runRenderer(const std::string& shaderName,
409405
CHECK(validated);
410406
}
411407
}
412-
return true;
408+
return result;
413409
}
414410

415411
TEST_CASE("Render: GLSL TestSuite", "[renderglsl]")

0 commit comments

Comments
 (0)