Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Provide a frame transform for UV texture coordinates #1027

Merged
merged 5 commits into from
Aug 2, 2022
Merged

Provide a frame transform for UV texture coordinates #1027

merged 5 commits into from
Aug 2, 2022

Conversation

vernalchen
Copy link
Contributor

@vernalchen vernalchen commented Jul 11, 2022 •
edited
Loading

A proposed fix for #1021
Adding a new place2dframe node that changes the order of the operations from
place2d: -pivot, scale, rotate, translate, pivot
into
place2dframe: -pivot, translate, rotate, scale, pivot
test_place2d_out_glsl:
test_place2d_out_glsl
test_place2dframe_out_glsl:
test_place2dframe_out_glsl

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jul 11, 2022 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@vernalchen vernalchen mentioned this pull request Jul 11, 2022
Closed
francis-wangfr
francis-wangfr approved these changes Jul 12, 2022
View reviewed changes
Copy link

@francis-wangfr francis-wangfr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ashwinbhat
Copy link
Contributor

Hi @kwokcb please let us know if you see any issues with this new addition of place2dframe node.

kwokcb
kwokcb reviewed Jul 13, 2022
View reviewed changes
Copy link
Contributor

@kwokcb kwokcb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. I have some minor comments.

libraries/stdlib/stdlib_defs.mtlx Outdated Show resolved Hide resolved
libraries/stdlib/stdlib_ng.mtlx Show resolved Hide resolved
resources/Materials/TestSuite/stdlib/math/transform.mtlx Outdated Show resolved Hide resolved
@vernalchen vernalchen force-pushed the adsk_contrib/vernalchen/place2dframe branch from 50fdc1e to 944de5d Compare July 13, 2022 05:37
@vernalchen vernalchen force-pushed the adsk_contrib/vernalchen/place2dframe branch from 944de5d to e356b76 Compare July 13, 2022 06:55
kwokcb
kwokcb approved these changes Jul 14, 2022
View reviewed changes
Copy link
Contributor

@kwokcb kwokcb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, if Jonathan is good with this.

@francis-wangfr
Copy link

That sounds almost done @kwokcb, thank you！
Hi @jstone-lucasfilm, would you like to share your ideas and suggestions about this fix? Thank you very much!

jstone-lucasfilm
jstone-lucasfilm reviewed Jul 19, 2022
View reviewed changes
Copy link
Member

@jstone-lucasfilm jstone-lucasfilm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, thanks @vernalchen, and I've posted two minor questions to resolve.

libraries/stdlib/stdlib_ng.mtlx Outdated Show resolved Hide resolved
libraries/stdlib/stdlib_defs.mtlx Outdated Show resolved Hide resolved
@vernalchen vernalchen force-pushed the adsk_contrib/vernalchen/place2dframe branch from 13f319f to c8b2b3d Compare July 26, 2022 09:34
jstone-lucasfilm
jstone-lucasfilm reviewed Jul 26, 2022
View reviewed changes
Copy link
Member

@jstone-lucasfilm jstone-lucasfilm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, @vernalchen, though I'd like to get feedback from @dbsmythe on the proposed language for the new operationorder input. Once the language has been worked out, we'll likely want to copy it directly into the 1.39 specification.

resources/Materials/TestSuite/stdlib/math/transform.mtlx Outdated Show resolved Hide resolved
@vernalchen vernalchen requested a review from dbsmythe July 27, 2022 05:12
dbsmythe
dbsmythe reviewed Aug 1, 2022
View reviewed changes
Copy link
Contributor

@dbsmythe dbsmythe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!
Leaving to @jstone-lucasfilm to officially approve the changes.

jstone-lucasfilm
jstone-lucasfilm approved these changes Aug 2, 2022
View reviewed changes
Copy link
Member

@jstone-lucasfilm jstone-lucasfilm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, thanks @vernalchen!

@jstone-lucasfilm jstone-lucasfilm merged commit 52b5339 into AcademySoftwareFoundation:main Aug 2, 2022
@vernalchen vernalchen mentioned this pull request Aug 4, 2022
Merged
@vernalchen vernalchen deleted the adsk_contrib/vernalchen/place2dframe branch August 5, 2022 01:36
@vernalchen vernalchen mentioned this pull request Aug 5, 2022
Merged
Michaelredaa pushed a commit to Michaelredaa/MaterialX that referenced this pull request Oct 21, 2023
@vernalchen
Provide a frame transform for UV texture coordinates (AcademySoftware…
a5bbba2 
…Foundation#1027)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@francis-wangfr francis-wangfr francis-wangfr approved these changes

@kwokcb kwokcb kwokcb approved these changes

@dbsmythe dbsmythe dbsmythe left review comments

@jstone-lucasfilm jstone-lucasfilm jstone-lucasfilm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@vernalchen @ashwinbhat @francis-wangfr @jstone-lucasfilm @dbsmythe @kwokcb