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.

Sign up for GitHub

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

Jump to bottom

Clamp overload for scalar min and max with vector value #4815

Open
mehmetoguzderin opened this issue Aug 13, 2024 · 3 comments
Open

Clamp overload for scalar min and max with vector value #4815

mehmetoguzderin opened this issue Aug 13, 2024 · 3 comments
Labels
wgsl WebGPU Shading Language Issues
Milestone
Milestone 2

Comments

@mehmetoguzderin
Copy link
Member

Issue 2 extracted from #4750 by @jacobgorm

The WGSL clamp function currently requires all arguments to be of the same type, including when used with vectors. Please see: https://gpuweb.github.io/gpuweb/wgsl/#clamp

This behavior differs from some other shading languages that allow scalar min/max arguments with a vector input.

The lack of such overloads for clamp complicates code porting and readability. Developers currently need to create vector versions of scalar values when using clamp with mixed vector and scalar arguments. Supporting such an overload could improve developer experience of WGSL.
@mehmetoguzderin mehmetoguzderin added the wgsl WebGPU Shading Language Issues label Aug 13, 2024
@jimblandy
Copy link
Contributor

jimblandy commented Aug 27, 2024 •
edited
Loading

The suggested behavior here seems very good. It would make sense to allow splats on all three arguments. But there are two issues:

  • It's not just clamp; smoothstep and mix would also be reasonable to update. We should actually put together a full list of everything we want to change, and not do it piecemeal.
  • In that light, given that the language is usable, if awkward, we may want to treat this as a nice treat for M2, unless the list is very short.

@alan-baker
Copy link
Contributor

mix already has a version that takes two scalars and vector because of a previous request.

I don't think you generally need an overload for clamp where the data is splatted. Usually it is vector data and scalar min/max arguments.

I also agree that, at this point, we should revisit splats generally before making a specific change. I'm not advocating one way or another, but it would be good to decide on the overall issue first.

@kdashg
Copy link
Contributor

kdashg commented Aug 27, 2024

WGSL 2024-08-27 Minutes
  • JB: Mozilla’s thinking:these sound like good changes, with real improvements. Have a lot of places in the spec where it makes sense. Going through them piecemeal seems not optimal. Should review systematically. At that point it looks like a bigger project. The language is usable as-is. We feel it makes sense to treat this as M2. For now focus on shipping what we have right now. When we come to M2 and address all these issue they’ll be very happy with the improvement.
  • AB: We basically agree. We did do one like this, for mix. Do we want splatting in general, or in special cases. Look at it in a batch.
  • JB: For this and the next two items, generally.

@kdashg kdashg mentioned this issue Aug 27, 2024
Open
@kdashg kdashg added this to the Milestone 2 milestone Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
wgsl WebGPU Shading Language Issues
Projects
None yet
Milestone
Milestone 2
Development

No branches or pull requests
4 participants
@jimblandy @kdashg @mehmetoguzderin @alan-baker