Add gfx-backend-gl support by kyren · Pull Request #183 · gfx-rs/wgpu

kyren · 2019-05-19T19:11:47Z

Also adds glutin-specific support methods to wgpu-native

kvark

Thank you for the PR!
Our goal for GL integration should be - minimal disruption on the API, we need to reduce the divergence of code paths as much as possible, and in the long term remove it entirely (i.e. have the code path to work equally well for all backends).
Please let me know what you think of my notes

kvark · 2019-05-21T17:13:12Z

wgpu-native/src/instance.rs

-pub fn instance_get_adapter(instance_id: InstanceId, desc: &AdapterDescriptor) -> AdapterHandle {
-    let instance_guard = HUB.instances.read();
-    let instance = &instance_guard[instance_id];
+fn get_adapter(


nit: let's rename to choose_adapter

kvark · 2019-05-21T17:13:40Z

wgpu-native/src/instance.rs

-    let instance_guard = HUB.instances.read();
-    let instance = &instance_guard[instance_id];
+fn get_adapter(
+    adapters: Vec<hal::Adapter<back::Backend>>,


let's turn this into a slice: &[hal::Adapter<B>]

It returns an adapter by value not by reference, so it can't take the possible set of adapters by reference. I can change this to I: IntoIterator<Item = hal::Adapter<B>>?

Actually it won't matter anymore since the methods will be combined again.

kvark · 2019-05-21T17:17:04Z

wgpu-native/src/instance.rs

+
+#[cfg(feature = "gfx-backend-gl")]
+#[no_mangle]
+pub extern "C" fn wgpu_gl_surface_get_adapter(


I wonder if we can still have Instance types and (some) entry points on GL but merged with surfaces, i.e. like today we do type CommandEncoderId = CommandBufferId, we should consider type SurfaceId = InstanceId for GL only. Of course, this should be removed once GL gets proper support for headless instances...

I actually tried to do this the first time, but I think I tried incorrectly to keep SurfaceId and InstanceId separate and was stymied by Surface not being clone-able. I'll see if there is more of the API surface that can be kept common with type SurfaceId = InstanceId.

by making InstanceId a type alias to SurfaceId

kyren · 2019-05-21T21:42:54Z

The gl API is now somewhat closer to the vanilla, at least Instance still exists now.

kvark · 2019-05-24T14:45:16Z

Thanks @kyren , let us know when it's ready to merge.
I really hope @msiglreith (or someone else's) work would eventually bring back the Instance on GL backend.

kyren · 2019-05-24T15:46:53Z

Thanks @kyren , let us know when it's ready to merge.
I really hope @msiglreith (or someone else's) work would eventually bring back the Instance on GL backend.

If you're okay with this as it is, I think this is fine for the time being?. Once the gl backend is in line with the others all of this can go away, but in the meantime something like this has to exist (especially when you include webgl support).

kvark · 2019-05-24T15:47:12Z

bors r+

183: Add gfx-backend-gl support r=kvark a=kyren Also adds glutin-specific support methods to wgpu-native Co-authored-by: kyren <kerriganw@gmail.com>

kyren · 2019-05-24T15:53:01Z

For others following along, this won't be super useful until this pr is merged into gfx-hal and then this pr is merged into wgpu-rs to use the updated wgpu.

kvark · 2019-05-24T15:54:10Z

bors r-
Actually, can we get some CI coverage here?

bors · 2019-05-24T15:54:11Z

Canceled

kyren · 2019-05-24T17:04:24Z

I'm not entirely sure whether that was the sort of thing you were thinking, or whether I did that correctly.

Edit: Oh, I seee, the makefile is ONLY used on nightly anyway... and there's a GLFW error that I don't understand yet AND this can't work from C anyway because the gl backend is unusable from C. Hmm, how exactly would you like the gl backend added to CI... I'm not sure if I understand what the best way to do that is.

kvark · 2019-05-24T17:53:00Z

@kyren current CI uses glfw, which also supports GL, so we should be testing GL backend the same way we are currently testing Metal/Vulkan from C API.
At the very least, having CI to build the feature gfx-backend-gl would be useful.

kyren · 2019-05-24T18:29:14Z

But the gl API isn't usable from C because it requires glutin types to be passed in, so there can't really be a C API test for it.

I'll try and make CI at least build the gfx-backend-gl feature.

kvark · 2019-05-24T18:57:53Z

Thank you!
bors r+

183: Add gfx-backend-gl support r=kvark a=kyren Also adds glutin-specific support methods to wgpu-native Co-authored-by: kyren <kerriganw@gmail.com>

bors · 2019-05-24T19:16:26Z

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/wgpu#183) is merged in wgpu, but at least this way we can discuss it. Co-authored-by: kyren <kerriganw@gmail.com>

183: Expose `enumerate_adapters`. r=kvark a=daxpedda Depends on gfx-rs#505. Use case was to give end user the ability to choose which GPU to use for an application. Co-authored-by: daxpedda <daxpedda@gmail.com>

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>

183: Expose `enumerate_adapters`. r=kvark a=daxpedda Depends on gfx-rs#505. Use case was to give end user the ability to choose which GPU to use for an application. Co-authored-by: daxpedda <daxpedda@gmail.com>

Add gfx-backend-gl support

8680195

Also adds glutin-specific support methods to wgpu-native

kyren mentioned this pull request May 20, 2019

Update API for new wgpu gl backend support gfx-rs/wgpu-rs#10

Merged

kvark reviewed May 21, 2019

View reviewed changes

Try to keep the API slightly closer to the standard API

af9aa78

by making InstanceId a type alias to SurfaceId

kvark approved these changes May 24, 2019

View reviewed changes

kyren changed the title ~~[DRAFT] Add gfx-backend-gl support~~ Add gfx-backend-gl support May 24, 2019

bors bot added a commit that referenced this pull request May 24, 2019

Merge #183

7ade514

183: Add gfx-backend-gl support r=kvark a=kyren Also adds glutin-specific support methods to wgpu-native Co-authored-by: kyren <kerriganw@gmail.com>

Only test building the gfx-backend-gl feature for now

0fe94fc

kyren force-pushed the wgpu-gl branch from f44dc3c to 0fe94fc Compare May 24, 2019 18:41

bors bot added a commit that referenced this pull request May 24, 2019

Merge #183

925718f

183: Add gfx-backend-gl support r=kvark a=kyren Also adds glutin-specific support methods to wgpu-native Co-authored-by: kyren <kerriganw@gmail.com>

bors bot merged commit 0fe94fc into gfx-rs:master May 24, 2019

kyren deleted the wgpu-gl branch May 24, 2019 19:29

zicklag mentioned this pull request Jan 14, 2020

Is OpenGL Supported now? #450

Closed

Conversation

kyren commented May 19, 2019

Uh oh!

kvark left a comment

Choose a reason for hiding this comment

Uh oh!

kvark May 21, 2019

Choose a reason for hiding this comment

Uh oh!

kvark May 21, 2019

Choose a reason for hiding this comment

Uh oh!

kyren May 21, 2019

Choose a reason for hiding this comment

Uh oh!

kyren May 21, 2019

Choose a reason for hiding this comment

Uh oh!

kvark May 21, 2019

Choose a reason for hiding this comment

Uh oh!

kyren May 21, 2019

Choose a reason for hiding this comment

Uh oh!

kyren commented May 21, 2019

Uh oh!

kvark commented May 24, 2019

Uh oh!

kyren commented May 24, 2019

Uh oh!

kvark commented May 24, 2019

Uh oh!

kyren commented May 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kvark commented May 24, 2019

Uh oh!

bors bot commented May 24, 2019

Canceled

Uh oh!

kyren commented May 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kvark commented May 24, 2019

Uh oh!

kyren commented May 24, 2019

Uh oh!

kvark commented May 24, 2019

Uh oh!

bors bot commented May 24, 2019

Build succeeded

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kyren commented May 24, 2019 •

edited

Loading

kyren commented May 24, 2019 •

edited

Loading