Skip to content
This repository was archived by the owner on Nov 20, 2025. It is now read-only.

Add hex interpolator#121

Merged
davesmith00000 merged 1 commit intoPurpleKingdomGames:mainfrom
dariusj:hex-interpolator
Dec 18, 2024
Merged

Add hex interpolator#121
davesmith00000 merged 1 commit intoPurpleKingdomGames:mainfrom
dariusj:hex-interpolator

Conversation

@dariusj
Copy link
Copy Markdown
Contributor

@dariusj dariusj commented Nov 27, 2024

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

@davesmith00000
Copy link
Copy Markdown
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
Comment thread ultraviolet/shared/src/main/scala/ultraviolet/syntax.scala Outdated
Comment thread ultraviolet/shared/src/test/scala/ultraviolet/SyntaxTests.scala Outdated
Comment thread ultraviolet/shared/src/test/scala/ultraviolet/SyntaxTests.scala Outdated
Comment thread ultraviolet/shared/src/test/scala/ultraviolet/SyntaxTests.scala
Comment thread ultraviolet/shared/src/main/scala/ultraviolet/syntax.scala Outdated
Copy link
Copy Markdown
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!

Comment thread ultraviolet/shared/src/main/scala/ultraviolet/syntax.scala Outdated
Comment thread ultraviolet/shared/src/main/scala/ultraviolet/syntax.scala Outdated
Comment thread ultraviolet/shared/src/test/scala/ultraviolet/SyntaxTests.scala Outdated
Comment thread ultraviolet/shared/src/test/scala/ultraviolet/SyntaxTests.scala Outdated
Comment thread ultraviolet/shared/src/test/scala/ultraviolet/SyntaxTests.scala
Comment thread ultraviolet/shared/src/test/scala/ultraviolet/macros/ShaderMacrosTests.scala Outdated
@davesmith00000
Copy link
Copy Markdown
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!

Comment thread ultraviolet/shared/src/main/scala/ultraviolet/macros/CreateShaderAST.scala Outdated
Comment thread ultraviolet/shared/src/main/scala/ultraviolet/syntax.scala Outdated
davesmith00000
davesmith00000 previously approved these changes Dec 17, 2024
Copy link
Copy Markdown
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.

Comment thread ultraviolet/shared/src/main/scala/ultraviolet/macros/CreateShaderAST.scala Outdated
Comment thread ultraviolet/shared/src/main/scala/ultraviolet/syntax.scala Outdated
@davesmith00000
Copy link
Copy Markdown
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
@dariusj dariusj deleted the hex-interpolator branch December 18, 2024 14:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants