Generate C header by grovesNL · Pull Request #10 · gfx-rs/wgpu

grovesNL · 2018-09-20T18:46:01Z

Fixes #6

~~WIP: there are a few issues I've encountered already for the few functions we expose.~~

grovesNL · 2018-09-20T18:46:38Z

wgpu-bindings/bindings.h

+  T *_0;
+};
+
+using DeviceHandle = Handle<Device<B>>;


Generics either aren't working here, or they're expected to be resolved manually by us.

grovesNL · 2018-09-20T18:48:08Z

wgpu-bindings/bindings.h

+
+struct BufferDescriptor {
+  uint32_t size;
+  BufferUsageFlags usage;


bitflags aren't supported yet, see mozilla/cbindgen#100

We could just expose uint32_t everywhere for flags, and map back on the Rust side...

grovesNL · 2018-09-20T18:49:51Z

wgpu-bindings/bindings.h

+  T *_0;
+};
+
+using DeviceHandle = Handle<Device<B>>;


Do we expect to receive pointers or handle structs for these? For example, if we want this to exactly match the API used for IPC, pointers might be better

not sure I understand the question, please clarify

Just wondering if the intent would be to reuse this header as the interface for IPC too (not sure how it's planned), or whether this header would only used directly (within the graphics process) with a separate interface for IPC.

It's a great question! Here is the thing: we need wgpu-native C header so that native apps (written in C or anything) can target both native and WASM. And we need wgpu-remote C header so that Gecko can call into us. Technically, these are two completely different things, but I believe we'd have a much easier time if these were exactly the same. So I think we should use the header generated here for both.

kvark

I do wonder though if we should generate the header as a build step for wgpu-native instead of a separate project.

grovesNL · 2018-09-20T19:01:22Z

Yeah that would work well too, although it will add extra time to regular builds even if the signatures haven't changed.

grovesNL · 2018-09-24T05:07:14Z

wgpu-native/src/registry.rs


+#[cfg(not(feature = "remote"))]
+lazy_static! {
+    pub(crate) static ref ADAPTER_REGISTRY: Mutex<LocalRegistry<AdapterHandle>> =


It would be great to drop this Mutex too but it's not obvious how to support both paths without it. We need a no-op mutex or some helper functions to wrap usage of the mutex for the remote case.

I think we should just move the mutability (i.e. Mutex) into the internals of the registry implementation, so that registry itself is Send+Sync and most methods are &self

grovesNL · 2018-09-24T05:09:03Z

wgpu-native/src/registry.rs

 pub(crate) type Id = u32;

-pub(crate) struct Registry<T> {
+pub(crate) trait Registry<T> {


I just used a trait here to ensure we keep both registry implementations in sync, but it's not really necessary.

makes sense. We might even go for Registry: IndexMut<Id>

grovesNL · 2018-09-24T05:11:24Z

@kvark see my latest commit, we now avoid the map for "local" usage, and "local" vs. "remote" are distinguished by a Cargo feature and #ifdef in the header

kvark · 2018-09-24T13:57:15Z

wgpu-bindings/src/main.rs

    crate_dir.push("../wgpu-native");

    let config = cbindgen::Config {
+        header: Some(String::from("#ifdef WGPU_REMOTE\n    typedef uint32_t WGPUId;\n#else\n    typedef void *WGPUId;\n#endif")),


looks to be a good case for raw strings:

const HEADER: &str = #" #ifdef ... ... #endif ";

Regular strings will work fine too, unfortunately doesn't avoid the leading newline though. I'll probably move it to a constant and simply use trim when it's used.

kvark · 2018-09-24T13:59:28Z

wgpu-native/src/registry.rs

 pub(crate) type Id = u32;

-pub(crate) struct Registry<T> {
+pub(crate) trait Registry<T> {


makes sense. We might even go for Registry: IndexMut<Id>

kvark · 2018-09-24T14:01:41Z

wgpu-native/src/registry.rs


+#[cfg(not(feature = "remote"))]
+lazy_static! {
+    pub(crate) static ref ADAPTER_REGISTRY: Mutex<LocalRegistry<AdapterHandle>> =


I think we should just move the mutability (i.e. Mutex) into the internals of the registry implementation, so that registry itself is Send+Sync and most methods are &self

kvark

One nit about parking_lot, otherwise good to go!

kvark · 2018-09-25T02:34:12Z

wgpu-native/Cargo.toml

+
 [features]
 default = []
+remote = []


this should bring parking_lot dependency

grovesNL · 2018-09-25T03:21:45Z

bors r=kvark

10: Generate C header r=kvark a=grovesNL Fixes #6 ~~WIP: there are a few issues I've encountered already for the few functions we expose.~~ Co-authored-by: grovesNL <josh@joshgroves.com> Co-authored-by: Joshua Groves <josh@joshgroves.com>

bors · 2018-09-25T03:24:55Z

Build succeeded

continuous-integration/travis-ci/push

10: Update API for new wgpu gl backend support r=kvark a=kyren This won't work until [this pr](gfx-rs#183) is merged in wgpu, but at least this way we can discuss it. Co-authored-by: kyren <kerriganw@gmail.com>

Integrating latest trunk

24: One big cleanup for the 0.3.1 release r=kvark a=kvark Closes #10 Co-authored-by: Dzmitry Malyshau <dmalyshau@mozilla.com>

grovesNL requested a review from kvark September 20, 2018 18:46

grovesNL commented Sep 20, 2018

View reviewed changes

kvark approved these changes Sep 20, 2018

View reviewed changes

grovesNL added 6 commits September 22, 2018 16:54

Add initial bindings

5759f76

Format all files

105a71d

Target C for header generation

d94d45c

Create C example

08ad0f4

Create shader modules from C

211189a

Add remote feature to control ID type

7d35607

grovesNL commented Sep 24, 2018

View reviewed changes

kvark reviewed Sep 24, 2018

View reviewed changes

Move mutex inside registry

7e67651

kvark approved these changes Sep 25, 2018

View reviewed changes

wgpu-native/Cargo.toml Outdated

[features]

default = []

remote = []

Copy link
Copy Markdown

Member

kvark Sep 25, 2018

Choose a reason for hiding this comment

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

this should bring parking_lot dependency

grovesNL changed the title ~~WIP: Generate C header~~ Generate C header Sep 25, 2018

Fix remote get_mut

19ddb63

bors bot merged commit 19ddb63 into gfx-rs:master Sep 25, 2018

grovesNL deleted the bindings branch September 25, 2018 03:26

grovesNL mentioned this pull request Sep 27, 2018

Add render pipeline creation #11

Merged

victorvde pushed a commit to victorvde/wgpu that referenced this pull request Apr 23, 2023

Merge pull request gfx-rs#10 from gfx-rs/trunk

4a2ff45

Integrating latest trunk

cwfitzgerald pushed a commit that referenced this pull request Oct 26, 2023

Merge #24

2810498

24: One big cleanup for the 0.3.1 release r=kvark a=kvark Closes #10 Co-authored-by: Dzmitry Malyshau <dmalyshau@mozilla.com>

This was referenced Jan 17, 2024

Multithreaded Compute Segfaults in CI on GL/Linux #4285

Closed

Intermittent crashes in Vulkan tests on Linux #5084

Closed

grinapo mentioned this pull request Feb 27, 2026

Segmentation fault in halmark example with lots of bunnies #9121

Open

Conversation

grovesNL commented Sep 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kvark left a comment

Choose a reason for hiding this comment

Uh oh!

grovesNL commented Sep 20, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grovesNL commented Sep 24, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grovesNL Sep 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kvark left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grovesNL commented Sep 25, 2018

Uh oh!

bors bot commented Sep 25, 2018

Build succeeded

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

grovesNL commented Sep 20, 2018 •

edited

Loading

grovesNL Sep 24, 2018 •

edited

Loading