Wholesome update for wgpu-0.3 #70

kvark · 2019-08-21T13:58:47Z

This update is a little more opinionated, i.e. the BGL creation just accepts a slice of things instead of a descriptor. I believe the reason for descriptors in the upstream API is mostly techincal - it's more convenient to generate bindings in Googles giant codegen. Following it verbatim doesn't appear to be strictly necessary, especially if we can extract better usability from Rust features.

The change also disables "gl" feature for the release. We'll re-enable it once we can provide it nicely, it will not be a breaking change.

I also noticed that barriers are not working correctly on the mipmap example. Will investigate separately - the native part is already published anyway. Edit: fixed by gfx-rs/wgpu#302

seivan · 2019-08-21T14:45:44Z

src/lib.rs

-    }
-
-    #[cfg(not(feature = "gl"))]
-    pub fn create_surface_from_macos_layer(&self, layer: *mut std::ffi::c_void) -> Surface {


Is this being removed? I depend on this since there is no other option to let SDL2 set everything up and just pass you the layer. raw-window-handle doesn't feel fleshed out enough to rely on for now?

indeed, we should keep that one

Is it too late to give my feedback on the raw-window-handle stuff or is this something decided? It seems very unfinished as an idea, and I do mean just the idea alone sans implementation. rust-windowing/raw-window-handle#14

I think we can live with NSView/UIView, but the discussion about CAMetalLayer does still need to happen. Thanks for filing the issue!

rukai · 2019-08-21T21:03:37Z

src/lib.rs

    /// adapter.
    ///
    /// # Panics
    ///
    /// Panics if there are no available adapters. This will occur if none of the graphics backends
    /// are enabled.
-    pub fn get_adapter(&self, options: Option<&RequestAdapterOptions>) -> Adapter {
+    pub fn request_adapter(&self, options: &RequestAdapterOptions) -> Adapter {


How are you supposed to call request_adapter(None) now?

it's equivalent to request_adapter(&wgpu::RequestAdapterOptions::default())

Oh yeah fair enough.

I guess my only issue is one day when we are using wgpu-rs on the web, we wont be able to access that part of the API, if only for testing different browser implementations, but that seems pretty obscure.

(I would mark this as resolved if I could, I dont seem to have access)

The Option thing in WebIDL specification itself is defined to be equivalent of supplying a dictionary with the fields full of default values, so it's not like a browser has freedom to do anything other than this (it would be a bug!).

rukai · 2019-08-21T21:04:51Z

src/lib.rs

@@ -554,9 +542,9 @@ impl Adapter {
    /// # Panics
    ///
    /// Panics if the extensions specified by `desc` are not supported by this adapter.
-    pub fn request_device(&self, desc: Option<&DeviceDescriptor>) -> Device {
+    pub fn request_device(&self, desc: &DeviceDescriptor) -> Device {


How are you supposed to call request_device(None) now?

similarly, by doing request_device(&wgpu::DeviceDescriptor::default())

yanchith · 2019-08-21T21:09:00Z

Cargo.toml

@@ -20,12 +20,13 @@ metal = ["wgn/gfx-backend-metal"]
 dx11 = ["wgn/gfx-backend-dx11"]
 dx12 = ["wgn/gfx-backend-dx12"]
 vulkan = ["wgn/gfx-backend-vulkan"]
-gl = ["wgn/gfx-backend-gl"]
+#gl = ["wgn/gfx-backend-gl"]

 [dependencies]
 #TODO: only depend on the published version


This TODO is actually done now?

indeed, will fix that, thanks!

grovesNL · 2019-08-22T00:42:08Z

src/lib.rs

-///
-/// A `BindGroupLayoutDescriptor` can be passed to [`Device::create_bind_group_layout`] to obtain a
-/// [`BindGroupLayout`].
-pub struct BindGroupLayoutDescriptor<'a> {


I think one reason the descriptor was useful is because of the inheritance chain (GPUBindGroupLayoutDescriptor : GPUObjectDescriptorBase). GPUObjectDescriptorBase has DOMString? label (which we haven't implemented yet), so by accepting a slice here we won't be able to provide debug labels for bind group layouts.

aha, great point!

grovesNL · 2019-08-22T00:43:46Z

Cargo.toml

@@ -20,12 +20,13 @@ metal = ["wgn/gfx-backend-metal"]
 dx11 = ["wgn/gfx-backend-dx11"]
 dx12 = ["wgn/gfx-backend-dx12"]
 vulkan = ["wgn/gfx-backend-vulkan"]
-gl = ["wgn/gfx-backend-gl"]
+#gl = ["wgn/gfx-backend-gl"]


Is there an issue for the part that gl is blocked on here? It would probably be useful to clarify it in a comment for anyone using gl patches at the moment

issue is that we initialize all other backends without winit feature
for GL, there is currently no way to use it without glutin. Enabling glutin though on the backend would conflict with glutin in wgpu-rs itself, since the backends are from crates.io and they rely on the old glutin version...
This will be unlocked if/when we back-port raw-window-handle to gfx-backend-gl-0.3

kvark · 2019-08-22T03:53:22Z

I think everything got reviewed and addressed...
bors r=everyone

70: Wholesome update for wgpu-0.3 r=everyone a=kvark ~~This update is a little more opinionated, i.e. the BGL creation just accepts a slice of things instead of a descriptor. I believe the reason for descriptors in the upstream API is mostly techincal - it's more convenient to generate bindings in Googles giant codegen. Following it verbatim doesn't appear to be strictly necessary, especially if we can extract better usability from Rust features.~~ The change also disables "gl" feature for the release. We'll re-enable it once we can provide it nicely, it will not be a breaking change. I also noticed that barriers are not working correctly on the mipmap example. Will investigate separately - the native part is already published anyway. Edit: fixed by gfx-rs/wgpu#302 Co-authored-by: Dzmitry Malyshau <dmalyshau@mozilla.com>

bors · 2019-08-22T04:36:46Z

Build succeeded

continuous-integration/travis-ci/push

kvark requested a review from grovesNL August 21, 2019 13:58

seivan reviewed Aug 21, 2019

View reviewed changes

rukai reviewed Aug 21, 2019

View reviewed changes

yanchith reviewed Aug 21, 2019

View reviewed changes

grovesNL reviewed Aug 22, 2019

View reviewed changes

Wholesome update for wgpu-0.3

c2743c2

kvark force-pushed the release-0.3 branch from b8cb8a6 to c2743c2 Compare August 22, 2019 03:51

bors bot merged commit c2743c2 into gfx-rs:master Aug 22, 2019

kvark deleted the release-0.3 branch August 22, 2019 04:37

parasyte mentioned this pull request Aug 31, 2019

macOS: "acquire_image failed, re-creating" #74

Closed

kvark mentioned this pull request Sep 3, 2019

Please add support for the new raw-window-handle crate #64

Closed

Lokathor mentioned this pull request Dec 11, 2019

[Tracker] Better windowing/graphics inter-operation rust-gamedev/wg#26

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wholesome update for wgpu-0.3 #70

Wholesome update for wgpu-0.3 #70

kvark commented Aug 21, 2019 •

edited

seivan Aug 21, 2019 •

edited

kvark Aug 21, 2019

seivan Aug 21, 2019

kvark Aug 21, 2019

rukai Aug 21, 2019

kvark Aug 21, 2019

rukai Aug 21, 2019 •

edited

kvark Aug 21, 2019

rukai Aug 21, 2019

kvark Aug 21, 2019

yanchith Aug 21, 2019

kvark Aug 21, 2019

grovesNL Aug 22, 2019

kvark Aug 22, 2019

grovesNL Aug 22, 2019

kvark Aug 22, 2019

kvark commented Aug 22, 2019

bors bot commented Aug 22, 2019

Wholesome update for wgpu-0.3 #70

Wholesome update for wgpu-0.3 #70

Conversation

kvark commented Aug 21, 2019 • edited

seivan Aug 21, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rukai Aug 21, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kvark commented Aug 22, 2019

bors bot commented Aug 22, 2019

Build succeeded

kvark commented Aug 21, 2019 •

edited

seivan Aug 21, 2019 •

edited

rukai Aug 21, 2019 •

edited