WebGLRenderer: Remove material condition for unconditional uniforms.

[SoprachevAK]

Description

WebGLRenderer load default (unconditional) uniforms for not all built-in materials. This uniforms declare in common shader chunk and include for all built-in materials. Also material has onBeforeCompile callback witch

Useful for the modification of built-in materials.

If I completely legally modify built-in materials, I want to get access to default unconditional uniforms.

In WebGLRenderer before load some uniforms set conditional that check material type, and for some material not provide some uniforms. I am remove this conditionals.

This doesn't affect performance much, but will make the behavior more predictable.

[github-actions]

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev	Filesize PR	Diff
645.2 kB (159.9 kB)	644.9 kB (159.9 kB)	-282 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev	Filesize PR	Diff
438.4 kB (106 kB)	438.1 kB (106 kB)	-282 B

[Mugen87]

Do you mind demonstrating with a fiddle what use case is currently not possible? This will make it easier to understand the issue and how the PR fixes it.

[SoprachevAK]

I'm modify PointsMaterial shader, to convert model coordinates to worldPosition, after I do some manipulations in worldSpace, after i want convert worldSpace to screen gl_Position

const point = new BufferGeometry().setFromPoints([new Vector3(10, 10, 10)])
const material = new PointsMaterial({
  color: 0xff0000,
  size: 5,
  sizeAttenuation: false,
})

const points = new Points(point, material)
scene.add(points)

material.onBeforeCompile = (shader) => {
  shader.vertexShader = shader.vertexShader.replace('#include <worldpos_vertex>', /* glsl */`
  vec4 worldPosition = modelMatrix * vec4(transformed, 1.0);
  
  // Some manipulations in world space, for example multiply global Y to 0.1
  worldPosition.y *= 0.1;
  
  gl_Position = projectionMatrix * modelViewMatrix * inverse(modelMatrix) * worldPosition; // WORK
  gl_Position = projectionMatrix * viewMatrix * worldPosition; // NOT WORK
  `)
}

If I use classic way

gl_Position = projectionMatrix * viewMatrix * worldPosition;

to convert worldPosition to screenPosition, shader not working, because viewMatrix is always zero. Because WebGLRenderer not provide viewMatrix for built-in PointsMaterial

I know that modelViewMatrix = viewMatrix * modelMatrix and of course I can calculate viewMatrix = modelViewMatrix * inverse(modelMatrix), and use this instead viewMatrix:

gl_Position = projectionMatrix * modelViewMatrix * inverse(modelMatrix) * worldPosition;

and this work correctly. But this is weird, counterintuitive, and not optimal.

My fix removes the material check before passing viewMatrix uniform and loads it always for all materials as it is written in the documentation

[Mugen87]

I agree it's a bit unfortunate in context of viewMatrix.

Would you be okay to make this change just for the view matrix but not for the other two uniforms? There is a section called // common matrices further down the method where the view matrix update command could be moved.

[SoprachevAK]

I think all this uniforms must be load unconditional. This declare in global prefixVertex in WebGLProgram witch include for all non raw vertex shaders.

uniform mat4 modelMatrix;
uniform mat4 modelViewMatrix;
uniform mat4 projectionMatrix;
uniform mat4 viewMatrix;
uniform mat3 normalMatrix;
uniform vec3 cameraPosition;
uniform bool isOrthographic;

// common matrices section place outside if ( refreshProgram || _currentCamera !== camera ) block, but everything related to the camera should updated in this block. I'm moved all setValues to the common part inside refreshProgram block.

ps. WebGLUniforms.setValue already check uniform to existing. Additional check for cameraPosition need only for skip _vector3.setFromMatrixPosition( camera.matrixWorld ) for optimization purposes and this check not need to another uniforms

[Mugen87]

Just because they are declared does not mean a uniform update is required.

Sorry, I have to think about this change since we want to avoid uniform updates whenever possible. I would be okay to update the view matrix because it is listed in the documentation but not with the other ones.

[SoprachevAK]

All these matrices, except isOrthographic, are listed in the documentation and, therefore, they should be update unconditionally.

About isOrthographic I think that updating it in WebGLRenderer have zero impact for performance, because in the end it still leads to checking if the uniform is in the WebGLUniforms map

three.js/src/renderers/webgl/WebGLUniforms.js

Lines 1095 to 1101 in 39305cf

	setValue( gl, name, value, textures ) {
	const u = this.map[ name ];
	if ( u !== undefined ) u.setValue( gl, value, textures );
	}

And load to GPU only if needed

[Mugen87]

All these matrices, except isOrthographic, are listed in the documentation and, therefore, they should be update unconditionally.

You are right I have actually missed cameraPosition.

About isOrthographic I think that updating it in WebGLRenderer have zero impact for performance, because in the end it still leads to checking if the uniform is in the WebGLUniforms map

Since the uniform is always defined in the scaffold, the uniform is always present in the map and thus would be updated. However, it still might not be evaluated in the shader (e.g. for depth and normal materials). In this context, executing the update is wasted resources.

[Mugen87]

One could consider to define isOrthographic not in the scaffold but similar like other uniforms in UniformsLib. That would be probably less confusing.

[SoprachevAK]

Уes, I understand you, then it really needs to avoid updating. I think I should return the orthographic update as it was, but I can comment todo or open the issue with your note about transferring isOrthographic to UniformsLib