Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NodeMaterials: Remove or clarify usage of inherited varyings/uniforms from base materials #24410

Closed
robertlong opened this issue Jul 30, 2022 · 2 comments

Comments

@robertlong
Copy link
Contributor

robertlong commented Jul 30, 2022

Is your feature request related to a problem? Please describe.

Currently the node materials inject default uniforms and varyings from their base material (ShaderMaterial) and from their main shader node (Physical, Standard, Basic, etc.). This ends up adding a bunch of uniforms and varyings that may or may not be used and no clear way on how we might use them>

From the generated code in the webgl_nodes_materials_standard example:

// Three.js r143 - NodeMaterial System

// <node_builder>

// uniforms
uniform sampler2D nodeUniform0;
uniform vec3 nodeUniform1;
uniform sampler2D nodeUniform4;
uniform sampler2D nodeUniform5; 

// attributes



// varyings
varying vec2 nodeVarying0;
varying vec4 nodeVarying1;
varying vec3 nodeVarying2;
varying vec3 nodeVarying3;
varying vec3 nodeVarying4; 

// vars

vec3 nodeVar0;
vec2 nodeVar1;
vec3 nodeVar2;
vec2 nodeVar3;
vec3 nodeVar4;
vec4 nodeVar5;
vec3 nodeVar6;
float nodeVar7; 

// codes



// </node_builder>

#define STANDARD
#ifdef PHYSICAL
    #define IOR
    #define SPECULAR
#endif
uniform vec3 diffuse;
uniform vec3 emissive;
uniform float roughness;
uniform float metalness;
uniform float opacity;

// ...

#if defined( USE_COLOR_ALPHA )
    varying vec4 vColor;
    #elif defined( USE_COLOR )
    varying vec3 vColor;
#endif
#if ( defined( USE_UV ) && ! defined( UVS_VERTEX_ONLY ) )
    varying vec2 vUv;
#endif
#if defined( USE_LIGHTMAP ) || defined( USE_AOMAP )
    varying vec2 vUv2;
#endif
#ifdef USE_MAP
    uniform sampler2D map;
#endif
#ifdef USE_ALPHAMAP
    uniform sampler2D alphaMap;
#endif
#ifdef USE_ALPHATEST
    uniform float alphaTest;
#endif
#ifdef USE_AOMAP
    uniform sampler2D aoMap;
    uniform float aoMapIntensity;
#endif
#ifdef USE_LIGHTMAP
    uniform sampler2D lightMap;
    uniform float lightMapIntensity;
#endif
#ifdef USE_EMISSIVEMAP
    uniform sampler2D emissiveMap;
#endif

Describe the solution you'd like

I'd really like a way to be able to properly define and reference these uniforms/varyings. Perhaps nodes like UniformNode and AttributeNode can check to see if there is already a defined uniform/attribute and reuse those?

Alternately, these uniforms/varyings could only be user defined. And I think there's a decent case to be made that this is the better option.

#if ( defined( USE_UV ) && ! defined( UVS_VERTEX_ONLY ) )
    varying vec2 vUv;
#endif
#if defined( USE_LIGHTMAP ) || defined( USE_AOMAP )
    varying vec2 vUv2;
#endif

// ...

#ifdef USE_AOMAP
    float ambientOcclusion = ( texture2D( aoMap, vUv2 ).r - 1.0 ) * aoMapIntensity + 1.0;
    reflectedLight.indirectDiffuse *= ambientOcclusion;
    #if defined( USE_ENVMAP ) && defined( STANDARD )
        float dotNV = saturate( dot( geometry.normal, geometry.viewDir ) );
        reflectedLight.indirectSpecular *= computeSpecularOcclusion( dotNV, ambientOcclusion, material.roughness );
    #endif
#endif

The issue I'm currently trying to solve with node materials is being able to dynamically assign which uv gets used for each texture so that i can properly support glTF's texCoord property (See #12608). In the above snippet, you can see that because the node material is based on the other material, it pulls in the same assumptions about which uvs to use for each texture. This means, even if I wanted to write a node shader that dynamically assigned which uv set to use per texture, I couldn't for these maps.

I think NodeMaterial may be better off extending RawShaderMaterial and using a simplified ShaderLib to get rid of these assumptions and allow for maximum developer flexibility.

Describe alternatives you've considered

Perhaps this is already possible for uniforms with the MaterialNode? I really can't tell how that works.

Additional context

None

@sunag
Copy link
Collaborator

sunag commented Jul 30, 2022

The issue I'm currently trying to solve with node materials is being able to dynamically assign which uv gets used for each texture so that i can properly support glTF's texCoord property...

Perhaps this is already possible for uniforms with the MaterialNode? I really can't tell how that works.

Have you tried something like?

import { texture, uv } from 'three-nodes/Nodes.js';

const map = new THREE.TextureLoader().load( 'map.jpg' );

/* 0=uv, 1=uv2 */
nodeMaterial.colorNode = texture( map, uv( 1 ) );

Currently the node materials inject default uniforms and varyings from their base material (ShaderMaterial) and from their main shader node (Physical, Standard, Basic, etc.). This ends up adding a bunch of uniforms and varyings that may or may not be used and no clear way on how we might use them>

In most cases these defaults defines like USE_MAP of Three.js will never be used because if the user uses material.colorNode shouldn't use material.map. NodeMaterial will always replace the default behavior related. So this pattern below will not be processed by the GPU:

#ifdef USE_MAP
    uniform sampler2D map;
#endif

The clean version of NodeBuilder is being developed based on the WebGPURenderer but could be absorbed into the WebGLRenderer as well at some point.

@robertlong
Copy link
Contributor Author

Yeah I think having NodeBuilder built on top of RawShaderMaterial in the future will solve this. I've moved away from this solution and am now just patching the ShaderChunks to get what I need.

The solution you pose above will work so long as you don't use any of the built in uniforms. This just becomes tough when trying to reproduce the behaviors of MeshStandardMaterial.

Anyway, I think this is a nice thing to have in the future. I think we should close this and whoever tries implementing MeshStandardMaterial on top of NodeMaterial in the future should ping me with their solution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants