Move some types into shared wgpu-types crate #514

grovesNL · 2020-03-10T02:32:40Z

As we discussed a while ago, we need to be able to share some types between wgpu-core/wgpu-native/wgpu-remote/wgpu-rs.

The problem is that we want to avoid a dependency on wgpu-core and wgpu-native when building wgpu-rs for the wasm32-unknown-unknown target. We can avoid this by moving all shared types into a separate crate which is exposed on all targets.

Let me know if we should use some other approach or organize the types somehow. This isn't complete yet, but it might be easier to integrate this over several PRs instead of diverging my branch too far.

kvark

Nice work! A few corrections are needed

kvark · 2020-03-10T02:56:48Z

wgpu-types/src/lib.rs

@@ -0,0 +1,506 @@
+use std::{io, slice};


let's have a license header like we do in wgpu source files, so that we don't have a problem vendoring this in Gecko

kvark · 2020-03-10T02:57:20Z

wgpu-types/src/lib.rs

+#[repr(u8)]
+#[derive(Clone, Copy, Debug, PartialEq)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+pub enum Backend {


This one doesn't sounds like you'd want from the Web side?

Yeah I debated whether to include this here. It's part of the public API wgpu-rs, so it's slightly easier to include another BrowserWebGpu variant (possibly even BrowserWebGl later) instead of trying to redefine Backend for the wasm32 target.

wgpu-core/Cargo.toml

kvark

ship it!
bors r+

514: Move some types into shared wgpu-types crate r=kvark a=grovesNL As we discussed a while ago, we need to be able to share some types between wgpu-core/wgpu-native/wgpu-remote/wgpu-rs. The problem is that we want to avoid a dependency on wgpu-core and wgpu-native when building [wgpu-rs for the wasm32-unknown-unknown target](gfx-rs/wgpu-rs#101). We can avoid this by moving all shared types into a separate crate which is exposed on all targets. Let me know if we should use some other approach or organize the types somehow. This isn't complete yet, but it might be easier to integrate this over several PRs instead of diverging my branch too far. Co-authored-by: Joshua Groves <josh@joshgroves.com>

bors · 2020-03-10T04:11:30Z

Build failed

continuous-integration/travis-ci/push

grovesNL · 2020-03-10T17:57:45Z

@kvark did you want to take another look at the updated Cargo.tomls? I think we need it to be something like this for serde (at least for now without introducing a separate feature)

kvark · 2020-03-10T19:16:20Z

wgpu-core/Cargo.toml

+path = "../wgpu-types"
+package = "wgpu-types"
+version = "0.1"
+features = ["serde"]


so serde is now unconditional? that seems wrong - native users don't need it

aloucks · 2020-03-11T01:23:43Z

wgpu-core/src/swap_chain.rs

@@ -71,7 +72,7 @@ pub enum PresentMode {
 #[derive(Clone, Debug)]


Should the SwapChainDescriptor and PresentMode move as well?

I didn't do an exhaustive check, but I would think it should be everything re-exported here:

https://github.com/gfx-rs/wgpu-rs/blob/master/src/lib.rs#L19-L84

Yeah most likely, this PR isn't comprehensive because I didn't want this branch to diverge too much. I'm just moving types across whenever I need to use them with the web-sys backend (gfx-rs/wgpu-rs#193), so there will be some smaller PRs later

grovesNL · 2020-03-11T01:23:55Z

@kvark I added a serde1 feature to workaround this, and added a few more types

aloucks · 2020-03-11T01:29:49Z

@kvark I added a serde1 feature to workaround this, and added a few more types

You could make the feature name serde with the serde crate attribute and package renaming.

Example: https://github.com/RustCrypto/RSA/pull/41/files

grovesNL · 2020-03-11T01:36:35Z

Interesting, let's try doing that instead. Thanks!

aloucks · 2020-03-11T01:40:46Z

need to update the attributes too:

- #[cfg_attr(feature = "serde1", derive(Serialize, Deserialize))]
+ #[cfg_attr(feature = "serde", derive(Serialize, Deserialize), serde(crate="serde_crate"))]

kvark · 2020-03-11T02:12:58Z

@grovesNL please proceed when ready, unless you have specific spots that need a look/review.

grovesNL · 2020-03-11T02:30:28Z

bors r=kvark

514: Move some types into shared wgpu-types crate r=kvark a=grovesNL As we discussed a while ago, we need to be able to share some types between wgpu-core/wgpu-native/wgpu-remote/wgpu-rs. The problem is that we want to avoid a dependency on wgpu-core and wgpu-native when building [wgpu-rs for the wasm32-unknown-unknown target](gfx-rs/wgpu-rs#101). We can avoid this by moving all shared types into a separate crate which is exposed on all targets. Let me know if we should use some other approach or organize the types somehow. This isn't complete yet, but it might be easier to integrate this over several PRs instead of diverging my branch too far. Co-authored-by: Joshua Groves <josh@joshgroves.com>

kvark · 2020-03-11T02:45:02Z

Funny how we are getting 2x more crates in the repo between 0.4 and 0.5 releases :)

grovesNL · 2020-03-11T03:07:01Z

It looks like bors couldn't authenticate with GitHub for some reason:

{{:badmatch,
  {:error, :post_commit_status, 401,
   "{\"message\":\"Bad credentials\",\"documentation_url\":\"https://developer.github.com/v3\"}"}},
 [
   {BorsNG.GitHub, :post_commit_status!, 2,
    [file: 'lib/github/github.ex', line: 227]},
   {BorsNG.Worker.Batcher, :maybe_complete_batch, 1,
    [file: 'lib/worker/batcher.ex', line: 535]},
   {BorsNG.Worker.Batcher, :handle_cast, 2,
    [file: 'lib/worker/batcher.ex', line: 90]},
   {:gen_server, :try_dispatch, 4,
    [file: 'gen_server.erl', line: 637]},
   {:gen_server, :handle_msg, 6,
    [file: 'gen_server.erl', line: 711]},
   {:proc_lib, :init_p_do_apply, 3,
    [file: 'proc_lib.erl', line: 249]}
 ]}

I guess we should try again – I don't think we configure bors credentials manually anywhere.

bors retry

bors · 2020-03-11T03:39:30Z

Build succeeded

continuous-integration/travis-ci/push

514: Release version 0.6 r=kvark a=kvark Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>

grovesNL requested a review from kvark March 10, 2020 02:32

kvark requested changes Mar 10, 2020

View reviewed changes

kvark approved these changes Mar 10, 2020

View reviewed changes

kvark reviewed Mar 10, 2020

View reviewed changes

Move some types into shared wgpu-types crate

9940aef

aloucks reviewed Mar 11, 2020

View reviewed changes

Rename serde feature

24caf76

bors bot merged commit a74de20 into gfx-rs:master Mar 11, 2020

grovesNL deleted the wasm1 branch March 11, 2020 04:00

kvark added a commit to kvark/wgpu that referenced this pull request Jun 3, 2021

[rs] Merge gfx-rs#514

01a019f

514: Release version 0.6 r=kvark a=kvark Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move some types into shared wgpu-types crate #514

Move some types into shared wgpu-types crate #514

grovesNL commented Mar 10, 2020 •

edited

Loading

kvark left a comment

kvark Mar 10, 2020

kvark Mar 10, 2020

grovesNL Mar 10, 2020

kvark left a comment

bors bot commented Mar 10, 2020

grovesNL commented Mar 10, 2020

kvark Mar 10, 2020

aloucks Mar 11, 2020 •

edited

Loading

grovesNL Mar 11, 2020

grovesNL commented Mar 11, 2020

aloucks commented Mar 11, 2020

grovesNL commented Mar 11, 2020

aloucks commented Mar 11, 2020

kvark commented Mar 11, 2020

grovesNL commented Mar 11, 2020

kvark commented Mar 11, 2020

grovesNL commented Mar 11, 2020

bors bot commented Mar 11, 2020

		@@ -71,7 +72,7 @@ pub enum PresentMode {
		#[derive(Clone, Debug)]

Move some types into shared wgpu-types crate #514

Move some types into shared wgpu-types crate #514

Conversation

grovesNL commented Mar 10, 2020 • edited Loading

kvark left a comment

Choose a reason for hiding this comment

kvark Mar 10, 2020

Choose a reason for hiding this comment

kvark Mar 10, 2020

Choose a reason for hiding this comment

grovesNL Mar 10, 2020

Choose a reason for hiding this comment

kvark left a comment

Choose a reason for hiding this comment

bors bot commented Mar 10, 2020

Build failed

grovesNL commented Mar 10, 2020

kvark Mar 10, 2020

Choose a reason for hiding this comment

aloucks Mar 11, 2020 • edited Loading

Choose a reason for hiding this comment

grovesNL Mar 11, 2020

Choose a reason for hiding this comment

grovesNL commented Mar 11, 2020

aloucks commented Mar 11, 2020

grovesNL commented Mar 11, 2020

aloucks commented Mar 11, 2020

kvark commented Mar 11, 2020

grovesNL commented Mar 11, 2020

kvark commented Mar 11, 2020

grovesNL commented Mar 11, 2020

bors bot commented Mar 11, 2020

Build succeeded

grovesNL commented Mar 10, 2020 •

edited

Loading

aloucks Mar 11, 2020 •

edited

Loading