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

Refactor of the shader validation #6081

Merged
merged 3 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/platform/graphics/graphics-device.js
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ class GraphicsDevice extends EventHandler {
this.indexBuffer = null;
this.vertexBuffers = [];
this.shader = null;
this.shaderValid = undefined;
this.renderTarget = null;
}

Expand Down
1 change: 0 additions & 1 deletion src/platform/graphics/null/null-graphics-device.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ class NullGraphicsDevice extends GraphicsDevice {
}

setShader(shader) {
return true;
}

setBlendState(blendState) {
Expand Down
36 changes: 22 additions & 14 deletions src/platform/graphics/webgl/webgl-graphics-device.js
Original file line number Diff line number Diff line change
Expand Up @@ -2101,6 +2101,10 @@ class WebglGraphicsDevice extends GraphicsDevice {
draw(primitive, numInstances, keepBuffers) {
const gl = this.gl;

this.activateShader();
if (!this.shaderValid)
return;

let sampler, samplerValue, texture, numTextures; // Samplers
let uniform, scopeId, uniformVersion, programVersion; // Uniforms
const shader = this.shader;
Expand Down Expand Up @@ -2722,30 +2726,34 @@ class WebglGraphicsDevice extends GraphicsDevice {
/**
* Sets the active shader to be used during subsequent draw calls.
*
* @param {Shader} shader - The shader to set to assign to the device.
* @returns {boolean} True if the shader was successfully set, false otherwise.
* @param {Shader} shader - The shader to assign to the device.
*/
setShader(shader) {
if (shader !== this.shader) {
if (shader.failed) {
return false;
} else if (!shader.ready && !shader.impl.finalize(this, shader)) {
shader.failed = true;
return false;
}

this.shader = shader;

// Set the active shader
this.gl.useProgram(shader.impl.glProgram);
this.shaderValid = undefined; // need to run activation / validation

// #if _PROFILER
this._shaderSwitchesPerFrame++;
// #endif
}
}

activateShader() {

this.attributesInvalidated = true;
if (this.shaderValid === undefined) {
const { shader } = this;
if (shader.failed) {
this.shaderValid = false;
} else if (!shader.ready && !shader.impl.finalize(this, shader)) {
shader.failed = true;
this.shaderValid = false;
} else {
// Set the active shader
this.gl.useProgram(shader.impl.glProgram);
this.shaderValid = true;
}
}
return true;
}

/**
Expand Down
7 changes: 2 additions & 5 deletions src/platform/graphics/webgl/webgl-shader.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { now } from '../../../core/time.js';
import { WebglShaderInput } from './webgl-shader-input.js';
import { SHADERTAG_MATERIAL, semanticToLocation } from '../constants.js';
import { DeviceCache } from '../device-cache.js';
import { DebugGraphics } from '../debug-graphics.js';

let _totalCompileTime = 0;

Expand Down Expand Up @@ -257,10 +258,6 @@ class WebglShader {
return true;
}

// if the program wasn't linked yet (shader was not created in batch)
if (!this.glProgram)
this.link(device, shader);

const glProgram = this.glProgram;
const definition = shader.definition;

Expand Down Expand Up @@ -398,7 +395,7 @@ class WebglShader {
if (!gl.getShaderParameter(glShader, gl.COMPILE_STATUS)) {
const infoLog = gl.getShaderInfoLog(glShader);
const [code, error] = this._processError(source, infoLog);
const message = `Failed to compile ${shaderType} shader:\n\n${infoLog}\n${code}`;
const message = `Failed to compile ${shaderType} shader:\n\n${infoLog}\n${code} while rendering ${DebugGraphics.toString()}`;
// #if _DEBUG
error.shader = shader;
console.error(message, error);
Expand Down
14 changes: 7 additions & 7 deletions src/platform/graphics/webgpu/webgpu-graphics-device.js
Original file line number Diff line number Diff line change
Expand Up @@ -481,14 +481,14 @@ class WebgpuGraphicsDevice extends GraphicsDevice {

setShader(shader) {

this.shader = shader;
if (shader !== this.shader) {
this.shader = shader;

// #if _PROFILER
// TODO: we should probably track other stats instead, like pipeline switches
this._shaderSwitchesPerFrame++;
// #endif

return true;
// #if _PROFILER
// TODO: we should probably track other stats instead, like pipeline switches
this._shaderSwitchesPerFrame++;
// #endif
}
}

setBlendState(blendState) {
Expand Down
3 changes: 2 additions & 1 deletion src/platform/graphics/webgpu/webgpu-shader.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Debug, DebugHelper } from '../../../core/debug.js';
import { SHADERLANGUAGE_WGSL } from '../constants.js';
import { DebugGraphics } from '../debug-graphics.js';

import { ShaderProcessor } from '../shader-processor.js';
import { WebgpuDebug } from './webgpu-debug.js';
Expand Down Expand Up @@ -146,7 +147,7 @@ class WebgpuShader {
const spirv = this.shader.device.glslang.compileGLSL(src, shaderType);
return this.shader.device.twgsl.convertSpirV2WGSL(spirv);
} catch (err) {
console.error(`Failed to transpile webgl ${shaderType} shader [${this.shader.label}] to WebGPU: [${err.message}]`, {
console.error(`Failed to transpile webgl ${shaderType} shader [${this.shader.label}] to WebGPU: [${err.message}] while rendering ${DebugGraphics.toString()}`, {
processed: src,
original: originalSrc,
shader: this.shader
Expand Down
18 changes: 3 additions & 15 deletions src/scene/renderer/forward-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,6 @@ class ForwardRenderer extends Renderer {
const clusteredLightingEnabled = this.scene.clusteredLightingEnabled;

// Render the scene
let skipMaterial = false;
const preparedCallsCount = preparedCalls.drawCalls.length;
for (let i = 0; i < preparedCallsCount; i++) {

Expand All @@ -552,22 +551,11 @@ class ForwardRenderer extends Renderer {
const lightMaskChanged = preparedCalls.lightMaskChanged[i];
const shaderInstance = preparedCalls.shaderInstances[i];
const material = drawCall.material;
const objDefs = drawCall._shaderDefs;
const lightMask = drawCall.mask;

if (newMaterial) {

const shader = shaderInstance.shader;
if (!shader.failed && !device.setShader(shader)) {
Debug.error(`Error compiling shader [${shader.label}] for material=${material.name} pass=${pass} objDefs=${objDefs}`, material);
}

// skip rendering with the material if shader failed
skipMaterial = shader.failed;
if (skipMaterial)
break;

DebugGraphics.pushGpuMarker(device, `Material: ${material.name}`);
device.setShader(shaderInstance.shader);

// Uniforms I: material
material.setParameters(device);
Expand All @@ -585,11 +573,10 @@ class ForwardRenderer extends Renderer {
device.setBlendState(material.blendState);
device.setDepthState(material.depthState);
device.setAlphaToCoverage(material.alphaToCoverage);

DebugGraphics.popGpuMarker(device);
}

DebugGraphics.pushGpuMarker(device, `Node: ${drawCall.node.name}`);
DebugGraphics.pushGpuMarker(device, `Material: ${material.name}`);

this.setupCullMode(camera._cullFaces, flipFactor, drawCall);

Expand Down Expand Up @@ -649,6 +636,7 @@ class ForwardRenderer extends Renderer {
}

DebugGraphics.popGpuMarker(device);
DebugGraphics.popGpuMarker(device);
}
}

Expand Down
4 changes: 1 addition & 3 deletions src/scene/renderer/shadow-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -336,9 +336,7 @@ class ShadowRenderer {
// sort shadow casters by shader
meshInstance._key[SORTKEY_DEPTH] = shadowShader.id;

if (!shadowShader.failed && !device.setShader(shadowShader)) {
Debug.error(`Error compiling shadow shader for material=${material.name} pass=${shadowPass}`, material);
}
device.setShader(shadowShader);

// set buffers
renderer.setVertexBuffers(device, mesh);
Expand Down