-
Notifications
You must be signed in to change notification settings - Fork 202
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
Fix MaterialX compilation on M1 Macs #2579
Merged
seando-adsk
merged 2 commits into
dev
from
t_gamaj/MAYA-122706/remove_global_variable_usage
Sep 7, 2022
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,8 @@ | |
#include <MaterialXGenGlsl/GlslShaderGenerator.h> | ||
#include <MaterialXGenShader/Shader.h> | ||
|
||
#include <regex> | ||
|
||
MATERIALX_NAMESPACE_BEGIN | ||
|
||
namespace Stage { | ||
|
@@ -25,6 +27,74 @@ const string LIGHT_LOOP_RESULT = "lightLoopResult"; | |
const string MAYA_ENV_IRRADIANCE_SAMPLE = "diffuseI"; | ||
const string MAYA_ENV_RADIANCE_SAMPLE = "specularI"; | ||
const string MAYA_ENV_ROUGHNESS = "roughness"; | ||
|
||
// The Apple shader compiler found on M1/M2 machines does not allow using a global variable as a | ||
// temporary buffer. We will need to handle these inputs differently. | ||
void fixupVertexDataInstance(ShaderStage& stage) | ||
{ | ||
// All the strings start with $. This is an anchor character in regex, so skip it. | ||
auto d = [](auto const& s) { return s.substr(1); }; | ||
|
||
// Find keywords: (as text) | ||
// | ||
// vec3 HW::T_POSITION_WORLD vec3 $normalWorld | ||
// | ||
// And replace with: | ||
// | ||
// vec3 unused_T_POSITION_WORLD vec3 unused_normalWorld | ||
// | ||
|
||
static const std::string paramSource = "vec3 [$](" + d(HW::T_POSITION_WORLD) + "|" | ||
+ d(HW::T_POSITION_OBJECT) + "|" + d(HW::T_NORMAL_WORLD) + "|" + d(HW::T_NORMAL_OBJECT) | ||
+ "|" + d(HW::T_TANGENT_WORLD) + "|" + d(HW::T_TANGENT_OBJECT) + "|" | ||
+ d(HW::T_BITANGENT_WORLD) + "|" + d(HW::T_BITANGENT_OBJECT) + ")"; | ||
|
||
static const std::regex paramRegex(paramSource.c_str()); | ||
|
||
// Find keywords: (as text) | ||
// | ||
// HW::T_VERTEX_DATA_INSTANCE.HW::T_POSITION_WORLD $vd.$normalWorld | ||
// | ||
// And replace with: | ||
// | ||
// HW::T_POSITION_WORLD(PIX_IN.HW::T_POSITION_WORLD) $normalWorld( PIX_IN.$normalWorld ) | ||
// | ||
|
||
static const std::string vtxSource = "[$]" + d(HW::T_VERTEX_DATA_INSTANCE) + "[.][$](" | ||
+ d(HW::T_POSITION_WORLD) + "|" + d(HW::T_POSITION_OBJECT) + "|" + d(HW::T_NORMAL_WORLD) | ||
+ "|" + d(HW::T_NORMAL_OBJECT) + "|" + d(HW::T_TANGENT_WORLD) + "|" | ||
+ d(HW::T_TANGENT_OBJECT) + "|" + d(HW::T_BITANGENT_WORLD) + "|" + d(HW::T_BITANGENT_OBJECT) | ||
+ ")"; | ||
|
||
static const std::regex vtxRegex(vtxSource.c_str()); | ||
|
||
// Find keywords: (as text) | ||
// | ||
// HW::T_VERTEX_DATA_INSTANCE. $vd.$inGeomprop_st | ||
// | ||
// And replace with: | ||
// | ||
// (nothing) $inGeomprop_st | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are captured in the shader main and passed as parameters, so they do not require the |
||
// | ||
|
||
static const std::string vdCleanupSource = "[$]" + d(HW::T_VERTEX_DATA_INSTANCE) + "[.]"; | ||
|
||
static const std::regex vdCleanupRegex(vdCleanupSource.c_str()); | ||
|
||
std::string code = stage.getSourceCode(); | ||
code = std::regex_replace(code, paramRegex, "vec3 unused_$1"); | ||
code = std::regex_replace(code, vtxRegex, "$$$1( PIX_IN.$$$1 )"); | ||
code = std::regex_replace(code, vdCleanupRegex, ""); | ||
|
||
#if MX_COMBINED_VERSION >= 13804 | ||
stage.setSourceCode(code); | ||
#else | ||
// setSourceCode was added in 1.38.4. Use const casting in previous versions to modify the | ||
// member variable directly (since getSourceCode returns by reference). | ||
std::string& ncCode(const_cast<std::string&>(stage.getSourceCode())); | ||
ncCode = code; | ||
#endif | ||
} | ||
} // namespace | ||
|
||
const string& HwSpecularEnvironmentSamples::name() | ||
|
@@ -67,8 +137,6 @@ GlslFragmentGenerator::GlslFragmentGenerator() | |
_tokenSubstitutions[HW::T_TANGENT_OBJECT] = "Tm"; | ||
_tokenSubstitutions[HW::T_BITANGENT_WORLD] = "Bw"; | ||
_tokenSubstitutions[HW::T_BITANGENT_OBJECT] = "Bm"; | ||
_tokenSubstitutions[HW::T_VERTEX_DATA_INSTANCE] | ||
= "g_mxVertexData"; // name of a global non-const variable | ||
if (OgsXmlGenerator::useLightAPI() >= 2) { | ||
// Use a Maya 2022.1-aware surface node implementation. | ||
registerImplementation( | ||
|
@@ -126,34 +194,6 @@ ShaderPtr GlslFragmentGenerator::generate( | |
ScopedFloatFormatting floatFormatting(Value::FloatFormatFixed); | ||
|
||
const VariableBlock& vertexData = pixelStage.getInputBlock(HW::VERTEX_DATA); | ||
if (!vertexData.empty()) { | ||
// GLSL shader input blocks are essentially global constants accessible | ||
// from any function in the shader, not just the entry point. The | ||
// MaterialX GLSL generator makes use of this global access pattern. | ||
// We could make it work for VP2 GLSL fragments by referencing the | ||
// PIX_IN input block generated by VP2 in the final effect but this | ||
// approach has two problems: | ||
// 1. VP2 pre-processes some inputs (e.g. Nw) before passing them as | ||
// arguments to the fragment's root function. These are the correct | ||
// values to use for shading, not the original ones from PIX_IN. | ||
// 2. This pattern is not portable to HLSL where shader stage inputs | ||
// are passed as arguments to the entry point function and have to be | ||
// passed to other functions explicitly. | ||
// | ||
// Since passing vertex inputs to all functions as arguments would be | ||
// too intrusive in GLSL codegen, we instead define a non-const global | ||
// struct variable which gets populated from the function arguments in | ||
// the fragment's root function and is read from in all other functions | ||
|
||
emitLine("struct", pixelStage, false); | ||
emitScopeBegin(pixelStage); | ||
emitVariableDeclarations( | ||
vertexData, EMPTY_STRING, Syntax::SEMICOLON, context, pixelStage, false); | ||
emitScopeEnd(pixelStage, false, false); | ||
emitString(" " + HW::T_VERTEX_DATA_INSTANCE, pixelStage); | ||
emitLineEnd(pixelStage, true); | ||
emitLineBreak(pixelStage); | ||
} | ||
|
||
// Add global constants and type definitions | ||
const unsigned int maxLights = std::max(1u, context.getOptions().hwMaxActiveLightSources); | ||
|
@@ -328,21 +368,6 @@ ShaderPtr GlslFragmentGenerator::generate( | |
// to a surface shader, so just output black. | ||
emitLine("return vec3(0.0)", pixelStage); | ||
} else { | ||
// Copy vertex data values from the root function's arguments to a | ||
// global struct variable accessible from all other functions. | ||
// | ||
for (size_t i = 0; i < vertexData.size(); ++i) { | ||
const string& name = vertexData[i]->getVariable(); | ||
|
||
emitLineBegin(pixelStage); | ||
emitString(HW::T_VERTEX_DATA_INSTANCE, pixelStage); | ||
emitString(".", pixelStage); | ||
emitString(name, pixelStage); | ||
emitString(" = ", pixelStage); | ||
emitString(name, pixelStage); | ||
emitLineEnd(pixelStage, true); | ||
} | ||
|
||
if (lighting && OgsXmlGenerator::useLightAPI() < 2) { | ||
// Store environment samples from light rig: | ||
emitLine( | ||
|
@@ -424,6 +449,8 @@ ShaderPtr GlslFragmentGenerator::generate( | |
// End function | ||
emitScopeEnd(pixelStage); | ||
|
||
fixupVertexDataInstance(pixelStage); | ||
|
||
// Replace all tokens with real identifier names | ||
replaceTokens(_tokenSubstitutions, pixelStage); | ||
|
||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
To be further replaced later so we get the familiar
Nw( PIX_IN.Nw )
that can be observed in the shader main function if looking with APITrace.