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

sampleType: "float" is not valid with multisampled: true #3364

Closed
kainino0x opened this issue Aug 24, 2022 · 1 comment
Closed

sampleType: "float" is not valid with multisampled: true #3364

kainino0x opened this issue Aug 24, 2022 · 1 comment
Milestone

Comments

@kainino0x
Copy link
Contributor

kainino0x commented Aug 24, 2022

In the Great Bind Group Layout Refactor of #1223, we made a rule that multisampled: true could not be used with sampleType: "float". This topic was discussed on that PR, starting here: #1223 (review)
but we left it as is and deferred the topic.

There's no strong reason for this restriction, except that you can't actually filter a multisampled texture binding at all, and "unfilterable-float" is a strictly less constrained binding type. The only reason I can think of this being useful is if in the future it becomes possible to actually filter multisampled textures. I don't know if that's a thing, but it seems unlikely, and probably wouldn't quite fit into the API in that way anyway.

The actual problem is that sampleType defaults to "float", which makes it less ergonomic: in order to use multisampled: true at all, you must set the sampleType. Plus, using it with sampleType: "float" sounds perfectly valid, but isn't.

On the other hand:

multisampled: true, type: "float" would be extra restrictive for no reason (wouldn't allow unfilterable float formats)

So perhaps it would be even better to automatically upgrade "float" to "unfilterable-float" when multisampled is true, or at least make the sampleType default depend on multisampled.

Options:

  • 0. Leave as is, { multisampled: true } is invalid because sampleType defaults to "float"
  • 1. Allow "float" with multisampled, { multisampled: true } is valid, but has a sampleType of "float" so it won't be valid with some textures (e.g. "r32float")
  • 2a. Change default sampleType to depend on multisampled (multisampled true -> "unfilterable-float", false -> "float")
  • 2b. Auto upgrade "float" to "unfilterable-float" when using multisampled

Based on some test results in gpuweb/cts#1777, I think Chromium/Dawn isn't even implementing this validation. So it's likely people are currently happily using "float" with multisampled.

@kainino0x kainino0x added the tacit resolution candidate Editors may be able to resolve and move to tacit resolution queue label Aug 24, 2022
@kainino0x kainino0x added this to the V1.0 milestone Aug 24, 2022
@kainino0x
Copy link
Contributor Author

Editors: Given that this is a fairly advanced usage of the API (binding multisampled textures to shaders), and the error message should be immediate and clear, choose option 0 for v1. If there is developer input or any further thoughts from members of the group, we can consider reopening this for post-V1 or even V1.

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

No branches or pull requests

1 participant