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

Can no longer create an Arc<FramebufferAbstract>> in vulkano (due to x11 IME changes) #472

Closed
rukai opened this issue Apr 18, 2018 · 3 comments
Labels

Comments

@rukai
Copy link
Contributor

rukai commented Apr 18, 2018

@francesca64 Thanks for your continued work on winit. This appears to be introduced by: 09c809003 Any ideas on how to fix it?

This is preventing PF Sandbox from compiling on linux: https://travis-ci.org/rukai/PF_Sandbox

ImeSender is an alias for std::sync::mpsc::Sender<(u64, i16, i16)> which is not Sync.
There is a field of type ImeSender on the winit::os::unix::x11::Window struct: https://github.com/tomaka/winit/blob/master/src/platform/linux/x11/mod.rs#L1007
This means Window is not Sync and therefore vulkano::image::SwapchainImage<Window> is not sync.
This prevents us from creating an Arc<FramebufferAbstract> which is the only way to store a frame buffer in a struct in vulkano.

The following code no longer compiles with winit and vulkano sourced from github

extern crate vulkano;
extern crate winit;

use vulkano::framebuffer::{Framebuffer, FramebufferAbstract, RenderPassAbstract};
use vulkano::image::SwapchainImage;
use winit::Window;

use std::sync::Arc;

pub fn foo(images: &[Arc<SwapchainImage<Window>>], render_pass: Arc<RenderPassAbstract + Sync + Send>) {
    images.iter().map(|image| {
        Arc::new(
            Framebuffer::start(render_pass.clone()).add(image.clone()).unwrap().build().unwrap()
        ) as Arc<FramebufferAbstract + Send + Sync>
    });
}

We get this compilation error

error[E0277]: the trait bound `std::sync::mpsc::Sender<(u64, i16, i16)>: std::marker::Sync` is not satisfied in `vulkano::swapchain::Surface<winit::Window>`
  --> src/main.rs:18:9
   |
18 | /         Arc::new(
19 | |             Framebuffer::start(render_pass.clone()).add(image.clone()).unwrap().build().unwrap()
20 | |         ) as Arc<FramebufferAbstract + Send + Sync>
   | |_________^ `std::sync::mpsc::Sender<(u64, i16, i16)>` cannot be shared between threads safely
   |
   = help: within `vulkano::swapchain::Surface<winit::Window>`, the trait `std::marker::Sync` is not implemented for `std::sync::mpsc::Sender<(u64, i16, i16)>`
   = note: required because it appears within the type `winit::os::unix::x11::Window`
   = note: required because it appears within the type `winit::platform::platform::Window`
   = note: required because it appears within the type `winit::Window`
   = note: required because it appears within the type `vulkano::swapchain::Surface<winit::Window>`
   = note: required because of the requirements on the impl of `std::marker::Send` for `std::sync::Arc<vulkano::swapchain::Surface<winit::Window>>`
   = note: required because it appears within the type `vulkano::swapchain::Swapchain<winit::Window>`
   = note: required because of the requirements on the impl of `std::marker::Send` for `std::sync::Arc<vulkano::swapchain::Swapchain<winit::Window>>`
   = note: required because it appears within the type `vulkano::image::SwapchainImage<winit::Window>`
   = note: required because of the requirements on the impl of `std::marker::Sync` for `std::sync::Arc<vulkano::image::SwapchainImage<winit::Window>>`
   = note: required because it appears within the type `((), std::sync::Arc<vulkano::image::SwapchainImage<winit::Window>>)`
   = note: required because it appears within the type `vulkano::framebuffer::Framebuffer<std::sync::Arc<vulkano::framebuffer::RenderPassAbstract + std::marker::Sync + std::marker::Send>, ((), std::sync::Arc<vulkano::image::SwapchainImage<winit::Window>>)>`
   = note: required for the cast to the object type `vulkano::framebuffer::FramebufferAbstract + std::marker::Sync + std::marker::Send`

error: aborting due to previous error

If you want more information on this error, try using "rustc --explain E0277"
error: Could not compile `foo`.
@francesca64
Copy link
Member

  1. PF Sandbox sounds really cool!
  2. I think I can probably just wrap ImeSender in a mutex and be good to go.
  3. Thanks for catching this, though the sender was added back in 1c4973d, so it looks like this regression unfortunately made it into the 0.12.0 release.
  4. I'll add a test to make sure this can't happen again.

@francesca64
Copy link
Member

Alright, give this a shot: https://github.com/francesca64/winit/tree/x11-imesender-sync

I can't test PF Sandbox myself, since it fails at OutOfDeviceMemory (my VRAM usage sits around 1795/1999 MiB, thanks to having 135 Chrome tabs open at once), but it definitely compiles now.

@rukai
Copy link
Contributor Author

rukai commented Apr 19, 2018

Yes that fixes the issue, thankyou!

francesca64 added a commit to francesca64/winit that referenced this issue Apr 19, 2018
francesca64 added a commit that referenced this issue Apr 21, 2018
* x11: Windows are Sync again

Fixes #472

* Add test ensuring that Window is Sync

Window must be Sync for Vulkano's Arc<FramebufferAbstract> to be usable.
@rukai rukai mentioned this issue Jun 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants