attempt to simplify the phong shaders and lights.

[bhouston]

This is an attempt to clean up the lighting code. I got this far -- and I think the results are beautiful:

https://github.com/bhouston/three.js/blob/simplified_lighting/src/renderers/shaders/ShaderChunk/lights_phong_fragment.glsl

[bhouston]

These functions would also be amazing for hiding the use of a texture map to access the light parameters. :) #7060

[tschw]

Could scan for your muncrions with a regex, then scan for a matching brace, substituting all newlines with spaces on the way...

Why did they have to omit the \ functionality from the GLSL preprocessor?!!

[tschw]

muncrions

As I just read you on the other PR. Not meant to insult your submission. I really like what you are doing! 👍
Been looking at the code first and thought you might have written a preprocessor already. I'd be great if it worked out of the box, though.

[bhouston]

Old preprocessor, not merged, is here: #6273

[bhouston]

I think the solution is to move towards arrays of structs for lights. This WebGL conformance test for uniform struct arrays seems to work on IE, Firefox, Chrome and on the mobile devices I've tested. I'll give this a go.

https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/misc/shader-with-array-of-structs-uniform.html?webglVersion=%5Bobject%20Object%5D

[tschw]

Old preprocessor, not merged, is here: #6273

Why did it not get merged? Too heavy? I guess it could have been a giant step forward at that time. By now, I think we should aim straight for the graphs on that front, however.

This project is the WebGL library, so it deserves something more flexible / powerful than glsify. What we build will be the standard.

These functions would also be amazing for hiding the use of a texture map to access the light parameters. :) #7060

I do like the idea of Muncrions...

I think the solution is to move towards arrays of structs for lights

How do they scale? Did you check the spec on their packing rules? Will they automatically address #7037, maybe ?

[bhouston]

Why did it not get merged? Too heavy? I guess it could have been a giant step forward at that time. By now, I think we should aim straight for the graphs on that front, however.

I think it is still good to get a preprocessor into ThreeJS. The same preprocessor can be used by a graph-based system if the graph-based system is modular.

The main reason it didn't get merged is that I have a ton of projects on the go and can not consistently contribute to ThreeJS unless my work objectives are well aligned with my ThreeJS contributions.

I am pretty sure that any shader graph that doesn't allow for other types of shaders to co-exist will not get merged.

I think if a graph-based system is built correctly (@unconed's is a good example) the shader graphs can create ThreeJS shaders that work side by side with shaders created in different fashions -- allowing both types of shaders to continue to evolve. There is tons of existing shaders around and we shouldn't break compatibility.

It may be worthwhile at some point to figure out a modular design for a graph-based system so that you do not get a lot of opposition once it is presented as a PR. Notice how I proposed the complete Animation System revamp first before coding it and got feedback and incorporated it. This avoids pulling a "fait accompli" on everyone.

[bhouston]

@tschw This PR now allows WebGLRenderer to support structs and arrays of structs for uniforms, which allowed me to use the nicer functions for light access.

[tschw]

Is this really such a good idea, instead of accumulating outgoingLight right away?

A smart GLSL compiler will optimize it right. Less smart (true ES, on-target) compilers may take the code more literally, in this case, spilling registers to slower memory, limiting the number of parallel threads, or both.

[bhouston]

Yes, it is critical that they are accumulated separately this way. Remember you are looking at a work in progress. I still need to extend the usage of this division in the accumulation outside of just this *.glsl file, but I am doing it incrementally so I have working code at all the intermediate steps.

When one wants to incorporate SAO, SSAO, aoMaps, one needs to dim only the indirect components, and sometimes only indirect diffuse. In a later commit, I have introduced a ReflectedLight struct to make this a bit easier to deal with.

[WestLangley]

After looking at the entire PR, I am thinking this structure may be cleaner:

struct ReflectedLight {
    vec3 indirectDiffuse;
    vec3 indirectSpecular;
    vec3 directDiffuse;
    vec3 directSpecular;
};

ReflectedLight reflectedLight

It can be changed later if you agree.

[mrdoob]

I agree 😛

[WestLangley]

@bhouston My overall impression is what you have done is a significant improvement. I support your objective.

However, I wish you had implemented your idea without changing the models.

There are significant problems here with MeshPhysicalMaterial, and your attempt to make the code plug-and-play has made it very difficult to follow, IMO. Large parts of MeshPhysicalMaterial have been removed. IMO, the function names need to be improved for clarity. I will try to think of something.

[mrdoob]

I'm merging this by now.

@WestLangley fell free to add back the removed code when you get a chance. We should have more than enough time to get this stable for r74 😊

Thanks everyone!

[mrdoob]

@bhouston webgl_loader_sea3d_hierarchy looks different after this PR. Do you mind taking a look?

Before:

After:

[bhouston]

@mrdoob, I have fixed the Sea3D loader here: #7487 They were doing a search and replace based on the old shader code.

[bhouston]

Also thanks for merging the code! Wohoo!

[mrdoob]

@bhouston Seems like we lost SpotLight's angle.

Could you take a look when you get a chance?

[mrdoob]

I was researching #7359 (comment) and I have tracked it down to this PR.

Seems like MeshPhongMaterial is now broken on Android.

https://dl.dropboxusercontent.com/u/7508542/temp/bug.html

Desktop:

Android (Nexus 5):

Any ideas? No errors in the console.

iOS seems fine...

I hope is not something like Android not supporting structs 😟

[bhouston]

Two things. One we can run conformance tests on andriod to find out if there is support:

https://github.com/KhronosGroup/WebGL/blob/master/sdk/tests/conformance/glsl/misc/shader-with-array-of-structs-uniform.html

Two, didn't he identify the error:

#7359 (comment)

I'll check this later today.

[bhouston]

I'm reproducing the bug with no error messages on Android. Interestingly, Lambert variations work - which uses light structs but in the vertex shader.

[bhouston]

WebGL conformance tests for arrays:

https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/misc/shader-with-array-of-structs-containing-arrays.html?webglVersion=1&quiet=0
https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/misc/shader-with-attrib-struct.vert.html?webglVersion=1&quiet=0
https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/misc/shader-with-array-of-structs-uniform.html?webglVersion=1&quiet=0

And they all pass on my Nexus 5. And they use fragment shader struct arrays.

[MasterJames]

Passing those tests I'll try to go through them but I'm not sure what I'm looking for here I get one fail on the phone I screen grabbed the original seemingly lightess problem.
https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/bugs/array-of-struct-with-int-first-position.html

fragment_shader translated for driver
FAIL at (532, 0) expected: 0,127,0,255 was 255,255,255,255

[bhouston]

@MasterJames I solved the issue: #7556

[MasterJames]

Okay awesome!

[Zob1]

Just curious, will this process make the transition to lights on layers easier or harder?

Sent from my iPhone

On Nov 8, 2015, at 7:15 PM, Master James notifications@github.com wrote:

Okay awesome!

—
Reply to this email directly or view it on GitHub.