Fix swizzle edge cases in version upgrade #1957
Merged
Conversation
This file contains 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
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
nadult
commented
Aug 1, 2024
nadult
commented
Aug 1, 2024
|
@jstone-lucasfilm sorry for the formatting errors. I actually thought that the code was properly formatted, because I'm using an automatic formatting plugin in Visual Studio based on clang-format. But it turns out, that I'm using a newer version (16) than the version which is used in MaterialX (13 I guess?). Unfortunately even with the same .clang-format configuration file, there are slight differences in formatting between different versions... |
In this case, I think it's fine to omit the new unit test for these specific swizzle patterns, following the approach we use for validating earlier version upgrades. If there are production examples of materials leveraging these swizzle patterns, it would be fine to include them in our test suite with other version upgrade examples: https://github.com/AcademySoftwareFoundation/MaterialX/tree/main/resources/Materials/TestSuite/stdlib/upgrade Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
jstone-lucasfilm
approved these changes
Aug 8, 2024
jstone-lucasfilm
changed the title
jstone-lucasfilm
merged commit 3ec6e87
into
AcademySoftwareFoundation:main
34 checks passed
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.
This PR fixes 2 edge cases when upgrading materials from version 1.38 to 1.39 involving swizzle nodes:
Tests are also added to cover these cases.