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

Change type of mip_level_count and array_layer_count #3445

Merged
merged 1 commit into from
Feb 3, 2023

Conversation

teoxoy
Copy link
Member

@teoxoy teoxoy commented Feb 2, 2023

Change type of mip_level_count and array_layer_count (members of TextureViewDescriptor and ImageSubresourceRange) from Option<NonZeroU32> to Option<u32>.

Follow up to #3410 (comment).

Also cleans up duplicated code related to texture layers/mips.

@teoxoy teoxoy added this to the WebGPU Specification V1 milestone Feb 2, 2023
@teoxoy teoxoy requested a review from crowlKats as a code owner February 2, 2023 18:21
@teoxoy teoxoy force-pushed the change-type branch 2 times, most recently from 2d39966 to 9a1a663 Compare February 2, 2023 18:38
Copy link
Collaborator

@crowlKats crowlKats left a comment

Choose a reason for hiding this comment

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

Could the deno change instead be to use a #[serde(flatten)] with wgpu_types::ImageSubresourceRange in the CreateTextureViewArgs struct?

…`TextureViewDescriptor` and `ImageSubresourceRange`) from `Option<NonZeroU32>` to `Option<u32>`

Clean up duplicated code related to texture layers/mips.
Copy link
Collaborator

@crowlKats crowlKats left a comment

Choose a reason for hiding this comment

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

Deno part looks good to me

@crowlKats crowlKats dismissed their stale review February 3, 2023 11:59

adressed

@nical
Copy link
Contributor

nical commented Feb 3, 2023

LGTM

@nical nical merged commit 41de797 into gfx-rs:master Feb 3, 2023
@teoxoy teoxoy deleted the change-type branch February 3, 2023 14:07
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.

3 participants