Skip to content

Add WebGPUTextureAspectFlags#89

Merged
Kangz merged 1 commit intogpuweb:masterfrom
grovesNL:aspects
Sep 21, 2018
Merged

Add WebGPUTextureAspectFlags#89
Kangz merged 1 commit intogpuweb:masterfrom
grovesNL:aspects

Conversation

@grovesNL
Copy link
Contributor

The changes in #69 reference WebGPUTextureAspect but it wasn't added in the PR.

Not sure if it has been discussed outside #69 yet, but I guess something like this definition would be reasonable.

};

typedef u32 WebGPUTextureAspectFlags;
interface WebGPUTextureAspect {
Copy link
Contributor

Choose a reason for hiding this comment

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

I assumed this would be an enum instead of a bitfield because of the following line in the vulkan spec for the validation of VkBufferImageCopy:

The aspectMask member of imageSubresource must only have a single bit set

That said it would be ok to allow both aspects to be copied at once and split the copy on the Vulkan backend

Copy link
Contributor

@kvark kvark Sep 18, 2018

Choose a reason for hiding this comment

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

FYI, Vulkan doesn't require an implementation to support buffer-to-texture (or texture-to-texture for the matter) copies with depth-stencil involved. See Table 31.19. Mandatory format support: 64-bit/uneven channels and depth/stencil.

I don't think we want to support copying depth/stencil separately, although I'd be open to discuss this :)
Maybe this aspect flag isn't needed here at all then?

Copy link

@magcius magcius Sep 18, 2018

Choose a reason for hiding this comment

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

The aspect flag is also used for texture views and sampling. It's how you choose between sampling the depth or stencil components of a DS texture.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, and for texture views it doesn't make sense for it to be an enum. I suggest to leave it as bitflags and remove the use of it from WebGPUTextureCopyView

@Kangz
Copy link
Contributor

Kangz commented Sep 21, 2018

This seems non-controversial, landing to unblock #90

@Kangz Kangz merged commit 6a42e71 into gpuweb:master Sep 21, 2018
@grovesNL grovesNL deleted the aspects branch December 4, 2018 17:32
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.

4 participants