-
Notifications
You must be signed in to change notification settings - Fork 3
feat(webgl): implement vertexAttribI4i, vertexAttribI4ui, vertexAttri… #399
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
Conversation
…bI4iv, and vertexAttribI4uiv methods with argument validation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements four WebGL2 vertex attribute methods that were previously marked as "Not implemented": vertexAttribI4i, vertexAttribI4ui, vertexAttribI4iv, and vertexAttribI4uiv. These methods set integer vertex attributes with proper argument validation.
Key changes:
- Added complete implementations for all four methods with argument count and type validation
- Implemented support for both typed arrays (Int32Array/Uint32Array) and regular JavaScript arrays in the vector variants
- Added minimum component count validation (4 components required) for array-based methods
| int index = args[0]->Int32Value(context).ToChecked(); | ||
| int v0 = args[1]->Int32Value(context).ToChecked(); | ||
| int v1 = args[2]->Int32Value(context).ToChecked(); | ||
| int v2 = args[3]->Int32Value(context).ToChecked(); | ||
| int v3 = args[4]->Int32Value(context).ToChecked(); | ||
|
|
||
| handle()->vertexAttribI4i(index, v0, v1, v2, v3); |
Copilot
AI
Oct 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The index parameter should be validated to ensure it's non-negative and within valid bounds (typically < MAX_VERTEX_ATTRIBS, usually 16 or 32 depending on implementation). Invalid index values could cause undefined behavior in the underlying OpenGL calls.
| uint32_t v1 = args[2]->Uint32Value(context).ToChecked(); | ||
| uint32_t v2 = args[3]->Uint32Value(context).ToChecked(); | ||
| uint32_t v3 = args[4]->Uint32Value(context).ToChecked(); | ||
|
|
Copilot
AI
Oct 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The index parameter should be validated to ensure it's non-negative and within valid bounds (typically < MAX_VERTEX_ATTRIBS). This matches the validation pattern needed in vertexAttribI4i.
| // Validate index parameter | |
| constexpr int MAX_VERTEX_ATTRIBS = 16; // WebGL2 minimum, replace with actual value if available | |
| if (index < 0 || index >= MAX_VERTEX_ATTRIBS) { | |
| isolate->ThrowException(Exception::RangeError( | |
| String::NewFromUtf8(isolate, "vertexAttribI4ui: index out of range").ToLocalChecked())); | |
| return; | |
| } |
| return; | ||
| } | ||
|
|
||
| int index = args[0]->Int32Value(context).ToChecked(); |
Copilot
AI
Oct 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The index parameter should be validated to ensure it's non-negative and within valid bounds (typically < MAX_VERTEX_ATTRIBS). This validation is missing across all four methods.
| int index = args[0]->Int32Value(context).ToChecked(); | |
| int index = args[0]->Int32Value(context).ToChecked(); | |
| // Validate index is non-negative and within bounds | |
| constexpr int MAX_VERTEX_ATTRIBS = 16; // WebGL spec minimum, adjust if available from implementation | |
| if (index < 0 || index >= MAX_VERTEX_ATTRIBS) | |
| { | |
| isolate->ThrowException(Exception::TypeError( | |
| MakeMethodError(isolate, "vertexAttribI4iv", "index out of range"))); | |
| return; | |
| } |
| return; | ||
| } | ||
|
|
||
| handle()->vertexAttribI4iv(index, values); |
Copilot
AI
Oct 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The index parameter should be validated to ensure it's non-negative and within valid bounds (typically < MAX_VERTEX_ATTRIBS). This validation is missing across all four methods.
| return; | ||
| } | ||
|
|
||
| int index = args[0]->Int32Value(context).ToChecked(); |
Copilot
AI
Oct 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The index parameter should be validated to ensure it's non-negative and within valid bounds (typically < MAX_VERTEX_ATTRIBS). This validation is missing across all four methods.
| return; | ||
| } | ||
|
|
||
| handle()->vertexAttribI4uiv(index, values); |
Copilot
AI
Oct 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The index parameter should be validated to ensure it's non-negative and within valid bounds (typically < MAX_VERTEX_ATTRIBS). This validation is missing across all four methods.
|
@copilot please support vertexAttrib[1234]f[v] https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/vertexAttrib |
|
@EndlessJour9527 I've opened a new pull request, #400, to work on those changes. Once the pull request is ready, I'll request review from you. |
…t/support-vertext-attribI4x
implement vertexAttribI4i, vertexAttribI4ui, vertexAttrbI4iv, and vertexAttribI4uiv methods with argument validation