-
Notifications
You must be signed in to change notification settings - Fork 408
Fix dot node crash in code generation #2505
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
jstone-lucasfilm
merged 1 commit into
AcademySoftwareFoundation:main
from
kwokcb:fix_dot_crash
Jul 27, 2025
Merged
Fix dot node crash in code generation #2505
jstone-lucasfilm
merged 1 commit into
AcademySoftwareFoundation:main
from
kwokcb:fix_dot_crash
Jul 27, 2025
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jstone-lucasfilm
approved these changes
Jul 27, 2025
Member
jstone-lucasfilm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, @kwokcb, and this looks like a straightforward fix to include in 1.39.4.
91ffea0
into
AcademySoftwareFoundation:main
32 checks passed
ashwinbhat
pushed a commit
to autodesk-forks/MaterialX
that referenced
this pull request
Jul 28, 2025
Code attempts to use `in` on `dot` not without checking for existence on the node. It will not be exposed (added) as a uniform input if it's connected as is the case with the `dot` nodes used in the open_pbr to standard_surface conversion graph. Simply add in a firewall check for existence first.
zicher3d
added a commit
to autodesk-forks/MaterialX
that referenced
this pull request
Aug 27, 2025
commit c65288d Merge: 36eef1a b1ac08e Author: Ashwin Bhat <[email protected]> Date: Thu Aug 21 15:29:59 2025 -0700 Merge pull request #1444 from autodesk-forks/adsk-merge-branch Merge main into adsk_contrib/dev upto Aug 19, 2025 commit b1ac08e Merge: 36eef1a 131d9f5 Author: Ashwin Bhat <[email protected]> Date: Wed Aug 20 13:33:42 2025 -0700 Merge branch 'main' into adsk-merge-branch commit 131d9f5 Author: Jonathan Stone <[email protected]> Date: Tue Aug 19 08:08:56 2025 -0700 Improve UDIM support in viewer (AcademySoftwareFoundation#2507) This changelist improves the support for multi-UDIM materials in the MaterialX Viewer, allowing materials with explicit UDIM-associated names to be bound to implicit geometric UDIMs. Although explicit UDIM-associated names are not a recommended convention for multi-UDIM materials, they are common enough in studio pipelines that it's valuable to support them as an alternative. commit ec66ab3 Author: Jonathan Stone <[email protected]> Date: Tue Aug 19 07:46:22 2025 -0700 Fix grammar errors (AcademySoftwareFoundation#2524) This changelist fixes a handful of grammatical errors that were detected in text analysis of the MaterialX project. commit 36eef1a Author: zicher3d <[email protected]> Date: Mon Aug 18 12:04:06 2025 -0700 Additional input units (#1442) These inputs are unitless in OGS, so they were left unitless in the MaterialX graphs too. But they obviously need to have a unit to keep a consistent size if the scene unit is changed. Experiments in Inventor seem to suggest all these values are in Inches, but we will check them again once we can do a side-by-side comparison. Also renamed an input for Knurl, which was not consistent with the rest of the procedurals. commit c440c61 Author: JGamache-autodesk <[email protected]> Date: Fri Aug 15 14:58:50 2025 -0400 Assign source URI to generated OCIO NodeDefs (AcademySoftwareFoundation#2522) Also Implementations. Without a source URI these can not be distinguished from user created nodes and filtered out when writing. commit 7581aa5 Author: Lee Kerley <[email protected]> Date: Fri Aug 15 11:54:23 2025 -0700 Remove unneeded DocumentPtr from MDL ShaderGen (AcademySoftwareFoundation#2518) Remove unneeded ConstDocumentPtr being passed around inside the MDL shader generator. While poking around with some Shader Generator refactor experiments I noticed this argument not being used anywhere. commit d2beda7 Author: zicher3d <[email protected]> Date: Wed Aug 13 11:17:32 2025 -0700 USD compatibility updates (#1441) Changing some inputs to make the graphs more USD friendly. These changes will not affect any of the graphs functionality, they will just shift the default values somewhere else. Checked the unit tests and there are no changes needed there. commit 936e4bb Author: hybridherbst <[email protected]> Date: Fri Aug 8 01:50:08 2025 +0200 Move uniform updates to object scope in Web Viewer (AcademySoftwareFoundation#2446) The current handling of uniforms was per-material, which means that when a material is used on multiple objects, they all share uniforms – thus, they were all rendered with the uniforms of the last user of that material. Any multi-object files were affected. It just happened to work for the default shader ball because there all objects have the same pivot. As a side effect of this, the previous on-demand updating of uniforms (using `#_updateTransforms`) isn't possible anymore, so I removed the related code. commit ec218ae Author: JGamache-autodesk <[email protected]> Date: Thu Aug 7 15:20:03 2025 -0400 Introduce NanoColor names to default color management (AcademySoftwareFoundation#2513) For 1.39 we only remap to existing color spaces. Todo: For 1.40: - rename node defs/graphs to make nanocolor names the default - implement an upgrade path for legacy color spaces names - provide new nodegraphs for lin_ap0_scene, lin_rec2020_scene, and srgb_ap1_scene commit 7640c3e Author: zicher3d <[email protected]> Date: Mon Aug 4 14:24:02 2025 -0700 Adding legacy knurl (#1439) * Adding legacy knurl Adding nodedef and nodegraph for Legacy Knurl. This implements a generic knurl that is a superset, usable standalone, of the knurl used as a Metal pattern. The original knurl in RapidRT was not usable standalone. This solution is different and has more features, making it a worthwhile extra procedural to add to the library. A coupe of extra fixes are included - defaults for a couple of size parameters were incorrectly set to 0,0 after changing them to vector2. - A converter node from LookdevX was removed. It was unconnected, so no real change, and it might be recreated if that graph is re-exported. I'll try to keep an eye on it. * Adding diagonal option The diagonal option was in my original version, but was removed when I added separate X and Y scales, as the pattern was distorting and it was not necessary for matching the RRT version. But then I realized this was an important feature of Knurl in manufacturing, and spend some time trying to make it work again. This change adds just a few nodes and the feature is back, even if there is a small approximation in the normals direction when stretched. * Adding unit test for knurl Simple unit test commit ae521c4 Author: Luis Ehlen <[email protected]> Date: Mon Aug 4 17:12:33 2025 -0400 Disable 'Deploy Web Viewer' job (#1438) * Disable 'Deploy Web Viewer' job * Commenting out instead of deleting commit 8bcd1a0 Author: zicher3d <[email protected]> Date: Mon Aug 4 14:12:15 2025 -0700 Removing space input (#1440) * Removing space input The "space" input has been deprecated. To avoid issue we remove the input as the default value (tangent) is the desired one, so this will work with the current and the new nodedef. * Removing space also form translation graphs These graphs used "object" as the space input for normalmap. After checking the correct input should have been "tangent" and as such the inputs can be removed and rely on the default value too, and no space-conversion is necessary. commit 4d5a884 Author: Jonathan Stone <[email protected]> Date: Thu Jul 31 17:30:21 2025 -0700 Add CVEs from OSTIF review (AcademySoftwareFoundation#2511) This changelist adds the CVEs addressed during a recent OSTIF security review of MaterialX. commit 1c77369 Author: Johan Bowald <[email protected]> Date: Thu Jul 31 23:43:59 2025 +0200 Support anisotropy in Standard Surface to glTF PBR (AcademySoftwareFoundation#2440) Adds translations for specular_anisotropy and specular_rotation from `standardSurface` to `gltf_pbr` commit 91ffea0 Author: Bernard Kwok <[email protected]> Date: Sun Jul 27 17:29:02 2025 -0400 Fix dot node crash in code generation (AcademySoftwareFoundation#2505) Code attempts to use `in` on `dot` not without checking for existence on the node. It will not be exposed (added) as a uniform input if it's connected as is the case with the `dot` nodes used in the open_pbr to standard_surface conversion graph. Simply add in a firewall check for existence first. commit be1b675 Author: Jonathan Stone <[email protected]> Date: Thu Jul 24 14:11:19 2025 -0700 Add glTF anisotropy to changelog (AcademySoftwareFoundation#2502) commit 04cc053 Author: Jonathan Stone <[email protected]> Date: Wed Jul 23 13:00:33 2025 -0700 Additional changelog update (AcademySoftwareFoundation#2501) commit 8d9bf65 Author: Jonathan Stone <[email protected]> Date: Wed Jul 23 09:24:06 2025 -0700 Update changelog for release candidate (AcademySoftwareFoundation#2500) commit 5e4f4fc Author: Jonathan Stone <[email protected]> Date: Tue Jul 22 15:22:21 2025 -0700 Remove blur helper functions (AcademySoftwareFoundation#2498) Following up on AcademySoftwareFoundation#2411, this changelist removes the blur helper functions in GLSL, MSL, and OSL that are no longer used by the codebase. commit e81fbab Author: Lee Kerley <[email protected]> Date: Mon Jul 21 16:19:03 2025 -0700 Remove Blur node implementation (AcademySoftwareFoundation#2411) I'm submitting this PR - because I think it's the best we can do for now. I looked in to trying to find a portable robust implementation for this, and couldn't really come up with anything. I think it's better to be explicitly do nothing, than to do something that is confusing. MDL currently just passes through the `in` input as the output, so I'm using that as the template for the others as it seems like a reasonable fallback - implemented as a nodegraph that is now shared across all languages. commit 24cf7fc Author: JGamache-autodesk <[email protected]> Date: Mon Jul 21 11:05:25 2025 -0400 Fix MaterialXRender private dependency to OIIO (AcademySoftwareFoundation#2497) Also declare downstream dependencies when building static libraries. commit 531ded4 Author: Jonathan Stone <[email protected]> Date: Sun Jul 20 13:50:02 2025 -0700 Update JavaScript packages (AcademySoftwareFoundation#2496) This changelist updates all referenced JavaScript packages to their latest version via `npm update`. commit 0415e58 Author: Jonathan Stone <[email protected]> Date: Sun Jul 20 11:36:09 2025 -0700 Additional optimization to glTF PBR graph (AcademySoftwareFoundation#2493) This changelist implements one additional optimization to the graph for glTF PBR, ensuring that the render cost of the transmissive `dielectric_bsdf` node can be skipped for materials with no transmissive component. Performance tests were conducted on an NVIDIA RTX A6000 at 4K resolution, and the following timing improvements were seen: glTF PBR Carpaint: 11ms -> 8ms glTF PBR Plastic: 8ms -> 5ms glTF PBR Gold: 9ms -> 6ms glTF PBR Boombox: 4ms -> 3ms commit 9d31418 Author: JGamache-autodesk <[email protected]> Date: Sat Jul 19 21:00:05 2025 -0400 Modernize CMake handling of OIIO dependency (AcademySoftwareFoundation#2495) By asking CMake to write MATERIALX_BUILD_OIIO in the generated CMake files if MaterialXRender was built with OpenImageIO support. commit f46bf25 Author: Warren Moore <[email protected]> Date: Thu Jul 17 14:55:56 2025 -0700 Add guard to clean up Metal objects on exception (AcademySoftwareFoundation#2484) This PR addresses the crash reported in AcademySoftwareFoundation#2473. It introduces a scope guard object that automatically ends the current command encoder and command buffer if the `renderFrame` method exits abnormally due to an exception. Without such a precaution, the viewer crashes shortly after the exception is handled, because the encoder is released by its autorelease pool before `endEncoding` is called on it, causing a hard assert in Metal. commit bc3f8ea Author: Lee Kerley <[email protected]> Date: Thu Jul 17 14:09:29 2025 -0700 Replace OSL C++ Material node with data driven implementation (AcademySoftwareFoundation#2477) Remove OSL C++ Material node in favour of data driven implementation. Simplification of code export allows ignoring of node classification, which the OSL code generator doesn't need to emit the correct code. commit 0888099 Author: Jonathan Stone <[email protected]> Date: Thu Jul 17 13:17:03 2025 -0700 Update changelog for recent work (AcademySoftwareFoundation#2494) commit a6d5348 Author: Jonathan Stone <[email protected]> Date: Tue Jul 15 13:30:17 2025 -0700 Optimizations to Standard Surface graph (AcademySoftwareFoundation#2483) This changelist implements optimizations to the graph for Standard Surface, replacing `mix` operations on leaf BSDFs with pre-multiplied `add` operations. Pre-multiplied `add` operations take better advantage of dynamic branching in hardware shading languages, and should have a neutral or positive impact on software shading languages. Performance tests were conducted on an NVIDIA RTX A6000 at 4K resolution, and the following timing improvements were seen: Standard Surface Marble: 3ms -> 1ms Standard Surface Carpaint: 3ms -> 1ms Standard Surface Glass: 5ms -> 3ms Standard Surface Plastic: 2ms -> 1ms Standard Surface Brushed Metal: 2ms -> 1ms Standard Surface Chess Set: 9ms -> 4ms commit 0e12b19 Author: Pavlo Penenko <[email protected]> Date: Sun Jul 13 15:39:53 2025 -0400 Update punctuation in ShaderGeneration.md (AcademySoftwareFoundation#2490) Just a comma for better readability. commit 540c183 Author: Jonathan Stone <[email protected]> Date: Sat Jul 12 17:16:05 2025 -0700 Update changelog for recent work (AcademySoftwareFoundation#2491) commit 99c76dd Author: hybridherbst <[email protected]> Date: Sat Jul 12 19:13:48 2025 +0200 Keep original object and material names in Web Viewer (AcademySoftwareFoundation#2445) Previously, object names were taken directly from the three.js scene. However, three.js enforces specific constraints for object names (e.g. spaces, braces and so on are replaced with `_`). This renaming breaks DAG matching for geom expressions, as the compared name might not be right. three.js keeps the original name around though, so this PR makes sure the original name is used for path matching instead of the in-scene name. The PR also sets the material name properly from the `materialassign` instead of keeping it empty. commit c33985f Author: hybridherbst <[email protected]> Date: Sat Jul 12 03:56:08 2025 +0200 Fix assignment of geomprops in Web Viewer (AcademySoftwareFoundation#2471) The web viewer did assign vertex attributes no matter if they existed, leading to exceptions when a loaded file e.g. didn't have UV coordinates. I also added more properties – UV0, UV1, UV2, Color, Color1 – so that they can be leveraged in loaded materials. commit 08163d3 Author: tdavidovicNV <[email protected]> Date: Fri Jul 11 23:52:19 2025 +0200 Add minimum error threshold to render tests (AcademySoftwareFoundation#2482) Added an error (`-e`) parameter to `tests_to_html`. When set to any non-negative values, all tests with all of the RMS errors lower or equal to this threshold are skipped, producing html with only failing tests. It is off by default. commit bc2b2cf Author: Jonathan Stone <[email protected]> Date: Fri Jul 11 08:53:04 2025 -0700 Remove CI workaround for CMake 4+ (AcademySoftwareFoundation#2488) This changelist removes a workaround in GitHub CI for the requirements of CMake 4+, which have now been met by updating to a newer version of NanoGUI. commit 48766ca Author: tdavidovicNV <[email protected]> Date: Fri Jul 11 16:54:13 2025 +0200 Update NanoGUI version to support CMake 4+ (AcademySoftwareFoundation#2487) The bin2c.cmake requirement for cmake 2.8 caused the NanoGUI compilation to fail to configure with any CMake 4.x.y The newest NanoGUI fixes this, and the rest of the updates seems rather innocuous. Without this, MaterialXView does not compile in the latest vscode on Windows (and possibly elsewhere). commit 8b9d0fd Author: tdavidovicNV <[email protected]> Date: Thu Jul 10 23:15:13 2025 +0200 Add support for individual files in render tests (AcademySoftwareFoundation#2478) The `renderTestPaths` could only accept directories, and would run all tests within that directory. This modification allows adding `.mtlx` files as well, useful for debugging individual tests. commit 1248c41 Author: tdavidovicNV <[email protected]> Date: Thu Jul 10 22:00:03 2025 +0200 Revert TextureBakerPtr changes from AcademySoftwareFoundation#2301 (AcademySoftwareFoundation#2485) This reverts commit cf6b196 for reasons outlined in AcademySoftwareFoundation#2301 commit b74787d Author: Jonathan Stone <[email protected]> Date: Tue Jul 8 16:44:35 2025 -0700 Update changelog for recent work (AcademySoftwareFoundation#2481) commit d02b1e3 Author: Scot Brew <[email protected]> Date: Tue Jul 8 16:02:38 2025 -0500 Use shader code token replacement instead of #ifdef (AcademySoftwareFoundation#2476) Produce cleaner generated shader code by removing use of #ifdef HW_SEPARATE_SAMPLERS to handle both combined and separate texture samplers. This restores the generated shader code to look like it did before adding the ability to handle separate texture + sampler (for WGSL). This is an enhancement to more cleanly address the issue: * AcademySoftwareFoundation#2379 commit f832d96 Author: Jonathan Stone <[email protected]> Date: Tue Jul 8 13:28:33 2025 -0700 Optimizations to glTF PBR graph (AcademySoftwareFoundation#2467) This changelist implements optimizations to the graph for glTF PBR, replacing `mix` operations on leaf BSDFs with pre-multiplied `add` operations, and removing duplicate computations. Performance tests were conducted on an NVIDIA RTX A6000 at 4K resolution, and the following timing improvements were seen: glTF PBR Carpaint: 25ms -> 11ms glTF PBR Glass: 23ms -> 9ms glTF PBR Gold: 23ms -> 10ms glTF PBR Boombox: 11ms -> 4ms commit 657ef1d Author: tdavidovicNV <[email protected]> Date: Mon Jul 7 23:34:27 2025 +0200 Fixed a typo in GLSL rotation matrix used in hextiling (AcademySoftwareFoundation#2475) There is a typo in the GLSL rotation matrix used in hextiling. The 1st row, 2nd column should be `-a.z` instead of `-a.x`. See `mx_rotate_vector3.glsl` for the other axis rotation matrix implementation (or [`Rotation matrix from axis and angle` ](https://en.wikipedia.org/wiki/Rotation_matrix#Rotation_matrix_from_axis_and_angle) on wikipedia) commit 96bae48 Author: Lee Kerley <[email protected]> Date: Mon Jul 7 10:52:29 2025 -0700 Add `fractal2d` node (AcademySoftwareFoundation#2183) Mirroring the existing behavior for `<fractal3d>`, we add the corresponding `<fractal2d> node that uses the `texcoord` input as its manifold. Add support for OSL, MSL, GLSL, MDL (note : MDL not tested). Updated specification documents to add `<fractal2d>` to the specification and remove it from the proposals document. Also remove `period` inputs for the noises that don't have those inputs in the specification. This is already documented in the proposals document. Note : in the GLSL noise library code I renamed the existing `mx_fractal_noise_xxx()` functions to `mx_fractal3d_noise_xxx()` to make the differentiation cleaner with the new `mx_fractal2d_noise_xxx()`. But I retained wrapper functions with the old name that just call through to the new name for backwards compatibility. I was unsure how much custom customer code might be referencing those functions. We can remove the shim functions when we have another breaking API release. commit 2224621 Author: krohmerNV <[email protected]> Date: Mon Jul 7 19:47:16 2025 +0200 Fix MDL code gen after removing ClosureSourceCodeNode (AcademySoftwareFoundation#2465) In AcademySoftwareFoundation#2400 the ClosureSourceCodeNode has been removed and replaced by the regular SourceCodeNode. For that to work, the SourceCodeNode needs to emit dependent function calls to generate code for the inputs.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Code attempts to use
inondotnot without checking for existence on the node.It will not be exposed (added) as a uniform input if it's connected as is the case with the
dotnodesused in the open_pbr to standard_surface conversion graph.
Simply add in a firewall check for existence first.
Fixes #2504.
Test
Tried running with and without changes in MaterialXView. File loads and renders properly now.