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

Electron overriding High DPI support on macOS with SDL #20656

Closed
3 tasks done
Suhail opened this issue Oct 21, 2019 · 6 comments
Closed
3 tasks done

Electron overriding High DPI support on macOS with SDL #20656

Suhail opened this issue Oct 21, 2019 · 6 comments

Comments

@Suhail
Copy link

Suhail commented Oct 21, 2019

Preflight Checklist

  • I have read the Contributing Guidelines for this project.
  • I agree to follow the Code of Conduct that this project adheres to.
  • I have searched the issue tracker for an issue that matches the one I want to file, without success.

Issue Details

When running our C++ node addon in Electron high DPI settings/flags are not respected. The node addon on its own works fine until it's packaged with Electron.

Our node addon separately, in another process but called from within Electron, creates a new window with SDL that attempts to be DPI enabled via SDL_WINDOW_ALLOW_HIGHDPI

We aren't using new BrowserWindow and expecting Chrome to resize in this case!

Does not work in the following scenario:

  • Running via electron .
  • Running after a packaged .dmg

The Key in Info.plist High Resolution Capable is set to YES -- electron appears to auto-generate a default info.plist with it. (We do not set anything on our own) nor are we setting --extend-info

  • Electron Version:

    • "electron": "^6.0.9",
      "electron-builder": "^21.2.0",
  • Operating System:
    macOS - 10.14.6

Expected Behavior

The program should be displayed in High DPI mode and respect what is being passed to SDL

For example, we should receive a SDL_WINDOWEVENT that tells us to resize the texture at a new width and height.

Actual Behavior

When moving a program window (not Electron created window) over to a Retina screen (macbook pro), it doesn't receive a new renderer output to resize too and doesn't appear high DPI aware

When we run our node addon separately, moving the window reconfigures the texture each time as soon as we move the screen enough to a different DPI screen.

To Reproduce

Set the following up in SDL:

// init
SDL_Window *window = SDL_CreateWindow(
      "App name", SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, width, height,
      SDL_WINDOW_RESIZABLE | SDL_WINDOW_ALLOW_HIGHDPI)

 SDL_Renderer *renderer =
      SDL_CreateRenderer(window, -1, SDL_RENDERER_ACCELERATED);

// configure texture
SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, "linear");

  int o_width = 0, o_height = 0;
  int ret = SDL_GetRendererOutputSize(ctx->renderer, &o_width, &o_height);
  if (ret != 0) {
    throw runtime_error("Could not get output renderer size\n");
  }

  printf("Output renderer size: %dx%d\n", o_width, o_height);

Questions

  • How do ensure this works in dev when we do electron .
  • How do make this successful work when packaged?
@Suhail
Copy link
Author

Suhail commented Oct 23, 2019

I tried using the ELECTRON_RUN_AS_NODE env variable:

window = fork(require.resolve(this.module_path), [], {
        env: { ELECTRON_RUN_AS_NODE: 1 },
      });

I noticed the child process does get the var:

{ ELECTRON_RUN_AS_NODE: '1', __CF_USER_TEXT_ENCODING: '0x1F5:0x0:0x0' }

However, it still runs differently than if I just ran the program with node on its own instead via electron.

Is this window we're creating inheriting something from the other windows?

Some people have suggested that the window created by SDL is inheriting the window that is forking it in the first place -- taking all the Cocoa inherited properties with it. It's unclear to me why it still wouldn't work with high DPI though given that.

@Suhail
Copy link
Author

Suhail commented Oct 23, 2019

Ok, I think I got to this bottom of this:

  • A fork'd process is not inheriting the properties of the Info.plist of its parent process
  • That's because it can't find it as per: If the agent that launched the program did not specify the full path to the program's executable in the argv parameters, the main bundle value might be NULL. Bundles rely on either the path to the executable being in argv[0] or the presence of the executable's path in the PATH environment variable. If neither of these is present, the bundle routines might not be able to find the main bundle directory. Programs launched by xinetd often experience this problem when xinetd changes the current directory to /.
  • When printing out the NSHighResolutionCapable key from the bundle it returns null. The path of the bundle is also: /usr/local/bin in my case. That's not surprising since that's likely where node is located.

So, we have to find a way to pass the true process' path so the program can find the right bundle and Info.plist file such that when calling NSWindow in SDL2 it will display things at High DPI.

It'd be nice if Electron handled this part where perhaps it would send the right path along with the fork'd call.

macOS does not appear to default to high dpi.

@Suhail
Copy link
Author

Suhail commented Oct 24, 2019

Solution: Whatever you set in the Info.plist for the main process, equivalently apply it to Electron Helper processes since they have their own Info.plist.

We patched this via:

cd dist/mac/Mighty.app/Contents/Frameworks/Mighty\ Helper.app/Contents
/usr/libexec/PlistBuddy -c "add NSHighResolutionCapable bool true" Info.plist

@miniak
Copy link
Contributor

miniak commented Oct 27, 2019

did you try loading the native module in the main process instead of the renderer?

@Suhail
Copy link
Author

Suhail commented Oct 27, 2019

Yes

@codebytere
Copy link
Member

Closed by #21872 (comment)

@sofianguy sofianguy moved this from Unsorted Issues to Fixed for Next Release in 6.1.x Feb 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
6.1.x
Fixed for Next Release
Development

No branches or pull requests

4 participants