Skip to content

Conversation

@toji
Copy link
Member

@toji toji commented Feb 3, 2021

Fixes #1397


Preview | Diff

@toji toji requested review from Kangz, kainino0x and kvark February 3, 2021 01:04
Copy link
Contributor

@Kangz Kangz left a comment

Choose a reason for hiding this comment

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

LGTM

@toji toji merged commit d305f2c into gpuweb:main Feb 3, 2021
@toji toji deleted the create-view branch February 3, 2021 16:59
Copy link
Contributor

@kainino0x kainino0x left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, here are some comments for a followup :)

- If |this|.{{GPUTexture/[[dimension]]}} is
<dl class="switch">
: {{GPUTextureDimension/"1d"}}
:: Let |arrayCount| be |this|.{{GPUTexture/[[textureSize]]}}.[=Extent3D/depth=].
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This is a little misleading since we don't have 1d-array textures. If this were encapsulated in an algorithm (see below) then I would explicitly call out that texture.textureSize.depth is expected to be 1.

set |resolved|.{{GPUTextureViewDescriptor/dimension}} to {{GPUTextureViewDimension/"1d"}}.
- If |texture|.{{GPUTexture/[[dimension]]}} is {{GPUTextureDimension/"2d"}}:
- If |texture|.{{GPUTexture/[[textureSize]]}}.[=Extent3D/depth=] is greater than `1`
and |resolved|.{{GPUTextureViewDescriptor/arrayLayerCount}} is greater than `1`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Found another thing while reviewing a CTS change.

The meaning here has changed from the original (before this PR), though that was outdated because it special-cased 0. I think this should be refactored to say:

  • If texture.dimension is 2d:
    • If texture.textureSize.depth is 1 or (the original, pre-defaulting) arrayLayerCount is defined, view dimension is 2d.
    • else view dimension is 2d-array.

Pretty sure that's the original intent. Though it's kind of weird? Maybe it should have been:

  • If texture.dimension is 2d:
    • If texture.textureSize.depth is 1 or (the original, pre-defaulting) arrayLayerCount is 1, view dimension is 2d.
    • else view dimension is 2d-array.

ben-clayton pushed a commit to ben-clayton/gpuweb that referenced this pull request Sep 6, 2022
This PR adds unimplemented tests for the `textureNumSamples` builtin.

Issue gpuweb#1265
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

Successfully merging this pull request may close these issues.

Missing specification for GPUTexture.createView

4 participants