Skip to content

Add integer signature to floor and ceil#1362

Merged
jstone-lucasfilm merged 3 commits intoAcademySoftwareFoundation:mainfrom
sideeffects:integer_floor_ceil
May 26, 2023
Merged

Add integer signature to floor and ceil#1362
jstone-lucasfilm merged 3 commits intoAcademySoftwareFoundation:mainfrom
sideeffects:integer_floor_ceil

Conversation

@crydalch
Copy link
Copy Markdown
Contributor

Adds support to ceil and floor to output integers. There will be other tickets/discussions around improving integer support across MaterialX, but adding integer output to these nodes first would be a big help to some nodegraph contributions we'd like to contribute soon.

Thanks!

@crydalch
Copy link
Copy Markdown
Contributor Author

crydalch commented May 23, 2023

Locally it builds, but it's got some test failures because implementationUseCount is not matching count, in GenShaderUtil. I'm not expecting it to pass CI here, but I'm not sure why the count is off in the first place, so maybe it'll just work? :)

@jstone-lucasfilm
Copy link
Copy Markdown
Member

This looks really promising, thanks @crydalch, and I believe we just need to add examples of these new nodes to our unit tests, since they're new primitives with their own unique source code. I should be able to help set this up, and I'll take a look when I have a moment.

@crydalch crydalch mentioned this pull request May 24, 2023
Copy link
Copy Markdown
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 @crydalch!

@jstone-lucasfilm jstone-lucasfilm merged commit 99ebdf5 into AcademySoftwareFoundation:main May 26, 2023
Michaelredaa pushed a commit to Michaelredaa/MaterialX that referenced this pull request Oct 21, 2023
Adds support to ceil and floor to output integers. There will be other tickets/discussions around improving integer support across MaterialX, but adding integer output to these nodes first would be a big help to some nodegraph contributions we'd like to contribute soon.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants