From dfdf2a027c875371faef318697cadad9bdb24a43 Mon Sep 17 00:00:00 2001 From: hybridherbst Date: Wed, 1 Apr 2026 08:43:44 +0200 Subject: [PATCH] Add hwTexcoordVerticalFlip option for UV convention Add GenOptions::hwTexcoordVerticalFlip to flip the V component at the texcoord node output before UVs enter the hardware shader graph. This allows applications to preserve mesh UV data while selecting the target shader convention through generator options. Expose the option in JavaScript bindings, apply it in HwTexCoordNode, and update CgltfLoader so texcoordVerticalFlip=false preserves loaded UV data while texcoordVerticalFlip=true performs a data-level flip. Geometry loaders now expose whether their source texture coordinate convention requires a shader-side vertical flip. MaterialXView and MaterialXGraphEditor query that through GeometryHandler, so glTF and GLB keep their previous visual orientation while OBJ remains unflipped, including when switching mesh formats. Add loader coverage for OBJ and glTF texcoordVerticalFlip behavior. --- .../JsMaterialXGenShader/JsGenOptions.cpp | 1 + .../MaterialXGenHw/Nodes/HwTexCoordNode.cpp | 17 ++++++- source/MaterialXGenShader/GenOptions.h | 6 +++ source/MaterialXGraphEditor/RenderView.cpp | 7 +++ source/MaterialXRender/CgltfLoader.cpp | 9 ++-- source/MaterialXRender/CgltfLoader.h | 5 ++ source/MaterialXRender/GeometryHandler.cpp | 13 +++++ source/MaterialXRender/GeometryHandler.h | 11 +++++ .../MaterialXTest/MaterialXRender/Render.cpp | 48 +++++++++++++++++++ source/MaterialXView/Viewer.cpp | 7 +++ 10 files changed, 117 insertions(+), 7 deletions(-) diff --git a/source/JsMaterialX/JsMaterialXGenShader/JsGenOptions.cpp b/source/JsMaterialX/JsMaterialXGenShader/JsGenOptions.cpp index 972035d7ca..97d08d33bf 100644 --- a/source/JsMaterialX/JsMaterialXGenShader/JsGenOptions.cpp +++ b/source/JsMaterialX/JsMaterialXGenShader/JsGenOptions.cpp @@ -32,6 +32,7 @@ EMSCRIPTEN_BINDINGS(GenOptions) ems::class_("GenOptions") .property("shaderInterfaceType", &mx::GenOptions::shaderInterfaceType) .property("fileTextureVerticalFlip", &mx::GenOptions::fileTextureVerticalFlip) + .property("hwTexcoordVerticalFlip", &mx::GenOptions::hwTexcoordVerticalFlip) .property("targetColorSpaceOverride", &mx::GenOptions::targetColorSpaceOverride) .property("targetDistanceUnit", &mx::GenOptions::targetDistanceUnit) .property("addUpstreamDependencies", &mx::GenOptions::addUpstreamDependencies) diff --git a/source/MaterialXGenHw/Nodes/HwTexCoordNode.cpp b/source/MaterialXGenHw/Nodes/HwTexCoordNode.cpp index 996928e395..5fe205c9ec 100644 --- a/source/MaterialXGenHw/Nodes/HwTexCoordNode.cpp +++ b/source/MaterialXGenHw/Nodes/HwTexCoordNode.cpp @@ -44,9 +44,24 @@ void HwTexCoordNode::emitFunctionCall(const ShaderNode& node, GenContext& contex VariableBlock& vertexData = stage.getOutputBlock(HW::VERTEX_DATA); const string prefix = shadergen.getVertexDataPrefix(vertexData); ShaderPort* texcoord = vertexData[variable]; + + const string inTexcoord = HW::T_IN_TEXCOORD + "_" + index; + const bool flipV = context.getOptions().hwTexcoordVerticalFlip; + + auto emitTexcoordAssign = [&](const string& lhs) { + if (flipV) + { + if (output->getType() == Type::VECTOR2) + return lhs + " = vec2(" + inTexcoord + ".x, 1.0 - " + inTexcoord + ".y)"; + else + return lhs + " = vec3(" + inTexcoord + ".x, 1.0 - " + inTexcoord + ".y, " + inTexcoord + ".z)"; + } + return lhs + " = " + inTexcoord; + }; + if (!texcoord->isEmitted()) { - shadergen.emitLine(prefix + texcoord->getVariable() + " = " + HW::T_IN_TEXCOORD + "_" + index, stage); + shadergen.emitLine(emitTexcoordAssign(prefix + texcoord->getVariable()), stage); texcoord->setEmitted(); } } diff --git a/source/MaterialXGenShader/GenOptions.h b/source/MaterialXGenShader/GenOptions.h index 93f5f7677d..40a0359631 100644 --- a/source/MaterialXGenShader/GenOptions.h +++ b/source/MaterialXGenShader/GenOptions.h @@ -78,6 +78,7 @@ class MX_GENSHADER_API GenOptions GenOptions() : shaderInterfaceType(SHADER_INTERFACE_COMPLETE), fileTextureVerticalFlip(false), + hwTexcoordVerticalFlip(false), addUpstreamDependencies(true), libraryPrefix("libraries"), emitColorTransforms(true), @@ -117,6 +118,11 @@ class MX_GENSHADER_API GenOptions /// texture space convention. By default this option is false. bool fileTextureVerticalFlip; + /// If true the y-component of texture coordinates will be flipped at + /// the texcoord node output, before entering the shader graph. This + /// affects all downstream UV consumers. By default this option is false. + bool hwTexcoordVerticalFlip; + /// An optional override for the target color space. /// Shader fragments will be generated to transform /// input values and textures into this color space. diff --git a/source/MaterialXGraphEditor/RenderView.cpp b/source/MaterialXGraphEditor/RenderView.cpp index d270cc270c..3ba8c64d38 100644 --- a/source/MaterialXGraphEditor/RenderView.cpp +++ b/source/MaterialXGraphEditor/RenderView.cpp @@ -256,6 +256,9 @@ void RenderView::loadMesh(const mx::FilePath& filename) _geometryHandler->clearGeometry(); if (_geometryHandler->loadGeometry(filename)) { + const bool hwTexcoordVerticalFlip = _geometryHandler->requiresTexcoordVerticalFlip(filename); + const bool reloadRequired = _genContext.getOptions().hwTexcoordVerticalFlip != hwTexcoordVerticalFlip; + _genContext.getOptions().hwTexcoordVerticalFlip = hwTexcoordVerticalFlip; _meshFilename = filename; if (_splitByUdims) { @@ -299,6 +302,10 @@ void RenderView::loadMesh(const mx::FilePath& filename) _imageHandler->releaseRenderResources(_shadowMap); _shadowMap = nullptr; } + if (reloadRequired && !_materials.empty()) + { + reloadShaders(); + } } } diff --git a/source/MaterialXRender/CgltfLoader.cpp b/source/MaterialXRender/CgltfLoader.cpp index 5543520a72..790bcd5b86 100644 --- a/source/MaterialXRender/CgltfLoader.cpp +++ b/source/MaterialXRender/CgltfLoader.cpp @@ -396,13 +396,10 @@ 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 - if (isTexCoordStream && v == 1) + // Preserve texture coordinates unless a data-level flip is requested. + if (isTexCoordStream && v == 1 && texcoordVerticalFlip) { - 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..7d32f4ebfa 100644 --- a/source/MaterialXRender/CgltfLoader.h +++ b/source/MaterialXRender/CgltfLoader.h @@ -34,6 +34,11 @@ class MX_RENDER_API CgltfLoader : public GeometryLoader /// Load geometry from file path bool load(const FilePath& filePath, MeshList& meshList, bool texcoordVerticalFlip = false) override; + bool requiresTexcoordVerticalFlip() const override + { + return true; + } + private: unsigned int _debugLevel; }; diff --git a/source/MaterialXRender/GeometryHandler.cpp b/source/MaterialXRender/GeometryHandler.cpp index 184b998595..45297777f8 100644 --- a/source/MaterialXRender/GeometryHandler.cpp +++ b/source/MaterialXRender/GeometryHandler.cpp @@ -118,6 +118,19 @@ bool GeometryHandler::loadGeometry(const FilePath& filePath, bool texcoordVertic return loaded; } +bool GeometryHandler::requiresTexcoordVerticalFlip(const FilePath& filePath) const +{ + std::pair range; + string extension = filePath.getExtension(); + range = _geometryLoaders.equal_range(extension); + for (auto it = range.second; it != range.first;) + { + --it; + return it->second->requiresTexcoordVerticalFlip(); + } + return false; +} + MeshPtr GeometryHandler::findParentMesh(MeshPartitionPtr part) { for (MeshPtr mesh : getMeshes()) diff --git a/source/MaterialXRender/GeometryHandler.h b/source/MaterialXRender/GeometryHandler.h index 5dfc561e0e..942e675695 100644 --- a/source/MaterialXRender/GeometryHandler.h +++ b/source/MaterialXRender/GeometryHandler.h @@ -46,6 +46,13 @@ class MX_RENDER_API GeometryLoader /// @return True if load was successful virtual bool load(const FilePath& filePath, MeshList& meshList, bool texcoordVerticalFlip = false) = 0; + /// Return true if this loader's source texture coordinate convention + /// requires a vertical flip for hardware shader generation. + virtual bool requiresTexcoordVerticalFlip() const + { + return false; + } + protected: // List of supported string extensions StringSet _extensions; @@ -95,6 +102,10 @@ class MX_RENDER_API GeometryHandler /// @param texcoordVerticalFlip Flip texture coordinates in V. Default is to not flip. bool loadGeometry(const FilePath& filePath, bool texcoordVerticalFlip = false); + /// Return true if the loader for the given file expects a vertical texture + /// coordinate flip for hardware shader generation. + bool requiresTexcoordVerticalFlip(const FilePath& filePath) const; + /// Get list of meshes const MeshList& getMeshes() const { diff --git a/source/MaterialXTest/MaterialXRender/Render.cpp b/source/MaterialXTest/MaterialXRender/Render.cpp index 94ed79479d..35bc5cbc0b 100644 --- a/source/MaterialXTest/MaterialXRender/Render.cpp +++ b/source/MaterialXTest/MaterialXRender/Render.cpp @@ -7,6 +7,8 @@ #include #include +#include +#include #include #include #include @@ -114,6 +116,37 @@ void testGeomHandler(GeomHandlerTestOptions& options) CHECK(loadFailed == 0); } +mx::MeshFloatBuffer loadTexcoords(mx::GeometryLoaderPtr loader, const mx::FilePath& filePath, bool texcoordVerticalFlip) +{ + mx::MeshList meshes; + REQUIRE(loader->load(filePath, meshes, texcoordVerticalFlip)); + REQUIRE(!meshes.empty()); + + mx::MeshStreamPtr texcoordStream = meshes[0]->getStream(mx::MeshStream::TEXCOORD_ATTRIBUTE, 0); + REQUIRE(texcoordStream); + return texcoordStream->getData(); +} + +void testTexcoordVerticalFlip(mx::GeometryLoaderPtr loader, const mx::FilePath& filePath) +{ + const size_t texcoordStride = 2; + mx::MeshFloatBuffer texcoords = loadTexcoords(loader, filePath, false); + mx::MeshFloatBuffer flippedTexcoords = loadTexcoords(loader, filePath, true); + REQUIRE(texcoords.size() == flippedTexcoords.size()); + REQUIRE(texcoords.size() >= texcoordStride); + + bool foundNonBoundaryTexcoord = false; + for (size_t i = 1; i < texcoords.size(); i += texcoordStride) + { + CHECK(std::abs(flippedTexcoords[i] - (1.0f - texcoords[i])) < 1e-6f); + if (texcoords[i] > 0.0f && texcoords[i] < 1.0f) + { + foundNonBoundaryTexcoord = true; + } + } + CHECK(foundNonBoundaryTexcoord); +} + TEST_CASE("Render: Geometry Handler Load", "[rendercore]") { std::ofstream geomHandlerLog; @@ -149,6 +182,21 @@ TEST_CASE("Render: Geometry Handler Load", "[rendercore]") geomHandlerLog.close(); } +TEST_CASE("Render: Geometry Loader Texcoord Flip", "[rendercore]") +{ + mx::FileSearchPath searchPath = mx::getDefaultDataSearchPath(); + mx::FilePath geometryPath = searchPath.find("resources/Geometry/"); + + testTexcoordVerticalFlip(mx::TinyObjLoader::create(), geometryPath / "sphere.obj"); + testTexcoordVerticalFlip(mx::CgltfLoader::create(), geometryPath / "shaderball.glb"); + + mx::GeometryHandlerPtr handler = mx::GeometryHandler::create(); + handler->addLoader(mx::TinyObjLoader::create()); + handler->addLoader(mx::CgltfLoader::create()); + CHECK(!handler->requiresTexcoordVerticalFlip(geometryPath / "sphere.obj")); + CHECK(handler->requiresTexcoordVerticalFlip(geometryPath / "shaderball.glb")); +} + struct ImageHandlerTestOptions { mx::ImageHandlerPtr imageHandler; diff --git a/source/MaterialXView/Viewer.cpp b/source/MaterialXView/Viewer.cpp index 1b7bff04e1..399e6a72de 100644 --- a/source/MaterialXView/Viewer.cpp +++ b/source/MaterialXView/Viewer.cpp @@ -1275,6 +1275,9 @@ void Viewer::loadMesh(const mx::FilePath& filename) _geometryHandler->clearGeometry(); if (_geometryHandler->loadGeometry(filename)) { + const bool hwTexcoordVerticalFlip = _geometryHandler->requiresTexcoordVerticalFlip(filename); + const bool reloadRequired = _genContext.getOptions().hwTexcoordVerticalFlip != hwTexcoordVerticalFlip; + _genContext.getOptions().hwTexcoordVerticalFlip = hwTexcoordVerticalFlip; _meshFilename = filename; if (_splitByUdims) { @@ -1308,6 +1311,10 @@ void Viewer::loadMesh(const mx::FilePath& filename) } invalidateShadowMap(); + if (reloadRequired && !_materials.empty()) + { + reloadShaders(); + } } else {