[SOLVED] PBRLighting has two PostShadow Techniques?

Hi guys, sorry if I am asking a noob question!

I noticed there are two PostShadow Techniques in PBRLighting.j3md. (Looked at Lighting.j3md and it has just one)

Can someone explain the reason, please? Shouldn’t there be just one?

read this:

:slight_smile: in short: in PBR Transparency shadow was not working, and he done it with new file to avoid issues.

I think you got me wrong.

I mean

and this:

why? see this topic i linked, there is:

https://github.com/jMonkeyEngine/jmonkeyengine/pull/1265

i think its related? if you just want to know why there are 2 instead of 1, idk exactly, but @yaRnMcDonuts were recently touching this so it might be easier if he could explain.

to be specific:

both PBRLighting.j3md and Lighting.j3md Have

  • Technique PreShadow {
    and
  • Technique PostShadow {

The only difference is that PBR one have:

  • PreShadowPBR.frag
    and
  • PostShadowPBR.frag

The reason is first topic i wanted you to read it.
Reason was that PBR transparency for shadows was not working, so @yaRnMcDonuts fixed it, but he dupplicated files to avoid issues (well i also was against dupplicating files)

Sorry, I updated the Github permalink in my first post. (I wrongly pasted it from Ryan PR when I was reviewing his PR).

Yes this is what I want to know.

seconds before your post now i posted full explaination, well imo first topic i linked you is explaination but please read post above yours now :slight_smile:

ah sorry, now i noticed…

it just have dupplicated:

Technique PostShadow{

you dont mean frag/vert files.

I think it might be issue (almost dupplicated code issue)

Yes

There are more differences than just GLSL versions.

One even have SHADOWMAP_SIZE dupplicated.

But looking, it trully change in just GLSL versions so i think it could be just:
GLSL310 GLSL300 GLSL150 GLSL100
?

Or maybe someone know hidden reason of it.(i will search when it was added)

1 Like

After a little investigation.

When Nehon added PBR shader.

There were:

  • Technique PostShadow15{
    and
  • Technique PostShadow{

first was using:

    VertexShader GLSL150:   Common/MatDefs/Shadow/PostShadow15.vert
    FragmentShader GLSL150: Common/MatDefs/Shadow/PostShadow15.frag

second:

    VertexShader GLSL100:   Common/MatDefs/Shadow/PostShadow.vert
    FragmentShader GLSL100: Common/MatDefs/Shadow/PostShadow.frag

later in this commit:

changed first one (15 one) it to use:

    VertexShader GLSL150:   Common/MatDefs/Shadow/PostShadow.vert
    FragmentShader GLSL150: Common/MatDefs/Shadow/PostShadow.frag

the description contains:

Fixed the name of shadow shaders in PBRLighting.j3md, it has changed …

…since the merge with master

so i belive it has come from Merging some changes.

and later in:

he changed Technique name to same as second one. but i think it dont matter much here, because it was only about backface shadows. (passing BACKFACE_SHADOWS: BackfaceShadows)

So in summary main reason was:

Fixed the name of shadow shaders in PBRLighting.j3md, it has changed …

…since the merge with master

i think its not intended and probably after merge there can be just one Technique, Nehon probably just dont seen there are 2 of them. (initially PostShadow15 and PostShadow)

When i look now, there is still PostShadowFilter15 , but im not sure if this is used anywhere:

3 Likes

@Ali_RS you could create Github issue for this i think :slight_smile:

i dont see reason why need 2 of them. it more looks like merge with master made 2 of them.

But also please have in mind @yaRnMcDonuts changed it too, so to avoid again Technique dupplication, need to make sure Where(in which one) he added PBR frag file change.

1 Like

Yes, I think so. I guess this can even be confirmed in below commit:

See that the extra “PostShadow” Techniques are removed in the Lighting.j3md and Unshaded.j3md files:

1 Like

Yes, going to fill an issue for this.

Edit:
The issue is submitted:

1 Like

I wonder what happens when you define two techniques with the same name?
The second one will be picked?
Is this something to warn about or even throw an exception?

My understanding is that you can have more than one technique with the same name because some may support different GLSL versions.

…in that case, I imagine the code just uses the first one that matches or something.

1 Like

Unless someone is opposing, I may go ahead and make a patch to remove the extra technique (the one that uses GLSL100) and append GLSL100 to the other one, just like it was done in Lighting.j3md, but before that, I am going to merge PR 1265 to prevent a file conflict issue that is going to be introduced with new patch.

4 Likes

PR is ready in case anyone wants to review it

https://github.com/jMonkeyEngine/jmonkeyengine/pull/1273

I did a quick test with HoverTank model with both DirectionalLightShadowRenderer and DirectionalLightShadowFilter and seems working fine.

1 Like