-
Notifications
You must be signed in to change notification settings - Fork 63
Improve screen-space shaders, some minor clean-up #1593
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
Conversation
Wow, all those copy-pasted shaders looked like cargo cult. Thank you very much for deduplicating that! |
386e4c8
to
461af9a
Compare
Yeah, it's been on my mind for some time now, especially as I keep needing to either re-build existing shaders with different graphics settings, or create new ones. |
I've also added a couple more commits with some clean-up. |
6a89a29
to
e14fc72
Compare
7714ca5
to
5c823f8
Compare
GLSL doesn't build
|
5c823f8
to
e0f0efc
Compare
Should be fixed now. |
e0f0efc
to
10fc03d
Compare
I'm getting crashes when enabling bloom. |
Fixed now. |
Changes `GLShader` constructors, removing unnecessary stuff from them, and making it only have 2 distinct constructors - for rendering shaders and compute shaders. Changes the relevant shaders to use a new `screenSpace` shader, which is almost a no-op. NUKE the the other .glsl shaders that just repeat the same functionality. Also NUKES the `u_ModelViewProjectionMatrix` from them, because the result is always a full-screen quad anyway. Adds `Tess_InstantScreenSpaceQuad()` for use with this, because with some other shaders we still do need `u_ModelViewProjectionMatrix`.
Also NUKE `u_ModelViewProjectionMatrix` and `u_TextureMatrix`, because they have no effect there.
Fix screen-space shaders. Use a triangle IBO with vertexes being set directly in the vertex shader based on `gl_VertexID`. Re-order the shader stuff that was becoming a bit of a mess.
f0bf2a3
to
a67058e
Compare
Removed |
I got this with the
|
Fixed. |
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.
Requires #1586
Changes
GLShader
constructors, removing unnecessary stuff from them, and making it only have 2 distinct constructors - for rendering shaders and compute shaders.Changes the relevant shaders to use a new
screenSpace
shader, which is almost a no-op.NUKE the the other .glsl shaders that just repeat the same functionality.
Also NUKES the
u_ModelViewProjectionMatrix
from them, because the result is always a full-screen quad anyway.Adds
Tess_InstantScreenSpaceQuad()
for use with this, because with some other shaders we still do needu_ModelViewProjectionMatrix
. This actually draws a single triangle that gets clipped to the screen (this has a good explanation why).The screen-space shaders in this pr are ones that always process the whole screen, and where the vertex shaders were the same anyway, while sending the
u_ModelViewProjectionMatrix
uniform and multiplying the vertex position with it, and, indeed, even having a VBO for them at all, was useless.I didn't touch the
screen
shader here, although it really is just another screen-space shader. It is, however, also used forST_SCREENMAP
stage, which I don't think we use anywhere, and seems to be intended for use on something like an in-game camera screen. That wouldn't work properly though, due to how the texture coordinates are calculated (relative to the screen position of the fragment, not the world/model position). We could probably just use thegeneric
shader for that.Also cleaned-up the formatting and ordering of shaders/shader files.
This brings down the total amount of shader compilation, shader program linking (with #1587), and shader files.