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.

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

Add hex interpolator #121

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

dariusj
Copy link
Contributor

@dariusj dariusj commented Nov 27, 2024

Add hex, hexa, rgb, and rgba custom interpolators.

@davesmith00000
Copy link
Member

(This work relates to issue #94.)

Good to see you are the hack event yesterday!

So to get this over the line, I think we need the following:

  • Ensure the code works for vec3's and vec4's, i.e. hex"#FF00FFFF" ends up as vec4(1.0f, 0.0f, 1.0f, 1.0f).
  • Standard issue simple unit tests for both to prove it.
  • An (can be just one) acceptance test with both of them in it, that produces the expected GLSL. There's a pattern that's easy enough to follow I think. I'd expect syntax like vec4(hex"#FF00FF", 1.0f) to produce vec4(vec3(1.0,0.0,1.0),1.0). If it doesn't, we'll need to work out why not. Fair warning, if things are going to unexpectedly go sideways, this is where it'll happen, so don't struggle with it for too long if it's not happening for you - I'll help! 😄

I'm very happy if the work ends there, but the follow on piece is the same thing but for something like: rgb"255,0,0", and rgba"255,0,0,255".

@dariusj dariusj force-pushed the hex-interpolator branch 2 times, most recently from e0dfe4e to fa82f84 Compare November 28, 2024 23:28
Copy link
Member

@davesmith00000 davesmith00000 left a comment

Choose a reason for hiding this comment

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

Great work! 💪

I've tried to answer your comments, give me a shout if you need help!

@davesmith00000
Copy link
Member

When you get back to this @dariusj, can you please sync your fork and rebase. I've fiddled with the build a bit. Thanks!

davesmith00000
davesmith00000 previously approved these changes Dec 17, 2024
Copy link
Member

@davesmith00000 davesmith00000 left a comment

Choose a reason for hiding this comment

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

Looks great! Assuming the build passes I'm happy to get this merged. I have left a couple of comments on the error messaging, but they aren't show stoppers.

@davesmith00000
Copy link
Member

If you're happy, you could take this out of draft?

@dariusj dariusj marked this pull request as ready for review December 17, 2024 16:53
@davesmith00000 davesmith00000 merged commit a61041e into PurpleKingdomGames:main Dec 18, 2024
1 check passed
@dariusj dariusj deleted the hex-interpolator branch December 18, 2024 14:39
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