Skip to content

Clarify: texture+sampling validation applies to pairs used together in a builtin#4945

Merged
dneto0 merged 1 commit into
gpuweb:mainfrom
dneto0:issue-4944
Oct 31, 2024
Merged

Clarify: texture+sampling validation applies to pairs used together in a builtin#4945
dneto0 merged 1 commit into
gpuweb:mainfrom
dneto0:issue-4944

Conversation

@dneto0

@dneto0 dneto0 commented Oct 30, 2024

Copy link
Copy Markdown
Contributor

Fixed: #4944

@dneto0 dneto0 added this to the Milestone 0 milestone Oct 30, 2024
@dneto0 dneto0 requested a review from kainino0x October 30, 2024 23:46
@dneto0

dneto0 commented Oct 30, 2024

Copy link
Copy Markdown
Contributor Author

cc: @greggman @Kangz

@github-actions

Copy link
Copy Markdown
Contributor

Previews, as seen when this build job started (823f156):
WebGPU webgpu.idl | Explainer | Correspondence Reference
WGSL grammar.js | wgsl.lalr.txt

@dneto0 dneto0 merged commit cfb4914 into gpuweb:main Oct 31, 2024
@kainino0x kainino0x added bug api WebGPU API copyediting Pure editorial stuff (copyediting, *.bs file syntax, etc.) labels Nov 1, 2024
@kainino0x

Copy link
Copy Markdown
Contributor

For the record, this was where we said we couldn't use filtering with depth: #2094 (comment)

@greggman

greggman commented Nov 1, 2024

Copy link
Copy Markdown
Contributor

So FYI, in the WGSL spec there's this

https://gpuweb.github.io/gpuweb/wgsl/#texturesamplecompare

If the sampler uses bilinear filtering then the returned value is the filtered average of these values, otherwise the comparison result of a single texel is returned.

textureSampleCompare only takes depth textures so I guess the sentence above should be removed? Or leave it for someday when depth textures are allowed to be filtered since today the WebGPU spec disallows it.

Note: I was already testing this (averaging of depth texture compare values) and it seemed to be mostly working though not 100% sure it works everywhere. They've been filtered out now.

@kainino0x

Copy link
Copy Markdown
Contributor

Depth textures can definitely be used with filtering sampler_comparison. Maybe the spec text needs to explicitly exclude sampler_comparison since it only cares about sampler.

@greggman

greggman commented Nov 1, 2024

Copy link
Copy Markdown
Contributor

ah, ... hmmm. I think we need a table

texture_?d with depth texture texture_depth_??
sampler 'linear' bad bad
sampler_comparison 'linear' N/A OK

correct?

@kainino0x

Copy link
Copy Markdown
Contributor

yeah, that table seems correct. IMO just tweaking the language to specify it includes sampler but not sampler_comparison is enough though. I'll draft a PR.

(There was a second fix in #4949 but I don't think that one needs any tweak.)

kainino0x added a commit to kainino0x/gpuweb that referenced this pull request Nov 1, 2024
kainino0x added a commit to kainino0x/gpuweb that referenced this pull request Nov 1, 2024
kainino0x added a commit that referenced this pull request Nov 4, 2024
kainino0x added a commit to kainino0x/gpuweb that referenced this pull request Nov 4, 2024
Kangz pushed a commit that referenced this pull request Nov 4, 2024
jrprice pushed a commit to jrprice/gpuweb that referenced this pull request Nov 7, 2024
jrprice pushed a commit to jrprice/gpuweb that referenced this pull request Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api WebGPU API bug copyediting Pure editorial stuff (copyediting, *.bs file syntax, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

define "texture sampling" builtins/operations so it can be refernced from validating GPUProgrammableStage

4 participants