In D3205#78192, @Clément Foucault (fclem) wrote:

In D3205#78183, @Kanzaki Wataru (kanzwataru) wrote:

@Clément Foucault (fclem)

What should I do for the ifdef? There doesn't seem to be anywhere else in the code that detects if EEVEE is used or not.

Well after searching, it seems these #ifdef were removed at some point so I believed it's not usefull to add them back now.

So I think I'll accept the patch as is.

Good job and thank you for your contribution :) will commit this tomorrow after final review.

• Added SSS to node_shadertorgb

What should I do for the ifdef? There doesn't seem to be anywhere else in the code that detects if EEVEE is used or not.

In D3205#78174, @Kinouti Takahiro (dskjal) wrote:

I think that ssrToggle in gpu_shader_material.glsl is unnecessary.

Why don't you move cubemap code into if clause. This avoid normal decode and transform when no need to do that.

• Removed Split Shader and Smooth BSDF (to be implemented as node groups in a NPR add-on)
• Fixed up Shader to RGBA to not throw away screen-space data
• Moved fallback_cubemap() to a more shared place to be used by Shader to RGBA

In D3205#78068, @Clément Foucault (fclem) wrote:

In D3205#78057, @Kanzaki Wataru (kanzwataru) wrote:

Thanks for the hint, but I admit this SSR stuff is out of my league. From what I gather, the SSR reflections/refractions are a post-process effect? I'm not sure how I can use the fallback_cubemap(). I understand that in theory I need to calculate the SSR and bake it into the output in node_shadertorgb(), but I'm having some trouble seeing the big picture.

I don't want you to do the SSR stuff but only to compute the fallback lighting when doing the conversion in node_shadertorgb().

The fallback function can be shared by putting it somewhere like lightprobe_lib.glsl.

fallback_cubemap basically evaluate all reflections probes. So you want to evaluate them with what should have been the SSR data sent to the post process pass.

In D3205#78024, @Clément Foucault (fclem) wrote:

In D3205#78002, @Kanzaki Wataru (kanzwataru) wrote:

@Clément Foucault (fclem)

So I took a look at fallback_cubemap() and how it evaluates the screen space inputs, but it seems that it depends on a lot of things that get calculated in the main() function. Is there a way to split this out into its own function so that it can be cleanly called from the Shader to RGBA node without duplication?

void fallback_cubemap(vec3 N, vec3 V, vec3 W, vec3 viewPosition, float roughness, float roughnessSquared, inout vec4 spec_accum)

V is world space view vector.
W is world space position.
viewPosition is view space position.
All of these are just varyings attributes converted (or not) to world space, but you do need to recompute them yes.

N is world space normal vector.
You just need to decode the normal from the SSR parameters and convert to world space orientation.

roughness and roughnessSquared can be derived from the SSR inputs.

There will be no redundancy because all the inputs needs to precomputed from different sources.

So I took a look at fallback_cubemap() and how it evaluates the screen space inputs, but it seems that it depends on a lot of things that get calculated in the main() function. Is there a way to split this out into its own function so that it can be cleanly called from the Shader to RGBA node without duplication?

In D3205#75839, @Clément Foucault (fclem) wrote:

In D3205#75797, @Kanzaki Wataru (kanzwataru) wrote:

Are the first two nodes not to be accepted at all? Or would they be fine if the implementation is fixed? I guess the challenge is implementing NPR stuff in a PBR engine without breaking things...

I think it would be better to distribute them as groupnode inside an addon once the closure to rgba conversion is handled.

In D3205#75791, @Clément Foucault (fclem) wrote:

I really like the results. However I have concerns about the implementation.

The first 2 nodes can be done with relatively simple nodegroups. They break PBR badly but still use Closures as inputs and outputs.
Closures are not to be treated as simple RGBA that's the whole point of having the conversion.
Also they won't work in Cycles.
So I'm not really ok with adding them. We need to keep things somewhat consistent and predictable. Your nodes don't suggest they break PBR and loose the screenspace datas.

For the Closure to RGBA conversion see my comment.

Resubmitted here: https://developer.blender.org/D3205

In D3190#75549, @Clément Foucault (fclem) wrote:

In D3190#75529, @Kinouti Takahiro (dskjal) wrote:

@Clément Foucault (fclem)
Would you imprement correct Closure to RGBA node for Eevee?

I will, if no one claims it. So let me know.

In D3190#75529, @Kinouti Takahiro (dskjal) wrote:

@Clément Foucault (fclem)
Would you imprement correct Closure to RGBA node for Eevee?