-
Notifications
You must be signed in to change notification settings - Fork 8
refactor: Ran formatting on fragment shaders #334
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
base: main
Are you sure you want to change the base?
Conversation
|
Should we re-run this and get it merged? Or just close it... |
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.
Pull request overview
This PR applies automated code formatting to all fragment shader files (.frag) to establish consistent style. The main changes involve standardizing whitespace, removing the f suffix from float literals, and reformatting function signatures and control flow statements to follow a consistent pattern.
Key changes:
- Removed
fsuffix from float literals (e.g.,1.0f→1.0) - Standardized function signature formatting with parameters on separate lines
- Normalized whitespace and spacing in expressions and control flow statements
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/constants/shaders/volumePick.frag |
Applied formatting to function signatures, spacing, and removed float literal suffixes |
src/constants/shaders/slice.frag |
Standardized whitespace, control flow formatting, and expression spacing |
src/constants/shaders/raymarch.frag |
Applied consistent formatting to function signatures and control flow statements |
src/constants/shaders/pathtrace_denoise.frag |
Removed float literal suffixes and standardized function signature formatting |
src/constants/shaders/pathtrace.frag |
Comprehensive formatting including float literal suffixes, function signatures, and control flow statements |
src/constants/shaders/fuseI.frag |
Standardized function signature formatting |
src/constants/shaders/fuseF.frag |
Standardized function signature formatting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Thanks for the reminder! Checking this, will reopen it in a bit! |
| float r = sqrt(max(0.f, 1.f - z*z)); | ||
| float phi = 2.f * PI * U.y; | ||
| vec3 getUniformSphereSample(in vec2 U) { | ||
| float z = 1. - 2. * U.x; |
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.
did the formatter know it was running on GLSL code? what language does it think it was formatting? Just double checking when i see the removal of the .f suffixes on these floating point literals.
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.
Yes! I'm using this extension here: https://marketplace.visualstudio.com/items?itemName=raczzalan.webgl-glsl-editor
https://wikis.khronos.org/opengl/Data_Type_(GLSL)#Literals
A numeric literal that uses a decimal is by default of type float. To create a float literal from an integer value, use the suffix f or F as in C/C++.
Floats can also be defined using exponential notation, using the e or E to separate the base from the exponent. The exponent is always base-10.
In GLSL 4.00 and above, double-precision floats are available. By default, all floating-point literals are of type float. To create a double-precision float, use the lf or LF suffixes. This will force it to the double type.
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.
I think it's automatically removing them because not every version of GLSL supports f suffixes. If we included a version directive in the shader I think it would add them back in.
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.
ah, I believe threejs might auto-insert a version directive
toloudis
left a comment
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.
I guess this is better than not formatting!
Problem
I ran the formatter on all the
.fragshaders, so edits to them in the future won't incur huge diffs.Screenshots
Shaders still work: