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

macOS metal deadlock (multithreaded renderer) #2036

Open
sinkingsugar opened this issue Feb 11, 2020 · 7 comments
Open

macOS metal deadlock (multithreaded renderer) #2036

sinkingsugar opened this issue Feb 11, 2020 · 7 comments
Labels

Comments

@sinkingsugar
Copy link
Contributor

sinkingsugar commented Feb 11, 2020

since #2001 my framework stopped working properly on macOS.
as explained in the comment of the PR above there is a deadlock.
I saw that the fix got reverted but that's really confusing.
CC: @attilaz

I will try to workaround by using different threads, but the easy and usual behavior is definitely broken.

Workaround:
Revert ecc8f791f2ed53225e000eb57f8109660c011ee0 (ecc8f79)

@bkaradzic bkaradzic added the bug label Feb 11, 2020
@kattkieru
Copy link

I think I just hit the same issue-- working on using bgfx with an SDL2 window, the app locks hard unless I disable threading in the build.

@loryxia
Copy link

loryxia commented Mar 21, 2020

i stuck at the same issue too, single thread is ok.

@mfxdeveloper
Copy link

I have this too. Apart from disabling multithreading on Mac (which lowers performance), is the workaround to go back to an earlier version of the bgfx metal renderer which did work with multithreading?

@sinkingsugar
Copy link
Contributor Author

Fyi the workaround is to create a .mm file and instead of passing a window or view to bgfx, pass directly a metal layer.
Basic solution (no cleanups):

#import <QuartzCore/CAMetalLayer.h>
#import <Metal/Metal.h>
#import <MetalKit/MetalKit.h>
#import <Cocoa/Cocoa.h>

namespace BGFX {
void *cbSetupMetalLayer(void *wnd) {
  NSWindow *window = (NSWindow*)wnd;
  NSView *contentView = [window contentView];
  [contentView setWantsLayer:YES];
  CAMetalLayer *res = [CAMetalLayer layer];
  [contentView setLayer:res];
  return res;
}
}

Passing directly a metal layer prevents the (a bit silly semaphore and workaround) deadlock

@mfxdeveloper
Copy link

mfxdeveloper commented Jun 7, 2020

Thank you, that metal layer fix looks pretty easy!

@Kurapikov
Copy link

Fyi the workaround is to create a .mm file and instead of passing a window or view to bgfx, pass directly a metal layer. Basic solution (no cleanups):

#import <QuartzCore/CAMetalLayer.h>
#import <Metal/Metal.h>
#import <MetalKit/MetalKit.h>
#import <Cocoa/Cocoa.h>

namespace BGFX {
void *cbSetupMetalLayer(void *wnd) {
  NSWindow *window = (NSWindow*)wnd;
  NSView *contentView = [window contentView];
  [contentView setWantsLayer:YES];
  CAMetalLayer *res = [CAMetalLayer layer];
  [contentView setLayer:res];
  return res;
}
}

Passing directly a metal layer prevents the (a bit silly semaphore and workaround) deadlock

This deadlock issue still persists in the most recent version of bgfx on macOS 14.0, and the workaround by sinkingsugar still effectively addresses it. Given that this issue has been present for over three years and many people (including myself) have wasted a lot of time on it, I believe it should be clearly labeled as either a bug or a feature.

If this issue is identified as a bug, then I think sinkingsugar's workaround should be integrated into the main code branch. If it's deemed a feature, then I believe it should be prominently highlighted multiple times in the documentation and examples, providing more information for those who encounter the problem.

Between the two, I would prefer it to be classified as a bug and have the workaround incorporated within the bgfx library. This ensures a consistent appearance of the API. Moreover, for pure C++ projects that don't use Objective-C, retaining an Objective-C file solely for the workaround is an unsightly implementation.

@bkaradzic
Copy link
Owner

@Kurapikov Create PR with fix you think it's appropriate...

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

No branches or pull requests

6 participants