diff --git a/javascript/MaterialXView/source/helper.js b/javascript/MaterialXView/source/helper.js index 417512f666..728b69db86 100644 --- a/javascript/MaterialXView/source/helper.js +++ b/javascript/MaterialXView/source/helper.js @@ -87,7 +87,7 @@ function fromMatrix(matrix, dimension) * @param {mx.Uniforms} uniforms * @param {THREE.textureLoader} textureLoader */ -function toThreeUniform(type, value, name, uniforms, textureLoader, searchPath, flipY) +function toThreeUniform(type, value, name, uniforms, textureLoader, searchPath) { let outValue = null; switch (type) @@ -164,7 +164,7 @@ function toThreeUniform(type, value, name, uniforms, textureLoader, searchPath, // Set address & filtering mode if (outValue) - setTextureParameters(outValue, name, uniforms, flipY); + setTextureParameters(outValue, name, uniforms); } } break; @@ -227,7 +227,7 @@ function getMinFilter(type, generateMipmaps) * @param {mx.Uniforms} uniforms * @param {mx.TextureFilter.generateMipmaps} generateMipmaps */ -function setTextureParameters(texture, name, uniforms, flipY = true, generateMipmaps = true) +function setTextureParameters(texture, name, uniforms, generateMipmaps = true) { const idx = name.lastIndexOf(IMAGE_PROPERTY_SEPARATOR); const base = name.substring(0, idx) || name; @@ -236,7 +236,10 @@ function setTextureParameters(texture, name, uniforms, flipY = true, generateMip texture.wrapS = THREE.RepeatWrapping; texture.wrapT = THREE.RepeatWrapping; texture.magFilter = THREE.LinearFilter; - texture.flipY = flipY; + + // Flip textures vertically on upload, compensating for the lower-left + // origin of texture space in MaterialX. + texture.flipY = true; if (uniforms.find(base + UADDRESS_MODE_SUFFIX)) { @@ -328,7 +331,7 @@ export function registerLights(mx, lights, genContext) * @param {mx.shaderStage} shaderStage * @param {THREE.TextureLoader} textureLoader */ -export function getUniformValues(shaderStage, textureLoader, searchPath, flipY) +export function getUniformValues(shaderStage, textureLoader, searchPath) { let threeUniforms = {}; @@ -343,7 +346,7 @@ export function getUniformValues(shaderStage, textureLoader, searchPath, flipY) const value = variable.getValue()?.getData(); const name = variable.getVariable(); threeUniforms[name] = new THREE.Uniform(toThreeUniform(variable.getType().getName(), value, name, uniforms, - textureLoader, searchPath, flipY)); + textureLoader, searchPath)); } } }); diff --git a/javascript/MaterialXView/source/viewer.js b/javascript/MaterialXView/source/viewer.js index 5cfce0bcb3..7b5723aea7 100644 --- a/javascript/MaterialXView/source/viewer.js +++ b/javascript/MaterialXView/source/viewer.js @@ -51,18 +51,6 @@ export class Scene this.#_worldViewPos = new THREE.Vector3(); } - // Set whether to flip UVs in V for loaded geometry - setFlipGeometryV(val) - { - this.#_flipV = val; - } - - // Get whether to flip UVs in V for loaded geometry - getFlipGeometryV() - { - return this.#_flipV; - } - // Utility to perform geometry file load loadGeometryFile(geometryFilename, loader) { @@ -144,10 +132,6 @@ export class Scene bbox.getBoundingSphere(bsphere); bboxTime = performance.now() - startBboxTime; - let theScene = viewer.getScene(); - let flipV = theScene.getFlipGeometryV(); - - this._scene.traverse((child) => { if (child.isMesh) @@ -167,8 +151,11 @@ export class Scene child.geometry.setAttribute('uv', new THREE.BufferAttribute(new Float32Array(uvs), 2)); } - else if (flipV) + else { + // Convert texture coordinates from the glTF convention, with the + // origin at the upper left, to the MaterialX convention, with the + // origin at the lower left. const uvCount = child.geometry.attributes.position.count; const uvs = child.geometry.attributes.uv.array; for (let i = 0; i < uvCount; i++) @@ -428,9 +415,6 @@ export class Scene #_geometryURL = ''; // Geometry loader #_gltfLoader = null; - // Flip V coordinate of texture coordinates. - // Set to true to be consistent with desktop viewer. - #_flipV = true; // Scene #_scene = null; @@ -940,11 +924,9 @@ export class Material let vShader = shader.getSourceCode("vertex").replace(/^#version\s+.*\n/, ''); let fShader = shader.getSourceCode("pixel").replace(/^#version\s+.*\n/, ''); - let theScene = viewer.getScene(); - let flipV = theScene.getFlipGeometryV(); let uniforms = { - ...getUniformValues(shader.getStage('vertex'), textureLoader, searchPath, flipV), - ...getUniformValues(shader.getStage('pixel'), textureLoader, searchPath, flipV), + ...getUniformValues(shader.getStage('vertex'), textureLoader, searchPath), + ...getUniformValues(shader.getStage('pixel'), textureLoader, searchPath), } Object.assign(uniforms, { diff --git a/javascript/README.md b/javascript/README.md index e4ec449146..0c44d50639 100644 --- a/javascript/README.md +++ b/javascript/README.md @@ -237,7 +237,7 @@ stdlib.delete(); genContext.delete(); gen.delete(); ``` -Shader generation options may be changed by getting the options from the context and altering its properties. Changes to these options must occur after the standard libraries have been loaded as the call to `mx.loadStandardLibraries(genContext)` sets the options to some defaults. +Shader generation options may be changed by getting the options from the context and altering its properties. Changes to these options must occur after the standard libraries have been loaded as the call to `mx.loadStandardLibraries(genContext)` sets the options to some defaults. As an example, `fileTextureVerticalFlip` may be left false by web clients that flip images vertically on upload (as in the MaterialX Web Viewer), compensating for the lower-left origin of texture space in MaterialX. ```javascript genContext.getOptions().fileTextureVerticalFlip = false; ``` diff --git a/source/JsMaterialX/JsMaterialXGenShader/JsGenContext.cpp b/source/JsMaterialX/JsMaterialXGenShader/JsGenContext.cpp index 461f84b31d..97972ec148 100644 --- a/source/JsMaterialX/JsMaterialXGenShader/JsGenContext.cpp +++ b/source/JsMaterialX/JsMaterialXGenShader/JsGenContext.cpp @@ -24,7 +24,9 @@ void initContext(mx::GenContext& context, mx::FileSearchPath searchPath, mx::Doc // Register the search path for shader source code. context.registerSourceCodeSearchPath(searchPath); - // Set shader generation options. + // Set shader generation options, with file texture lookups left + // unflipped, since web clients are expected to flip images vertically + // on upload. context.getOptions().targetColorSpaceOverride = "lin_rec709"; context.getOptions().fileTextureVerticalFlip = false; context.getOptions().hwMaxActiveLightSources = 1; diff --git a/source/MaterialXGenOsl/LibsToOso.cpp b/source/MaterialXGenOsl/LibsToOso.cpp index aca0e7c6ca..d6f4a4f1d5 100644 --- a/source/MaterialXGenOsl/LibsToOso.cpp +++ b/source/MaterialXGenOsl/LibsToOso.cpp @@ -371,12 +371,13 @@ int main(int argc, char* const argv[]) // Register types from the libraries on the OSL shader generator. oslShaderGen->registerTypeDefs(librariesDoc); - // Setup the context of the OSL shader generator. + // Setup the context of the OSL shader generator. File texture lookups are + // generated with a vertical flip, matching the reference OSL shaders and + // compensating for the top-left image origin of OSL texture lookups. mx::GenContext context(oslShaderGen); context.registerSourceCodeSearchPath(argSearchPath); - // TODO: It might be good to find a way to not hardcode these options, especially the texture flip. context.getOptions().addUpstreamDependencies = false; - context.getOptions().fileTextureVerticalFlip = false; + context.getOptions().fileTextureVerticalFlip = true; context.getOptions().oslImplicitSurfaceShaderConversion = false; OslCompileOptions options; diff --git a/source/MaterialXGenShader/GenOptions.h b/source/MaterialXGenShader/GenOptions.h index 93f5f7677d..7d9ad22da6 100644 --- a/source/MaterialXGenShader/GenOptions.h +++ b/source/MaterialXGenShader/GenOptions.h @@ -112,9 +112,13 @@ class MX_GENSHADER_API GenOptions ShaderInterfaceType shaderInterfaceType; /// If true the y-component of texture coordinates used for sampling - /// file textures will be flipped before sampling. This can be used if - /// file textures need to be flipped vertically to match the target's - /// texture space convention. By default this option is false. + /// file textures will be flipped before sampling. Set this option when + /// the target samples images with their origin at the upper left (e.g. + /// textures uploaded to the GPU in scanline order), compensating for + /// the lower-left origin of texture space in MaterialX. Leave it false + /// when the target samples images with their origin at the lower left + /// (e.g. MDL), or when the client compensates by flipping images + /// vertically at upload time, as web clients typically do. bool fileTextureVerticalFlip; /// An optional override for the target color space. diff --git a/source/MaterialXRender/CgltfLoader.cpp b/source/MaterialXRender/CgltfLoader.cpp index 5a74f90a5a..23f2285f9c 100644 --- a/source/MaterialXRender/CgltfLoader.cpp +++ b/source/MaterialXRender/CgltfLoader.cpp @@ -128,7 +128,7 @@ void decodeVec4Tangents(MeshStreamPtr vec4TangentStream, MeshStreamPtr normalStr } // anonymous namespace -bool CgltfLoader::load(const FilePath& filePath, MeshList& meshList, bool texcoordVerticalFlip) +bool CgltfLoader::load(const FilePath& filePath, MeshList& meshList) { const string input_filename = filePath.asString(); const string ext = stringToLower(filePath.getExtension()); @@ -398,13 +398,12 @@ bool CgltfLoader::load(const FilePath& filePath, MeshList& meshList, bool texcoo for (cgltf_size v = 0; v < desiredVectorSize; v++) { float floatValue = (v < vectorSize) ? input[v] : 0.0f; - // Perform v-flip + // Convert texture coordinates from the glTF convention, with the + // origin at the upper left, to the MaterialX convention, with the + // origin at the lower left. if (isTexCoordStream && v == 1) { - if (!texcoordVerticalFlip) - { - floatValue = 1.0f - floatValue; - } + floatValue = 1.0f - floatValue; } buffer.push_back(floatValue); } diff --git a/source/MaterialXRender/CgltfLoader.h b/source/MaterialXRender/CgltfLoader.h index fb63cb6102..02da91b997 100644 --- a/source/MaterialXRender/CgltfLoader.h +++ b/source/MaterialXRender/CgltfLoader.h @@ -32,7 +32,7 @@ class MX_RENDER_API CgltfLoader : public GeometryLoader static CgltfLoaderPtr create() { return std::make_shared(); } /// Load geometry from file path - bool load(const FilePath& filePath, MeshList& meshList, bool texcoordVerticalFlip = false) override; + bool load(const FilePath& filePath, MeshList& meshList) override; private: unsigned int _debugLevel; diff --git a/source/MaterialXRender/GeometryHandler.cpp b/source/MaterialXRender/GeometryHandler.cpp index 184b998595..3bff646407 100644 --- a/source/MaterialXRender/GeometryHandler.cpp +++ b/source/MaterialXRender/GeometryHandler.cpp @@ -85,7 +85,7 @@ void GeometryHandler::computeBounds() } } -bool GeometryHandler::loadGeometry(const FilePath& filePath, bool texcoordVerticalFlip) +bool GeometryHandler::loadGeometry(const FilePath& filePath) { // Early return if already loaded if (hasGeometry(filePath)) @@ -102,7 +102,7 @@ bool GeometryHandler::loadGeometry(const FilePath& filePath, bool texcoordVertic GeometryLoaderMap::iterator last = --range.first; for (auto it = first; it != last; --it) { - loaded = it->second->load(filePath, _meshes, texcoordVerticalFlip); + loaded = it->second->load(filePath, _meshes); if (loaded) { break; diff --git a/source/MaterialXRender/GeometryHandler.h b/source/MaterialXRender/GeometryHandler.h index 5dfc561e0e..132e2317e8 100644 --- a/source/MaterialXRender/GeometryHandler.h +++ b/source/MaterialXRender/GeometryHandler.h @@ -40,11 +40,12 @@ class MX_RENDER_API GeometryLoader } /// Load geometry from disk. Must be implemented by derived classes. + /// Texture coordinates are returned in the MaterialX convention, with the + /// origin at the lower left of texture space. /// @param filePath Path to file to load /// @param meshList List of meshes to update - /// @param texcoordVerticalFlip Flip texture coordinates in V when loading /// @return True if load was successful - virtual bool load(const FilePath& filePath, MeshList& meshList, bool texcoordVerticalFlip = false) = 0; + virtual bool load(const FilePath& filePath, MeshList& meshList) = 0; protected: // List of supported string extensions @@ -92,8 +93,7 @@ class MX_RENDER_API GeometryHandler /// Load geometry from a given location /// @param filePath Path to geometry - /// @param texcoordVerticalFlip Flip texture coordinates in V. Default is to not flip. - bool loadGeometry(const FilePath& filePath, bool texcoordVerticalFlip = false); + bool loadGeometry(const FilePath& filePath); /// Get list of meshes const MeshList& getMeshes() const diff --git a/source/MaterialXRender/TinyObjLoader.cpp b/source/MaterialXRender/TinyObjLoader.cpp index b01af1ebf9..b2ad2490ca 100644 --- a/source/MaterialXRender/TinyObjLoader.cpp +++ b/source/MaterialXRender/TinyObjLoader.cpp @@ -48,7 +48,7 @@ using VertexIndexMap = std::unordered_map shapes; @@ -117,10 +117,6 @@ bool TinyObjLoader::load(const FilePath& filePath, MeshList& meshList, bool texc { texcoord[k] = attrib.texcoords[indexObj.texcoord_index * MeshStream::STRIDE_2D + k]; } - if (texcoordVerticalFlip && k == 1) - { - texcoord[k] = 1.0f - texcoord[k]; - } } // Check for duplicate vertices. diff --git a/source/MaterialXRender/TinyObjLoader.h b/source/MaterialXRender/TinyObjLoader.h index e3934f9e41..ed16465a27 100644 --- a/source/MaterialXRender/TinyObjLoader.h +++ b/source/MaterialXRender/TinyObjLoader.h @@ -31,7 +31,7 @@ class MX_RENDER_API TinyObjLoader : public GeometryLoader static TinyObjLoaderPtr create() { return std::make_shared(); } /// Load geometry from disk - bool load(const FilePath& filePath, MeshList& meshList, bool texcoordVerticalFlip = false) override; + bool load(const FilePath& filePath, MeshList& meshList) override; }; MATERIALX_NAMESPACE_END diff --git a/source/MaterialXRender/Util.cpp b/source/MaterialXRender/Util.cpp index 89c6e4f0a8..967108dbc3 100644 --- a/source/MaterialXRender/Util.cpp +++ b/source/MaterialXRender/Util.cpp @@ -114,7 +114,9 @@ ShaderPtr createBlurShader(GenContext& context, OutputPtr output = nodeGraph->addOutput(); output->setConnectedNode(blurNode); - // Generate the shader + // Generate the shader, with file texture lookups left unflipped, since + // the blur input is a texture rendered on the GPU rather than an image + // uploaded in scanline order. GenContext blurContext = context; blurContext.getOptions().fileTextureVerticalFlip = false; return createShader(shaderName, blurContext, output); diff --git a/source/MaterialXRenderOsl/OslRenderer.cpp b/source/MaterialXRenderOsl/OslRenderer.cpp index 2bf1a43619..1d31b4d379 100644 --- a/source/MaterialXRenderOsl/OslRenderer.cpp +++ b/source/MaterialXRenderOsl/OslRenderer.cpp @@ -114,9 +114,13 @@ void OslRenderer::renderOSL(const FilePath& dirPath, const string& shaderName, c const string INPUT_SHADER_PARAMETER_OVERRIDES("%input_shader_parameter_overrides%"); const string INPUT_SHADER_OUTPUT_STRING("%input_shader_output%"); const string BACKGROUND_COLOR_STRING("%background_color%"); + const string RENDER_GEOMETRY_STRING("%render_geometry%"); StringMap replacementMap; replacementMap[OUTPUT_SHADER_TYPE_STRING] = outputShader; + // Format the geometry path with POSIX-style separators, which testrender + // supports on all platforms, including Windows. + replacementMap[RENDER_GEOMETRY_STRING] = _renderGeometry.asString(FilePath::FormatPosix); replacementMap[OUTPUT_SHADER_INPUT_STRING] = OUTPUT_SHADER_INPUT_VALUE_STRING; replacementMap[INPUT_SHADER_TYPE_STRING] = shaderName; string overrideString; @@ -250,8 +254,12 @@ void OslRenderer::renderOSLNetwork(const FilePath& dirPath, const string& shader const string ENVIRONMENT_SHADER_PARAMETER_OVERRIDES("%environment_shader_parameter_overrides%"); const string BACKGROUND_COLOR_STRING("%background_color%"); const string OSL_COMMANDS("%oslCmd%"); + const string RENDER_GEOMETRY_STRING("%render_geometry%"); StringMap replacementMap; + // Format the geometry path with POSIX-style separators, which testrender + // supports on all platforms, including Windows. + replacementMap[RENDER_GEOMETRY_STRING] = _renderGeometry.asString(FilePath::FormatPosix); string envOverrideString; for (const auto& param : _envOslShaderParameterOverrides) diff --git a/source/MaterialXRenderOsl/OslRenderer.h b/source/MaterialXRenderOsl/OslRenderer.h index 73ad13ffbd..e98e8b43d7 100644 --- a/source/MaterialXRenderOsl/OslRenderer.h +++ b/source/MaterialXRenderOsl/OslRenderer.h @@ -136,8 +136,8 @@ class MX_RENDEROSL_API OslRenderer : public ShaderRenderer /// Set the OSL shader output. /// This is used during render validation if "testshade" or "testrender" is executed. - /// For testrender this value is used to replace the %shader_output% token in the - /// input scene file. + /// For testrender this value is used to replace the %input_shader_output% token in + /// the scene template file. /// @param outputName Name of shader output /// @param outputType The MaterialX type of the output void setOslShaderOutput(const string& outputName, const string& outputType) @@ -164,16 +164,31 @@ class MX_RENDEROSL_API OslRenderer : public ShaderRenderer /// Set the XML scene file to use for testrender. This is a template file /// with the following tokens for replacement: - /// - %shader% : which will be replaced with the name of the shader to use - /// - %shader_output% : which will be replace with the name of the shader output to use + /// - %input_shader_type% : replaced with the name of the shader to render + /// - %input_shader_output% : replaced with the name of the shader output to use + /// - %input_shader_parameter_overrides% : replaced with input shader parameter overrides + /// - %output_shader_type% : replaced with the name of the output (remapping) shader + /// - %output_shader_input% : replaced with the name of the output shader input + /// - %environment_shader_parameter_overrides% : replaced with environment shader parameter overrides + /// - %background_color% : replaced with the background color + /// - %render_geometry% : replaced with the absolute path to the geometry to render /// @param templateFilePath Scene file name void setOslTestRenderSceneTemplateFile(const FilePath& templateFilePath) { _oslTestRenderSceneTemplateFile = templateFilePath; } + /// Set the geometry to be rendered with testrender, replacing the + /// %render_geometry% token in the scene template file. + /// @param geometryFilePath Absolute path to the geometry file + void setRenderGeometry(const FilePath& geometryFilePath) + { + _renderGeometry = geometryFilePath; + } + /// Set the name of the shader to be used for the input XML scene file. - /// The value is used to replace the %shader% token in the file. + /// The value is used to replace the %input_shader_type% token in the + /// scene template file. /// @param shaderName Name of shader void setOslShaderName(const string& shaderName) { @@ -265,6 +280,7 @@ class MX_RENDEROSL_API OslRenderer : public ShaderRenderer FilePath _oslTestShadeExecutable; FilePath _oslTestRenderExecutable; FilePath _oslTestRenderSceneTemplateFile; + FilePath _renderGeometry; string _oslShaderName; StringVec _oslShaderParameterOverrides; StringVec _envOslShaderParameterOverrides; diff --git a/source/MaterialXTest/MaterialXGenMdl/GenMdl.cpp b/source/MaterialXTest/MaterialXGenMdl/GenMdl.cpp index 2353703e7a..699a4bd12b 100644 --- a/source/MaterialXTest/MaterialXGenMdl/GenMdl.cpp +++ b/source/MaterialXTest/MaterialXGenMdl/GenMdl.cpp @@ -301,17 +301,8 @@ TEST_CASE("GenShader: MDL Shader Generation", "[genmdl]") mx::GenOptions genOptions; genOptions.targetColorSpaceOverride = "lin_rec709"; - // Flipping the texture lookups for the test renderer only. - // This is because OSL testrender does not allow to change the UV layout of their sphere (yet) and the MaterialX test suite - // adopts the OSL behavior in order to produce comparable results. This means that raw texture coordinates, or procedurals - // that use the texture coordinates, do not match what might be expected when reading the MaterialX spec: - // "[...] the image is mapped onto the geometry based on geometry UV coordinates, with the lower-left corner of an image - // mapping to the (0,0) UV coordinate [...]" - // This means for MDL: here, and only here in the test suite, we flip the UV coordinates of mesh using the `--uv_flip` option - // of the renderer, and to correct the image orientation, we apply `fileTextureVerticalFlip`. - // In regular MDL integrations this is not needed because MDL and MaterialX define the texture space equally with the origin - // at the bottom left. - genOptions.fileTextureVerticalFlip = true; + // No texture coordinate flips are required for MDL, since MDL and MaterialX + // define texture space equally, with the origin at the lower left. mx::FilePath optionsFilePath = searchPath.find("resources/Materials/TestSuite/_options.mtlx"); diff --git a/source/MaterialXTest/MaterialXRender/RenderUtil.cpp b/source/MaterialXTest/MaterialXRender/RenderUtil.cpp index a3be5ec62a..93f1017c2c 100644 --- a/source/MaterialXTest/MaterialXRender/RenderUtil.cpp +++ b/source/MaterialXTest/MaterialXRender/RenderUtil.cpp @@ -147,6 +147,21 @@ void ShaderRenderTester::loadDependentLibraries(TestRunState& runState) loadAdditionalLibraries(runState.dependLib, runState.options); } +mx::FilePath findRenderGeometry(const mx::FilePath& renderGeometry, + const mx::FileSearchPath& searchPath) +{ + mx::FilePath geomPath = renderGeometry; + if (geomPath.isEmpty()) + { + geomPath = "sphere.obj"; + } + if (!geomPath.isAbsolute()) + { + geomPath = searchPath.find("resources/Geometry") / geomPath; + } + return geomPath; +} + // Create a list of generation options based on unit test options // These options will override the original generation context options. void ShaderRenderTester::getGenerationOptions(const GenShaderUtil::TestSuiteOptions& testOptions, @@ -169,6 +184,21 @@ void ShaderRenderTester::getGenerationOptions(const GenShaderUtil::TestSuiteOpti } } +void ShaderRenderTester::loadRenderGeometry(mx::GeometryHandlerPtr geomHandler, const mx::FilePath& geomPath) +{ + if (geomHandler->hasGeometry(geomPath)) + { + return; + } + + geomHandler->clearGeometry(); + geomHandler->loadGeometry(geomPath); + for (mx::MeshPtr mesh : geomHandler->getMeshes()) + { + addAdditionalTestStreams(mesh); + } +} + void ShaderRenderTester::addAdditionalTestStreams(mx::MeshPtr mesh) { size_t vertexCount = mesh->getVertexCount(); @@ -510,6 +540,11 @@ void ShaderRenderTester::initializeGeneratorContext(TestRunState& runState, Test // Set target unit space runState.context->getOptions().targetDistanceUnit = "meter"; + // Set the target color space, along with any target-specific + // generation options. + runState.context->getOptions().targetColorSpaceOverride = "lin_rec709"; + setTargetGenerationOptions(runState.context->getOptions()); + // Register shader metadata defined in the libraries. _shaderGenerator->registerShaderMetadata(runState.dependLib, *runState.context); diff --git a/source/MaterialXTest/MaterialXRender/RenderUtil.h b/source/MaterialXTest/MaterialXRender/RenderUtil.h index 7bedd49dcb..b043afd450 100644 --- a/source/MaterialXTest/MaterialXRender/RenderUtil.h +++ b/source/MaterialXTest/MaterialXRender/RenderUtil.h @@ -8,6 +8,7 @@ #include +#include #include #include #include @@ -205,6 +206,11 @@ struct RenderProfileResult bool success = true; }; +// Resolve the given render geometry to a full file path, falling back to the +// default sphere when no geometry is specified. +mx::FilePath findRenderGeometry(const mx::FilePath& renderGeometry, + const mx::FileSearchPath& searchPath); + // Base class used for performing compilation and render tests for a given // shading language and target. // @@ -246,6 +252,11 @@ class ShaderRenderTester const GenShaderUtil::TestSuiteOptions &/*options*/, mx::GenContext& /*context*/) {}; + // Set any target-specific generation options, e.g. the vertical flip of + // file texture lookups compensating for the image origin of the target. + // Called once when the generation context for the test run is created. + virtual void setTargetGenerationOptions(mx::GenOptions& /*options*/) {}; + // // Code validation methods (compile and render) // @@ -273,6 +284,10 @@ class ShaderRenderTester // If these streams don't exist add them for testing purposes void addAdditionalTestStreams(mx::MeshPtr mesh); + // Load the given geometry into the given handler, if not already present, + // adding any additional streams required for testing. + void loadRenderGeometry(mx::GeometryHandlerPtr geomHandler, const mx::FilePath& geomPath); + // Add any paths to explicitly skip here virtual void addSkipFiles() { diff --git a/source/MaterialXTest/MaterialXRenderGlsl/RenderGlsl.cpp b/source/MaterialXTest/MaterialXRenderGlsl/RenderGlsl.cpp index 7915a51638..10d78e97a5 100644 --- a/source/MaterialXTest/MaterialXRenderGlsl/RenderGlsl.cpp +++ b/source/MaterialXTest/MaterialXRenderGlsl/RenderGlsl.cpp @@ -39,6 +39,13 @@ class GlslShaderRenderTester : public RenderUtil::ShaderRenderTester void registerLights(mx::DocumentPtr document, const GenShaderUtil::TestSuiteOptions &options, mx::GenContext& context) override; + void setTargetGenerationOptions(mx::GenOptions& options) override + { + // Flip file texture lookups vertically, compensating for the top-left + // image origin of textures uploaded to the GPU in scanline order. + options.fileTextureVerticalFlip = true; + } + void createRenderer(std::ostream& log) override; RenderUtil::RenderProfileResult runRenderer( @@ -202,9 +209,7 @@ RenderUtil::RenderProfileResult GlslShaderRenderTester::runRenderer( mx::ScopedTimer generationTimer(&result.languageTimes.generationTime); MX_TRACE_SCOPE(mx::Tracing::Category::ShaderGen, "GenerateShader"); - mx::GenOptions& contextOptions = context.getOptions(); - contextOptions = options; - contextOptions.targetColorSpaceOverride = "lin_rec709"; + context.getOptions() = options; shader = shadergen.generate(shaderName, element, context); generationTimer.endTimer(); } @@ -245,36 +250,8 @@ RenderUtil::RenderProfileResult GlslShaderRenderTester::runRenderer( try { // Set geometry - mx::GeometryHandlerPtr geomHandler = _renderer->getGeometryHandler(); - mx::FilePath geomPath; - if (!testOptions.renderGeometry.isEmpty()) - { - if (!testOptions.renderGeometry.isAbsolute()) - { - geomPath = searchPath.find("resources/Geometry") / testOptions.renderGeometry; - } - else - { - geomPath = testOptions.renderGeometry; - } - } - else - { - geomPath = searchPath.find("resources/Geometry/sphere.obj"); - } - - if (!geomHandler->hasGeometry(geomPath)) - { - // For test sphere and plane geometry perform a V-flip of texture coordinates. - const std::string baseName = geomPath.getBaseName(); - bool texcoordVerticalFlip = baseName == "sphere.obj" || baseName == "plane.obj"; - geomHandler->clearGeometry(); - geomHandler->loadGeometry(geomPath, texcoordVerticalFlip); - for (mx::MeshPtr mesh : geomHandler->getMeshes()) - { - addAdditionalTestStreams(mesh); - } - } + mx::FilePath geomPath = RenderUtil::findRenderGeometry(testOptions.renderGeometry, searchPath); + loadRenderGeometry(_renderer->getGeometryHandler(), geomPath); bool isShader = mx::elementRequiresShading(element); _renderer->setLightHandler(isShader ? _lightHandler : nullptr); diff --git a/source/MaterialXTest/MaterialXRenderMdl/RenderMdl.cpp b/source/MaterialXTest/MaterialXRenderMdl/RenderMdl.cpp index 32b604e6ea..c78c0ce7a9 100644 --- a/source/MaterialXTest/MaterialXRenderMdl/RenderMdl.cpp +++ b/source/MaterialXTest/MaterialXRenderMdl/RenderMdl.cpp @@ -29,6 +29,13 @@ class MdlShaderRenderTester : public RenderUtil::ShaderRenderTester } protected: + void setTargetGenerationOptions(mx::GenOptions& /*options*/) override + { + // No texture coordinate flips are required for MDL, since MDL and + // MaterialX define texture space equally, with the origin at the + // lower left. + } + void createRenderer(std::ostream& /*log*/) override { }; RenderUtil::RenderProfileResult runRenderer( @@ -82,10 +89,7 @@ RenderUtil::RenderProfileResult MdlShaderRenderTester::runRenderer( try { mx::ScopedTimer genTimer(&result.languageTimes.generationTime); - mx::GenOptions& contextOptions = context.getOptions(); - contextOptions = options; - contextOptions.targetColorSpaceOverride = "lin_rec709"; - contextOptions.fileTextureVerticalFlip = true; + context.getOptions() = options; // Specify the MDL target version to be the latest which is also the default. mx::GenMdlOptionsPtr genMdlOptions = std::make_shared(); @@ -167,7 +171,14 @@ RenderUtil::RenderProfileResult MdlShaderRenderTester::runRenderer( // Set scene command += " --camera 0 0 3 0 0 0"; command += " --fov 45"; - command += " --materialxtest_mode"; // align texcoord space with OSL + + // Since no scene geometry is specified, df_vulkan renders its + // built-in sphere, whose texture coordinates span (0, 0) to + // (2, 1). Halve its U range to align it with the sphere + // geometry used by other renderers in the test suite. The + // default V orientation of df_vulkan matches the MaterialX + // convention, with the origin at the lower left. + command += " --uv_scale 0.5 1"; // Application setup command += " --noaux"; diff --git a/source/MaterialXTest/MaterialXRenderMsl/RenderMsl.mm b/source/MaterialXTest/MaterialXRenderMsl/RenderMsl.mm index 4ed56fe215..8862165782 100644 --- a/source/MaterialXTest/MaterialXRenderMsl/RenderMsl.mm +++ b/source/MaterialXTest/MaterialXRenderMsl/RenderMsl.mm @@ -42,6 +42,13 @@ void loadAdditionalLibraries(mx::DocumentPtr document, void registerLights(mx::DocumentPtr document, const GenShaderUtil::TestSuiteOptions &options, mx::GenContext& context) override; + void setTargetGenerationOptions(mx::GenOptions& options) override + { + // Flip file texture lookups vertically, compensating for the top-left + // image origin of textures uploaded to the GPU in scanline order. + options.fileTextureVerticalFlip = true; + } + void createRenderer(std::ostream& log) override; RenderUtil::RenderProfileResult runRenderer( @@ -206,9 +213,7 @@ void registerLights(mx::DocumentPtr document, const GenShaderUtil::TestSuiteOpti transpTimer.endTimer(); mx::ScopedTimer generationTimer(&result.languageTimes.generationTime); - mx::GenOptions& contextOptions = context.getOptions(); - contextOptions = options; - contextOptions.targetColorSpaceOverride = "lin_rec709"; + context.getOptions() = options; shader = shadergen.generate(shaderName, element, context); generationTimer.endTimer(); } @@ -247,36 +252,8 @@ void registerLights(mx::DocumentPtr document, const GenShaderUtil::TestSuiteOpti try { // Set geometry - mx::GeometryHandlerPtr geomHandler = _renderer->getGeometryHandler(); - mx::FilePath geomPath; - if (!testOptions.renderGeometry.isEmpty()) - { - if (!testOptions.renderGeometry.isAbsolute()) - { - geomPath = searchPath.find("resources/Geometry") / testOptions.renderGeometry; - } - else - { - geomPath = testOptions.renderGeometry; - } - } - else - { - geomPath = searchPath.find("resources/Geometry/sphere.obj"); - } - - if (!geomHandler->hasGeometry(geomPath)) - { - // For test sphere and plane geometry perform a V-flip of texture coordinates. - const std::string baseName = geomPath.getBaseName(); - bool texcoordVerticalFlip = baseName == "sphere.obj" || baseName == "plane.obj"; - geomHandler->clearGeometry(); - geomHandler->loadGeometry(geomPath, texcoordVerticalFlip); - for (mx::MeshPtr mesh : geomHandler->getMeshes()) - { - addAdditionalTestStreams(mesh); - } - } + mx::FilePath geomPath = RenderUtil::findRenderGeometry(testOptions.renderGeometry, searchPath); + loadRenderGeometry(_renderer->getGeometryHandler(), geomPath); bool isShader = mx::elementRequiresShading(element); _renderer->setLightHandler(isShader ? _lightHandler : nullptr); diff --git a/source/MaterialXTest/MaterialXRenderOsl/RenderOsl.cpp b/source/MaterialXTest/MaterialXRenderOsl/RenderOsl.cpp index 41e25afca3..cebe01f1c4 100644 --- a/source/MaterialXTest/MaterialXRenderOsl/RenderOsl.cpp +++ b/source/MaterialXTest/MaterialXRenderOsl/RenderOsl.cpp @@ -90,6 +90,17 @@ class OslShaderRenderTester : public RenderUtil::ShaderRenderTester } protected: + void setTargetGenerationOptions(mx::GenOptions& options) override + { + // Flip file texture lookups vertically, compensating for the top-left + // image origin of OSL texture lookups. + options.fileTextureVerticalFlip = true; + + // Generate a wrapper shader connecting the output to Ci, as required + // for rendering with testrender. + options.oslConnectCiWrapper = true; + } + void createRenderer(std::ostream& log) override; RenderUtil::RenderProfileResult runRenderer( @@ -223,10 +234,7 @@ RenderUtil::RenderProfileResult OslShaderRenderTester::runRenderer( try { mx::ScopedTimer genTimer(&result.languageTimes.generationTime); - mx::GenOptions& contextOptions = context.getOptions(); - contextOptions = options; - contextOptions.targetColorSpaceOverride = "lin_rec709"; - contextOptions.oslConnectCiWrapper = true; + context.getOptions() = options; // Apply local overrides for shader generation. shadergen.registerImplementation("IM_tangent_vector3_" + mx::OslShaderGenerator::TARGET, TangentOsl::create); @@ -339,6 +347,17 @@ RenderUtil::RenderProfileResult OslShaderRenderTester::runRenderer( sceneTemplatePath = sceneTemplatePath / sceneTemplateFile; _renderer->setOslTestRenderSceneTemplateFile(sceneTemplatePath.asString()); + // Set the render geometry. OSL testrender currently loads + // only OBJ geometry, so fall back to the default sphere + // for other formats. + mx::FilePath renderGeometry = testOptions.renderGeometry; + if (renderGeometry.getExtension() != "obj") + { + renderGeometry = mx::FilePath(); + } + mx::FilePath geomPath = RenderUtil::findRenderGeometry(renderGeometry, searchPath); + _renderer->setRenderGeometry(geomPath); + // Validate rendering { mx::ScopedTimer renderTimer(&result.languageTimes.renderTime); diff --git a/source/MaterialXTest/MaterialXRenderOsl/Utilities/scene_template.xml b/source/MaterialXTest/MaterialXRenderOsl/Utilities/scene_template.xml index 87db0dac2e..feecb3f5ef 100644 --- a/source/MaterialXTest/MaterialXRenderOsl/Utilities/scene_template.xml +++ b/source/MaterialXTest/MaterialXRenderOsl/Utilities/scene_template.xml @@ -34,5 +34,5 @@ connect inputShader.%input_shader_output% outputShader.%output_shader_input%; - + diff --git a/source/MaterialXTest/MaterialXRenderOsl/Utilities/scene_template_oslcmd.xml b/source/MaterialXTest/MaterialXRenderOsl/Utilities/scene_template_oslcmd.xml index 4451bb7853..2cdff45532 100644 --- a/source/MaterialXTest/MaterialXRenderOsl/Utilities/scene_template_oslcmd.xml +++ b/source/MaterialXTest/MaterialXRenderOsl/Utilities/scene_template_oslcmd.xml @@ -22,5 +22,5 @@ %oslCmd% - + diff --git a/source/MaterialXTest/MaterialXRenderSlang/RenderSlang.cpp b/source/MaterialXTest/MaterialXRenderSlang/RenderSlang.cpp index de1b4f8d7d..594b071992 100644 --- a/source/MaterialXTest/MaterialXRenderSlang/RenderSlang.cpp +++ b/source/MaterialXTest/MaterialXRenderSlang/RenderSlang.cpp @@ -38,6 +38,13 @@ class SlangShaderRenderTester : public RenderUtil::ShaderRenderTester void registerLights(mx::DocumentPtr document, const GenShaderUtil::TestSuiteOptions& options, mx::GenContext& context) override; + void setTargetGenerationOptions(mx::GenOptions& options) override + { + // Flip file texture lookups vertically, compensating for the top-left + // image origin of textures uploaded to the GPU in scanline order. + options.fileTextureVerticalFlip = true; + } + void createRenderer(std::ostream& log) override; RenderUtil::RenderProfileResult runRenderer( @@ -198,9 +205,7 @@ RenderUtil::RenderProfileResult SlangShaderRenderTester::runRenderer( transpTimer.endTimer(); mx::ScopedTimer generationTimer(&result.languageTimes.generationTime); - mx::GenOptions& contextOptions = context.getOptions(); - contextOptions = options; - contextOptions.targetColorSpaceOverride = "lin_rec709"; + context.getOptions() = options; shader = shadergen.generate(shaderName, element, context); generationTimer.endTimer(); } @@ -240,36 +245,8 @@ RenderUtil::RenderProfileResult SlangShaderRenderTester::runRenderer( try { // Set geometry - mx::GeometryHandlerPtr geomHandler = _renderer->getGeometryHandler(); - mx::FilePath geomPath; - if (!testOptions.renderGeometry.isEmpty()) - { - if (!testOptions.renderGeometry.isAbsolute()) - { - geomPath = searchPath.find("resources/Geometry") / testOptions.renderGeometry; - } - else - { - geomPath = testOptions.renderGeometry; - } - } - else - { - geomPath = searchPath.find("resources/Geometry/sphere.obj"); - } - - if (!geomHandler->hasGeometry(geomPath)) - { - // For test sphere and plane geometry perform a V-flip of texture coordinates. - const std::string baseName = geomPath.getBaseName(); - bool texcoordVerticalFlip = baseName == "sphere.obj" || baseName == "plane.obj"; - geomHandler->clearGeometry(); - geomHandler->loadGeometry(geomPath, texcoordVerticalFlip); - for (mx::MeshPtr mesh : geomHandler->getMeshes()) - { - addAdditionalTestStreams(mesh); - } - } + mx::FilePath geomPath = RenderUtil::findRenderGeometry(testOptions.renderGeometry, searchPath); + loadRenderGeometry(_renderer->getGeometryHandler(), geomPath); bool isShader = mx::elementRequiresShading(element); _renderer->setLightHandler(isShader ? _lightHandler : nullptr); diff --git a/source/MaterialXView/Viewer.cpp b/source/MaterialXView/Viewer.cpp index 3030574f8f..9a154d9f8c 100644 --- a/source/MaterialXView/Viewer.cpp +++ b/source/MaterialXView/Viewer.cpp @@ -263,18 +263,23 @@ Viewer::Viewer(const std::string& materialFilename, #else _renderPipeline = GLRenderPipeline::create(this); - // Set Essl generator options + // Set Essl generator options, with file texture lookups left unflipped, + // matching web clients such as the MaterialX Web Viewer, which flip + // images vertically on upload. _genContextEssl.getOptions().targetColorSpaceOverride = "lin_rec709"; _genContextEssl.getOptions().fileTextureVerticalFlip = false; _genContextEssl.getOptions().hwMaxActiveLightSources = 1; #endif #if MATERIALX_BUILD_GEN_OSL - // Set OSL generator options. + // Set OSL generator options, with file texture lookups compensating for + // the top-left image origin of OSL texture lookups. _genContextOsl.getOptions().targetColorSpaceOverride = "lin_rec709"; - _genContextOsl.getOptions().fileTextureVerticalFlip = false; + _genContextOsl.getOptions().fileTextureVerticalFlip = true; #endif #if MATERIALX_BUILD_GEN_MDL - // Set MDL generator options. + // Set MDL generator options, with file texture lookups left unflipped, + // since MDL and MaterialX define texture space equally, with the origin + // at the lower left. _genContextMdl.getOptions().targetColorSpaceOverride = "lin_rec709"; _genContextMdl.getOptions().fileTextureVerticalFlip = false; #endif diff --git a/source/PyMaterialX/PyMaterialXRender/PyGeometryHandler.cpp b/source/PyMaterialX/PyMaterialXRender/PyGeometryHandler.cpp index 08560254bc..fbbf1895cd 100644 --- a/source/PyMaterialX/PyMaterialXRender/PyGeometryHandler.cpp +++ b/source/PyMaterialX/PyMaterialXRender/PyGeometryHandler.cpp @@ -18,15 +18,14 @@ class PyGeometryLoader : public mx::GeometryLoader { } - bool load(const mx::FilePath& filePath, mx::MeshList& meshList, bool texcoordVerticalFlip = false) override + bool load(const mx::FilePath& filePath, mx::MeshList& meshList) override { PYBIND11_OVERLOAD_PURE( bool, mx::GeometryLoader, load, filePath, - meshList, - texcoordVerticalFlip + meshList ); } }; diff --git a/source/PyMaterialX/PyMaterialXRenderOsl/PyOslRenderer.cpp b/source/PyMaterialX/PyMaterialXRenderOsl/PyOslRenderer.cpp index 9ae042cced..73d93bc1a6 100644 --- a/source/PyMaterialX/PyMaterialXRenderOsl/PyOslRenderer.cpp +++ b/source/PyMaterialX/PyMaterialXRenderOsl/PyOslRenderer.cpp @@ -29,6 +29,7 @@ void bindPyOslRenderer(py::module& mod) .def("setOslTestShadeExecutable", &mx::OslRenderer::setOslTestShadeExecutable) .def("setOslTestRenderExecutable", &mx::OslRenderer::setOslTestRenderExecutable) .def("setOslTestRenderSceneTemplateFile", &mx::OslRenderer::setOslTestRenderSceneTemplateFile) + .def("setRenderGeometry", &mx::OslRenderer::setRenderGeometry) .def("setOslShaderName", &mx::OslRenderer::setOslShaderName) .def("setOslUtilityOSOPath", &mx::OslRenderer::setOslUtilityOSOPath) .def("useTestRender", &mx::OslRenderer::useTestRender)