Enhancements to the JME3 Rendering Engine

Development Engine Development
162
  • The integration is not done at all. See all stuff hardcoded into RenderManager.java

“all…” Just link every code in that file that is “all” for you and propose solution

  • This PR add methods/types that are not used in any code

Not linked related code

  • Everything in the FrameGraph package is poorly documented

Would be better to suggest changes per exact line, what exactly in documentation per lines you do not like. You want us guess or what?

  • Naming inside the shaders (If i have to look up a java file and a project desription to see whats inside a texture it is bad to maintain)

Again. Zero related names or code. Everyone can just say “this code names are bad” without pointing out even examples. Maybe someone could just agree/disagree with you if you could even just give example for this. Even more, explaining why this specific name is bad or not.
Maybe it is possible to guess, but code check should not force peopel to guess what exact namings you mean.

  • It offers no more flexibility than the current backend. (Only moar lights)

Only this point is fine, since you explained here in HUB what you mean, tho it should also be mentioned in github.

  • The api is far from stable and v2 in his repository is quite different than this

Same like some random guy will say “JME code is crap! everything is unstable! fix it!”
Professional would be comments per line of code you feel are reason for non-stability of code.

about “his repository is quite different than this” there was already reply to that i seen.

  • The public api added trough this PR it will break code once it hopefully gets integrated cleanly.

Again again again… Again… Where specific reason? What code exactly break? Why hide it?

  • The current implementations skips still important topics (shadows/post processing)

Again… Should PR add more features related to shadows/post processing or what? need more info like in every point you said.

What code exactly make you thinking this PR cant skip shadow/post processing topic.
We do not read on your mind you know.

If shadows are for example broken in this PR, say it.

  • The usual formatting issues

when you seen other people Code checks, they usually point out even specific formatting issues, why you dont?

You might know, but main issue for Code Check is like “guess what i think and what i refer to”

It is possible to guess, but code check should list exact code and even fix proposals when able.

Just someone answered your comment and ofc i see “i guess” there.

163

Most of the stuff added in RenderManager.java

Everything in Viewport for example

There is zero doc in that package


#if __VERSION__ >= 120
layout(location = 0) out vec4 outColor0;
layout(location = 1) out vec4 outColor1;
layout(location = 2) out vec4 outColor2;
layout(location = 3) out vec4 outColor3;
layout(location = 4) out vec4 outColor4;
#define Context_OutGBuff0 outColor0
#define Context_OutGBuff1 outColor1
#define Context_OutGBuff2 outColor2
#define Context_OutGBuff3 outColor3
#define Context_OutGBuff4 outColor4
#else
#define Context_OutGBuff0 gl_FragData[0]
#define Context_OutGBuff1 gl_FragData[1]
#define Context_OutGBuff2 gl_FragData[2]
#define Context_OutGBuff3 gl_FragData[3]
#define Context_OutGBuff4 gl_FragData[4]
#endif


uniform sampler2D m_Context_InGBuff0;
uniform sampler2D m_Context_InGBuff1;
uniform sampler2D m_Context_InGBuff2;
uniform sampler2D m_Context_InGBuff3;
uniform sampler2D m_Context_InGBuff4;
#define Context_InGBuff0 m_Context_InGBuff0
#define Context_InGBuff1 m_Context_InGBuff1
#define Context_InGBuff2 m_Context_InGBuff2
#define Context_InGBuff3 m_Context_InGBuff3
#define Context_InGBuff4 m_Context_InGBuff4

so what does that shader do?

Not in the sense that it breaks code, but in the sense that the integration is done differently in the next version

Everything added in the RenderManager.java again will probably get removed again.
All setters of the various techniques have to go to some VarSinks at some point

Because i am not going to comment on every god damn whitespace

164

Most of the stuff added in RenderManager.java

i still dont see difference from this 2, exept now its most not all.
while anyway “hardcoded” is not same as “The integration is not done at all.”

Also what exactly is hardcoded for you, there are settable fields there, variables no constants.
or maybe you just mean to make some protected not private?
You see, hardcoded can mean a lot, i would like see example you mean.
If its related to some method contents that should be done more elastic/extendable way, then tell exactly which ones.

Also if you mean your:

if(viewport.getFg!=null){
renderWithFG
}else{
renderTheOldWay
}

then its more about topic that was discussed, if author should also support old way or not.

There is only “renderPath” variable related, so again why not just comment specific code?
And anyway it probably could be added in later stage, ofc.
Proper comment should be
“110 line of Viewport change(github allow you comment specific line) → unused, proposal: wait with this for 2.0 PR”
This is how Code check is done.

True, i think @JhonKkk will add docs for it. So here you might no need refer to anything specific, right.

you want check my Deferred knowledge here or what.
So even more - you should explain which names are bad and which would be better instead of this provocation.

Author already explained this topic before as i remember.

now:

And this is how code check comment should look, well done!

So now at least someone can refer to this, fixing or explaining.

Depend on amount. If you say there are just too many of this, usually provide just 1-2 examples and refer to rest to code having same/similar.

But ofc best if there would be auto-formatter so imo its not important topic. More important would be adding auto-formatter or “format verification step” on commit. This should be real topic in general imo.(ofc not here, and it was little discussed already - John also made a comment about this)

1 Like
165

I’m sorry I’m late, I read through the discussions on github and the relevant information in this post. I made relevant replies on github. Here I will reply to some relevant contents again:

For this PR, I first tried to integrate the basic FG framework, render pipeline, old JME3 code, and make it work out of the box for users in SimpleApplication with almost no modifications needed. Of course, I assumed most JME3 users would not do secondary development based on the source engine, meaning most people do game development based on application layer code like SimpleApplication. So I could confidently adjust and gradually refactor in subsequent feature PRs (as long as I keep SimpleApplication needing no modifications, it should allow most users to easily use the new features, right? Please point out if I’m wrong, thanks);
This PR is about the render pipeline (but it also contains the basic FG code, but FG is not externally usable in this PR). Maybe @zzuegg hopes SimpleApplication could use FG interfaces in this PR? I will consider @zzuegg ‘s suggestions to expose FG interfaces for use in SimpleApplication, but I’m not sure if I should add this code in this PR or in another FG PR (I planned to provide the feature of external FG interface usage in the multi-pass PR). If @zzuegg hopes to access FG interfaces in SimpleApplication in this PR, I can submit this part of the code here, but I haven’t completed FG yet, would it be allowed for me to complete FG gradually in this and future PRs?
Could this PR provide the application layer interface that is finally used by SimpleApplication with custom FG, but allow me to improve FG internal logic subsequently (as long as I keep the application layer FG interface unchanged for SimpleApplication, it won’t break users’ SimpleApplication, right?)

5 Likes
166

Thank you for pointing it out. I will spend time this weekend to write relevant documentation, and annotate the meaning of those intermediate artifact codes. This is indeed an omission on my part.

3 Likes