Library upgrades and updates

tychedelia commented 1 year ago

Mostly working, but need to double check a few things around egui. And make sure all the shaders are still working with wgsl changes.

Switch from async-std to tokio.
Switch from egui_wgpu_backend to the in-tree egui-wgpu.
Upgrade to egui version 0.23.
Upgrade to wgpu version 0.17.
Upgrade to winit version 0.28.

TODO:

[x] Ensure all targets (i.e. WASM) still working.
[x] Figure out whether we can use eframe as a replacement for epi in the new egui version. (answer: no)
[x] Resolve issue from https://github.com/nannou-org/nannou/pull/940#discussion_r1365303258 in the draw_capture_hi_res example.
[ ] Figure out resize issue on Ubuntu.
[ ] Establish egui pattern to replace epi.
[ ] Egui demo colors.

infinity-creative commented 1 year ago

I can confirm that egui seems to be working correctly in my project. I will let you know if anything comes up.

infinity-creative commented 1 year ago

Wasm builds now break.

I think this is due to converting to tokio.

tokio+mio - seems to break on WASM

 Compiling mio v0.8.8
error[E0432]: unresolved import `crate::sys::IoSourceState`

infinity-creative commented 1 year ago

@tychedelia I attempted to test WASM compiling by pulling out tokio. But got suck (new to rust) - I have left a PR on your repo it might help it might not :)

tychedelia commented 1 year ago

@tychedelia I attempted to test WASM compiling by pulling out tokio. But got suck (new to rust) - I have left a PR on your repo it might help it might not :)

Thanks! I totally blanked on testing WASM. I'll give your change a shot today or otherwise make sure things are working with the WASM builds.

tychedelia commented 1 year ago

@infinitylunacreative just pushed some fixes that should allow tokio to run on wasm targets. will you give it a shot and let me know if that works (on the head of my fork)?

tychedelia commented 1 year ago

Looks like @mitchmindtree ran into some of the same questions re: eframe in this previous upgrade attempt here https://github.com/nannou-org/nannou/pull/861. It does appear that the upstream might be willing to accept changes to eframe to make it work better with our use case. It would be wonderful if there was a more fully featured example of a nannou app that uses the previous epi functionality, but I couldn't find one.

infinity-creative commented 1 year ago

@infinitylunacreative just pushed some fixes that should allow tokio to run on wasm targets. will you give it a shot and let me know if that works (on the head of my fork)?

Thanks, I will take a look today.

infinity-creative commented 1 year ago

@tychedelia I can confirm the WASAM build is working for me 🥳 - Thank you!

infinity-creative commented 1 year ago

@tychedelia I decided to do some more testing. I tried it on linux. WASAM works great.

However the "normal" builds throw this error.

error[E0432]: unresolved import `winit::platform::unix`
   --> /home/infinity/work/nannou/nannou/src/window.rs:783:34
    |
783 |             use winit::platform::unix::WindowBuilderExtUnix;
    |                                  ^^^^ could not find `unix` in `platform`

error[E0599]: no method named `with_class` found for struct `WindowBuilder` in the current scope
   --> /home/infinity/work/nannou/nannou/src/window.rs:784:29
    |
784 |             window = window.with_class("nannou".to_string(), "nannou".to_string());
    |                             ^^^^^^^^^^ method not found in `WindowBuilder`

tychedelia commented 1 year ago

@tychedelia I decided to do some more testing. I tried it on linux. WASAM works great.

However the "normal" builds throw this error.

error[E0432]: unresolved import `winit::platform::unix`
   --> /home/infinity/work/nannou/nannou/src/window.rs:783:34
    |
783 |             use winit::platform::unix::WindowBuilderExtUnix;
    |                                  ^^^^ could not find `unix` in `platform`

error[E0599]: no method named `with_class` found for struct `WindowBuilder` in the current scope
   --> /home/infinity/work/nannou/nannou/src/window.rs:784:29
    |
784 |             window = window.with_class("nannou".to_string(), "nannou".to_string());
    |                             ^^^^^^^^^^ method not found in `WindowBuilder`

Looks like there was a breaking change in winit: https://github.com/rust-windowing/winit/blob/master/CHANGELOG.md?plain=1#L255.

Not sure the best approach here. I think I'm going to enable xorg by default and make wayland an additional feature flag.

tychedelia commented 1 year ago

Should be fixed now, but I haven't tested wayland.

infinity-creative commented 1 year ago

Can con firm it is fixed. - Thank you.

tychedelia commented 1 year ago

Can con firm it is fixed. - Thank you.

Thank you for all the testing!!

infinity-creative commented 1 year ago

Can con firm it is fixed. - Thank you.

Thank you for all the testing!!

You are welcome. However I also really want this update and think it is a much needed contribution. I don't have the deep rust knowledge required to do this this type of update yet. So and this is the least I can do to help.

I am also really enjoying Rust/nannou over Processing and OpenFrameworks and want to continue using it.

Spacing Issue

One thing I just noticed is there is a dimension "space/padding" around the whole window on the Linux build windows vs WASAM. But the canvas extends past as you can see in my squares on the right of the window screenshot.

FYI: In these I am using the cords example code on a 800x800 sketch. I don't know if this is a setting or a bug? - I think a bug :shrug:

~~Haven't checked OSX yet. I will post when I do.~~ It is correct, screenshot attached

Linux (Ubuntu) - Space Issue

Screenshot from 2023-10-14 09-31-18

WASAM - Correct

Screenshot from 2023-10-14 09-38-26

OSX - Correct

tychedelia commented 1 year ago

One thing I just noticed is there is a dimension "space/padding" around the whole window on the Linux build windows vs WASAM. But the canvas extends past as you can see in my squares on the right of the window screenshot.

Yeah, this seems like a bug. Let's see if we can fix this... Would you mind sharing your example sketch so I can try to reproduce? The few examples I've tried don't seem to have this padding on a fresh copy of Ubuntu 22.04.2 ARM64.

infinity-creative commented 1 year ago

Here is a repo with a base sketch in in:

https://github.com/infinitylunacreative/nannou_update_test

Hopefully In linux you can see the cords are at -410, 410, on an 800,800.

I'm running Ubuntu 22.04.3 64bit

infinity-creative commented 1 year ago

I also I found the following while testing the high res output example. - sorry

Found on Ubuntu 22.04.3 64bit.

Stack trace

/home/infinity/work/nannou/target/debug/examples/draw_capture_hi_res
thread 'main' panicked at 'wgpu error: Validation Error

Caused by:
    In Device::create_shader_module
      note: label = `shaders/fs_msaa4.wgsl`

Shader 'shaders/fs_msaa4.wgsl' parsing error: the type of `tex_size` is expected to be `vec2<i32>`, but got `vec2<u32>`
   ┌─ wgsl:15:9
   │
15 │     let tex_size: vec2<i32> = textureDimensions(tex);
   │         ^^^^^^^^ definition of `tex_size`

    the type of `tex_size` is expected to be `vec2<i32>`, but got `vec2<u32>`

', /home/infinity/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.17.1/src/backend/direct.rs:3056:5
stack backtrace:
   0: rust_begin_unwind
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/std/src/panicking.rs:593:5
   1: core::panicking::panic_fmt
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/core/src/panicking.rs:67:14
   2: wgpu::backend::direct::default_error_handler
             at /home/infinity/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.17.1/src/backend/direct.rs:3056:5
   3: core::ops::function::Fn::call
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/core/src/ops/function.rs:79:5
   4: <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/alloc/src/boxed.rs:1999:9
   5: wgpu::backend::direct::ErrorSinkRaw::handle_error
             at /home/infinity/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.17.1/src/backend/direct.rs:3042:17
   6: wgpu::backend::direct::Context::handle_error
             at /home/infinity/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.17.1/src/backend/direct.rs:324:9
   7: <wgpu::backend::direct::Context as wgpu::context::Context>::device_create_shader_module
             at /home/infinity/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.17.1/src/backend/direct.rs:931:13
   8: <T as wgpu::context::DynContext>::device_create_shader_module
             at /home/infinity/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.17.1/src/context.rs:2241:37
   9: wgpu::Device::create_shader_module
             at /home/infinity/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.17.1/src/lib.rs:2251:26
  10: nannou_wgpu::texture::reshaper::Reshaper::new
             at ./nannou_wgpu/src/texture/reshaper/mod.rs:54:22
  11: draw_capture_hi_res::model
             at ./examples/draw/draw_capture_hi_res.rs:68:28
  12: nannou::app::Builder<M>::new::{{closure}}
             at ./nannou/src/app.rs:256:59
  13: core::ops::function::FnOnce::call_once{{vtable.shim}}
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/core/src/ops/function.rs:250:5
  14: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/alloc/src/boxed.rs:1985:9
  15: nannou::app::Builder<M,E>::run_async::{{closure}}
             at ./nannou/src/app.rs:517:31
  16: tokio::runtime::park::CachedParkThread::block_on::{{closure}}
             at /home/infinity/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.33.0/src/runtime/park.rs:282:63
  17: tokio::runtime::coop::with_budget
             at /home/infinity/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.33.0/src/runtime/coop.rs:107:5
  18: tokio::runtime::coop::budget
             at /home/infinity/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.33.0/src/runtime/coop.rs:73:5
  19: tokio::runtime::park::CachedParkThread::block_on
             at /home/infinity/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.33.0/src/runtime/park.rs:282:31
  20: tokio::runtime::context::blocking::BlockingRegionGuard::block_on
             at /home/infinity/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.33.0/src/runtime/context/blocking.rs:66:9
  21: tokio::runtime::scheduler::multi_thread::MultiThread::block_on::{{closure}}
             at /home/infinity/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.33.0/src/runtime/scheduler/multi_thread/mod.rs:87:13
  22: tokio::runtime::context::runtime::enter_runtime
             at /home/infinity/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.33.0/src/runtime/context/runtime.rs:65:16
  23: tokio::runtime::scheduler::multi_thread::MultiThread::block_on
             at /home/infinity/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.33.0/src/runtime/scheduler/multi_thread/mod.rs:86:9
  24: tokio::runtime::runtime::Runtime::block_on
             at /home/infinity/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.33.0/src/runtime/runtime.rs:350:45
  25: nannou::app::Builder<M,E>::run
             at ./nannou/src/app.rs:456:9
  26: draw_capture_hi_res::main
             at ./examples/draw/draw_capture_hi_res.rs:7:5
  27: core::ops::function::FnOnce::call_once
             at /rustc/eb26296b556cef10fb713a38f3d16b9886080f26/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

I looked at the shaders to see if I could fix but not to sure which side of the fence to even attempt.

RobWalt commented 1 year ago

Real cool iniziative. I'd like to join you two to build up some more momentum since the project seems to be quiet inactive otherwise. I'll give the branch some tests and a review as well as soon as I'm on the PC.

The last panic looks like a type error in the shader and should be rather easy to fix.

infinity-creative commented 1 year ago

@RobWalt that would be great. It is all @tychedelia. Im just here breaking things 😬 🤣 .

I am new to shaders as a whole and don't really know how to fix, but yes please get involved. I am guessing fixes you put in should come as a PR to this working repo. This way hopefully we can get it all squared away.

tychedelia commented 1 year ago

@RobWalt Going to look at some of these today, but feel free to PR into my fork (aka this PR)!

tychedelia commented 1 year ago

Here is a repo with a base sketch in in:

https://github.com/infinitylunacreative/nannou_update_test

Hopefully In linux you can see the cords are at -410, 410, on an 800,800.

I'm running Ubuntu 22.04.3 64bit

Update:

The first resize event sent by winit incorrectly includes 20px, which seems to be the case regardless of user specified setting. This feels like a potential bug in winit itself, maybe https://github.com/rust-windowing/winit/issues/2094. Next steps are likely to try upgrading to the 29.1-beta release and see if it's been fixed there. Otherwise I'll file a ticket on winit and see if they have any advice.

infinity-creative commented 1 year ago

@RobWalt @tychedelia - Is there anything you need/want me to test?

anselanza commented 11 months ago

I can't offer much help here, but just want to commend the effort to bring Nannou up to date regarding WGPU and Egui. I really like this framework - and have used it in at least one project in production - but I was getting concerned about the apparent lack of updates recently.

tychedelia commented 10 months ago

Since I opened this, winit has released an 0.29 version which includes their event loop 3.0 rework that has a substantial number of changes. I've attempted an upgrade, and while there's a lot of little fiddly changes that aren't too bad, there are some substantial changes that are somewhat difficult to integrate.

Key has been split into an an enum representing differences between character keys and "named" keys.
LOTS of changes to events emitted by winit. The biggest is probably moving RedrawRequested to WindowEvent. These are probably not too bad to refactor but there's lots of changes here and so it's a bit hard to analyze what events might be relevant to users.

It would be really nice to be able to land a 29.x upgrade in order to get nannou up to a stable state with regards to one of its most important dependencies, but this potentially will incur a lot of user visible breakage for what's ultimately pretty minimal gain for the user.

tychedelia commented 10 months ago

https://github.com/nannou-org/nannou/pull/940/commits/36a724e9783656bf5a197f4b2ece8160f053f017 fixes the issue with the egui demo app, but isn't the right pattern for our users. likely more to do here, it's rendering correctly now though.

tychedelia commented 10 months ago

@mitchmindtree yeah, seeing the colors are super noticeable here in the color picker example.

i'm good with this plan! we can fast follow anything that might come up from further user testing.

mitchmindtree commented 10 months ago

Okydokey let's land this!

I'll follow-up with a PR bumping versions so we can publish to crates.io :+1:

nannou-org / nannou