Modularize Shaders

I have been browsing through the new module code. Currently for me it adds another layer of complexity since i have to have open even more files to follow the flow. During that i noticed that you implemented structs. So here is kind of a introduction to modern rendering techniques, and why i do not like the current struct. I know it is a WIP and an early test and a whole lot of work so consider this as food for the brain and not critic to the work done.

When designing the data model for the shaders there are two goals:

  • Reusable code
  • Decoupling the data-source from the data the shaders work on. Usually to actually be able to have reusable code.

Now, when you design the data structures you should also take into account ā€œwhere the data gets generatedā€, ā€œcan the data be reused?ā€, ā€œhow long is the data validā€

As i can see in the PBRSurface code you already noticed that because you grouped the values:

    #struct StdPBRSurface
        // from geometry
        vec3 position; // position in w space
        vec3 viewDir; // view dir in worldSpace
        vec3 geometryNormal; // normals w/o normalmap
        vec3 normal; // normals w/ normalmap
        bool frontFacing; //gl_FrontFacing
        float depth;

        // from texture
        vec3 albedo;
        float alpha;
        float metallic;              // metallic value at the surface
        float roughness;
        vec3 diffuseColor;
        vec3 specularColor;
        vec3 fZero;
        vec3 ao;
        float exposure;
        vec3 emission;


        // computed
        float NdotV;

        // from env
        vec3 bakedLightContribution; // light from light map or other baked sources
        vec3 directLightContribution; // light from direct light sources
        vec3 envLightContribution; // light from environment 

        float brightestLightStrength;
    #endstruct
    #define PBRSurface StdPBRSurface 

No consider, i we split the struct into different ones.

first, extract the cam information:

struct CameraInfo{

}

then we could extract separate SurfaceInfo since that shold be reusable between different materials.

struct CameraInfo{

}

struct SurfaceInfo{

}

struct MaterialInfo{

}

struct FrameInfo{

}

struct GeometryInfo{

}

ultimately we probably need FrameInfo too. That would hold the frame info like tpf …
if we want to support instancing we might need another struct GeometryInfo.

Now let’s forget about the MaterialInfo for a while, because that one likely is going to hold material specific stuff, but keep in mind that we have that.

Now lets get to the reason why i would prefer a such fine grained data.

Start considering the validity time and reusability between different shaders and materials.

FrameInfo is produced by the cpu, updated each frame, and reusable between all shaders. This makes it a good candidate to pass the parameters as uniform buffer. One api interaction per frame and we are good.

CameraInfo is also valid for a render pass at least, as a matter of fact, you need access to not only the current camera in use, but for other cameras too when applying shadow maps. So we probably and up with something like:

struct CameraInfo{

}

layout (std140) uniform CameraInfos{
    CameraInfo camera[NR_OF_CAMERAS];
};

So from the above listed struct’s, SurfaceInfo might be the only one that is actually produced on the fragment shader and passed to later stages, all other are ultimately just input decouplers for the fragment shader.

Why all that?

Decoupling the GeometryInfo allows for instancing as we have it currently.
Decoupling and passing the MaterialInfo trough a UBO allows for instancing with different material parameters.
Ultimately in the drawindirect methods from gl4.6 you have to preload everything.
And then for each object you want to draw you only have to upload very little data.

struct{
 int geometryIndex; //Access to geometryInfo[geometryIndex];
 int cameraIndex; //Access to cameraInfo[cameraIndex];
 int materialIndex; //Access to materialInfo[materialIndex];
}

Since you preloaded everything it allows you to render everything that shares the same VAO and shares the same shader variant in one drawcall.

I am very bad at explaining things, so i hope it does at least make a little bit of sense

4 Likes

Yeah that is the one unfortunate side effect of this refactoring, between the glsllibs and struct approach, it ends up requiring a lot of swapping back and forth between files to work with the PBR fragment shader now.

Do you think it would help if the .glsllibs and pbr struct were located in a different place or had more straightforward names? I originally had 2 glsllibs with more obvious names: PBRTextureReads.glsllib and PBRLightCalculations.glsllibs (or something along those lines), i think now its called PBRLightingUtils which I personally think is more obscure

Also I should mention that most of the struct related stuff int hat PR was added by @RiccardoBlb so there may be a few things with that that I don’t have as good of a grasp on. And he might have some more insightful things to say in regards to your proposed changes too.

My original knee jerk reaction was to dislike the struct system, and I personally didn’t mind leaving all the variable declarations and uniforms in the .frag file instead of using a struct. I already felt like my original changes (which only split up the texture reading and lighting calculation into 2 .glsllibs by using basic inOut functions) was enough to solve the issues I’ve ran into. But I also see the benefit of the struct approach too, so as of now I’m personally okay with doing it either way.


I think I understand what you’re saying about adjusting the struct system, to pretty much just split the current StdPBRSurface struct up into a few different structs, and then also add the capability to have more structs for places beyond the .frag shader? If I’m following correctly then I think that sounds like a good approach for modularizing things even more. It would just need to be done carefully so we don’t overcomplicate things too much.

That has been my main concern, that all this modularization and refactoring could end up making it very difficult for someone to look at and understand how to fork or debug the pbr shader unless they are already fairly experienced in shaders. But maybe I’m wrong idk, I’m curious to hear more opinions on whether all the added complexity is worth it or if jme users feel like this will make them less likely to delve into forking or troubleshooting the PBR shader.

Yeah, currently with the uber struct you are limiting reusability.
Let the pbr light calculation work only on data that they actually need. PBRMaterial struct and SurfaceData struct. SurfaceData is generated on the vertex shader and could be send to the frag shader without knowing that it’s pbr. then phong lighting would of course require PhongMaterial struct.

Then, the various vertex operations could also work on the same data and can be reused across different shaders

1 Like

The work on the modular shader was just to make maintaining and creating new pbr based shaders easier by creating an idea of an extensible PBRSurface with some required fields that the dev has to fill in. So that as long as you can provide a PBRSurface and PBRLight, you can reuse the code.
Eg. you want triplanar mapping? You just create 3 surfaces, no need to rewrite the lightning logic at all.

That said, not only i 101% agree with everything in your post, it is also my personal preference when it comes to implementing this kind of shaders. It’ll probably need to be tailored to the way jme work to not overwhelm people trying to migrate their shaders, but i like the idea very much.

2 Likes

I’m close to finishing adapting the modular PBRSurface.glsl and PBRLightingUtils.glsllib to work with the PBR Terrains.

I didn’t modify too much in those 2 files, mostly just cleanup and minor refactoring to make those modular parts work with the terrain shaders, and then I added a new PBRTerrainUtils glsllib that does all the unique terrain param reading and assigns the final pbr values to the PBRSurface.


I also made some improvements to the new ā€œsun exposureā€ param (name still pending) that can be used to simulate smooth transitions in the directionalLight while going in and out of caves and indoor areas.

I’ll try to put a simple example with a model using this feature in the future, but here’s an old gif I have that shows how I use this in my project for smooth lighting going into dark caves or buildings

This screenshot shows how I painted the model’s vertex colors to prepare it:

and here’s how the directional lighting smoothly transitions at the cave entry, but the PointLight remains un-affected and still lights up the dark parts of the the cave:
https://i.imgur.com/M1XexXe.gif

I’ve also updated it so you can now also use either the vertex colors or a gray-scale texture to represent a model’s ā€œexposureā€. This new param is essentially just a secondary AmbientOcclusion value that is used exclusively for dimming the DirectionalLight.

And, of course, similar results could be achieved without this new feature, simply by attaching caves to a node with separate lights, but then you don’t get the benefits of smooth lighting when transitioning between the dark cave and the bright outdoors.


I also notice that Lighting.j3md uses this glsllib for some basic in-shader fog:

But it was never added to PBRLighting.frag. I personally use the FogFilter in my game, but if there’s any interest in this in-shader fog, I could add it to the PBR shader as well.

6 Likes

Also in regards to this idea:

I still think this is a good idea, but I wanted to get used to using the structs and get everything working with the current basic PBRSurface in the terrain shader first, so I didn’t start splitting up the PBRSurface struct yet in the big PR I’m about to submit. Then once this PR is reviewed and the foundation is finalized we can continue working on modularizing things even more.

2 Likes

I submitted a PR that adapts PBRLightingUtils and PBRSurface to be able to work with PBRTerrain shaders, and also added the new exposure param and fog to PBRLighting:

Review and feedback is appreciated. But do not feel rushed to review everything right away, there will still be more changes to this modular system while its in infancy, at least until 3.8 reaches stable. And I plan to merge this PR fairly soon so it can start being tested in the next alpha release. So any delayed feedback or review will still have plenty of time to be addressed.

2 Likes

Hi @yaRnMcDonuts
Could you please leave a comment before merging code, similar to how @sgold does it? Mentioning that you’ll proceed with merging in 48 or 72 hours if no revisions are requested would be great. This gives everyone visibility into your plans and the remaining time for suggestions. It’s important for community collaboration and getting help. Thanks!

2 Likes

Sorry if it seems like this PR moved fast, I did mention on github that I’d be merging it very soon and that I will be making more future PRs to address reviews and further modularize the new shader system before 3.8 reaches stable.

But in the future, I will make sure to give a time frame or a last call prior to merging something.

I have done a last call like that for most PRs I’ve merged so far. However in this case I felt it was an urgent PR that only primarily needed tested for functionality and then passed quickly to fix a bunch of severe bugs with the struct implementation that were overlooked in riccardos original PR that caused some of my specGloss models and models without normal maps to crash.

It turned out that much of my original work from that PR was accidentally reverted and lots of my code was broken and some even deleted when he overwrote my PR to add the struct system, so this PR was a big collection of bug fixes and restoring things that I thought had mostly already been discussed over the past year.

I also thoroughly tested it as much as I could on my own device to make sure the crashes were gone before merging it. I just wanted to get the bugs fixed and merged for a new alpha release asap, before anyone else runs into these bugs while trying to help test the new modular design in the current alpha release that’s out there.

So I hope it does not appear that I rushed this PR to avoid feedback and review, and I apologize if I was not clear enough in regards to why the PR was urgent in my original post.

I just did not want to let the extra discussion in regards to the API delay an urgent PR to fix a crash. I also did my best to split as much of my non-urgent changes as possible into second PR that I did not merge yet, and will still be adding onto and requesting more in-depth reiveiw on: Per-Layer TriPlanar and Refactoring for PBR Terrain Shaders by yaRnMcDonuts Ā· Pull Request #2353 Ā· jMonkeyEngine/jmonkeyengine Ā· GitHub

I hope this does not make you feel discouraged to leave reviews or feedback on anything. I very much appreciate the review and feedback, I just did not want to dedicate that particular PR to in-depth review since it was urgent to fix a few bugs that managed to slip through the big PR that riccardo and I collaborated on.

2 Likes

I plan to merge this PR in the next 24-36 hours so that the bug fixes to PBRLighting and the new modular terrain shader can both start being tested in the next alpha release asap.

This PR only modularizes AdvancedPBRTerrain.frag, not PBRTerrain.frag yet. After I have determined that there are no issues during testing and am sure that everyone is okay with my approach, then I will apply the same approach to PBRTerrain.frag.


In the meantime, any review on this file (PbrLightingUtils.glsllib) is also appreciated, as this is the file where nearly all of the functionality for PBRLighting.j3md now takes place.

This file contains params and texture reads for PBRLighting, as well as lighting calculation for any all PBR shaders that use the new PBRSurface struct

PBRLightingUtils.glsllib was originally based on my modular design, then riccardo adapted it to use structs and added a few more useful things, then he passed it back to me to test and finalize everything. I have tested it heavily by this point, however PBRLightingUtils is still the most likely to contain mistakes that were made in the collaboration process as well as areas where the modular design could potentially still be further improved.