Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade wgpu #830

Merged
merged 14 commits into from
May 24, 2021
Merged

Upgrade wgpu #830

merged 14 commits into from
May 24, 2021

Conversation

Dispersia
Copy link
Contributor

@Dispersia Dispersia commented Apr 13, 2021

Draft PR to upgrade wgpu and use wgsl so cross can stop being compiled for android, includes temporary patches until fix for staging belt goes out.

Fixes #511.
Closes #523.

@hecrj hecrj added the feature New feature or request label May 19, 2021
@hecrj hecrj added this to the 0.4.0 milestone May 19, 2021
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

I got this to compile using the latest releases of wgpu and wgpu_glyph.

However, I am getting some shader validation errors once I try to run the tour.

     Running `target/debug/tour`
wgpu error: Validation Error

Caused by:
    In Device::create_render_pipeline
      note: label = `iced_wgpu::quad pipeline`
    error matching VERTEX shader requirements against the pipeline
    shader global ResourceBinding { group: 0, binding: 0 } is not available in the layout pipeline layout
    buffer structure size 80, added to one element of an unbound array, if it's the last field, ended up greater than the given `min_binding_size`

@Dispersia Dispersia marked this pull request as ready for review May 19, 2021 14:16
@Dispersia
Copy link
Contributor Author

let me take a look, thanks for the patch so hopefully not too bad to fix

@Dispersia
Copy link
Contributor Author

talked to wgpu team and had me make an issue: gfx-rs/naga#882 will be waiting on that. I was suggested to just pad it up to the size it wants, but if i do that the uniform size doesn't match, and then if i pad the actual byte array it gives the original error again, so going to look into it further. Seems to be something introduced in 0.8.1

@Dispersia
Copy link
Contributor Author

Dispersia commented May 20, 2021

As was mentioned in that issue, the error wasn't super helpful but structs in uniforms need to align to their largest member, in this case mat4x4 which aligns to 16: https://gpuweb.github.io/gpuweb/wgsl/#alignment-and-size. Modified to fix padding for quad, also noticed triangle was doing the same thing, but it was already aligned to 16 so the current padding could just be removed. I'm not sure what issue that caused previously, but everything runs correctly on my system without it so looks like its ok to me

@hecrj
Copy link
Member

hecrj commented May 20, 2021

Thanks for fixing that @Dispersia! 🎉 I believe this should probably fix #511 too.

However, I still get a panic caused by a validation error on macOS Catalina:

wgpu error: Validation Error

Caused by:
    In Device::create_render_pipeline
      note: label = `iced_wgpu::quad pipeline`
    Internal error in VERTEX shader: Error compiling the shader "\"Compilation failed: \\n\\nprogram_source:106:17: error: declaration with attribute \\\'position\\\' already specified\\n, metal::float4 coord [[position]]\\n  ~~~~~~~~~~~~~~^~~~~\\nprogram_source:105:17: note: previous declaration with attribute \\\'position\\\' here\\n, metal::float4 position [[position]]\\n                ^\\n\""

@Dispersia
Copy link
Contributor Author

Appreciate you testing on mac! I'll make sure to try and cover all of the other platforms then (since I don't have a mac yet, unfortunately). Created an issue here: gfx-rs/naga#888, unsure how metal solves this problem

@kaimast
Copy link

kaimast commented May 20, 2021

I just tested your branch on Linux/X11. The wgpu integration example works fine but the solar system one crashes on startup.

~/dev/tmp/iced upgrade-wgpu ❯ env RUST_BACKTRACE=full cargo run --package solar_system                                                                                                    ✘ SEGV  48s
    Finished dev [unoptimized + debuginfo] target(s) in 0.15s
     Running `target/debug/solar_system`
MESA-INTEL: warning: Performance support disabled, consider sysctl dev.i915.perf_stream_paranoid=0

wgpu error: Validation Error

Caused by:
    In Device::create_bind_group
      note: label = `iced_wgpu::triangle::msaa uniforms bind group`
    sampler binding 0 expects filtering = false, but given a sampler with filtering = true


thread 'main' panicked at 'Handling wgpu errors as fatal by default', /home/kai/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.8.1/src/backend/direct.rs:1955:5
stack backtrace:
   0:     0x55ff96e89910 - std::backtrace_rs::backtrace::libunwind::trace::hdcf4f90f85129e83
                               at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/../../backtrace/src/backtrace/libunwind.rs:90:5
   1:     0x55ff96e89910 - std::backtrace_rs::backtrace::trace_unsynchronized::h2669e30cb82f6732
                               at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x55ff96e89910 - std::sys_common::backtrace::_print_fmt::hfbda19e17f6db318
                               at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/sys_common/backtrace.rs:67:5
   3:     0x55ff96e89910 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h1a8751bf59281272
                               at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/sys_common/backtrace.rs:46:22
   4:     0x55ff96eace2f - core::fmt::write::h7aa6cd0067dca82a
                               at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/core/src/fmt/mod.rs:1094:17
   5:     0x55ff96e85215 - std::io::Write::write_fmt::hd7dd3a1df9b6befb
                               at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/io/mod.rs:1584:15
   6:     0x55ff96e8ba1b - std::sys_common::backtrace::_print::h551e9ec8a9fa8106
                               at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/sys_common/backtrace.rs:49:5
   7:     0x55ff96e8ba1b - std::sys_common::backtrace::print::ha4b1c5e95fa040b3
                               at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/sys_common/backtrace.rs:36:9
   8:     0x55ff96e8ba1b - std::panicking::default_hook::{{closure}}::h0b34c9ab7fb9f857
                               at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/panicking.rs:208:50
   9:     0x55ff96e8b4fd - std::panicking::default_hook::h3067e8318decd17a
                               at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/panicking.rs:225:9
  10:     0x55ff96e8bfcd - std::panicking::rust_panic_with_hook::h81b8facc50f34daa
                               at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/panicking.rs:591:17
  11:     0x55ff964620d3 - std::panicking::begin_panic::{{closure}}::h321d8656493a5d60
                               at /home/kai/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:520:9
  12:     0x55ff964675f9 - std::sys_common::backtrace::__rust_end_short_backtrace::hc25627b7e193d4b7
                               at /home/kai/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:141:18
  13:     0x55ff96462009 - std::panicking::begin_panic::h4c8176da4b4ced29
                               at /home/kai/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:519:12
  14:     0x55ff95e91cec - wgpu::backend::direct::default_error_handler::hc8c00d5eac853cc2
                               at /home/kai/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.8.1/src/backend/direct.rs:1955:5
  15:     0x55ff95db4b9c - core::ops::function::Fn::call::h34ea71e30a2804b6
                               at /home/kai/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:70:5
  16:     0x55ff95ddb8e2 - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::h09bd3d9b27d73596
                               at /home/kai/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1589:9
  17:     0x55ff95e91be1 - wgpu::backend::direct::ErrorSinkRaw::handle_error::h45e9772f94dc527c
                               at /home/kai/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.8.1/src/backend/direct.rs:1942:9
  18:     0x55ff95e85041 - wgpu::backend::direct::Context::handle_error::h50ddd10a85e06df5
                               at /home/kai/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.8.1/src/backend/direct.rs:93:9
  19:     0x55ff95e8bded - <wgpu::backend::direct::Context as wgpu::Context>::device_create_bind_group::hfd81f9a80093d338
                               at /home/kai/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.8.1/src/backend/direct.rs:938:13
  20:     0x55ff96064727 - wgpu::Device::create_bind_group::h02f63f4d3e860868
                               at /home/kai/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.8.1/src/lib.rs:1571:17
  21:     0x55ff95c3c178 - iced_wgpu::triangle::msaa::Blit::new::h6701d55dc97e3afd
                               at /home/kai/dev/tmp/iced/wgpu/src/triangle/msaa.rs:44:13
  22:     0x55ff95be3189 - iced_wgpu::triangle::Pipeline::new::{{closure}}::h1bcd634956b74d1d
                               at /home/kai/dev/tmp/iced/wgpu/src/triangle.rs:206:40
  23:     0x55ff95bdc528 - core::option::Option<T>::map::h2ae83fcd3493eb27
                               at /home/kai/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/option.rs:487:29
  24:     0x55ff95c4dcde - iced_wgpu::triangle::Pipeline::new::h61ef4c793b378576
                               at /home/kai/dev/tmp/iced/wgpu/src/triangle.rs:206:19
  25:     0x55ff95c3e2fa - iced_wgpu::backend::Backend::new::hd1c028397630a962
                               at /home/kai/dev/tmp/iced/wgpu/src/backend.rs:37:33
  26:     0x55ff95c29a19 - iced_wgpu::window::compositor::Compositor::create_backend::h80365374c72c668d
                               at /home/kai/dev/tmp/iced/wgpu/src/window/compositor.rs:77:9
  27:     0x55ff95ac66b8 - <iced_wgpu::window::compositor::Compositor as iced_graphics::window::compositor::Compositor>::new::h2b7333615a06b36f
                               at /home/kai/dev/tmp/iced/wgpu/src/window/compositor.rs:97:23
  28:     0x55ff95b926e5 - iced_winit::application::run::h7e3e96481d560caf
                               at /home/kai/dev/tmp/iced/winit/src/application.rs:151:34
  29:     0x55ff95b6b36b - iced::application::Application::run::hda86147dacd7badc
                               at /home/kai/dev/tmp/iced/src/application.rs:219:16
  30:     0x55ff95b14277 - solar_system::main::h316013c452806dd8
                               at /home/kai/dev/tmp/iced/examples/solar_system/src/main.rs:18:5
  31:     0x55ff95b4e512 - core::ops::function::FnOnce::call_once::h9ea1938231e87869
                               at /home/kai/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
  32:     0x55ff95ab220a - std::sys_common::backtrace::__rust_begin_short_backtrace::h3f9f1432cd692bd1
                               at /home/kai/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:125:18
  33:     0x55ff95b3a096 - std::rt::lang_start::{{closure}}::h737492ca953cccca
                               at /home/kai/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:49:18
  34:     0x55ff96e8c3ca - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::h3b8c329143d7638a
                               at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/core/src/ops/function.rs:259:13
  35:     0x55ff96e8c3ca - std::panicking::try::do_call::h4b72c261b4eefc1b
                               at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/panicking.rs:379:40
  36:     0x55ff96e8c3ca - std::panicking::try::h703d31b7896cbd49
                               at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/panicking.rs:343:19
  37:     0x55ff96e8c3ca - std::panic::catch_unwind::h37cad9b35388a915
                               at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/panic.rs:431:14
  38:     0x55ff96e8c3ca - std::rt::lang_start_internal::hab5a8a909af4f90e
                               at /rustc/5c029265465301fe9cb3960ce2a5da6c99b8dcf2/library/std/src/rt.rs:34:21
  39:     0x55ff95b3a070 - std::rt::lang_start::ha2345f6196cfc1ee
                               at /home/kai/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:48:5
  40:     0x55ff95b148ac - main
  41:     0x7fb6ce547565 - __libc_start_main
  42:     0x55ff95aaee9e - _start
  43:                0x0 - <unknown>
fish: “env RUST_BACKTRACE=full cargo r…” terminated by signal SIGSEGV (Address boundary error)

@Dispersia
Copy link
Contributor Author

interesting, that was passing before 0.8 release but i guess new validation broke it, which is good! However, looking into that pipeline, there's actually several things im having a hard time understanding what it's doing, the main thing is the uniform, it's a dynamically sized transform, but isn't defined as an array in the shader. I'm not sure how glsl is even handling that currently, but once i figure out the intent I'll try to get a fix for that. Thanks for testing it!

@hecrj
Copy link
Member

hecrj commented May 21, 2021

I have reverted 2d549d8, as removing the padding in triangle was causing this error:

wgpu error: Validation Error

Caused by:
    In a RenderPass
      note: encoder = `iced_wgpu encoder`
    In a set_bind_group command
      note: bind group = `iced_wgpu::triangle uniforms bind group`
    dynamic binding at index 0: offset 64 does not respect `BIND_BUFFER_ALIGNMENT`

I think I have also fixed the issue reported by @kaimast in 4cbc345. Let me know!

All the examples seem to work on my end now.

@Dispersia
Copy link
Contributor Author

ha, you would think it would be obvious that i removed padding and then had alignment issues, but oh well haha. Thanks for the patch, tested on x11 and windows and all examples seem good to me too

@PolyMeilex PolyMeilex mentioned this pull request May 22, 2021
1 task
@mvilim mvilim mentioned this pull request May 23, 2021
22 tasks
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

Thanks again everyone! I think we can merge this 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

metal validation crash on macos
3 participants