-
Notifications
You must be signed in to change notification settings - Fork 419
Refactor runRenderer to use structured types #2869
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 6 commits
97cae93
1904ded
56aacf1
ad7a4af
1c17ae3
d5e0b55
ac55117
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,25 @@ 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)); | ||
| } | ||
|
|
||
| bool ShaderRenderTester::validate(const mx::FilePath optionsFilePath) | ||
| { | ||
| // per-run state objects, logger, profiler | ||
|
|
@@ -57,24 +76,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 +104,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; | ||
| } | ||
|
Comment on lines
+108
to
114
|
||
| } | ||
|
|
||
| // 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 +509,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 +611,6 @@ void TestRunLogger::start(const std::string& target, const GenShaderUtil::TestSu | |
| #endif | ||
| } | ||
|
|
||
| TestRunLogger::~TestRunLogger() | ||
| { | ||
| _renderLog.reset(); | ||
| _validationLog.reset(); | ||
| _profilingLog.reset(); | ||
| } | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // TestRunProfiler | ||
| // --------------------------------------------------------------------------- | ||
|
|
@@ -646,10 +661,7 @@ void TestRunTracer::start(const std::string& target, const GenShaderUtil::TestSu | |
| } | ||
| } | ||
|
|
||
| TestRunTracer::~TestRunTracer() | ||
| { | ||
| _state.reset(); | ||
| } | ||
| TestRunTracer::~TestRunTracer() = default; | ||
|
|
||
| #endif // MATERIALX_BUILD_PERFETTO_TRACING | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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,40 @@ 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::FileSearchPath imageSearchPath; | ||||||||||||||||||||||||||||
| mx::FilePath outputPath; | ||||||||||||||||||||||||||||
| mx::ImageVec* imageVec = nullptr; | ||||||||||||||||||||||||||||
| std::string shaderName; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| mx::DocumentPtr doc() const { return element->getDocument(); } | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
| mx::FileSearchPath imageSearchPath; | |
| mx::FilePath outputPath; | |
| mx::ImageVec* imageVec = nullptr; | |
| std::string shaderName; | |
| mx::DocumentPtr doc() const { return element->getDocument(); } | |
| mx::DocumentPtr document = element ? element->getDocument() : nullptr; | |
| mx::FileSearchPath imageSearchPath; | |
| mx::FilePath outputPath; | |
| mx::ImageVec* imageVec = nullptr; | |
| std::string shaderName; | |
| mx::DocumentPtr doc() const { return document; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validate()now returnsallSucceededbased onRenderProfileResult::success, butsuccessis only set to false on shader-generation failure. In the renderer implementations, render/compile failures typically result invalidatedstaying false and onlyCHECK(validated)firing, leavingresult.successtrue—sovalidate()can still return true despite failures. Consider settingresult.success = result.success && validated(or setting it false in the exception paths) so the boolean return value reliably reflects test success.