Skip to content

Commit

Permalink
WebGLObject creation is now infallible, and starts Lost iff context L…
Browse files Browse the repository at this point in the history
…ost. (#3642)

* WebGLObject creation is infallible, and starts Lost iff context Lost.

* Object creation in extensions made infallible.

* Update tests: Infallible object creation.

* Test isObject() functions; Fix context-lost-worker choice of testFailed/testPassed

* isObject(x) -> true only after bindObject(x).
  • Loading branch information
kdashg authored Jul 29, 2024
1 parent 571f865 commit 97b78aa
Show file tree
Hide file tree
Showing 11 changed files with 119 additions and 73 deletions.
5 changes: 4 additions & 1 deletion extensions/EXT_disjoint_timer_query/extension.xml
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ interface EXT_disjoint_timer_query {
const GLenum TIMESTAMP_EXT = 0x8E28;
const GLenum GPU_DISJOINT_EXT = 0x8FBB;

WebGLTimerQueryEXT? createQueryEXT();
WebGLTimerQueryEXT createQueryEXT();
undefined deleteQueryEXT(WebGLTimerQueryEXT? query);
[WebGLHandlesContextLoss] boolean isQueryEXT(WebGLTimerQueryEXT? query);
undefined beginQueryEXT(GLenum target, WebGLTimerQueryEXT query);
Expand Down Expand Up @@ -421,5 +421,8 @@ interface EXT_disjoint_timer_query {
<revision date="2023/06/01">
<change>Added error codes for invalid API usage.</change>
</revision>
<revision date="2024/04/19">
<change><code>createQueryEXT</code> made infallible.</change>
</revision>
</history>
</extension>
5 changes: 4 additions & 1 deletion extensions/OES_vertex_array_object/extension.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ interface WebGLVertexArrayObjectOES : WebGLObject {
interface OES_vertex_array_object {
const GLenum VERTEX_ARRAY_BINDING_OES = 0x85B5;

WebGLVertexArrayObjectOES? createVertexArrayOES();
WebGLVertexArrayObjectOES createVertexArrayOES();
undefined deleteVertexArrayOES(WebGLVertexArrayObjectOES? arrayObject);
[WebGLHandlesContextLoss] GLboolean isVertexArrayOES(WebGLVertexArrayObjectOES? arrayObject);
undefined bindVertexArrayOES(WebGLVertexArrayObjectOES? arrayObject);
Expand Down Expand Up @@ -167,5 +167,8 @@ interface OES_vertex_array_object {
<revision date="2022/07/09">
<change>Added error codes for invalid API usage.</change>
</revision>
<revision date="2024/04/19">
<change><code>createVertexArrayOES</code> made infallible.</change>
</revision>
</history>
</ratified>
17 changes: 14 additions & 3 deletions sdk/tests/conformance/context/context-lost-restored.html
Original file line number Diff line number Diff line change
Expand Up @@ -239,13 +239,24 @@
function testOESVertexArrayObject() {
if (OES_vertex_array_object) {
// Extension must still be lost.
shouldBeNull("OES_vertex_array_object.createVertexArrayOES()");
let vao = OES_vertex_array_object.createVertexArrayOES();
assertMsg(vao, "[with extension lost] createVertexArrayOES() -> non-null");
assertMsg(!OES_vertex_array_object.isVertexArrayOES(vao), "[with context lost] isVertexArrayOES(createVertexArrayOES()) -> false");
// Try re-enabling extension

old_OES_vertex_array_object = OES_vertex_array_object;
OES_vertex_array_object = reGetExtensionAndTestForProperty(gl, "OES_vertex_array_object", false);
shouldBeTrue("OES_vertex_array_object.createVertexArrayOES() != null");
shouldBeTrue("old_OES_vertex_array_object.createVertexArrayOES() == null");

vao = OES_vertex_array_object.createVertexArrayOES();
assertMsg(vao, "[with new non-lost extension] createVertexArrayOES() -> non-null");
assertMsg(!OES_vertex_array_object.isVertexArrayOES(vao), "[with new non-lost extension, before bindVAO] isVertexArrayOES(createVertexArrayOES()) -> false");
OES_vertex_array_object.bindVertexArrayOES(vao);
OES_vertex_array_object.bindVertexArrayOES(null);
assertMsg(OES_vertex_array_object.isVertexArrayOES(vao), "[with new non-lost extension, after bindVAO] isVertexArrayOES(createVertexArrayOES()) -> true");

vao = old_OES_vertex_array_object.createVertexArrayOES();
assertMsg(vao, "[with old lost extension] createVertexArrayOES() -> non-null");
assertMsg(!old_OES_vertex_array_object.isVertexArrayOES(vao), "[with old lost extension] isVertexArrayOES(createVertexArrayOES()) -> false");
}
}

Expand Down
43 changes: 27 additions & 16 deletions sdk/tests/conformance/context/context-lost.html
Original file line number Diff line number Diff line change
Expand Up @@ -308,12 +308,6 @@

// Functions return nullable values should all return null.
var nullTests = [
"gl.createBuffer()",
"gl.createFramebuffer()",
"gl.createProgram()",
"gl.createRenderbuffer()",
"gl.createShader(gl.GL_VERTEX_SHADER)",
"gl.createTexture()",
"gl.getActiveAttrib(program, 0)",
"gl.getActiveUniform(program, 0)",
"gl.getAttachedShaders(program)",
Expand All @@ -336,14 +330,24 @@
];
testFunctionsThatReturnNULL(nullTests);

[
[() => gl.createBuffer(), x => gl.bindBuffer(gl.ARRAY_BUFFER, x), x => gl.isBuffer(x)],
[() => gl.createFramebuffer(), x => gl.bindFramebuffer(gl.FRAMEBUFFER, x), x => gl.isFramebuffer(x)],
[() => gl.createProgram(), x => undefined, x => gl.isProgram(x)],
[() => gl.createRenderbuffer(), x => gl.bindRenderbuffer(gl.RENDERBUFFER, x), x => gl.isRenderbuffer(x)],
[() => gl.createShader(gl.VERTEX_SHADER), x => undefined, x => gl.isShader(x)],
[() => gl.createTexture(), x => gl.bindTexture(gl.TEXTURE_2D, x), x => gl.isTexture(x)],
].forEach(([fn_create, fn_bind, fn_is]) => {
const x = fn_create();
assertMsg(x, `${fn_create.toString()} -> non-null`);
assertMsg(fn_is(x) === false, `[before bind] ${fn_is.toString()} -> false`);
fn_bind(x);
fn_bind(null);
assertMsg(fn_is(x) === false, `[after bind] ${fn_is.toString()} -> false`);
});

// "Is" queries should all return false.
shouldBeFalse("gl.isBuffer(buffer)");
shouldBeFalse("gl.isEnabled(gl.BLEND)");
shouldBeFalse("gl.isFramebuffer(framebuffer)");
shouldBeFalse("gl.isProgram(program)");
shouldBeFalse("gl.isRenderbuffer(renderbuffer)");
shouldBeFalse("gl.isShader(shader)");
shouldBeFalse("gl.isTexture(texture)");

shouldBe("gl.getError()", "gl.NO_ERROR");

Expand All @@ -355,10 +359,17 @@
"OES_vertex_array_object.isVertexArrayOES(vertexArrayObject)",
"OES_vertex_array_object.deleteVertexArrayOES(vertexArrayObject)",
]);
testFunctionsThatReturnNULL(
[
"OES_vertex_array_object.createVertexArrayOES()",
]);

[
[() => OES_vertex_array_object.createVertexArrayOES(), x => OES_vertex_array_object.isVertexArrayOES(x), x => OES_vertex_array_object.isVertexArrayOES(x)],
].forEach(([fn_create, fn_bind, fn_is]) => {
const x = fn_create();
assertMsg(x, `${fn_create.toString()} -> non-null`);
assertMsg(fn_is(x) === false, `[before bind] ${fn_is.toString()} -> false`);
fn_bind(x);
fn_bind(null);
assertMsg(fn_is(x) === false, `[after bind] ${fn_is.toString()} -> false`);
});
}

testUploadingLostContextToTexture();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@
finishTest();
return;
}
testLosingAndRestoringContext().then(function() {
testLosingAndRestoringContext().then(function(s) {
testPassed("Test passed");
finishTest();
return;
}, function() {
testFailed("Some test failed");
}, function(s) {
testFailed("Test failed: " + s);
finishTest();
return;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,11 @@
var worker = new Worker('context-lost-worker.js');
worker.postMessage("Start worker");
worker.onmessage = function(e) {
if (e.data == "Test passed") {
testPassed("All tests have passed");
} else {
testFailed("Some tests failed");
const fn = e.data.fail ? testFailed : testPassed;
fn(e.data.msg);
if (e.data.finishTest) {
finishTest();
}
finishTest();
return;
}
}
Expand Down
18 changes: 8 additions & 10 deletions sdk/tests/conformance/offscreencanvas/context-lost-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,23 @@ self.onmessage = function(e) {

// call testValidContext() before checking for the extension, because this is where we check
// for the isContextLost() method, which we want to do regardless of the extension's presence.
if (!testValidContext())
self.postMessage("Test failed");
self.postMessage({fail: !testValidContext(), msg: "testValidContext()"});

WEBGL_lose_context = gl.getExtension("WEBGL_lose_context");
self.postMessage({fail: !WEBGL_lose_context, msg: "WEBGL_lose_context"});

extension = gl.getExtension("WEBGL_lose_context");
// need an extension that exposes new API methods.
OES_vertex_array_object = gl.getExtension("OES_vertex_array_object");
if (extension == null || OES_vertex_array_object == null)
self.postMessage("Test failed");
self.postMessage({fail: !OES_vertex_array_object, msg: "OES_vertex_array_object"});

// We need to initialize |uniformLocation| before losing context.
// Otherwise gl.getUniform() when context is lost will throw.
uniformLocation = gl.getUniformLocation(program, "tex");
extension.loseContext();
WEBGL_lose_context.loseContext();

canvas.addEventListener("webglcontextlost", function() {
if (testLostContextWithoutRestore())
self.postMessage("Test passed");
else
self.postMessage("Test failed");
self.postMessage({fail: !testLostContextWithoutRestore(), msg: "testLostContextWithoutRestore()",
finishTest:true});
}, false);
}

10 changes: 5 additions & 5 deletions sdk/tests/conformance/offscreencanvas/context-lost.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,27 +33,27 @@
return;
}

extension = gl.getExtension("WEBGL_lose_context");
WEBGL_lose_context = gl.getExtension("WEBGL_lose_context");
// need an extension that exposes new API methods.
OES_vertex_array_object = gl.getExtension("OES_vertex_array_object");
if (extension == null || OES_vertex_array_object == null) {
testFailed("Some tests failed");
if (WEBGL_lose_context == null || OES_vertex_array_object == null) {
testFailed("extension && OES_vertex_array_object failed");
finishTest();
return;
}

// We need to initialize |uniformLocation| before losing context.
// Otherwise gl.getUniform() when context is lost will throw.
uniformLocation = gl.getUniformLocation(program, "tex");
extension.loseContext();
WEBGL_lose_context.loseContext();

canvas.addEventListener("webglcontextlost", function() {
if (testLostContextWithoutRestore()) {
testPassed("All tests passed");
finishTest();
return;
} else {
testFailed("Some tests failed");
testFailed("testLostContextWithoutRestore failed");
finishTest();
return;
}
Expand Down
44 changes: 30 additions & 14 deletions sdk/tests/js/tests/canvas-tests-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ var canvas;
var gl;
var OES_vertex_array_object;
var uniformLocation;
var extension;
var WEBGL_lose_context;
var buffer;
var framebuffer;
var program;
Expand Down Expand Up @@ -439,7 +439,7 @@ function testLostContextWithoutRestore()
return false;

// Test the extension itself.
if (!compareGLError(gl.INVALID_OPERATION, "extension.loseContext()"))
if (!compareGLError(gl.INVALID_OPERATION, "WEBGL_lose_context.loseContext()"))
return false;

imageData = new ImageData(1, 1);
Expand Down Expand Up @@ -567,13 +567,7 @@ function testLostContextWithoutRestore()
return false;

// Functions return nullable values should all return null.
if (gl.createBuffer() != null ||
gl.createFramebuffer() != null ||
gl.createProgram() != null ||
gl.createRenderbuffer() != null ||
gl.createShader(gl.GL_VERTEX_SHADER) != null ||
gl.createTexture() != null ||
gl.getActiveAttrib(program, 0) != null ||
if (gl.getActiveAttrib(program, 0) != null ||
gl.getActiveUniform(program, 0) != null ||
gl.getAttachedShaders(program) != null ||
gl.getBufferParameter(gl.ARRAY_BUFFER, gl.BUFFER_SIZE) != null ||
Expand All @@ -594,6 +588,22 @@ function testLostContextWithoutRestore()
gl.getExtension("WEBGL_lose_context") != null)
return false;

const failedTests = [
"gl.createBuffer()",
"gl.createFramebuffer()",
"gl.createProgram()",
"gl.createRenderbuffer()",
"gl.createShader(gl.VERTEX_SHADER)",
"gl.createTexture()",
].reduce(s => {
const v = eval(s);
return !v;
});
if (failedTests.length) {
console.log({failedTests});
return false;
}

// "Is" queries should all return false.
if (gl.isBuffer(buffer) || gl.isEnabled(gl.BLEND) || gl.isFramebuffer(framebuffer) ||
gl.isProgram(program) || gl.isRenderbuffer(renderbuffer) || gl.isShader(shader) ||
Expand All @@ -609,7 +619,7 @@ function testLostContextWithoutRestore()
!compareGLError(gl.NO_ERROR, "OES_vertex_array_object.isVertexArrayOES(vertexArrayObject)") ||
!compareGLError(gl.NO_ERROR, "OES_vertex_array_object.deleteVertexArrayOES(vertexArrayObject)"))
return false;
if (OES_vertex_array_object.createVertexArrayOES() != null)
if (!OES_vertex_array_object.createVertexArrayOES())
return false;
}
return true;
Expand Down Expand Up @@ -718,7 +728,7 @@ function testLosingAndRestoringContext()
});
canvas.addEventListener("webglcontextrestored", function() {
if (!testRestoredContext())
reject("Test failed");
reject("Test failed: !testRestoredContext()");
else
resolve("Test passed");
});
Expand Down Expand Up @@ -788,16 +798,22 @@ function testOESTextureFloat() {
function testOESVertexArrayObject() {
if (OES_vertex_array_object) {
// Extension must still be lost.
if (OES_vertex_array_object.createVertexArrayOES() != null)
if (!OES_vertex_array_object.createVertexArrayOES()) {
console.error("!OES_vertex_array_object.createVertexArrayOES()");
return false;
}
// Try re-enabling extension

var old_OES_vertex_array_object = OES_vertex_array_object;
OES_vertex_array_object = reGetExtensionAndTestForProperty(gl, "OES_vertex_array_object", false);
if (OES_vertex_array_object.createVertexArrayOES() == null)
if (!OES_vertex_array_object.createVertexArrayOES()) {
console.error("!OES_vertex_array_object.createVertexArrayOES() 2");
return false;
if (old_OES_vertex_array_object.createVertexArrayOES() != null)
}
if (!old_OES_vertex_array_object.createVertexArrayOES()) {
console.error("!old_OES_vertex_array_object.createVertexArrayOES()");
return false;
}
return true;
}
}
Expand Down
Loading

0 comments on commit 97b78aa

Please sign in to comment.