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

Move validation from Device::create_texture_from_hal() to Device::create_texture() #2001

Merged
merged 3 commits into from
Sep 27, 2021

Conversation

dwbrite
Copy link
Contributor

@dwbrite dwbrite commented Sep 26, 2021

Connections
It seems the mipmap validation was originally fixed in #893, and then moved to fn create_texture_from_hal in #1609?

Description
Users encountered a segfault upon creating a texture when mip_level_count was set to 0 with (at least) Vulkan.
This is due to wgpu's validation taking place after the API's texture is created.
This change moves wgpu's validation to before the API's texture is created, introducing a friendlier error instead of a segfault.

Testing
I ran the cube example with mip_level_count as 1, and again on 0.
With mip_level_count set to 0, wgpu now produces the error:

thread 'main' panicked at 'wgpu error: Validation Error

Caused by:
    In Device::create_texture
    texture descriptor mip level count (0) is invalid
    
'

notes:
format_features is being created twice (probably not worth an api change), and validation will no longer happen in the unsafe hal code-paths.

…create_texture()`

---

Users encountered a segfault upon creating a texture when mip_level_count was set to 0 with (at least) Vulkan.
This is due to wgpu's validation taking place after the API's texture is created.

This change moves wgpu's validation to before the API's texture is created, introducing a friendly error instead of a segfault.
Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

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

Nice! Just one small thing

@kvark kvark enabled auto-merge (squash) September 27, 2021 03:58
auto-merge was automatically disabled September 27, 2021 04:21

Head branch was pushed to by a user without write access

@kvark kvark enabled auto-merge (squash) September 27, 2021 13:19
@kvark kvark merged commit dbfdbf4 into gfx-rs:master Sep 27, 2021
Patryk27 pushed a commit to Patryk27/wgpu that referenced this pull request Nov 23, 2022
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.

2 participants