Skip to content

Fix validation errors + panics on empty buffers#720

Merged
bors[bot] merged 1 commit intogfx-rs:masterfrom
rukai:empty_buffers_fix
Jun 15, 2020
Merged

Fix validation errors + panics on empty buffers#720
bors[bot] merged 1 commit intogfx-rs:masterfrom
rukai:empty_buffers_fix

Conversation

@rukai
Copy link
Copy Markdown
Contributor

@rukai rukai commented Jun 14, 2020

Description
My previous PR left a vulkan validation error when creating an empty buffer.
This PR fixes that and also fixes a panic preventing the compute-example from running with no elements.

Testing
Unit test added in wgpu-rs PR. gfx-rs/wgpu-rs#373

Copy link
Copy Markdown
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.

Thank you for addressing this! I have a couple of concerns

Err(e) => {
log::error!("Mapping failed {:?}", e);
resource::BufferMapAsyncStatus::Error
let status = if mapping.sub_range.size.map(|x| x != 0).unwrap_or(true) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

bit: let's use map_or

Copy link
Copy Markdown
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.

Thanks!
the map_or thing is not a blocker, we can proceed
bors r+

bors bot added a commit that referenced this pull request Jun 15, 2020
720: Fix validation errors + panics on empty buffers r=kvark a=rukai

**Description**
My previous PR left a vulkan validation error when creating an empty buffer.
This PR fixes that and also fixes a panic preventing the compute-example from running with no elements.

**Testing**
Unit test added in wgpu-rs PR. gfx-rs/wgpu-rs#373
<!--
Non-trivial functional changes would need to be tested through:
  - [wgpu-rs](https://github.com/gfx-rs/wgpu-rs) - test the examples.
  - [wgpu-native](https://github.com/gfx-rs/wgpu-native/) - check the generated C header for sanity.

Ideally, a PR needs to link to the draft PRs in these projects with relevant modifications.
See #666 for an example.
If you can add a unit/integration test here in `wgpu`, that would be best.
-->


Co-authored-by: Rukai <rubickent@gmail.com>
@rukai rukai force-pushed the empty_buffers_fix branch from 88ddffd to b047552 Compare June 15, 2020 01:04
@bors
Copy link
Copy Markdown
Contributor

bors bot commented Jun 15, 2020

Canceled.

@rukai
Copy link
Copy Markdown
Contributor Author

rukai commented Jun 15, 2020

ah, well i've pushed it now

@kvark
Copy link
Copy Markdown
Member

kvark commented Jun 15, 2020

Thanks, that's even better!
bors r+

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Jun 15, 2020

@bors bors bot merged commit bb3e179 into gfx-rs:master Jun 15, 2020
bors bot added a commit to gfx-rs/wgpu-rs that referenced this pull request Jun 15, 2020
373: add test case for empty buffer r=kvark a=rukai

Depends on gfx-rs/wgpu#720

Co-authored-by: Rukai <rubickent@gmail.com>
kvark pushed a commit to kvark/wgpu that referenced this pull request Jun 3, 2021
373: add test case for empty buffer r=kvark a=rukai

Depends on gfx-rs#720

Co-authored-by: Rukai <rubickent@gmail.com>
kvark added a commit to kvark/wgpu that referenced this pull request Jun 3, 2021
720: Remove typed-arena dependency r=cwfitzgerald a=kvark

Also bumps the wgpu dependency to include gfx-rs#1160

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
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