Draft of wgpu-core bump #66

mkeeter · 2021-01-23T16:11:55Z

This is a draft PR to be merged after wgpu-core is bumped to use the latest gfx in this PR

Updates cbindgen instructions
Rebuilds wgpu/ffi.h header
Adds bool now to wgpu_texture_view_destroy to match other _destroy functions which take a bool
Renames now → wait in wgpu_texture_destroy to match other _destroy functions which take a bool

This required one hand-edit of wgpu.h (commit 8c29d4b) to fix an issue where it mis-translated an enum into a struct; not sure why this happened.

(the CI build is going to fail, because I'm waiting for the gfx PR to be merged before bumping hashes in Cargo.toml)

monocodus

This is an autogenerated code review.

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

The .monocodus config not found in your repo. Default config is used.
Check config documentation here

monocodus

This is an autogenerated code review.

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

The .monocodus config not found in your repo. Default config is used.
Check config documentation here

monocodus

This is an autogenerated code review.

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

The .monocodus config not found in your repo. Default config is used.
Check config documentation here

1163: Bump gfx to latest master r=kvark a=mkeeter **Connections** - This bumps `gfx` to [#3610](gfx-rs/gfx#3610), and also includes [#3608](gfx-rs/gfx#3608) and [#3609](gfx-rs/gfx#3608) - [Here](gfx-rs/wgpu-native#66) is a draft PR to `wgpu-native` - `wgpu-rs` requires a one-line fix to the `texture_view_drop` call (which now takes a boolean); I can PR this next. **Description** This fixes [pathological shader complexity in SPIRV-Cross](KhronosGroup/SPIRV-Cross#1594), as well as a few other `gfx` PRs. **Testing** I updated `wgpu-native` to use this branch, then updated my [toy raytracer](https://github.com/mkeeter/rayray) to use the resulting `dylibs` and confirmed that it no longer takes forever to compile the pathological shader. In addition, I updated `wgpu-rs` and went through the examples; nothing seems out of place. Co-authored-by: Matt Keeter <[email protected]>

kvark · 2021-01-23T17:06:17Z

Thank you for an amazing work!

This required one hand-edit of wgpu.h (commit 8c29d4b) to fix an issue where it mis-translated an enum into a struct; not sure why this happened.

Let's look at this closer, we aren't expected to hand-fix anything. At first sight, it does appear like a bug in cbindgen - it confused the wgt::BindingType with the local BindingType. Can we play with it to see what's going on? What if we try to name the local type as something else, like BindingType2? We may need to file this upstream after a bit of investigation.

For the purposes of this PR though, it might be OK to merge the hand-fix, as long as it unblocks you and other users.

monocodus

This is an autogenerated code review.

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

The .monocodus config not found in your repo. Default config is used.
Check config documentation here

mkeeter · 2021-01-23T18:12:34Z

Alright, I reduced it to a test case and reported upstream as cbindgen #649.

726: Bump wgpu-core and update texture_view_drop call r=kvark a=mkeeter This PR updates to the latest `wgpu-core` commit ([wgpu #1163](gfx-rs/wgpu#1163)), and is the counterpart to [wgpu-native #66](gfx-rs/wgpu-native#66). I'm using `wait = false` in the `texture_view_drop` call to match `buffer_drop` and `texture_drop` elsewhere `backend/direct.rs`, though I don't quite understand the implications 😅 Co-authored-by: Matt Keeter <[email protected]>

kvark · 2021-01-23T21:43:43Z

bors r+

66: Draft of wgpu-core bump r=kvark a=mkeeter This is a draft PR to be merged after `wgpu-core` is bumped to use the latest `gfx` in [this PR](gfx-rs/wgpu#1163) - Updates `cbindgen` instructions - Rebuilds `wgpu/ffi.h` header - Adds `bool now` to `wgpu_texture_view_destroy` to match other `_destroy` functions which take a bool - Renames `now` → `wait` in `wgpu_texture_destroy` to match other `_destroy` functions which take a bool This required one hand-edit of `wgpu.h` (commit 8c29d4b) to fix an issue where it mis-translated an `enum` into a `struct`; not sure why this happened. (the CI build is going to fail, because I'm waiting for the `gfx` PR to be merged before bumping hashes in `Cargo.toml`) Co-authored-by: Matt Keeter <[email protected]>

bors · 2021-01-23T21:45:45Z

Build failed:

Windows Stable

kvark · 2021-01-23T23:14:47Z

network issue
bors retry

66: Draft of wgpu-core bump r=kvark a=mkeeter This is a draft PR to be merged after `wgpu-core` is bumped to use the latest `gfx` in [this PR](gfx-rs/wgpu#1163) - Updates `cbindgen` instructions - Rebuilds `wgpu/ffi.h` header - Adds `bool now` to `wgpu_texture_view_destroy` to match other `_destroy` functions which take a bool - Renames `now` → `wait` in `wgpu_texture_destroy` to match other `_destroy` functions which take a bool This required one hand-edit of `wgpu.h` (commit 8c29d4b) to fix an issue where it mis-translated an `enum` into a `struct`; not sure why this happened. (the CI build is going to fail, because I'm waiting for the `gfx` PR to be merged before bumping hashes in `Cargo.toml`) Co-authored-by: Matt Keeter <[email protected]>

bors · 2021-01-23T23:21:34Z

Build failed:

Ubuntu Nightly

kvark · 2021-01-23T23:51:51Z

legit error:

/home/runner/work/wgpu-native/wgpu-native/examples/compute/../../ffi/wgpu.h:2016:10: error: ‘WGPUBindingType’ defined as wrong kind of tag
struct WGPUBindingType ty;
^~~~~~~~~~~~~~~
/home/runner/work/wgpu-native/wgpu-native/examples/compute/../../ffi/wgpu.h:2016:26: error: field ‘ty’ has incomplete type
struct WGPUBindingType ty;

mkeeter · 2021-01-24T02:02:54Z

Oh, that's the cbindgen issue which I fixed with a hand-edit.

It looks like the Ubuntu builder re-runs cbindgen as part of the build here, which the other OS's don't do?

That makes sense in principle, just annoying in this one case 😆

Let's wait to hear what cbindgen upstream thinks of the issue – now that I've built the library locally, this isn't blocking me any more.

mkeeter · 2021-02-02T00:25:29Z

This issue has now been fixed in cbindgen upstream (0.17.0 release), so CI should work!

I've regenerated ffi/wgpu.h, and removed a few types from the config file – for some reason the cbindgen update removed the need to define WGPUNonZeroU64/WGPUOption_NonZeroU32/WGPUOption_NonZeroU64 ourselves 🤷🏻‍♂️

monocodus

This is an autogenerated code review.

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

The .monocodus config not found in your repo. Default config is used.
Check config documentation here

kvark · 2021-02-02T06:28:56Z

bors r+

bors · 2021-02-02T06:44:15Z

Build succeeded:

726: Bump wgpu-core and update texture_view_drop call r=kvark a=mkeeter This PR updates to the latest `wgpu-core` commit ([wgpu gfx-rs#1163](gfx-rs#1163)), and is the counterpart to [wgpu-native gfx-rs#66](gfx-rs/wgpu-native#66). I'm using `wait = false` in the `texture_view_drop` call to match `buffer_drop` and `texture_drop` elsewhere `backend/direct.rs`, though I don't quite understand the implications 😅 Co-authored-by: Matt Keeter <[email protected]>

mkeeter added 2 commits January 23, 2021 10:57

Draft of updates for wgpu-core bump

be29bcd

Hand-edit to fix bad translation

8c29d4b

mkeeter mentioned this pull request Jan 23, 2021

Bump gfx to latest master gfx-rs/wgpu#1163

Merged

monocodus bot suggested changes Jan 23, 2021

View reviewed changes

kvark approved these changes Jan 23, 2021

View reviewed changes

Bump wgpu to latest master

e3140b0

monocodus bot suggested changes Jan 23, 2021

View reviewed changes

mkeeter mentioned this pull request Jan 23, 2021

Bump wgpu-core and update texture_view_drop call gfx-rs/wgpu-rs#726

Merged

kvark marked this pull request as ready for review January 23, 2021 21:43

mkeeter mentioned this pull request Jan 26, 2021

Enums shadowed by names in module mozilla/cbindgen#649

Closed

Remove unused types and rebuild with cbindgen 0.7.0

12b1f39

monocodus bot suggested changes Feb 2, 2021

View reviewed changes

kvark mentioned this pull request Feb 2, 2021

Draft: Add support for shader module chain #70

Merged

3 tasks

bors bot merged commit 3317a65 into gfx-rs:master Feb 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Draft of wgpu-core bump #66

Draft of wgpu-core bump #66

mkeeter commented Jan 23, 2021 •

edited

Loading

monocodus bot left a comment

monocodus bot left a comment

monocodus bot left a comment

kvark commented Jan 23, 2021

monocodus bot left a comment

mkeeter commented Jan 23, 2021

kvark commented Jan 23, 2021

bors bot commented Jan 23, 2021

kvark commented Jan 23, 2021

bors bot commented Jan 23, 2021

kvark commented Jan 23, 2021

mkeeter commented Jan 24, 2021

mkeeter commented Feb 2, 2021

monocodus bot left a comment

kvark commented Feb 2, 2021

bors bot commented Feb 2, 2021

Draft of wgpu-core bump #66

Draft of wgpu-core bump #66

Conversation

mkeeter commented Jan 23, 2021 • edited Loading

monocodus bot left a comment

Choose a reason for hiding this comment

monocodus bot left a comment

Choose a reason for hiding this comment

monocodus bot left a comment

Choose a reason for hiding this comment

kvark commented Jan 23, 2021

monocodus bot left a comment

Choose a reason for hiding this comment

mkeeter commented Jan 23, 2021

kvark commented Jan 23, 2021

bors bot commented Jan 23, 2021

kvark commented Jan 23, 2021

bors bot commented Jan 23, 2021

kvark commented Jan 23, 2021

mkeeter commented Jan 24, 2021

mkeeter commented Feb 2, 2021

monocodus bot left a comment

Choose a reason for hiding this comment

kvark commented Feb 2, 2021

bors bot commented Feb 2, 2021

mkeeter commented Jan 23, 2021 •

edited

Loading