Refactor runRenderer to use structured types

source/MaterialXTest/MaterialXRender/RenderUtil.cpp

@@ -32,6 +32,26 @@ ShaderRenderTester::~ShaderRenderTester()
 {
 }
 
+RenderItem::RenderItem(mx::TypedElementPtr elem,
+                       mx::FileSearchPath searchPath,
+                       mx::FilePath outPath,
+                       mx::ImageVec* images)
+    : element(std::move(elem)),
+      imageSearchPath(std::move(searchPath)),
+      outputPath(std::move(outPath)),
+      imageVec(images)
+{
+    static const mx::StringMap pathMap = []() {
+        mx::StringMap m;
+        m["/"] = "_";
+        m[":"] = "_";
+        return m;
+    }();
+    shaderName = mx::createValidName(
+        mx::replaceSubstrings(element->getNamePath(), pathMap));
+    document = element ? element->getDocument() : nullptr;
+}
+
 bool ShaderRenderTester::validate(const mx::FilePath optionsFilePath)
 {
     // per-run state objects, logger, profiler
@@ -57,24 +77,25 @@ bool ShaderRenderTester::validate(const mx::FilePath optionsFilePath)
     // Data search path
     runState.searchPath = mx::getDefaultDataSearchPath();
 
-    mx::ScopedTimer ioTimer(&profiler.times().ioTime);
-    mx::FilePathVec files = collectTestFiles(runState);
-    ioTimer.endTimer();
+    mx::FilePathVec files;
+    {
+        mx::ScopedTimer t(&profiler.times().ioTime);
+        files = collectTestFiles(runState);
+    }
 
     // Load in the library dependencies once.
     // This will be imported in each test document below.
-    ioTimer.startTimer();
-    loadDependentLibraries(runState);
-    ioTimer.endTimer();
+    {
+        mx::ScopedTimer t(&profiler.times().ioTime);
+        loadDependentLibraries(runState);
+    }
 
     // Create renderers and generators
     initializeGeneratorContext(runState, logger, profiler);
 
-    // Map to replace "/" in Element path and ":" in namespaced names with "_".
-    mx::StringMap pathMap;
-    pathMap["/"] = "_";
-    pathMap[":"] = "_";
+    RenderSession session { runState.options, logger.renderLog() };
 
+    bool allSucceeded = true;
     for (const mx::FilePath& filename : files)
     {
         DocumentInfo docInfo = loadAndValidateDocument(filename, runState, logger, profiler);
@@ -84,17 +105,20 @@ bool ShaderRenderTester::validate(const mx::FilePath optionsFilePath)
 
         for (const auto& element : docInfo.elements)
         {
-            mx::string elementName = mx::createValidName(mx::replaceSubstrings(element->getNamePath(), pathMap));
-            runRenderer(elementName, element, *runState.context, docInfo.doc, logger.renderLog(), runState.options,
-                        profiler.times(), docInfo.imageSearchPath, docInfo.outputPath, nullptr);
+            RenderItem item { element, docInfo.imageSearchPath, docInfo.outputPath, nullptr };
+            auto result = runRenderer(session, item, *runState.context);
+            profiler.times().languageTimes.accumulate(result.languageTimes);
+            profiler.times().elementsTested += result.elementsTested;
+            if (!result.success)
+                allSucceeded = false;
         }
     }
 
     // Print profiling summary on the normal path.
     // All resource cleanup is handled by destructors.
     profiler.printSummary(runState.options, logger.profilingLog());
 
-    return true;
+    return allSucceeded;
 }
 
 void ShaderRenderTester::loadDependentLibraries(TestRunState& runState)
@@ -486,11 +510,10 @@ DocumentInfo ShaderRenderTester::loadAndValidateDocument(const mx::FilePath& fil
 {
     DocumentInfo info;
 
-    mx::ScopedTimer ioTimer(&profiler.times().ioTime);
-
     if (_skipFiles.count(filename.getBaseName()) > 0)
         return info;
 
+    mx::ScopedTimer ioTimer(&profiler.times().ioTime);
     info.doc = mx::createDocument();
     try
     {
@@ -589,13 +612,6 @@ void TestRunLogger::start(const std::string& target, const GenShaderUtil::TestSu
 #endif
 }
 
-TestRunLogger::~TestRunLogger()
-{
-    _renderLog.reset();
-    _validationLog.reset();
-    _profilingLog.reset();
-}
-
 // ---------------------------------------------------------------------------
 // TestRunProfiler
 // ---------------------------------------------------------------------------
@@ -646,10 +662,7 @@ void TestRunTracer::start(const std::string& target, const GenShaderUtil::TestSu
     }
 }
 
-TestRunTracer::~TestRunTracer()
-{
-    _state.reset();
-}
+TestRunTracer::~TestRunTracer() = default;
 
 #endif // MATERIALX_BUILD_PERFETTO_TRACING
 

source/MaterialXTest/MaterialXRender/RenderUtil.h

@@ -16,6 +16,7 @@
 #include <fstream>
 #include <iostream>
 #include <memory>
+#include <utility>
 
 #define LOG_TO_FILE
 
@@ -53,6 +54,19 @@ class LanguageProfileTimes
         output << "\tI/O: " << ioTime << " seconds" << std::endl;
         output << "\tImage save: " << imageSaveTime << " seconds" << std::endl;
     }
+
+    void accumulate(const LanguageProfileTimes& other)
+    {
+        totalTime += other.totalTime;
+        setupTime += other.setupTime;
+        transparencyTime += other.transparencyTime;
+        generationTime += other.generationTime;
+        compileTime += other.compileTime;
+        renderTime += other.renderTime;
+        ioTime += other.ioTime;
+        imageSaveTime += other.imageSaveTime;
+    }
+
     double totalTime = 0.0;
     double setupTime = 0.0;
     double transparencyTime = 0.0;
@@ -104,8 +118,6 @@ struct DocumentInfo
 class TestRunLogger
 {
   public:
-    ~TestRunLogger();
-
     void start(const std::string& target, const GenShaderUtil::TestSuiteOptions& options);
 
     std::ostream& renderLog() { return _renderLog ? *_renderLog : std::cout; }
@@ -124,7 +136,7 @@ class TestRunLogger
 class TestRunProfiler
 {
   public:
-    ~TestRunProfiler() { _totalTimer.reset(); }
+    ~TestRunProfiler() = default;
 
     void start();
     void printSummary(const GenShaderUtil::TestSuiteOptions& options,
@@ -161,6 +173,41 @@ struct TestRunState
     std::unique_ptr<mx::GenContext> context;
 };
 
+// Read-only test configuration for the entire validate() run.
+// Note: the log stream requires synchronization for concurrent use.
+struct RenderSession
+{
+    const GenShaderUtil::TestSuiteOptions& testOptions;
+    std::ostream& log;
+};
+
+// Per-element data passed to each runRenderer call.
+struct RenderItem
+{
+    RenderItem(mx::TypedElementPtr elem,
+               mx::FileSearchPath searchPath,
+               mx::FilePath outPath,
+               mx::ImageVec* images = nullptr);
+
+    mx::TypedElementPtr element;
+    mx::DocumentPtr document;
+    mx::FileSearchPath imageSearchPath;
+    mx::FilePath outputPath;
+    mx::ImageVec* imageVec = nullptr;
+    std::string shaderName;
+
+    mx::DocumentPtr doc() const { return document; }
+};
+
+// Returned by runRenderer — each call produces its own isolated profiling data.
+// The caller accumulates results, making future parallelism straightforward.
+struct RenderProfileResult
+{
+    LanguageProfileTimes languageTimes;
+    unsigned int elementsTested = 0;
+    bool success = true;
+};
+
 // Base class used for performing compilation and render tests for a given
 // shading language and target.
 //
@@ -209,17 +256,13 @@ class ShaderRenderTester
     // Create a renderer for the generated code
     virtual void createRenderer(std::ostream& log) = 0;
 
-    // Run the renderer
-    virtual bool runRenderer(const std::string& shaderName,
-        mx::TypedElementPtr element,
-        mx::GenContext& context,
-        mx::DocumentPtr doc,
-        std::ostream& log,
-        const GenShaderUtil::TestSuiteOptions& testOptions,
-        RenderProfileTimes& profileTimes,
-        const mx::FileSearchPath& imageSearchPath,
-        const std::string& outputPath = ".",
-        mx::ImageVec* imageVec = nullptr) = 0;
+    // Run the renderer.
+    // GenContext is a separate argument because it is mutable (written per-element)
+    // and must be cloned per-thread for future parallel execution.
+    virtual RenderProfileResult runRenderer(
+        const RenderSession& session,
+        const RenderItem& item,
+        mx::GenContext& context) = 0;
 
     // Save an image
     virtual bool saveImage(const mx::FilePath&, mx::ConstImagePtr, bool) const { return false;  };