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

Creating TextureDimension::D1 depth/stencil formats makes metal panic (and does not fail) #2323

Closed
Wumpf opened this issue Dec 28, 2021 · 2 comments · Fixed by #2331
Closed
Assignees
Labels
area: validation Issues related to validation, diagnostics, and error handling help required We need community help to make this happen. type: enhancement New feature or request

Comments

@Wumpf
Copy link
Member

Wumpf commented Dec 28, 2021

Description
Creating a 1 dimensional depth(/stencil) format does not cause a failure but makes metal panic:

-[MTLTextureDescriptorInternal validateWithDevice:]:1248: failed assertion `Texture Descriptor Validation
MTLTextureDescriptor: depth/stencil pixel formats require texture type of MTLTextureType2D[Array] or MTLTextureTypeCube[Array].

Similar to #2045, #2057

Repro steps
As described. Seen when running clear_texture test on Metal with a depth format after depth formats got enabled as in #2307

Expected vs observed behavior
Should give a specialized error.
This is also lacking in the WebGPU spec I believe.

Extra materials
n/a

Platform
MacOS

@Wumpf
Copy link
Member Author

Wumpf commented Dec 28, 2021

Found it in the spec after all, it is called out on createTexture https://gpuweb.github.io/gpuweb/#dom-gpudevice-createtexture

"1d"
...
        descriptor.format must not be a compressed format or depth-or-stencil format.

Wumpf added a commit to Wumpf/wgpu that referenced this issue Dec 28, 2021
kvark pushed a commit that referenced this issue Dec 28, 2021
* CLEAR_COMMANDS extension is now more of a window into wgpu zero-init
this has mostly implications on the constraints, but also allows a more leaky documentation which makes sense for this non-standard function as there is no other place to look it up

* clear_texture via renderpasses wip

* 3D depth textures are no longer allowed, volumes are always cleared via CPY_DST

* cleanup texture's clear_views

* rename CLEAR_COMMANDS to CLEAR_TEXTURE

* separate clear_texture into reusable & more descriptive parts

* texture clear views are now created ahead of time

* discarded surface fixup goes through new clear_texture method now

* onq ueue texture initialization now goes threw clear_texture
pending inits need to store Stored textures now though, causing more ref count bumping

* texture init on queue_write_texture now also goes through new clear_texture

* transfer functions on commandbuffer use now new texture init route

* merge collect_zero_buffer_copies_for_clear_texture into clear_texture_via_buffer_copies

* clear functions now take TextureInitRange

* Fix clippy lints

* command_encoder_clear_texture no longer takes write lock on texture

* TextureClearMode encodes now is_color

* code cleanup, mostly about `use`

* Handle volume textures in clear_texture_via_render_passes properly

* texture clear no longer requires id::Stored

* init tracking fixes for volumes and init on partial subresource writes

* texture creation enforces COPY_DST only if absolutely necessary

* unrolled functional chain, reduce unsafe scope size

* fix clippy lints

* clear_texture test no longer creates 1D textures

see #2323

* 3D textures are no longer cleared as render target since this isn't supported on Metal

* fix deno building issue, fix formatting

* TextureInner::Surface can now be zero initialized
Wumpf added a commit to Wumpf/wgpu that referenced this issue Dec 29, 2021
* CLEAR_COMMANDS extension is now more of a window into wgpu zero-init
this has mostly implications on the constraints, but also allows a more leaky documentation which makes sense for this non-standard function as there is no other place to look it up

* clear_texture via renderpasses wip

* 3D depth textures are no longer allowed, volumes are always cleared via CPY_DST

* cleanup texture's clear_views

* rename CLEAR_COMMANDS to CLEAR_TEXTURE

* separate clear_texture into reusable & more descriptive parts

* texture clear views are now created ahead of time

* discarded surface fixup goes through new clear_texture method now

* onq ueue texture initialization now goes threw clear_texture
pending inits need to store Stored textures now though, causing more ref count bumping

* texture init on queue_write_texture now also goes through new clear_texture

* transfer functions on commandbuffer use now new texture init route

* merge collect_zero_buffer_copies_for_clear_texture into clear_texture_via_buffer_copies

* clear functions now take TextureInitRange

* Fix clippy lints

* command_encoder_clear_texture no longer takes write lock on texture

* TextureClearMode encodes now is_color

* code cleanup, mostly about `use`

* Handle volume textures in clear_texture_via_render_passes properly

* texture clear no longer requires id::Stored

* init tracking fixes for volumes and init on partial subresource writes

* texture creation enforces COPY_DST only if absolutely necessary

* unrolled functional chain, reduce unsafe scope size

* fix clippy lints

* clear_texture test no longer creates 1D textures

see gfx-rs#2323

* 3D textures are no longer cleared as render target since this isn't supported on Metal

* fix deno building issue, fix formatting

* TextureInner::Surface can now be zero initialized
kvark pushed a commit that referenced this issue Dec 29, 2021
* CLEAR_COMMANDS extension is now more of a window into wgpu zero-init
this has mostly implications on the constraints, but also allows a more leaky documentation which makes sense for this non-standard function as there is no other place to look it up

* clear_texture via renderpasses wip

* 3D depth textures are no longer allowed, volumes are always cleared via CPY_DST

* cleanup texture's clear_views

* rename CLEAR_COMMANDS to CLEAR_TEXTURE

* separate clear_texture into reusable & more descriptive parts

* texture clear views are now created ahead of time

* discarded surface fixup goes through new clear_texture method now

* onq ueue texture initialization now goes threw clear_texture
pending inits need to store Stored textures now though, causing more ref count bumping

* texture init on queue_write_texture now also goes through new clear_texture

* transfer functions on commandbuffer use now new texture init route

* merge collect_zero_buffer_copies_for_clear_texture into clear_texture_via_buffer_copies

* clear functions now take TextureInitRange

* Fix clippy lints

* command_encoder_clear_texture no longer takes write lock on texture

* TextureClearMode encodes now is_color

* code cleanup, mostly about `use`

* Handle volume textures in clear_texture_via_render_passes properly

* texture clear no longer requires id::Stored

* init tracking fixes for volumes and init on partial subresource writes

* texture creation enforces COPY_DST only if absolutely necessary

* unrolled functional chain, reduce unsafe scope size

* fix clippy lints

* clear_texture test no longer creates 1D textures

see #2323

* 3D textures are no longer cleared as render target since this isn't supported on Metal

* fix deno building issue, fix formatting

* TextureInner::Surface can now be zero initialized
@kvark kvark added area: validation Issues related to validation, diagnostics, and error handling help required We need community help to make this happen. type: enhancement New feature or request labels Dec 29, 2021
@kvark
Copy link
Member

kvark commented Dec 29, 2021

Yes, let's forbid depth-stencil 1D textures please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: validation Issues related to validation, diagnostics, and error handling help required We need community help to make this happen. type: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants