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

Nothing will be presented if a Surface is created by a non-ui thread (Metal backend) #2727

Closed
shi-yan opened this issue Jun 3, 2022 · 10 comments
Labels
api: metal Issues with Metal area: wsi Issues with swapchain management or windowing type: bug Something isn't working

Comments

@shi-yan
Copy link

shi-yan commented Jun 3, 2022

I'm trying to create a multi-window wgpu program.

My current design is a single producer multiple consumer architecture.

The main event loop is the producer, it accepts events and then dispatches them to corresponding windows.

Each window has its own event loop on a dedicated thread listening to the messages from the main thread. From its dedicated thread, a window also creates its own wgpu command buffer and presents it.

This design seems to work on Linux with the vulkan/xcb backend.

But it doesn't work on Mac with the Metal/Appkit backend.

I wonder if this is expected? because I can't find confirmation from anywhere.

To illustrate the problem, I have created a testing project using winit (I have also tried another windowing system. and The behavior is the same.) (See Archive.zip)

the sample app has 2 variants, in RUST_LOG=trace cargo run --bin multi, I follow the above design. As you can see, the window is empty. What's rendered cannot be seen.

the singlethread variant RUST_LOG=trace cargo run --bin single simply creates command buffers and presents them from the main thread. Things can be rendered properly.

If we check the logs printed by wgpu from both variants, we won't find any issues. Both seem to be able to successfully submit buffers.

I wonder if this is a bug? or if this is expected behavior on Mac? If this is not a bug, I hope this can be mentioned in the document.

Thanks.

Archive.zip

@shi-yan
Copy link
Author

shi-yan commented Jun 3, 2022

so I did more investigation. I read the wgpu-hal code and noticed that Surface has a field "main_thread_id".

In the above code, I created the surface in a non-ui thread. So the "main_thread_id" isn't really the main thread's id.

I adjusted my code to create the Surface on the main thread, and things could render now.

I now feel that this might be a bug of WGPU?

it is related to the following code:

        let current_extent = if surface.main_thread_id == thread::current().id() {
            log::warn!("get dimensions on main thread");
            Some(surface.dimensions())
        } else {
            log::warn!("Unable to get the current view dimensions on a non-main thread");
            None
        };

When a surface is created in the main thread, but used in another thread, current_extent will be None. However, things can be rendered.

When a surface is created in a non-ui thread, current_extent is not None, and the surface dimension is correct. But presenting will not work (nothing will be visible, but no errors).

@shi-yan shi-yan changed the title Is it allowed to present() from a non-ui thread on Mac using Appkit? nothing will be presented if a Surface is created by a non-ui thread Jun 3, 2022
@shi-yan shi-yan changed the title nothing will be presented if a Surface is created by a non-ui thread Nothing will be presented if a Surface is created by a non-ui thread (Metal backend) Jun 3, 2022
@cwfitzgerald cwfitzgerald added type: bug Something isn't working area: correctness We're behaving incorrectly api: metal Issues with Metal labels Jun 4, 2022
@shi-yan
Copy link
Author

shi-yan commented Jun 5, 2022

Yes, I double-checked, with raw Metal, a CAMetalLayer can be created in a non-ui thread.

// the following can be in a non-ui thread
  auto swap_chain = [CAMetalLayer layer];
  swap_chain.device = device;
  swap_chain.pixelFormat = MTLPixelFormatBGRA8Unorm_sRGB;
  swap_chain.framebufferOnly = YES;
  swap_chain.frame = native_window.frame;
    native_window.contentView.wantsLayer = YES;
  native_window.contentView.layer = swap_chain;
  native_window.opaque = YES;
  native_window.backgroundColor = nil;

@jinleili
Copy link
Contributor

jinleili commented Jun 5, 2022

Tested the wgpu instance creation and drawing in non-ui threads using wgpu-on-app and running on iPhone 12 , everything looks fine. Maybe the cause is not on wgpu.

winit is not a library that wgpu has to rely on, I didn't go deep into its code, from wgpu's point of view, whether the following line of code can run in a non-ui thread depends on whether the layer inside winit is already CAMetalLayer or not, because CAMetalLayer must be created in the ui thread:
self.surface = Some(unsafe { instance.create_surface(winit_win) })

// wgpu-on-app
// iOS ViewController
// create wgpu instance
DispatchQueue.global().async { [weak self] in
   self?.wgpuCanvas = create_wgpu_canvas(viewObj)
}

// draw frame
DispatchQueue.global().async {
   enter_frame(self.wgpuCanvas)
}

@jinleili
Copy link
Contributor

jinleili commented Jun 5, 2022

Yes, I double-checked, with raw Metal, a CAMetalLayer can be created in a non-ui thread.

// the following can be in a non-ui thread
  auto swap_chain = [CAMetalLayer layer];
  swap_chain.device = device;
  swap_chain.pixelFormat = MTLPixelFormatBGRA8Unorm_sRGB;
  swap_chain.framebufferOnly = YES;
  swap_chain.frame = native_window.frame;
    native_window.contentView.wantsLayer = YES;
  native_window.contentView.layer = swap_chain;
  native_window.opaque = YES;
  native_window.backgroundColor = nil;

That's not true: https://stackoverflow.com/questions/58333048/cametallayer-initialization-only-works-on-main-thread

@cwfitzgerald cwfitzgerald added area: wsi Issues with swapchain management or windowing and removed area: correctness We're behaving incorrectly labels Jun 5, 2022
@shi-yan
Copy link
Author

shi-yan commented Jun 6, 2022

  1. I tested multi-thread CAMetalLayer in an obj-c project. it worked. I can upload a sample later.

  2. I have tried other windowing libs than just winit. They behave the same with wgpu. The issue is not from winit IMO.

@shi-yan
Copy link
Author

shi-yan commented Jun 6, 2022

And this bug is filed for Mac, not for iOS. iOS uses UIKit, not APPkit. And they can trigger different code paths.

Please debug and test on Mac

@jinleili
Copy link
Contributor

jinleili commented Jun 6, 2022

With a slight change to your code, the multi thread demo works fine.

 let instance = wgpu::Instance::new(wgpu::Backends::all());
 let surface = Some(unsafe { instance.create_surface(&winit_win) });
 let handle = thread::spawn(move || {
      ...
       MyWindow::event_loop(&mut win, winit_win, instance, surface, &mut rx);
});
...
pub async fn init_visual(
        &mut self,
        winit_win: &Window,
        instance: Instance,
        surface: Option<Surface>,
    ) -> Result<(), String> 

winit CHANGELOG says:

On macOS, drop unused Metal dependency.

So, CAMetalLayer Created in wgpu inner, but wgpu currently does not have the infrastructure to directly specify ui thread, it defaults to the user calling create_surface in the appropriate thread.

@shi-yan
Copy link
Author

shi-yan commented Jun 7, 2022

Yes, I understand how to workaround this issue by creating surfaces in the main thread. But I'm still wondering if this should be treated as a bug of WGPU.

The attachment is my objc program that creates an NSWindow in the main thread but configures a CAMetalLayer in a separate thread and then renders a background to the window.

It works fine.

Archive.zip

@shi-yan
Copy link
Author

shi-yan commented Jun 7, 2022

Thank you for the help.

I received some help from an Apple engineer who works on Metal.

According to the authoritative answer, It is ok to create a CAMetalLayer on a non-ui thread. However, if the non-ui thread doesn't have a runloop, the draw call won't be submitted.

The right way to do it is using the presentsWithTransaction mode and enclosing CAMetalLayer creation as well as rendering/presenting into :

[CATransaction begin];
... // CAMetalLayer creation and rendering/presenting code.
[CATransaction commit];

Also, the above sample code that I claimed can create and render to a CAMetalLayer in a non-ui thread wasn't correct (because the non-ui thread doesn't contain a render loop. It quits as soon as the first buffer submission has finished, which for some reason triggers a successful present. If the non-ui thread has a render loop, presenting won't succeed, this behaves the same as the rust code.)

Given this, close this issue. This issue should be addressed by a windowing system.

@shi-yan
Copy link
Author

shi-yan commented Jun 7, 2022

Also, I'd like to provide two code samples.

the first "no runloop" sample reproduces the same issue of the rust code (failed to present in a non-ui thread).
Archive_norunloop.zip

the second sample can present in a non-ui thread by using the CATransaction block;
Archive_catransaction.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: metal Issues with Metal area: wsi Issues with swapchain management or windowing type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants