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

Report WebGL errors #281

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
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
17 changes: 17 additions & 0 deletions sample/js/error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
const canvas = document.getElementById("renderCanvas");
const gl = canvas.getContext("webgl");

function loop() {
gl.clearColor(0.3, 0.3, 0.3, 1.0); // Clear to black, fully opaque
gl.clearDepth(1.0); // Clear everything
gl.clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT)

console.log(gl.getError(), "before")
gl.clear(0xFFFF)
console.log(gl.getError(), "after")

window.requestAnimationFrame(function() {
loop();
});
}
loop();
7 changes: 5 additions & 2 deletions src/backend/commands/baseCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export abstract class BaseCommand {
// Includes uniform functions special cases to prevent lots of inheritence.
const text = (functionInformation.name.indexOf("uniform") === 0) ?
this.stringifyUniform(functionInformation.arguments) :
this.stringify(functionInformation.arguments, functionInformation.result);
this.stringify(functionInformation.arguments, functionInformation.result, functionInformation.errors);

const commandCapture = {
id: commandCaptureId,
Expand Down Expand Up @@ -81,14 +81,17 @@ export abstract class BaseCommand {
// Nothing by default.
}

protected stringify(args: IArguments, result: any): string {
protected stringify(args: IArguments, result: any, errors: any[]): string {
let stringified = this.spiedCommandName;
if (args && args.length > 0) {
stringified += ": " + this.stringifyArgs(args).join(", ");
}
if (result !== undefined && result !== null) {
stringified += " -> " + this.stringifyResult(result);
}
if (errors.length > 0) {
stringified += " ~> " + errors.map(error => this.stringifyValue(error)).join(", ");
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a poor way of displaying this (the same goes for how the return value gets displayed.. this should be formatted in the frontend / not in the backend)

return stringified;
}

Expand Down
32 changes: 30 additions & 2 deletions src/backend/spies/commandSpy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,41 @@ export class CommandSpy {
// tslint:disable-next-line:only-arrow-functions
return function () {
const before = Time.now;
const result = OriginFunctionHelper.executeOriginFunction(self.spiedCommandRunningContext, self.spiedCommandName, arguments);
let result = OriginFunctionHelper.executeOriginFunction(self.spiedCommandRunningContext, self.spiedCommandName, arguments);
const after = Time.now;

const functionInformation = {
// Initialize error logging
const gl = self.spiedCommandRunningContext;
if (!gl.currentErrors) {
gl.currentErrors = [];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This information should be stored elsewhere; where?

}

const newErrors = []
if (self.spiedCommandName === 'getError') {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very hacky way to check for this?

// Inject one of the collected errors
result = gl.currentErrors.shift() ?? gl.NO_ERROR;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gl.NO_ERROR might have been modified by the application. Ideally I'd be using something similar to OriginFunctionHelper.executeOriginFunction to get the original constant.

How do I do that?

} else {
// Record new errors
while (true) {
const errorResult = OriginFunctionHelper.executeOriginFunction(gl, "getError", undefined);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be a nicer way to run this; I'm also unsure about undefined for IArguments but wasn't sure how to make this work otherwise.

if (errorResult === gl.NO_ERROR) {
break;
}
newErrors.push(errorResult);
}
// Move new errors to the accumulated list
newErrors.forEach(newError => {
if (!gl.currentErrors.includes(newError)) {
gl.currentErrors.push(newError);
}
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might break in which order the errors are being returned, because it avoids duplicates. However, this is implementation defined.

I'm also generally not sure if this complicated FIFO for handling multiple errors is necessary.

}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entire error handling might have to live elsewhere / it should at least move into a separate function


const functionInformation: IFunctionInformation = {
name: self.spiedCommandName,
arguments,
result,
errors: newErrors,
startTime: before,
endTime: after,
};
Expand Down
1 change: 1 addition & 0 deletions src/backend/types/functionInformation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export interface IFunctionInformation {
readonly name: string;
readonly arguments: IArguments;
readonly result: any;
readonly errors: any[];
readonly startTime: number;
readonly endTime: number;
}