Wgpu 0.6 #655

olekspickle · 2020-09-25T19:03:13Z

Here is my attempt to migrate wgpu from 0.5 to 0.6.
Did it the most straightforward way I can come up with so feel free to disregard it.

But it would be cool if someone point me out on why it is stuck compiling nannou...

yutannihilation

Cool. I tried this and failed (just because my Rust skill is very poor)... Let me leave some quick comments that I've learned from the failure.

(The full diff of my attempt and the issues I couldn't handle are listed on #647)

nannou/src/wgpu/texture/mod.rs

yutannihilation · 2020-09-26T01:38:47Z

nannou/src/draw/renderer/mod.rs

@@ -413,7 +417,7 @@ impl Renderer {
        let uniforms = create_uniforms(output_attachment_size, output_scale_factor);
        let uniforms_bytes = uniforms_as_bytes(&uniforms);
        let usage = wgpu::BufferUsage::UNIFORM | wgpu::BufferUsage::COPY_DST;
-        let uniform_buffer = device.create_buffer_with_data(uniforms_bytes, usage);
+        let uniform_buffer = device.create_buffer(uniforms_bytes, usage);


The equivalent in wgpu 0.6 to create_buffer_with_data() is create_buffer_init().

yutannihilation · 2020-09-26T01:41:44Z

nannou/src/draw/renderer/mod.rs

@@ -395,7 +399,7 @@ impl Renderer {
            .usage(wgpu::TextureUsage::SAMPLED | wgpu::TextureUsage::COPY_DST)
            .format(Self::GLYPH_CACHE_TEXTURE_FORMAT)
            .build(device);
-        let glyph_cache_texture_view = glyph_cache_texture.create_default_view();
+        let glyph_cache_texture_view = glyph_cache_texture.view().build();


The equivalent in wgpu 0.6 to create_default_view() is create_view(&wgpu::TextureViewDescriptor::default()).

mitchmindtree · 2020-09-27T11:53:24Z

Amazing, thanks for such an epic PR!

Unfortunately I'm busy with a contract at the moment, but will take a proper look as soon as I can!

olekspickle · 2020-09-28T17:47:37Z

removed format_to_component
adjust buffers init and default view creation to latest wgpu

mitchmindtree · 2020-10-04T13:48:25Z

nannou/src/wgpu/texture/mod.rs

    handle: Arc<TextureViewHandle>,
-    descriptor: wgpu::TextureViewDescriptor,
+    descriptor: wgpu::TextureViewDescriptor<'t>,


Hey @alekspickle, it looks like you might be running into some issues with Rust's lifetimes, and my suspicion is that this field here is the source of the unexpected complexity :)

The lifetime parameter on the wgpu::TextureViewDescriptor is associated with the lifetime of its label field. The purpose of this label field is to make it easier to track TextureViews when debugging or using some inspector like renderdoc. By remaining generic over the lifetime, wgpu allows users to use a label that is a reference to some static slice of memory (e.g. string literals like "foo", const or static strs) or dynamically allocated one (e.g. a reference to a String).

Throughout nannou, the only descriptor labels we use are string literals (e.g. "frame-texture-vew") as these are almost always enough to quickly identify the resource in question when using a graphics debugger / inspector. As a result, our wgpu::TextureViewDescriptor's always have 'static lifetimes. In case you're unfamiliar, 'static lifetimes are special in that they indicate a lifetime is valid for the entire duration the program will run.

We should be able to get away without TextureView having a lifetime parameter, and instead declaring the descriptor field like this:

pub struct TextureView { handle: Arc<TextureViewHandle>, descriptor: wgpu::TextureViewDescriptor<'static>, // notice the 'static lifetime here texture_extent: wgpu::Extent3d, texture_id: TextureId, }

This should hopefully allow you to avoid this problem where the lifetime parameters seem to propagate up through the whole program and it becomes impossible to make the borrow checker happy.

I hope this makes sense / helps a bit!

Indeed, that could simplify things.
But I guess the reason is me not understanding wgpu/nannou code intricacies enough.

See the changelog below for an overview of the changes included in this version. For the most part, this is an update for wgpu 0.5. @alekspickle already has a WIP for wgpu 0.6 at nannou-org#655! I'm more than happy to publish a new version again as soon as that lands, but figured this one is already well overdue :) Also, find out how you can get new nannou versions published quicker in the new Contributing chapter of the guide [here](https://guide.nannou.cc/contributing/publishing-new-versions.html)).

… confusing

kazimuth · 2020-10-09T06:59:49Z

I was interested in this for my own reasons (wanted to try some new wgpu-0.6 stuff in my sketches :), spent some time poking through this PR.

@alekspickle:

But it would be cool if someone point me out on why it is stuck compiling nannou...

This happens on my machine too. It looks like rustc is hanging! Somewhere in the texture management code. It's weird, I've never seen that happen before. I think it's because type resolution gets confused inside one of the async methods? Definitely a compiler bug of some kind.

I made some changes here to fix that. I also went and changed some of the lifetimes assuming that the wgpu Descriptor objects would have 'static labels, which simplifies things a lot. There's a fresh wall of errors to dig through now, which is exciting :) I'm thinking of poking at that some more this weekend, but if you'd rather be in charge let me know @alekspickle .

Also, it does appear that there are some architectural changes in 0.6 that need to be handled.

create_surface is now an unsafe function on Instance. So I think we'll need to take the Instance created here and store it... somewhere, not sure where that should go. Maybe just in App, or as part of the device map? @mitchmindtree , do you have thoughts?
BufferReadMapping and BufferWriteMapping have been replaced with BufferSlice, which carries a lifetime. That means that ImageReadMapping and Rgba8ReadMapping will need to grow lifetimes as well. Not sure if that'll have any larger impact.

Wgpu 0.6 redux

mitchmindtree · 2020-10-09T12:19:41Z

This happens on my machine too

@kazimuth Likewise! I've never seen it before either - I ended up having a crack at this from the ground up, see #660.

Maybe just in App, or as part of the device map?

Yeah these were my thoughts too, I think it might best live alongside the adapters (if they're still needed that is, I haven't checked) in the App.

I'm not yet sure when I'll get a chance to continue on #660, but feel free to take what you like from it if or fork it if you're hacking on your own branch :)

BufferReadMapping and BufferWriteMapping have been replaced with BufferSlice, which carries a lifetime. That means that ImageReadMapping and Rgba8ReadMapping will need to grow lifetimes as well. Not sure if that'll have any larger impact.

Yeah I imagine this might be the trickier change. I haven't yet taken the time to see how much those lifetimes propagating through types will affect things (I guess this will depend a lot on whether or not we store them or if users are encouraged to store them), it might be doable but it's also possible we'll need to approach this in a different way. The wgpu users channel on matrix might be a great place to ask.

olekspickle · 2020-10-09T12:30:01Z

@kazimuth a lot of errors after your fix seem like those I resolved before. I dont understand where they come from though, because they should be eliminanted by using 'static lifetime.
I am trying to dig into it rn

kazimuth · 2020-10-09T21:18:12Z

@alekspickle I think that's because rustc was dying before it got to those errors, so you wouldn't get the error messages. I get them too, will hold off on trying to fix if you're working on it. (You're welcome to merge my changes into this PR if you want, no pressure though.)

@mitchmindtree Doing a more in-depth read of the code, lifetimes will need to be added to:

BufferBytes (maybe?)
BufferImage
Snapshot
Rgba8ReadMapping
Rgba8AsyncMappedImageBuffer

Specifically, they all need to have the lifetime of the Buffer that is being read. That Buffer is created in Capturer::capture(). So, sadly, there isn't really a way to pass them back to the user while using that lifetime. You'd need to either:

use one of those scary crates that allow making self-referential structs, and return them as a bundle
make the api two-step: have the user call one thing to hold the Buffer, and then another call receives that as a reference and returns a Snapshot that borrows it... asynchronously.
switch the API to only use callbacks that receive a Snapshot.

Of these options, I think the third is probably the simplest, both from an implementation and use perspective.

olekspickle · 2020-10-10T00:37:33Z

@kazimuth Yeah, you've got the point. feel free to poke it yourself, I stumbled at least 2 things I dont know how to solve, which require more of both nannou and wgpu-rs experience.
Btw merged your fix and resolved a few errors.

mitchmindtree · 2020-10-11T17:18:34Z

@kazimuth I wonder if we can offer a combination of your 2nd and 3rd options? E.g. having two separate methods for allowing the user to produce either a future which they can handle somehow themselves, or pass a callback which nannou can call internally once the future completes? I haven't yet done enough digging into the new async read/write API in wgpu to know how feasible this would be.

kazimuth · 2020-10-11T18:16:11Z

@mitchmindtree it's probably doable.

Other question: the array_layer_count attribute was removed from TextureDescriptor in 0.6, but it was kept on TextureViewDescriptor. Unfortunately the texture builder code seems to use that attribute a lot. I'm not sure what to substitute to keep the logic the same. Do you have any idea what's going on there? I'm not all that familiar with array textures. I'd read the spec, but... well...

e: oh, I'll ask on Matrix

kazimuth · 2020-10-12T18:24:26Z

Update: apparently array_layer_count now only exists on TextureViews. A Texture may be 1d, 2d, or 3d, but only a texture view can be an array, cubemap, or cube array. I think I'll update the API to reflect this.

kazimuth · 2020-10-13T16:50:04Z

New work: https://github.com/alekspickle/nannou/compare/wgpu-0.6...kazimuth:wgpu-0.6-3

Don't merge it yet, I still need to make the changes with Buffer borrows.

kazimuth · 2020-10-13T18:50:12Z

Update: all compile errors fixed on my branch!

...Now we just have to address the runtime errors 😅

Things that need to be addressed:

All of the examples result in blank screens. I'll do some poking around in RenderDoc, bet there's a new flag somewhere with the wrong value. Background clear colors do work correctly though. (Fixed! Depth buffer was getting cleared to the wrong value.)
There's now a 256-byte alignment requirement for texture-to-buffer and buffer-to-texture copies, which breaks Capturer (runtime panic). We might be able to fix this by just... padding and reshaping the textures? Not sure.
In general, the tires should probably get kicked a little more. Going through and running different examples is probably a good idea.

Note: Getting the Buffer reference lifetimes fixed was actually surprisingly straightforward. All I needed to do was add lifetimes to the various read mappings + make Snapshot::read_async take self by reference. Snapshot::read still takes self by value (and passes ownership to the thread pool.)

Aleks Pickle added 4 commits September 4, 2020 11:09

wgpu 0.5-0.6: on wgpu higher level wrapping

8c765b3

wgpu 0.5-0.6: draw fix

3a9a7bc

wgpu 0.5-0.6: iterate with global wrappers

45dc3d7

wgpu 0.5-0.6: finish with lifetimes

b54c865

yutannihilation reviewed Sep 26, 2020

View reviewed changes

Aleks Pickle added 3 commits September 27, 2020 20:52

wgpu 0.5-0.6: remove format_to_component_type

8da1877

wgpu 0.5-0.6: change buffer init

8ed73cb

wgpu 0.5-0.6: change default view creation

1cf9fce

mitchmindtree reviewed Oct 4, 2020

View reviewed changes

mitchmindtree mentioned this pull request Oct 4, 2020

Update wgpu to 0.6 #647

Open

mitchmindtree mentioned this pull request Oct 4, 2020

Publish version 0.15 #658

Merged

Aleks Pickle and others added 2 commits October 5, 2020 14:14

wgpu 0.5-0.6: declare static lifetime on texture

264051d

Purge old wgpus

7575ad6

kazimuth added a commit to kazimuth/nannou that referenced this pull request Oct 9, 2020

Borrow wgpu-0.6 work from nannou-org#655

61524c4

kazimuth added a commit to kazimuth/nannou that referenced this pull request Oct 9, 2020

Borrow wgpu-0.6 work from nannou-org#655

40e6e3b

kazimuth added 2 commits October 9, 2020 02:28

Rename wgpu dependency to wgpu_upstream to make error messages less…

4edf961

… confusing

Kill BufferBytes::read() + fix lifetimes to get to next error wall

14b7829

Merge pull request #1 from kazimuth/wgpu-0.6-redux

f2971d1

Wgpu 0.6 redux

mitchmindtree mentioned this pull request Oct 9, 2020

Updating to wgpu 0.6 - follow-up attempt to #655 #660

Closed

5 tasks

fix spirv

bee1780

create buffer new way; fix mapping field

2cfd3dc

This was referenced Oct 15, 2020

Consider moving wgpu helpers to separate module #663

Closed

wgpu 0.6, third time's the charm #665

Merged

mitchmindtree mentioned this pull request Oct 23, 2020

wasm? #657

Open

mitchmindtree merged commit 2cfd3dc into nannou-org:master Feb 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wgpu 0.6 #655

Wgpu 0.6 #655

olekspickle commented Sep 25, 2020

yutannihilation left a comment

yutannihilation Sep 26, 2020

olekspickle Sep 28, 2020

yutannihilation Sep 26, 2020

olekspickle Sep 28, 2020

mitchmindtree commented Sep 27, 2020

olekspickle commented Sep 28, 2020

mitchmindtree Oct 4, 2020

olekspickle Oct 5, 2020 •

edited

Loading

kazimuth commented Oct 9, 2020

mitchmindtree commented Oct 9, 2020 •

edited

Loading

olekspickle commented Oct 9, 2020 •

edited

Loading

kazimuth commented Oct 9, 2020 •

edited

Loading

olekspickle commented Oct 10, 2020 •

edited

Loading

mitchmindtree commented Oct 11, 2020

kazimuth commented Oct 11, 2020 •

edited

Loading

kazimuth commented Oct 12, 2020

kazimuth commented Oct 13, 2020

kazimuth commented Oct 13, 2020 •

edited

Loading

Wgpu 0.6 #655

Wgpu 0.6 #655

Conversation

olekspickle commented Sep 25, 2020

yutannihilation left a comment

Choose a reason for hiding this comment

yutannihilation Sep 26, 2020

Choose a reason for hiding this comment

olekspickle Sep 28, 2020

Choose a reason for hiding this comment

yutannihilation Sep 26, 2020

Choose a reason for hiding this comment

olekspickle Sep 28, 2020

Choose a reason for hiding this comment

mitchmindtree commented Sep 27, 2020

olekspickle commented Sep 28, 2020

mitchmindtree Oct 4, 2020

Choose a reason for hiding this comment

olekspickle Oct 5, 2020 • edited Loading

Choose a reason for hiding this comment

kazimuth commented Oct 9, 2020

mitchmindtree commented Oct 9, 2020 • edited Loading

olekspickle commented Oct 9, 2020 • edited Loading

kazimuth commented Oct 9, 2020 • edited Loading

olekspickle commented Oct 10, 2020 • edited Loading

mitchmindtree commented Oct 11, 2020

kazimuth commented Oct 11, 2020 • edited Loading

kazimuth commented Oct 12, 2020

kazimuth commented Oct 13, 2020

kazimuth commented Oct 13, 2020 • edited Loading

olekspickle Oct 5, 2020 •

edited

Loading

mitchmindtree commented Oct 9, 2020 •

edited

Loading

olekspickle commented Oct 9, 2020 •

edited

Loading

kazimuth commented Oct 9, 2020 •

edited

Loading

olekspickle commented Oct 10, 2020 •

edited

Loading

kazimuth commented Oct 11, 2020 •

edited

Loading

kazimuth commented Oct 13, 2020 •

edited

Loading