KhronosGroup / KhronosGroup/WebGL
change WebGL2 spec slightly to help devs find race conditions?
- Dominant language
- HTML
- Stars
- 2.9k
- Forks
- 703
- Avg merge
- 2d 9h
- Merged PRs (30d)
- 4
Description
Currently the WebGL2 spec for `getQueryParameter` says
> In order to ensure consistent behavior across platforms, queries' results must only be made available when the user agent's event loop is not executing a task. In other words:
>
> * A query's result must not be made available until control has returned to the user agent's main loop.
> * Repeatedly fetching a query's QUERY_RESULT_AVAILABLE parameter in a loop, without returning control to the user agent, must always return the same value.
`getSyncParameter` has similar wording
I'd like to suggest the spec change here that the user must call `getQueryParameter(QUERY_RESULT_AVAILABLE)` and receive `true` and that if they don't then calling `getQueryParameter(QUERY_RESULT)` generates `INVALID_OPERATION`
As it is a dev can just call `getQueryParameter(QUERY_RESULT)` and assume the result is available. It might not be and they have a race in their code that is nearly impossible to find. It might just be some intermittent flicker. It might also only happen on some user's hardware which the dev has no access too. They might not have waited a frame (porting native code) and just assumed that 50% of the way through a frame reading the query result was OK.
To help find these issues I can see at least 2 possible solutions.
1. Require the WebGL2 implementation to check `QUERY_RESULT_AVAILABLE` when the user queries `QUERY_RESULT`. You can imagine that implementation as
```
// pseudo code
bool WebGL2RenderingContext::getQueryParameter(WebGLQuery* query, GLenum pname) {
switch (pname) {
case GL_QUERY_RESULT: {
GLuint available;
glGetQueryObjectuiv(query->id(), GL_QUERY_RESULT_AVAILABLE, &available);
if (!available) {
WebGL2RenderingContext::generateError(GL_INVALID_OPERATION, "tried to query result when result not available");
return null;
}
GLuint result;
glGetQueryObjectuiv(query->id(), GL_QUERY_RESULT, &result);
return result;
....
}
}
```
The problem with this solution is there is no way to test it since it requires a query to not be ready in order to test. At best you can test that the user didn't get the result in the same frame but you can not test that having waited for the next frame a race was caught.
Vs the suggested solution of requiring the user to query QUERY_RESULT_AVAILABLE and get true could be something like
```
// pseudo code
bool WebGL2RenderingContext::getQueryParameter(WebGLQuery* query, GLenum pname) {
switch (pname) {
case GL_QUERY_RESULT_AVAILABLE: {
GLuint available;
glGetQueryObjectuiv(query->id(), GL_QUERY_RESULT_AVAILABLE, &available);
// mark the user correctly checked the query is available.
// (was cleared when the query is submitted)
query.setAvailable(available);
return available;
case QUERY_RESULT:
if (!query->getAvailable()) {
WebGL2RenderingContext::generateError(GL_INVALID_OPERATION, "tried to query result without checking if result available and getting true");
return null;
}
GLuint result;
glGetQueryObjectuiv(query->id(), GL_QUERY_RESULT, &result);
return result;
....
}
}
```
Now it's easily testable.
This shouldn't affect well written programs. It's trivial for devs to fix their code if they aren't currently calling `getQueryParameter(gl.QUERY_RESULT_AVAILABLE)`. And, even if they don't check the result of `QUERY_RESULT_AVAILABLE` WebGL at least has an easy way to generate an error telling them they have a race condition.
It seems like the same would apply to `getSyncParameter` and other future async stuff?
Contributor guide
Research direction
Start with the WebGL2 spec wording for getQueryParameter and the similar getSyncParameter wording quoted in the issue. Compare the two proposed behaviors and determine the intended scope, including future asynchronous operations. Done means the normative change and a testable conformance expectation are agreed.
Written by the indexing model from the issue text.
Assessment
- Domain
- documentation
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100