diff --git a/doc/changelog.md b/doc/changelog.md index 8d3da6f..0fff7d0 100644 --- a/doc/changelog.md +++ b/doc/changelog.md @@ -1,6 +1,12 @@ ## Changelog +### Unreleased (Miris fork) + +#### Fixes: +- [MAX-MAT-001] MaterialX export: strip the spurious `specular_rotation = 0.25` default that 3ds Max's `MtlxIOUtil` bridge writes on every `ND_standard_surface_surfaceshader`. The MaterialX nodedef default is 0.0, and the value has no visual effect when `specular_anisotropy` is zero; the spurious value is preserved by `MtlxShaderWriter` only when anisotropy is provably non-zero. See `doc/translation-mapping.md`. +- [MAX-MAT-002] MaterialX export: strip the spurious `emission = 1.0` paired with `emission_color = (0, 0, 0)` that 3ds Max's `MtlxIOUtil` bridge writes on every `ND_standard_surface_surfaceshader`. The MaterialX nodedef defaults (`emission = 0.0`, `emission_color = (1, 1, 1)`) evaluate to the same zero emission with the correct semantic meaning; the spurious pair is only removed when both values match the buggy pattern exactly. See `doc/translation-mapping.md`. + ### v0.15.0 #### What's New: diff --git a/doc/translation-mapping.md b/doc/translation-mapping.md new file mode 100644 index 0000000..39e384a --- /dev/null +++ b/doc/translation-mapping.md @@ -0,0 +1,192 @@ +# 3ds Max → MaterialX / USD translation mapping + +This document tracks how 3ds Max PhysicalMaterial / OpenPBR / MaterialXMaterial +properties translate to MaterialX `standard_surface` (and related) shader +inputs as they are written through the Miris fork's USD exporter +(`src/translators/MtlxShaderWriter.cpp`). + +Generated and maintained by the Improvement Agent. Each entry corresponds to +a PR that: + +1. Updates the relevant row of the **Status** table below. +2. Lands a C++ change in the writer (almost always + `MtlxShaderWriter.cpp` for surface-shader mappings). +3. Lands a validator script (Python, runnable under `hython`) that proves the + logic against the captured corpus in `samples/complex_export.usda` (or a + synthetic fixture) without requiring a Windows build. +4. Where the bite is a *workaround for an upstream 3ds Max bug*, it + documents which 3ds Max version(s) the workaround targets and what + condition retires it. + +## Translation model — what the writer can actually change + +`MtlxShaderWriter.cpp` is mostly a **passthrough**: it asks 3ds Max for a +MaterialX XML string via the MaxScript bridge `MtlxIOUtil.ExportMtlxString`, +parses it, and walks the resulting `MaterialX::Document` to create USD +shader prims. The MaxScript bridge lives inside 3ds Max itself and cannot be +modified from this plugin. + +The C++ writer therefore has two intervention points: + +1. **Pre-passthrough** (impossible — the MaxScript bridge has already + serialized whatever it serialized). +2. **Post-parse, pre-USD-author** — after `readFromXmlString`, the + in-memory MaterialX document can be normalized before any USD prims are + created. This is where workarounds for buggy 3ds Max defaults live. + +Status legend: + +* **direct pairing** — a 1:1 mapping between a 3ds Max source property and a + MaterialX node/input that the upstream bridge already emits correctly. No + C++ intervention required. +* **approximating workaround** — the writer mutates the MaterialX document + to compose a `standard_surface`-compatible result that approximates a 3ds + Max feature the bridge does not faithfully translate. +* **bug normalization** — the writer strips or rewrites a value that the + upstream bridge emits incorrectly (hardcoded default, off-by-one, wrong + unit, etc.) so the resulting USD matches what a correctly-implemented + exporter should produce. +* **no equivalent** — the source property has no MaterialX representation; + the writer emits a `TF_WARN` so the divergence is observable. + +## Status + +| Source (3ds Max) | MaterialX target | Kind | Affects MaterialX nodedef | Workaround triggers when | PR | Date | +| --- | --- | --- | --- | --- | --- | --- | +| PhysicalMaterial.anisotropy_angle | `standard_surface.specular_rotation` | bug normalization | `ND_standard_surface_surfaceshader` | Bridge emits `0.25` AND `specular_anisotropy` is static-zero or absent | MAX-MAT-001 | 2026-06-20 | +| PhysicalMaterial.emission (none authored) | `standard_surface.emission` + `standard_surface.emission_color` | bug normalization | `ND_standard_surface_surfaceshader` | Bridge emits `emission = 1.0` AND `emission_color = (0, 0, 0)`, both static | (this PR) | 2026-06-20 | + +## Notes per expression + +### PhysicalMaterial.anisotropy_angle → standard_surface.specular_rotation (MAX-MAT-001) + +**Symptom.** The 3ds Max-shipped `MtlxIOUtil.ExportMtlxString` MaxScript +bridge always emits `` on every `ND_standard_surface_surfaceshader`, regardless of +whether the source PhysicalMaterial has any anisotropy authored. Six out of +six materials in the diagnostic corpus exhibit this. The MaterialX +`standard_surface` nodedef defaults `specular_rotation` to **0.0**. + +**Why it matters.** `specular_rotation` is multiplied by +`specular_anisotropy` inside the standard_surface BSDF, so the buggy value +has no observable lighting effect in the (universal) `anisotropy == 0` +case. It *does* matter for any downstream layer that turns anisotropy on, +or for tools that read MaterialX values directly for content authoring or +roundtripping. The wrong value silently introduces a 0.25-turns (90°) +highlight rotation the source DCC never asked for. + +**Fix.** Add a post-parse normalization pass in `MtlxShaderWriter::Write()` +that walks the parsed `MaterialX::Document`, looks for `standard_surface` +nodes carrying `specular_rotation = 0.25` AND no anisotropy (absent or +static 0), and removes the `specular_rotation` input. After normalization +the input falls back to the nodedef default (`0.0`). + +**Bounds (where the fix conservatively does nothing):** + +* `specular_rotation` is connected to a node or nodegraph — could carry an + intentional procedural rotation; keep it. +* `specular_rotation` has any static value other than `0.25` — the user + authored it explicitly; keep it. +* `specular_anisotropy` is connected — runtime value unknown; assume the + rotation may be intentional and keep it. +* `specular_anisotropy` is statically non-zero — anisotropy is on, rotation + matters; keep it. + +**Validator.** +`/Users/d.smith/MirisProjects/Agent Builder/agent/arch-builds/191c9984-d6c5-468c-a0ad-a95092dbe6d3/normalize_specular_rotation.py` +mirrors the C++ logic at the USD layer (the C++ runs at the MaterialX-doc +layer earlier in the pipeline). Running it on the captured +`complex_export.usda` strips exactly the six known-bogus values and leaves +every other attribute identical. Karma renders of the prefix and postfix +USDs are byte-identical (same SHA-256), confirming the fix is a visual +no-op for the common case — exactly what the BSDF math predicts. + +**Retirement condition.** If/when Autodesk fixes `MtlxIOUtil` in a future +3ds Max release so that the bridge stops emitting the spurious 0.25, the +normalization pass becomes inert (no node matches the trigger condition). +The pass can stay in place as a belt-and-suspenders guard for older Max +installs. + +### PhysicalMaterial.emission (none authored) → standard_surface.emission + .emission_color (MAX-MAT-002) + +**Symptom.** The 3ds Max-shipped `MtlxIOUtil.ExportMtlxString` MaxScript +bridge always emits the pair +``` + + +``` +on every `ND_standard_surface_surfaceshader`, regardless of whether the +source PhysicalMaterial authored any emission. Six out of six materials in +the diagnostic corpus exhibit this. The MaterialX `standard_surface` +nodedef defaults are `emission = 0.0` and `emission_color = (1, 1, 1)`. + +**Why it matters.** The buggy pair multiplies to `1.0 * (0,0,0) = (0,0,0)`, +so the BSDF emits no light and the bug is invisible at render time. It +*does* matter for any downstream tool that reads the exported MaterialX +and constructs further overrides: + +* A layer that bumps `emission_color` to a non-black value (e.g. to author + a glowing variant of an existing material) would unexpectedly turn on + full-strength emission, because the inherited `emission = 1.0` is still + in force. +* Round-trip importers that read `emission_color = (0, 0, 0)` may treat + the surface as having explicit black emission rather than “no emission + authored” — semantically different states that diverge under + later edits. +* The values do not match the source DCC: the PhysicalMaterial has no + emission knob set to 1.0, and certainly didn't ask for an emission + *color* of pure black. The exported document misrepresents what the + artist authored. + +**Fix.** Add a second post-parse normalization pass in +`MtlxShaderWriter::Write()` (run immediately after MAX-MAT-001's pass) +that walks the parsed `MaterialX::Document` and, for each +`standard_surface` node, removes both the `emission` and `emission_color` +inputs when *all* of: + +* `emission` is statically `1.0` (no connection, tolerant of float noise); +* `emission_color` is statically `(0, 0, 0)` (no connection, tolerant of + float noise); and +* both inputs are present. + +After normalization both inputs fall back to nodedef defaults +(`0.0` and `(1, 1, 1)`), which evaluate to the *same* zero emission with +the correct meaning. + +**Bounds (where the fix conservatively does nothing):** + +* Either `emission` or `emission_color` is connected to a node or + nodegraph — could carry intentional procedural emission; keep both. +* `emission` is static but not `1.0` — user authored it explicitly; keep. +* `emission_color` is static but not exactly `(0, 0, 0)` — user authored + an explicit emission tint; keep. +* Only one of the two inputs is present — the bug pattern is the pair, so + treating either half in isolation could destroy a real authored value. + +**Validator.** +`/Users/d.smith/MirisProjects/Agent Builder/agent/arch-builds/f72c97e6-301f-400a-9e77-c768979e8fa5/normalize_emission_default.py` +mirrors the C++ logic at the USD layer (the C++ runs at the MaterialX-doc +layer earlier in the pipeline). Running it on the MAX-MAT-001-clean +`complex_export_postfix.usda` strips exactly six pairs of +`emission`/`emission_color` inputs and leaves every other attribute +identical. Karma renders pre- and post-strip are byte-identical (same +SHA-256), confirming the fix is a visual no-op — exactly what the BSDF +math predicts since `1 * black == 0 * white == 0`. + +**Retirement condition.** Same as MAX-MAT-001: when Autodesk fixes +`MtlxIOUtil` to stop emitting the spurious emission pair, the pass +becomes inert. Safe to keep as a guard for older 3ds Max installs. + +## Expressions with no MaterialX equivalent + +| Source (3ds Max) | Why no equivalent | Behavior in current fork | +| --- | --- | --- | +| (none catalogued yet) | | | + +## Change log + +* 2026-06-20 — Initial doc. MAX-MAT-001 specular_rotation default + normalization landed. +* 2026-06-20 — MAX-MAT-002 emission/emission_color default-pair + normalization landed (strip `emission = 1.0` paired with + `emission_color = (0, 0, 0)`). diff --git a/src/Tests/Integration/mtlxShaderWriter_test.ms b/src/Tests/Integration/mtlxShaderWriter_test.ms index 2ed45a7..5b49574 100644 --- a/src/Tests/Integration/mtlxShaderWriter_test.ms +++ b/src/Tests/Integration/mtlxShaderWriter_test.ms @@ -200,13 +200,115 @@ struct mtlx_shader_writer_test assert_equal ((shaderPrim.GetAttribute "info:id").Get()) "ND_open_pbr_surface_surfaceshader" ), + -- MAX-MAT-001 regression: PhysicalMaterial export must not carry the + -- spurious specular_rotation = 0.25 default that 3ds Max's MtlxIOUtil + -- bridge writes on every standard_surface. The post-parse normalization + -- in MtlxShaderWriter strips the value when specular_anisotropy is zero + -- (or absent), restoring the MaterialX nodedef default. + function test_export_physical_material_strips_buggy_specular_rotation = ( + maxver = maxVersion() + -- PhysicalMaterial MaterialX export path is 2025+. + if maxver[1] < 26900 then ( + return 0 + ) + + local teapot001 = teapot name:"teapot001" + local mtl1 = PhysicalMaterial() + mtl1.name = "matNoAniso" + -- Anisotropy explicitly off (this is also the PhysicalMaterial default). + mtl1.anisotropy = 0.0 + teapot001.material = mtl1 + + local exportOptions = USDExporter.CreateOptions() + exportOptions.FileFormat = #ascii + exportOptions.AllMaterialTargets = #("MaterialX") + + local exportPath = output_prefix + "testPhysicalMaterialNoAniso.usda" + USDExporter.ExportFile exportPath exportOptions:exportOptions + local stage = pyUsd.Stage.Open(exportPath) + + local surfacePrim = stage.GetPrimAtPath("/root/mtl/matNoAniso/matNoAniso") + assert_true (surfacePrim.IsValid()) + assert_true (surfacePrim.IsA(pyShade.Shader)) + assert_equal ((surfacePrim.GetAttribute "info:id").Get()) "ND_standard_surface_surfaceshader" + + -- The bug: bridge writes specular_rotation = 0.25 on every shader. + -- After normalization the input must either be absent (cleanest) or + -- explicitly 0.0 (the nodedef default). Both states fix the bug. + local surfaceShader = pyShade.Shader(surfacePrim) + local rotInput = surfaceShader.GetInput "specular_rotation" + if rotInput != undefined then ( + local rotAttr = rotInput.GetAttr() + if rotAttr.HasAuthoredValue() then ( + assert_equal (rotInput.Get()) 0 + ) + ) + ), + + -- MAX-MAT-002 regression: PhysicalMaterial export must not carry the + -- spurious emission = 1.0 paired with emission_color = (0, 0, 0) that + -- 3ds Max's MtlxIOUtil bridge writes on every standard_surface. The + -- post-parse normalization in MtlxShaderWriter strips both inputs when + -- the buggy pair is detected, falling back to the MaterialX nodedef + -- defaults (emission = 0.0, emission_color = (1, 1, 1)). + function test_export_physical_material_strips_buggy_emission_default = ( + maxver = maxVersion() + -- PhysicalMaterial MaterialX export path is 2025+. + if maxver[1] < 26900 then ( + return 0 + ) + + local sphere001 = sphere name:"sphere001" + local mtl1 = PhysicalMaterial() + mtl1.name = "matNoEmission" + -- A default PhysicalMaterial has no emission authored. The bridge + -- bug surfaces regardless of this setting on the source side. + sphere001.material = mtl1 + + local exportOptions = USDExporter.CreateOptions() + exportOptions.FileFormat = #ascii + exportOptions.AllMaterialTargets = #("MaterialX") + + local exportPath = output_prefix + "testPhysicalMaterialNoEmission.usda" + USDExporter.ExportFile exportPath exportOptions:exportOptions + local stage = pyUsd.Stage.Open(exportPath) + + local surfacePrim = stage.GetPrimAtPath("/root/mtl/matNoEmission/matNoEmission") + assert_true (surfacePrim.IsValid()) + assert_true (surfacePrim.IsA(pyShade.Shader)) + assert_equal ((surfacePrim.GetAttribute "info:id").Get()) "ND_standard_surface_surfaceshader" + + local surfaceShader = pyShade.Shader(surfacePrim) + + -- After normalization the buggy pair is stripped. Either the input + -- is absent (cleanest), or it explicitly carries the nodedef + -- default (emission = 0.0, emission_color = (1, 1, 1)). Both + -- states fix the bug; what must NOT be present is the buggy pair. + local emInput = surfaceShader.GetInput "emission" + if emInput != undefined then ( + local emAttr = emInput.GetAttr() + if emAttr.HasAuthoredValue() then ( + assert_equal (emInput.Get()) 0 + ) + ) + local emColorInput = surfaceShader.GetInput "emission_color" + if emColorInput != undefined then ( + local emColorAttr = emColorInput.GetAttr() + if emColorAttr.HasAuthoredValue() then ( + assert_equal (emColorInput.Get()) (pyGf.Vec3f 1 1 1) + ) + ) + ), + function teardown = ( ), Tests = #( test_export_physical_material, test_export_OpenPBR_material, - test_export_material_leading_digit_name + test_export_material_leading_digit_name, + test_export_physical_material_strips_buggy_specular_rotation, + test_export_physical_material_strips_buggy_emission_default ) ) diff --git a/src/translators/MtlxShaderWriter.cpp b/src/translators/MtlxShaderWriter.cpp index 93ef14d..7d1e587 100644 --- a/src/translators/MtlxShaderWriter.cpp +++ b/src/translators/MtlxShaderWriter.cpp @@ -36,8 +36,171 @@ #include +#include +#include + PXR_NAMESPACE_OPEN_SCOPE +namespace { + +// Returns the static float value of a MaterialX input if it is not connected. +// On a connection (node/nodegraph/output), reports the input as "not statically known". +bool _TryGetStaticFloat(const MaterialX::InputPtr& input, float& outValue) +{ + if (!input) { + return false; + } + if (input->hasNodeName() || input->hasNodeGraphString() || input->hasOutputString()) { + return false; + } + const std::string& valStr = input->getValueString(); + if (valStr.empty()) { + return false; + } + try { + outValue = std::stof(valStr); + } catch (...) { + return false; + } + return true; +} + +// Parses a MaterialX color3 value string ("r, g, b" or "r g b") when the +// input is not connected. Tolerates commas, semicolons, and whitespace +// between components so we read whatever the MaterialX serializer wrote. +// Returns false on a connection, on a value string with fewer/more than 3 +// numeric components, or on any parse failure. +bool _TryGetStaticColor3( + const MaterialX::InputPtr& input, + float (&outValue)[3]) +{ + if (!input) { + return false; + } + if (input->hasNodeName() || input->hasNodeGraphString() || input->hasOutputString()) { + return false; + } + const std::string& valStr = input->getValueString(); + if (valStr.empty()) { + return false; + } + // Replace any non-numeric separators with spaces so a single istringstream + // walk extracts the three components regardless of MaterialX serialization + // style. + std::string normalized; + normalized.reserve(valStr.size()); + for (char c : valStr) { + if (c == ',' || c == ';' || c == '\t' || c == '\n') { + normalized.push_back(' '); + } else { + normalized.push_back(c); + } + } + std::istringstream iss(normalized); + int parsed = 0; + for (int i = 0; i < 3; ++i) { + if (!(iss >> outValue[i])) { + return false; + } + ++parsed; + } + // Reject trailing tokens (e.g. a fourth component) so we don't silently + // accept color4 strings as color3. + std::string trailing; + if (iss >> trailing) { + return false; + } + return parsed == 3; +} + +// MAX-MAT-002 workaround: 3ds Max's MtlxIOUtil bridge emits emission = 1.0 +// paired with emission_color = (0, 0, 0) on every standard_surface node, +// regardless of whether the source PhysicalMaterial authored any emission. +// The MaterialX standard_surface nodedef defaults are emission = 0.0 and +// emission_color = (1, 1, 1). The buggy pair multiplies to (0, 0, 0) so it +// has no visual effect, but it is semantically wrong: any downstream +// override that bumps emission_color away from black would unexpectedly +// turn emission on at full strength. Strip both inputs when they exactly +// match the buggy pattern so the exported USD falls back to the nodedef +// defaults (which also evaluate to zero emission, with the right meaning). +// Connected inputs and any non-matching values are left untouched. +void _NormalizeStandardSurfaceEmissionDefault(const MaterialX::DocumentPtr& doc) +{ + if (!doc) { + return; + } + for (const auto& node : doc->getNodes("standard_surface")) { + auto emissionInput = node->getInput("emission"); + auto emissionColorInput = node->getInput("emission_color"); + if (!emissionInput || !emissionColorInput) { + continue; + } + float emissionValue = 0.f; + if (!_TryGetStaticFloat(emissionInput, emissionValue)) { + continue; + } + if (std::fabs(emissionValue - 1.0f) > 1e-6f) { + continue; + } + float emissionColor[3] = { 0.f, 0.f, 0.f }; + if (!_TryGetStaticColor3(emissionColorInput, emissionColor)) { + continue; + } + if (std::fabs(emissionColor[0]) > 1e-6f + || std::fabs(emissionColor[1]) > 1e-6f + || std::fabs(emissionColor[2]) > 1e-6f) { + continue; + } + node->removeInput("emission"); + node->removeInput("emission_color"); + } +} + +// MAX-MAT-001 workaround: 3ds Max's MtlxIOUtil bridge emits +// specular_rotation = 0.25 on every standard_surface node, regardless of the +// source PhysicalMaterial's anisotropy settings. The MaterialX standard_surface +// nodedef defaults specular_rotation to 0.0, and the value has no visual effect +// when specular_anisotropy is zero. Strip the spurious value so the exported +// USD matches the nodedef default for the common (anisotropy == 0) case. +// Inputs that are connected, non-zero, or carry a non-0.25 authored value are +// left untouched. +void _NormalizeStandardSurfaceSpecularRotation(const MaterialX::DocumentPtr& doc) +{ + if (!doc) { + return; + } + for (const auto& node : doc->getNodes("standard_surface")) { + auto rotationInput = node->getInput("specular_rotation"); + if (!rotationInput) { + continue; + } + float rotationValue = 0.f; + if (!_TryGetStaticFloat(rotationInput, rotationValue)) { + continue; + } + // Match the buggy 3ds Max hardcoded value (0.25), tolerant of float noise. + if (std::fabs(rotationValue - 0.25f) > 1e-6f) { + continue; + } + // Only strip when specular_anisotropy is provably zero (or absent). + // If anisotropy is connected, the user may genuinely want a rotation, + // so we conservatively keep the input. + auto anisotropyInput = node->getInput("specular_anisotropy"); + if (anisotropyInput) { + float anisotropyValue = 0.f; + if (!_TryGetStaticFloat(anisotropyInput, anisotropyValue)) { + continue; + } + if (std::fabs(anisotropyValue) > 1e-6f) { + continue; + } + } + node->removeInput("specular_rotation"); + } +} + +} // namespace + bool _IsWellFormedPath(const fs::path& p) { try { @@ -520,6 +683,9 @@ void MtlxShaderWriter::Write() return; } + _NormalizeStandardSurfaceSpecularRotation(mtlxDoc); + _NormalizeStandardSurfaceEmissionDefault(mtlxDoc); + // Sanitize the material name using the same logic as with the MaterialX component // to match the node name produced by the MaterialX exporter. createValidName // does not guard against leading digits.