microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
99.75k stars 12.34k forks source link

WebGL(2) context methods don't allow null for resource parameters #58200

Open Ragnar-Oock opened 4 months ago

Ragnar-Oock commented 4 months ago

🔎 Search Terms

WebGL2RenderingContext, attachShader, createShader, null

🕗 Version & Regression Information

⏯ Playground Link

https://www.typescriptlang.org/play?target=99&jsx=0&ts=5.5.0-dev.20240415#code/KYDwDg9gTgLgBDAnmYcDKALAhgE2FAFWVQF44B1YAIwHEAZAJgCVgA7PKAS1YHMBhCKxigYAbQDkAMSYBBGgFkAogDkCAfTQAJGQBFFTcQF04AHwrV6zNh279BwkGPEA1fQUUANDdr0HDAbgAoQNBIWDgAMwBXVgBjGE5BOAAbCFxMXHwACh5kgC5zWkYWdnxbASERABo4AGcIKKhY4ALamC5eGqQUAoyOIhQASgLKIr78UzhWKOTkuABvQIBIWME2uuwOODJcgDpYqGAsYXGoLO7gQaDlgHobuE4IuEQGuBjVgFsPtngYDAhaqhktxgLUHvAAO7QADWYKoUXgPAgoLgWB4WG46z+qEOn2sx0SrDBxzgGBgMDAtTydwhtN20IwUEEAN20B4Nwh1FyHM40M4N00WHYwN4FQcMDoAJgAGIdIIAKSKBjygAcAAZ5QBOTUwNSxDDAWLQtQRaBqaazNSCPWHAmCYJLO4PJ5ZWqbCYkT1TGbJQYLW73JZ-JkQqbAUOKKBMs4AIjdmSgDzBFuSNReUXEADcgVK4KtKo4Y1cA3AAL7LPbxjhoBpNYCu91QGr1RrNYtLPafMCcZLAU4NhPF5b59a1KKxZq1ME7ZK7HjAGCnAAKWCgWG+wjOVfwNT2fAA8vIlwBJOiKDQEGQEACqaHbjzgrvHk9qfsWSyWhxgjVYGwTQSWcthzWCBe12fBoxyWd50XRtj1YU1JR4AcOEGds9jwXsTkbFD8HbL8f29WYglLIA

💻 Code

export type ShaderType = WebGL2RenderingContext['FRAGMENT_SHADER'] | WebGL2RenderingContext['VERTEX_SHADER'];

export function loadShader(gl: WebGL2RenderingContext, source: string, type: ShaderType): WebGLShader | null {
    const shader = gl.createShader(type);

    // if you uncomment those lines it works but goes against the recomendations at https://www.khronos.org/webgl/wiki/HandlingContextLost#Don%E2%80%99t_check_for_null_on_creation

    // if (shader === null) {
    //  throw new Error("shader is null, you've lost context");
    // }
    gl.shaderSource(shader, source);
    gl.compileShader(shader);

    const success = gl.getShaderParameter(shader, gl.COMPILE_STATUS);
    if (success) {
        return shader;
    }

    console.error(gl.getShaderInfoLog(shader));
    gl.deleteShader(shader);
    return null;
}

🙁 Actual behavior

Context methods that take in WebGL resources (shader, program, buffer...) report errors when provided with null but create* methods return the resource (pointer) or null, and the spec says we should not check for null upon resource creation because all functions accept null as a parameter (no-op).

example :

gl.shaderSource(shader, source);

Argument of type 'WebGLShader | null' is not assignable to parameter of type 'WebGLShader'. Type 'null' is not assignable to type 'WebGLShader'.

🙂 Expected behavior

Context methods taking resources as parameters should allow for null to be passed in. This would delineate from MDN's documentation but fit closer to the actual behavior of the API and the WebGL(2) spec.

Additional information about the issue

No response

MartinJohns commented 4 months ago

TypeScript frequently forbids code that would be valid according to specification. The question to ask is: Would people consider it a potential bug in their code if they pass in null? I'd say yes.

Ragnar-Oock commented 4 months ago

In library-authored code, I would agree, but it's of a native API we are talking about. And, as I pointed out, the official wiki recommends not to check for null on resource creation because the API is meant to support null values in place of resources.

You should really only get null from these functions if the context is lost and in that case there’s no reason to check. WebGL is designed so that for the most part everything just runs as normal even when a program or shader is null. khronos.org --- https://www.khronos.org/webgl/wiki/HandlingContextLost#Don%E2%80%99t_check_for_null_on_creation

So because the type definitions from lib.dom don't include null for the methods accepting resources or null (in the example shaderSource, compileShader, typescript raises errors on code which is recommended on the official doc.