-
Notifications
You must be signed in to change notification settings - Fork 195
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
fix(hlsl): emit constructor functions for arrays #2281
Conversation
7d52937
to
29a87be
Compare
Looks like CI is only failing on #2279. 😮💨👍🏻 |
A rebase should get it working. |
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.
LGTM, but let's add/extend one of the test snapshots with the code that was previously not getting translated correctly (from #2184)
98e63d5
to
35f5996
Compare
@teoxoy: Added a new test! I broke out a commit for adding a deliberately failing test case in HLSL, and then fixing it in the subsequent commit. Not sure if that's how you'd like to commit it, but I'm open to changing it if you want. 😊 |
35f5996
to
e8032b6
Compare
That's great for PRs - makes it a lot clearer when looking at the commit that fixes the issue the things that changed in the snapshots. But I don't think we should merge the commits separately though since if you'd bisect on the first commit, the tests will fail. |
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 stuff!
Fixes #2184.
Before: https://shader-playground.timjones.io/834690cc5ef4adbce8eebfbcaf721f76
After: https://shader-playground.timjones.io/0c093b24eb01d3974e7e5d130ed5b021