-
-
Notifications
You must be signed in to change notification settings - Fork 22
feat: Make std functions more generic #1723
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
base: main
Are you sure you want to change the base?
Conversation
pkg.pr.new packages
benchmark commit |
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.
wgsl.MatchingBoolInstance<T>
can also simplify select
signature
|
||
const cpuLt = <T extends AnyNumericVecInstance>(lhs: T, rhs: T) => | ||
VectorOps.lt[lhs.kind](lhs, rhs); | ||
function cpuLt(lhs: number, rhs: number): boolean; |
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.
At some point we decided that if a plain js operator works, then we do not write overloads for std operators so as not to encourage users to complicate their code.
I assume we withdraw that decision to allow for overloaded functions, right? If so, the JSDocs would need updating
|
||
const cpuLt = <T extends AnyNumericVecInstance>(lhs: T, rhs: T) => | ||
VectorOps.lt[lhs.kind](lhs, rhs); | ||
function cpuLt(lhs: number, rhs: number): boolean; |
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.
I am not convinced about these new overloads. The following now becomes type safe, and it will generate invalid wgsl. It would be fine if we handled mixed comparisons though. Maybe we could make unify
cast numbers to vectors by wrapping them?
function cmpLt(a: number | AnyNumericVecInstance, b: number | AnyNumericVecInstance) {
"kernel";
return lt(a, b);
}
cmpLt(3, vec2f());
returnType: correspondingBooleanVectorSchema(uargs[0]), | ||
}); | ||
}, | ||
normalImpl: <T extends AnyNumericVecInstance>(lhs: T, rhs: T) => |
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.
This function's interface is now inconsistent with other boolean functions
returnType: correspondingBooleanVectorSchema(uargs[0]), | ||
}); | ||
}, | ||
normalImpl: <T extends AnyNumericVecInstance>(lhs: T, rhs: T) => |
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.
Here as well
No description provided.