three.js icon indicating copy to clipboard operation
three.js copied to clipboard

Material: It's currently not possible to determine if a material failed to compile

Open gkjohnson opened this issue 5 months ago • 7 comments

Description

In working on three-gpu-pathtracer I've frequently run into platform-specific shader compilation issues that cause the shader to crash even though all syntax is correct. On DirectX, for example, a shader will fail to compile if an array of float parameters is passed to a function (see https://github.com/gkjohnson/three-gpu-pathtracer/pull/350). In order to prevent crashes for the user in these unforeseeable scenarios I'd like to be able programmatically determine whether a shader was able to compile and enable the application to display a note. This isn't currently possible. I'm planning to use compileAsync for the detection since compilation for such a large shader can be extremely slow on Windows.

Here's a list of some of the behavior that's not conducive to this:

  • You cannot provide multiple onShaderError callbacks so any helper functions provided by a library must overwrite existing functions which is error prone.
  • onShaderError does not provide any information about which mesh and material caused the failure.
  • When calling "compile" or "compileAsync" while checkShaderErrors is true, no errors are checked. The shader error is only fired on first use which is already too late.
  • As far as I know it's not possible to get the WebGLProgram or Vertex / FragmentShader objects associated with a Material / Mesh whether they've been compiled successfully or not.

Solution

  • Provide a function to retrieve the Vertex / Fragment Shaders and program associated with a Mesh and Material to check the compilation status (ideal because there's no need to set the flag to check shader errors)
  • Fire an "on error" and / or "on compile" event on the Material and / or itself with both material and object as arguments so the compilation status can be checked.

Alternatives

None afaik

Additional context

No response

gkjohnson avatar Feb 07 '24 10:02 gkjohnson

I've tried to come up with something, let me know what you think.

If I understand correctly, ideally, you want an error report right after gl.compileShader( shader ); As far as I know that's the soonest possible.

So based on your suggested solution

Fire an "on error" and / or "on compile" event on the Material and / or itself with both material and object as arguments so the compilation status can be checked.

So in the Material class we can add onCompileError, I assume it doesn't need material since the function is attached to said material

class Material extends EventDispatcher {

        /** existing code **/

        onBuild( /* shaderobject, renderer */ ) {}

	onBeforeRender( /* renderer, scene, camera, geometry, object, group */ ) {}

	onBeforeCompile( /* shaderobject, renderer */ ) {}

+       onCompileError( /* shaderobject, shaderstring, object */ ) {}

        /** existing code **/

}

Then in src\renderers\webgl\WebGLShader.js we can check if it did compile successfully.

- function WebGLShader( gl, type, string) {
+ function WebGLShader( gl, type, string, parameters ) {

	const shader = gl.createShader( type );

	gl.shaderSource( shader, string );
	gl.compileShader( shader );

	// some flag to check only if debug enabled
+	if ( checkShaderErrors ) {
+
+		if ( gl.getShaderParameter( shader, gl.COMPILE_STATUS ) === false ) {
+
+			const errorMsg = gl.getShaderInfoLog( shader );
+			console.error( 'Shader compilation failed: ' + errorMsg );
+
+			// new method from material class
+			parameters.material.onCompileError( shader, string, parameters.object );
+
+		}
+
+	}

	return shader;

}

But to pass the object to onCompileError, we need to pass it to WebGLShader() in a way or the other. For that I passed the shader parameters in src\renderers\webgl\WebGLProgram.js

function WebGLProgram( renderer, cacheKey, parameters, bindingStates ) {

-	const glVertexShader = WebGLShader( gl, gl.VERTEX_SHADER, vertexGlsl );
-	const glFragmentShader = WebGLShader( gl, gl.FRAGMENT_SHADER, fragmentGlsl );
+	const glVertexShader = WebGLShader( gl, gl.VERTEX_SHADER, vertexGlsl, parameters );
+	const glFragmentShader = WebGLShader( gl, gl.FRAGMENT_SHADER, fragmentGlsl, parameters );

}

And in the getParameters function of src\renderers\webgl\WebGLPrograms.js we add the object in the parameters properties to be able to pass it to the onCompileError function

const parameters = {
     /** existing properties **/
     
+   object: object     
}

Then as a user you can do

material.onCompileError = function ( shaderObject, shaderstring, object ) {

	console.log( 'object', object );
	console.log( 'object.material', object.material );

	// do something with the error
	cube.material = new THREE.MeshBasicMaterial( { color: 0xff00ff } );

};

Happy to make the PR if everyone is satisfied with this solution. Maybe there is a preferable way to have the object available in src\renderers\webgl\WebGLShader.js but I didn't see any other way.

AlaricBaraou avatar Feb 09 '24 05:02 AlaricBaraou

Thanks @AlaricBaraou! An event would be good but as you mention it does look like it requires some deeper changes to the renderer. I'm starting to think a more flexible option might be a "getShaderStatus" function that returns the compilation state and error:

const status = renderer.getCompilationStatus( material, object );

// return value
{
  status: ERROR, // COMPILED, UNRENDERABLE, NOT_COMPILED
  error: '... error message...',
}

Perhaps other maintainers have some thoughts on a preferred API here.

gkjohnson avatar Feb 12 '24 01:02 gkjohnson

WebGLRenderer.getCompilationStatus() would need at least one more parameter so you are able to fetch the correct program. The renderer needs information from the current render state which you can't determine without the scene. So it needs to be:

const status = renderer.getCompilationStatus( material, object, scene );

I wonder how you would use this API. If there is a shader compilation error, would you test all material/objects combinations in your app for errors?

At first sight, it seems a callback on material level like onErrorCompile() is easier to use although the integration into WebGLProgram requires indeed some refactoring. I wouldn't touch the parameters object though but consider to enhance acquireProgram() and safe a reference to the material and object in WebGLProgram.

Mugen87 avatar Feb 13 '24 16:02 Mugen87

You're right it is maybe less useful in other contexts. In my use case I only need to check one material.

At first sight, it seems a callback on material level like onErrorCompile() is easier to use although the integration into WebGLProgram requires indeed some refactoring. I wouldn't touch the parameters object though but consider to enhance acquireProgram() and safe a reference to the material and object in WebGLProgram.

I think an event should work okay but I'd prefer an event fired with "addEventListener":

material.addEventListener( 'compilation-error', ( shader, material, object, scene, camera ) => {

  // ...

} );

renderer.compileAsync( scene, camera ).then( () => {

  // errors should be fired by now if any

} );

The only oddity here is that in order for errors to fire we need to have debug.checkShaderErrors set to true which can be a problem with an async function. You have to set it to true before calling compileAsync and then reset it to the original value afterward meaning if my library does this it's overriding the user setting for the duration of compilation potentially causing render performance problems while rendering in that time (compilation can take upwards of 30 seconds on Windows). In the worst case the user code adjusts the value during the compilation time and the value is out of sync.

This is always a problem with these kinds of global flags, though. I guess we can consider this a separate issue and deal with it later if needed, as well.

gkjohnson avatar Feb 16 '24 11:02 gkjohnson

@mrdoob should decide if Material.onErrorCompile() or an compilation-error based on EventDispatcher is better. I vote for the first one since we already have Material.onBeforeCompile().

I agree the checkShaderErrors usage in context of compileAsync() is a separate issue and I would focus on that when the above question is clear.

Mugen87 avatar Feb 16 '24 11:02 Mugen87

should decide if Material.onErrorCompile() or an compilation-error based on EventDispatcher is better. I vote for the first one since we already have Material.onBeforeCompile().

For events like "Material.onErrorCompile" you can't register multiple event functions which is a problem I've run into multiple times. So it's not just a superficial design change - using a member function as a callback is functionally lacking. OnBeforeCompile maybe makes a more sense since it's modifying the shader and if multiple "on before compile" shader adjustments are being done order is important so it should be managed by the application.

Generally if event callbacks don't have to return anything or modify an argument we should be using the event dispatcher paradigm, I think.

gkjohnson avatar Feb 16 '24 12:02 gkjohnson

That makes sense!

Mugen87 avatar Feb 16 '24 13:02 Mugen87