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

Refactor shaders to utilize shared .glslibs between multiple rendering engines #2122

Open
yaRnMcDonuts opened this issue Oct 19, 2023 · 1 comment

Comments

@yaRnMcDonuts
Copy link
Contributor

yaRnMcDonuts commented Oct 19, 2023

Following up on this discussion:

https://hub.jmonkeyengine.org/t/optimize-terrain-shaders-for-tri-planar-mode-per-layer-remove-fog-defines-to-make-room/46816/43

After this PR: #2090 introducing a new rendering engine is merged into jme, we will have a lot of identical shader code that can be refactored into .glslibs to make things easier to maintain.

For example, much of the code of PBRLighting.frag and PBRLightingGBufferPack.frag is the same code doing the same texture reads prior to the lighting calculations. So this could be put into something like PBRLighting.glslib. And same for Lighting.frag and all of the terrain frag shaders with their GBufferPack equivalent in the new rendering engine.

This will also make it easier for jme devs to fork the existing stock jme shaders without having to constantly update local code when the shader is updated on master. Instead of making changes to PBRLighting.frag and everyone having to update their local fork, we can just make changes to PBRLighting.glslib and then most forks using the glslib will automatically adopt the changes with no extra effort.

@JohnLKkk
Copy link
Contributor

Following up on this discussion:

https://hub.jmonkeyengine.org/t/optimize-terrain-shaders-for-tri-planar-mode-per-layer-remove-fog-defines-to-make-room/46816/42

After this PR: #2090 introducing a new rendering engine is merged into jme, we will have a lot of identical shader code that can be refactored into .glslibs to make things easier to maintain.

For example, much of the code of PBRLighting.frag and PBRLightingGBufferPack.frag is the same code doing the same texture reads prior to the lighting calculations. So this could be put into something like PBRLighting.glslib. And same for Lighting.frag and all of the terrain frag shaders with their GBufferPack equivalent in the new rendering engine.

This will also make it easier for jme devs to fork the existing stock jme shaders without having to constantly update local code when the shader is updated on master. Instead of making changes to PBRLighting.frag and everyone having to update their local fork, we can just make changes to PBRLighting.glslib and then most forks using the glslib will automatically adopt the changes with no extra effort.

I agree with this suggestion. JME3 already has the concept of glsllib, so encapsulating reusable code can help better maintain shaders.

yaRnMcDonuts added a commit to yaRnMcDonuts/jmonkeyengine_nrs that referenced this issue Feb 2, 2024
Addresses this issue: jMonkeyEngine#2122

To re-summarize:

This PR extracts all of the texture reads and basematParam assignment into a .glslib file (currently named PBRLightingParamReads.glsllib)

And then it also extracts all of the lighting calculation into a .glslib file (currently named PBRLighting.glsllib)

I also fixed alot of formatting and indentation mistakes, and overall reorganized the shader.

This should now make it as easy as possible for jme devs to fork PBRLighting to make changes, and they will no longer have to update the lighting or texReads to stay on par with master, as all future changes will all be in the glsllibs. This also reduces the chances that a lesser-skilled shader dev (or even skilled shader devs on a bad day) mistakenly mess up the lighting calculations when forking the shader.

Any feedback and review is greatly appreciated.
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

No branches or pull requests

2 participants