Support descriptor arrays which are shorter than the BGL #1995

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.

Sign up for GitHub

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

Jump to bottom

Merged

kvark merged 1 commit into gfx-rs:master from cwfitzgerald:descriptor-array-mismatch

Sep 30, 2021

Member

cwfitzgerald commented Sep 24, 2021 •

edited

Loading

Connections

Closes #1635
Closes #1111

Description

This allows BG arrays to have fewer elements than are in the BGLs. As long as that element count is non-zero.

Testing

Rend3

cwfitzgerald requested a review from kvark

September 24, 2021 20:46

cwfitzgerald force-pushed the descriptor-array-mismatch branch from ba0576e to 827c6e8 Compare

September 24, 2021 21:27

cwfitzgerald mentioned this pull request

Use massive size BGL BVE-Reborn/rend3#215

Merged

5 tasks

Member Author

cwfitzgerald commented Sep 26, 2021

Do not merge. There are vulkan issues, specifically we need to turn on VK_DESCRIPTOR_BINDING_PARTIALLY_BOUND_BIT.

cwfitzgerald marked this pull request as draft

September 26, 2021 03:38

cwfitzgerald force-pushed the descriptor-array-mismatch branch 4 times, most recently from da022d7 to b0e76a9 Compare

September 29, 2021 02:03

cwfitzgerald marked this pull request as ready for review

September 29, 2021 02:05

Member Author

cwfitzgerald commented Sep 29, 2021

Alright this is ready to go. Needs a thorough comb through as it blew up in scope quite a bit.

cwfitzgerald force-pushed the descriptor-array-mismatch branch from b0e76a9 to b74eba9 Compare

September 29, 2021 04:04

kvark reviewed

View reviewed changes

Member

kvark left a comment

Thank you!
I'm going to make another pass after this round of changes.

wgpu-core/src/device/mod.rs Outdated Show resolved Hide resolved

wgpu-hal/src/vulkan/device.rs Show resolved Hide resolved

wgpu-hal/src/vulkan/device.rs Outdated Show resolved Hide resolved

wgpu-hal/src/vulkan/device.rs Outdated Show resolved Hide resolved

wgpu-hal/src/vulkan/device.rs Show resolved Hide resolved

wgpu-hal/src/vulkan/device.rs Show resolved Hide resolved

wgpu-hal/src/vulkan/mod.rs Outdated Show resolved Hide resolved

wgpu-hal/src/vulkan/adapter.rs Outdated Show resolved Hide resolved

wgpu-hal/src/vulkan/adapter.rs Outdated Show resolved Hide resolved

cwfitzgerald force-pushed the descriptor-array-mismatch branch 4 times, most recently from 298bb74 to 4f65cbe Compare

September 29, 2021 20:17

Member Author

cwfitzgerald commented Sep 29, 2021

Alright, take another look at this, updated with comments and a new UAB strategy.

cwfitzgerald force-pushed the descriptor-array-mismatch branch 2 times, most recently from bf4b0fd to 4804d5e Compare

September 29, 2021 20:32

kvark reviewed

View reviewed changes

wgpu-hal/src/vulkan/adapter.rs Outdated Show resolved Hide resolved

wgpu-hal/src/vulkan/adapter.rs Show resolved Hide resolved

wgpu-hal/src/vulkan/adapter.rs Outdated Show resolved Hide resolved

wgpu-hal/src/vulkan/adapter.rs Outdated Show resolved Hide resolved

wgpu-hal/src/vulkan/device.rs Outdated Show resolved Hide resolved

wgpu-hal/src/vulkan/adapter.rs Outdated Show resolved Hide resolved

cwfitzgerald force-pushed the descriptor-array-mismatch branch 3 times, most recently from 09369ef to cb0827b Compare

September 30, 2021 00:29

Member Author

cwfitzgerald commented Sep 30, 2021

Alright, third time's the charm, will test this in a minute.

kvark reviewed

View reviewed changes

Member

kvark left a comment

This looks great! Just a few nits left

wgpu-hal/src/lib.rs Outdated Show resolved Hide resolved

wgpu-hal/src/vulkan/adapter.rs Outdated Show resolved Hide resolved

wgpu-hal/src/vulkan/device.rs Outdated Show resolved Hide resolved


          Support descriptor arrays shorter than binding length

bd4a9e6

cwfitzgerald force-pushed the descriptor-array-mismatch branch from cb0827b to bd4a9e6 Compare

September 30, 2021 15:55

Member Author

cwfitzgerald commented Sep 30, 2021

Nightly fail is unrelated.

kvark approved these changes

View reviewed changes

kvark merged commit b679342 into gfx-rs:master

kvark mentioned this pull request

Allow if/panic in wgpu-hal #2009

Merged

cwfitzgerald mentioned this pull request

Disable partially bound arrays on intel/windows #2019

Merged

PixelDust22 reviewed

View reviewed changes

wgpu-hal/src/vulkan/adapter.rs

    
                      raw_device: ash::Device,

                      handle_is_owned: bool,

                      enabled_extensions: &[&'static CStr],

                      uab_types: super::UpdateAfterBindTypes,

Contributor

PixelDust22 Oct 8, 2021

wgpu_hal::vulkan is private so I am unable to call the function with this type. We either have to make the vulkan module public or somehow export this in wgpu_hal::api::Vulkan.

Member Author

cwfitzgerald Oct 8, 2021

Ack, I didn't realize that was public. Could you file an issue and I'll get a patch going tomorrow.

Contributor

PixelDust22 Oct 9, 2021

Just filed the issue. Thank you!

Contributor

zmerp Oct 14, 2021

I didn't notice this discussion, I already opened a PR for the fix: #2069

PixelDust22 reviewed

View reviewed changes

wgpu-hal/src/vulkan/mod.rs

    
              }

              impl UpdateAfterBindTypes {

                  fn from_limits(limits: &wgt::Limits, phd_limits: &vk::PhysicalDeviceLimits) -> Self {

Contributor

PixelDust22 Oct 8, 2021

This will probably have to be public as well. I'd expect to be able to construct an UpdateAfterBindTypes if I need to pass it into a public api.

PixelDust22 mentioned this pull request

vulkan::Adapter::device_from_raw contains private enum #2043

Closed

zmerp mentioned this pull request

Fix raw Vulkan API #2069

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet