Sitelet https://github.com/gpuweb/gpuweb/pull/3403
Skip to content

Add texture sample function which clamps coordinates - #3403

Merged
dneto0 merged 2 commits into
gpuweb:mainfrom
kainino0x:clamp01
Sep 13, 2022
Merged

Add texture sample function which clamps coordinates#3403
dneto0 merged 2 commits into
gpuweb:mainfrom
kainino0x:clamp01

Conversation

@kainino0x

@kainino0x kainino0x commented Aug 31, 2022

Copy link
Copy Markdown
Contributor

Adds this function for both texture_2d and texture_external for
symmetry as agreed in discussion.

Name up for bikeshedding, but this is my proposal.

Fixes #2766

@kainino0x
Add texture sample function which clamps coordinates
8d8ebd7 
Adds this function for both texture_2d and texture_external for
symmetry as agreed in discussion.

Fixes issue 2766
@kainino0x kainino0x added the copyediting Pure editorial stuff (copyediting, *.bs file syntax, etc.) label Aug 31, 2022
@kainino0x kainino0x added this to the V1.0 milestone Aug 31, 2022
@github-actions

github-actions Bot commented Aug 31, 2022 •
edited by mehmetoguzderin
Loading

Copy link
Copy Markdown
Contributor

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

@kainino0x

kainino0x commented Aug 31, 2022

Copy link
Copy Markdown
Contributor Author

Name up for bikeshedding, but this is my proposal.

LOD naming:

  • Level: makes it sound like you're going to pass a level
  • Base: IMO a bit clearer because it uses the "base" of the texture view

clamping naming:

  • Clamp, Clamped: sounds like clamping the LOD
  • Clamped, Clamp01: sounds like clamping the output of the sampling operation
  • ClampToEdge: matches the language used in the API ("clamp-to-edge")

toji
toji approved these changes Aug 31, 2022
View reviewed changes

@toji toji left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was Winston Churchill who said "textureSampleBaseClampToEdge is the worst name for a function – except for all the others that have been considered."

LGTM.

@kainino0x kainino0x mentioned this pull request Sep 1, 2022
Closed
dneto0
dneto0 approved these changes Sep 13, 2022
View reviewed changes

@dneto0 dneto0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nit for removing a sentence.

Comment thread wgsl/index.bs
@kainino0x
kainino0x removed the request for review from alan-baker September 13, 2022 18:08
@kainino0x

kainino0x commented Sep 13, 2022

Copy link
Copy Markdown
Contributor Author

@litherum PTAL

dneto0
dneto0 approved these changes Sep 13, 2022
View reviewed changes

@dneto0 dneto0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dneto0

dneto0 commented Sep 13, 2022

Copy link
Copy Markdown
Contributor

Approved in WGSL meeting 2022-09-13.

@dneto0
dneto0 merged commit b60b410 into gpuweb:main Sep 13, 2022
@kdashg

kdashg commented Sep 13, 2022

Copy link
Copy Markdown
Contributor
WGSL 2022-09-13 Minutes
  • Resolved: Accept

@kainino0x
kainino0x deleted the clamp01 branch September 13, 2022 20:50
@kainino0x kainino0x added the needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet label Sep 13, 2022
@lokokung lokokung removed the needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet label Oct 13, 2022
ben-clayton added a commit to ben-clayton/cts that referenced this pull request Nov 17, 2022
@ben-clayton
textureSampleLevel(texture_external) -> `textureSampleBaseClampToEd…
bd36e41 
…ge()`

The `textureSampleLevel(texture_external)` overload has been replaced with `textureSampleBaseClampToEdge(texture_external)`

See gpuweb/gpuweb#3403
@ben-clayton ben-clayton mentioned this pull request Nov 17, 2022
Merged
3 of 7 tasks
ben-clayton added a commit to gpuweb/cts that referenced this pull request Nov 17, 2022
@ben-clayton
textureSampleLevel(texture_external) -> `textureSampleBaseClampToEd…
992c9a8 
…ge()`

The `textureSampleLevel(texture_external)` overload has been replaced with `textureSampleBaseClampToEdge(texture_external)`

See gpuweb/gpuweb#3403
jiangzhaoming added a commit to jiangzhaoming/gpuweb that referenced this pull request Nov 25, 2022
@jiangzhaoming
wgsl: access texture_external with textureSampleBaseClampToEdge i…
fb31dad 
…nstead of `textureSampleLevel`

As is discussed in gpuweb#2766 and added into WGSL spec in gpuweb#3403,
`textureSampleBaseClampToEdge` should be used when sampling from
`texture_external`.
@jiangzhaoming jiangzhaoming mentioned this pull request Nov 25, 2022
Merged
jiangzhaoming added a commit that referenced this pull request Nov 30, 2022
@jiangzhaoming
wgsl: access texture_external with textureSampleBaseClampToEdge i…
b8f633b 
…nstead of `textureSampleLevel` (#3637)

As is discussed in #2766 and added into WGSL spec in #3403,
`textureSampleBaseClampToEdge` should be used when sampling from
`texture_external`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@toji toji toji approved these changes
@dneto0 dneto0 dneto0 approved these changes
@litherum litherum Awaiting requested review from litherum

Assignees

No one assigned

Labels

copyediting Pure editorial stuff (copyediting, *.bs file syntax, etc.)

Projects

None yet

Milestone

Milestone 0

Development

Successfully merging this pull request may close these issues.

Specify that texture_external sampling functions clamp coords to [0, 1]

5 participants

@kainino0x @dneto0 @kdashg @toji @lokokung