Skip to content

Separate valid internal IDs from external ones#857

Merged
bors[bot] merged 1 commit intogfx-rs:masterfrom
kvark:error
Aug 4, 2020
Merged

Separate valid internal IDs from external ones#857
bors[bot] merged 1 commit intogfx-rs:masterfrom
kvark:error

Conversation

@kvark
Copy link
Copy Markdown
Member

@kvark kvark commented Jul 31, 2020

Connections
Closes #638
wgpu-rs update - gfx-rs/wgpu-rs#494

Description
The core change here is to allow user-specified IDs to be in the "Error" state that was introduced in #776 .

This is done by defining an internal Valid<I> wrapper. Now, the hub storages can be indexed by this valid wrapper only. For regular IDs, we have to go through storage.get(index), which returns a Result. It still panics if the ID is totally garbage though, we don't want to handle what can't be expected here.

All the other changes come mostly as a consequence of that:

  • new "Invalid*" error variants are added
  • the error types are undergone sever refactoring
  • new command/draw.rs module for stuff shared between render passes and bundles
  • functions to generate error IDs for all the types
  • various renames, e.g. comb -> cmd_buf

The expected use by wgpu-rs is unchanged. So far, I don't think we'd be generating Error IDs, but we can always reconsider.
For browsers though, if device_create_xxx failed, they would generate and error ID. It will occupy the slot up until the corresponding JS object is dropped.

Testing
Tested on wgpu-rs examples

@kvark kvark requested review from cwfitzgerald and grovesNL July 31, 2020 18:44
Copy link
Copy Markdown
Contributor

@monocodus monocodus bot left a comment

Choose a reason for hiding this comment

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

This is an autogenerated code review.

Checker summary (by rust_clippy):
The tool has found 0 warnings, 1 errors.

Copy link
Copy Markdown
Contributor

@monocodus monocodus bot left a comment

Choose a reason for hiding this comment

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

This is an autogenerated code review.

Checker summary (by rust_clippy):
The tool has found 0 warnings, 1 errors.

bors bot added a commit to gfx-rs/wgpu-rs that referenced this pull request Aug 4, 2020
494: Update wgpu with the error model changes r=cwfitzgerald a=kvark

Depends on gfx-rs/wgpu#857

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
Copy link
Copy Markdown
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

A lot of great changes in here! Skimmed everything, but looked at the core parts and saw nothing obviously wrong or smelly!

LGTM!

.expect("Unable to find an adapter for selected backend");

let info = gfx_select!(adapter => global.adapter_get_info(adapter));
let info = gfx_select!(adapter => global.adapter_get_info(adapter)).unwrap();
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.

Not in scope, but we likely want an unwrap_pretty type thing here too.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

These are actually not expected to fail at all, unless we are hand-writing them.
I would be happy to accept such PR anyway though :)

@kvark
Copy link
Copy Markdown
Member Author

kvark commented Aug 4, 2020

bors r=cwfitzgerald

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Aug 4, 2020

@bors bors bot merged commit 78546f4 into gfx-rs:master Aug 4, 2020
bors bot added a commit to gfx-rs/wgpu-rs that referenced this pull request Aug 5, 2020
494: Update wgpu with the error model changes r=cwfitzgerald a=kvark

Depends on gfx-rs/wgpu#857

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
kvark added a commit to kvark/wgpu that referenced this pull request Jun 3, 2021
494: Update wgpu with the error model changes r=cwfitzgerald a=kvark

Depends on gfx-rs#857

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
kvark pushed a commit to kvark/wgpu that referenced this pull request Jun 3, 2021
857: updated wgpu with more copyable types r=kvark a=adamnemecek



Co-authored-by: adamnemecek <adamnemecek@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.

Error model

3 participants