Skip to content

Allow GLBoolean in pixelStorei #583

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

Closed
wants to merge 3 commits into from
Closed

Conversation

AVGP
Copy link

@AVGP AVGP commented Oct 17, 2018

@msftclas
Copy link

msftclas commented Oct 17, 2018

CLA assistant check
All CLA requirements met.

@AVGP
Copy link
Author

AVGP commented Oct 17, 2018

The reason for this change is that while the GL spec specifies the type for param to be GLint, the table in https://www.khronos.org/registry/OpenGL-Refpages/gl4/html/glPixelStore.xhtml demonstrates that GLboolean is a valid type, depending on the parameter to be set.

@fatcerberus
Copy link

Is it possible to use overloads to enforce the correct type based on the specific parameter being set? That would be ideal if it's possible to do so.

@AVGP
Copy link
Author

AVGP commented Oct 24, 2018

I don't know how I could do that - any ideas?

@fatcerberus
Copy link

You’d have to use literal types, but this requires the value of the GLenum constants to be known at compile time - if they aren’t compile-time constants (e.g. members of an enum, or known numeric values) then you can’t do it.

Just to illustrate the concept if you aren’t familiar with it:

func(control: 0, value: number);
func(control: 1, value: string);

So for the above function, if you pass 0 as the first arg, the second must be a number, if the first arg is 1 you have to pass a string.

I have a feeling the gl.CONSTANT_NAME values might not be static constants and/or are browser-specific though (so you couldn’t put them into an enum). :(

@sandersn
Copy link
Member

@fatcerberus I looked at WebGl 1.widl and it defines GLenum as unsigned long. Then there are lots of related constants in WebGLRenderingContextBase, but the generated d.ts doesn't include their values. It should. But this requires a change to the generator code, which I'm not up to speed on yet.

I would also need to find out if widl supports overloads discriminated by integer literal types. If not, then the change would have to go in overridingTypes.json (I think).

@AVGP It turns out that this change WAS in dom.generated.d.ts, but it got lost when we switched away from IE's idl files to standard ones. Can you just copy the change from #178 instead of adding an overload? I think it's better for the simple fix.

@@ -653,6 +653,7 @@ interface mixin WebGLRenderingContextBase
void lineWidth(GLfloat width);
void linkProgram(WebGLProgram program);
void pixelStorei(GLenum pname, GLint param);
void pixelStorei(GLenum pname, GLboolean param);
Copy link
Contributor

Choose a reason for hiding this comment

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

IDL files shouldn't be manually changed, instead overridingTypes.json can.

@orta
Copy link
Contributor

orta commented Feb 25, 2020

This is deprecated by #638

@orta orta closed this Feb 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants