Conversation
There was a problem hiding this comment.
The .monocodus config not found in your repo. Default config is used.
Check config documentation here
There was a problem hiding this comment.
The .monocodus config not found in your repo. Default config is used.
Check config documentation here
There was a problem hiding this comment.
The .monocodus config not found in your repo. Default config is used.
Check config documentation here
kvark
left a comment
There was a problem hiding this comment.
Thank you for taking a stab at this!
I am becoming worried here. If suddenly any access to anything is going to be able to return an error.
Then there is this contagious aspect: say a texture became invalid at some point, and then we access the view of it at some later point. Technically, we should turn the view into error state too, but how and when are we going to even know that there is a view that needs to be errored?
I need to think about this more and talk to the group maybe.
There was a problem hiding this comment.
The .monocodus config not found in your repo. Default config is used.
Check config documentation here
kvark
left a comment
There was a problem hiding this comment.
I believe my concerns are all addressed now.
Thank you!
|
I didn't squashed the commits (was waiting for the final approval). But I guess that was okay..so cool. |
|
@kunalmohan now with fast CI this becomes less of a problem to just merge stuff outside of bors. So I squashed them on merge. |
782: Add check while trying to remove uninserted Ids r=kvark a=kunalmohan **Connections** _Link to the issues addressed by this PR, or dependent PRs in other repositories_ An attempt to fix #781 regression from #776 **Description** _Describe what problem this is solving, and how it's solved._ When we used `VecMap`, it simply returned `None` for even out of bounds access to the map (We depended on it returning `None`). After #776 , we get a panic. So adding a simple index check before accessing it fixes this issue. **Testing** _Explain how this change is tested._ Tested on wgpu-rs examples with the changes in gfx-rs/wgpu-rs#430. All examples run fine except the `cube` which segfaults. <!-- 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: Kunal Mohan <kunalmohan99@gmail.com>
857: Separate valid internal IDs from external ones r=cwfitzgerald a=kvark **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 Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
776: Update wgpu with the new vertex format r=kvark a=kvark Based on gfx-rs#1235 Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
Connections
Link to the issues addressed by this PR, or dependent PRs in other repositories
fix #772
Description
Describe what problem this is solving, and how it's solved.
This aims to replace
VecMapwith a simpleVec<Element<T>>to keep track of valid and error Ids in Storages. This lays the foundations for #638. A newenum Element<T>is introduced as described in the linked issue.The
ErrorVariant is not used anywhere at the moment.Testing
Explain how this change is tested.
A Successful build is the only check done at the moment.