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

ComputeEffect: Implement onError callback #14952

Merged
merged 1 commit into from
Apr 8, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
73 changes: 37 additions & 36 deletions packages/dev/core/src/Compute/computeEffect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { ShaderStore } from "../Engines/shaderStore";
import { ShaderLanguage } from "../Materials/shaderLanguage";

import type { Engine } from "../Engines/engine";
import type { ComputeCompilationMessages } from "../Engines/Extensions/engine.computeShader";

/**
* Defines the route to the shader code. The priority is as follows:
Expand Down Expand Up @@ -381,7 +382,11 @@ export class ComputeEffect {
this._entryPoint
);

engine._executeWhenComputeStateIsCompiled(this._pipelineContext, () => {
engine._executeWhenComputeStateIsCompiled(this._pipelineContext, (messages: Nullable<ComputeCompilationMessages>) => {
if (messages && messages.numErrors > 0) {
this._processCompilationErrors(messages, previousPipelineContext);
return;
}
this._compilationError = "";
this._isReady = true;
if (this.onCompiled) {
Expand All @@ -403,54 +408,50 @@ export class ComputeEffect {
}
}

private _getShaderCodeAndErrorLine(code: Nullable<string>, error: Nullable<string>): [Nullable<string>, Nullable<string>] {
const regexp = /COMPUTE SHADER ERROR: 0:(\d+?):/;
private _processCompilationErrors(e: ComputeCompilationMessages | string, previousPipelineContext: Nullable<IComputePipelineContext> = null) {
this._compilationError = "";

let errorLine = null;
Logger.Error("Unable to compile compute effect:");
if (this.defines) {
Logger.Error("Defines:\n" + this.defines);
}

if (error && code) {
const res = error.match(regexp);
if (res && res.length === 2) {
const lineNumber = parseInt(res[1]);
const lines = code.split("\n", -1);
if (lines.length >= lineNumber) {
errorLine = `Offending line [${lineNumber}] in compute code: ${lines[lineNumber - 1]}`;
}
if (ComputeEffect.LogShaderCodeOnCompilationError) {
const code = this._pipelineContext?._getComputeShaderCode();
if (code) {
Logger.Error("Compute code:");
Logger.Error(code);
}
}

return [code, errorLine];
}

private _processCompilationErrors(e: any, previousPipelineContext: Nullable<IComputePipelineContext> = null) {
this._compilationError = e.message;
if (typeof e === "string") {
this._compilationError = e;
Logger.Error("Error: " + this._compilationError);
} else {
for (const message of e.messages) {
let msg = "";
if (message.line) {
msg += "Line " + message.line + ", ";
}
msg += message.type + ": " + message.text;

// Let's go through fallbacks then
Logger.Error("Unable to compile compute effect:");
Logger.Error("Defines:\n" + this.defines);
if (ComputeEffect.LogShaderCodeOnCompilationError) {
let lineErrorVertex = null,
code = null;
if (this._pipelineContext?._getComputeShaderCode()) {
[code, lineErrorVertex] = this._getShaderCodeAndErrorLine(this._pipelineContext._getComputeShaderCode(), this._compilationError);
if (code) {
Logger.Error("Compute code:");
Logger.Error(code);
if (this._compilationError) {
this._compilationError += "\n";
}
}
if (lineErrorVertex) {
Logger.Error(lineErrorVertex);
this._compilationError += msg;
Logger.Error(msg);
}
}
Logger.Error("Error: " + this._compilationError);

if (previousPipelineContext) {
this._pipelineContext = previousPipelineContext;
this._isReady = true;
if (this.onError) {
this.onError(this, this._compilationError);
}
this.onErrorObservable.notifyObservers(this);
}

if (this.onError) {
this.onError(this, this._compilationError);
}
this.onErrorObservable.notifyObservers(this);
}

/**
Expand Down
33 changes: 30 additions & 3 deletions packages/dev/core/src/Engines/Extensions/engine.computeShader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,30 @@ export type ComputeBindingLocation = { group: number; binding: number };
*/
export type ComputeBindingMapping = { [key: string]: ComputeBindingLocation };

/**
* Types of messages that can be generated during compilation
*/
export type ComputeCompilationMessageType = "error" | "warning" | "info";

/**
* Messages generated during compilation
*/
export interface ComputeCompilationMessages {
/**
* Number of errors generated during compilation
*/
numErrors: number;
/**
* List of messages generated during compilation
*/
messages: {
type: ComputeCompilationMessageType;
text: string;
line?: number;
column?: number;
}[];
}

/** @internal */
export enum ComputeBindingType {
Texture = 0,
Expand Down Expand Up @@ -111,7 +135,7 @@ declare module "../../Engines/thinEngine" {
_rebuildComputeEffects(): void;

/** @internal */
_executeWhenComputeStateIsCompiled(pipelineContext: IComputePipelineContext, action: () => void): void;
_executeWhenComputeStateIsCompiled(pipelineContext: IComputePipelineContext, action: (messages: Nullable<ComputeCompilationMessages>) => void): void;

/** @internal */
_releaseComputeEffect(effect: ComputeEffect): void;
Expand Down Expand Up @@ -161,8 +185,11 @@ ThinEngine.prototype._prepareComputePipelineContext = function (

ThinEngine.prototype._rebuildComputeEffects = function (): void {};

ThinEngine.prototype._executeWhenComputeStateIsCompiled = function (pipelineContext: IComputePipelineContext, action: () => void): void {
action();
ThinEngine.prototype._executeWhenComputeStateIsCompiled = function (
pipelineContext: IComputePipelineContext,
action: (messages: Nullable<ComputeCompilationMessages>) => void
): void {
action(null);
};

ThinEngine.prototype._releaseComputeEffect = function (effect: ComputeEffect): void {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { ComputeEffect } from "../../../Compute/computeEffect";
import type { IComputeContext } from "../../../Compute/IComputeContext";
import type { IComputePipelineContext } from "../../../Compute/IComputePipelineContext";
import type { Nullable } from "../../../types";
import type { ComputeBindingList, ComputeBindingMapping } from "../../Extensions/engine.computeShader";
import type { ComputeBindingList, ComputeBindingMapping, ComputeCompilationMessages } from "../../Extensions/engine.computeShader";
import { WebGPUEngine } from "../../webgpuEngine";
import { WebGPUComputeContext } from "../webgpuComputeContext";
import { WebGPUComputePipelineContext } from "../webgpuComputePipelineContext";
Expand Down Expand Up @@ -157,6 +157,30 @@ WebGPUEngine.prototype._rebuildComputeEffects = function (): void {
}
};

WebGPUEngine.prototype._executeWhenComputeStateIsCompiled = function (
pipelineContext: WebGPUComputePipelineContext,
action: (messages: Nullable<ComputeCompilationMessages>) => void
): void {
pipelineContext.stage!.module.getCompilationInfo().then((info) => {
const compilationMessages: ComputeCompilationMessages = {
numErrors: 0,
messages: [],
};
for (const message of info.messages) {
if (message.type === "error") {
compilationMessages.numErrors++;
}
compilationMessages.messages.push({
type: message.type,
text: message.message,
line: message.lineNum,
column: message.linePos,
});
}
action(compilationMessages);
});
};

WebGPUEngine.prototype._deleteComputePipelineContext = function (pipelineContext: IComputePipelineContext): void {
const webgpuPipelineContext = pipelineContext as WebGPUComputePipelineContext;
if (webgpuPipelineContext) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ export class WebGPUComputePipelineContext implements IComputePipelineContext {
}

public get isReady(): boolean {
if (this.stage) {
return true;
if (this.isAsync) {
// When async mode is implemented, this should return true if the pipeline is ready
return false;
}

// In synchronous mode, we return false, the readiness being determined by ComputeEffect
return false;
}

Expand Down