Unify texcoord conventions across render targets

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));
             }
         }
     });

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, {

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;
 ```

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;

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;

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.

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);
                                 }

source/MaterialXRender/CgltfLoader.h

@@ -32,7 +32,7 @@ class MX_RENDER_API CgltfLoader : public GeometryLoader
     static CgltfLoaderPtr create() { return std::make_shared<CgltfLoader>(); }
 
     /// 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;

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;

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

source/MaterialXRender/TinyObjLoader.cpp

@@ -48,7 +48,7 @@ using VertexIndexMap = std::unordered_map<VertexVector, uint32_t, VertexVector::
 // TinyObjLoader methods
 //
 
-bool TinyObjLoader::load(const FilePath& filePath, MeshList& meshList, bool texcoordVerticalFlip)
+bool TinyObjLoader::load(const FilePath& filePath, MeshList& meshList)
 {
     tinyobj::attrib_t attrib;
     vector<tinyobj::shape_t> 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.

source/MaterialXRender/TinyObjLoader.h

@@ -31,7 +31,7 @@ class MX_RENDER_API TinyObjLoader : public GeometryLoader
     static TinyObjLoaderPtr create() { return std::make_shared<TinyObjLoader>(); }
 
     /// 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

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);

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)

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;

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");