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

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

Open
Ragnar-Oock opened this issue Apr 15, 2024 · 2 comments
Open

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

Ragnar-Oock opened this issue Apr 15, 2024 · 2 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@Ragnar-Oock
Copy link

πŸ”Ž Search Terms

WebGL2RenderingContext, attachShader, createShader, null

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried (Nightly, 5.4.5, 4.9.5, 3.9.7), and I reviewed the FAQ for entries about WebGL

⏯ 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
Copy link
Contributor

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.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Apr 15, 2024
@Ragnar-Oock
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants