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

Per monitor DPI awareness causes issues with window positioning and sizing #10862

Open
MarshallOfSound opened this issue Oct 20, 2017 · 11 comments

Comments

@MarshallOfSound
Copy link
Member

@MarshallOfSound MarshallOfSound commented Oct 20, 2017

As per #9560 where previous issues regarding this were collapsed into.

If you have one monitor with non-100% scaling and one monitor with 100% scaling both setSize and setPosition do not act as expected (they always set incorrectly by a factor exactly equal to the scale factor of the screen)

A proposed workaround is #9708 but that simply reverts the addition of the feature instead of fixing the underlying cause.

All future issues regarding DPI scaling and window positioning / sizing should be merged into this one.

@MarshallOfSound

This comment has been minimized.

Copy link
Member Author

@MarshallOfSound MarshallOfSound commented Oct 20, 2017

@dimdimdimdim

This comment has been minimized.

Copy link

@dimdimdimdim dimdimdimdim commented Oct 20, 2017

@MarshallOfSound Well, the #10659 problem I raised, although clearly related with DPI, isn't specific to "per-monitor" DPI, and you don't even need multiple monitors to reproduce it. So it doesn't fit this new issue as currently described, and the way to solve both these issues most certainly involves different fixes in the code.

I'm afraid merging all the DPI-related bugs into this one will just make everybody forget about the sub-issues, as they are now closed. If you want to merge all these issues into one, the description of the main issue should probably be made much more general than it currently is.

Just my 2 cents.

@MarshallOfSound

This comment has been minimized.

Copy link
Member Author

@MarshallOfSound MarshallOfSound commented Oct 20, 2017

@dimdimdimdim As far as I am aware, the underlying reason is the same (adding the flag that makes Electron executable per-monitor DPI aware). Not saying we should merge all high-DPI issues into this one, but everyone who is reporting individual things like "positioning doesn't work", "sizing doesn't work" and "things appear off screen" should all get merged into this one issue that keeps track of root-cause and progress made on that.

@felixrieseberg

This comment has been minimized.

Copy link
Member

@felixrieseberg felixrieseberg commented Oct 27, 2017

As a general tip: I'm currently fighting my way through this and have learned (so far) that calling setPosition on Windows 10 seems to do the right thing, even if calling new BrowserWindow({ x, y }) didn't result in the correct positioning.

@felixrieseberg

This comment has been minimized.

Copy link
Member

@felixrieseberg felixrieseberg commented Oct 30, 2017

A workaround that also doesn't address the underlying issue: #10972

@bpasero

This comment has been minimized.

Copy link
Contributor

@bpasero bpasero commented Oct 31, 2017

@felixrieseberg nice find, I can also confirm the workaround seems to work for VS Code when calling setPosition after the window has been created.

bpasero added a commit to microsoft/vscode that referenced this issue Feb 26, 2018
georgitodorov added a commit to danailbd/gpii-app that referenced this issue Mar 14, 2018
…1.8.1

There is a severe, still unresolved issue with window positioning and
dimensions that comes with the new versions of Electron. More information
about it can be found here:
electron/electron#10862. The fixes include:
* Activating the `resize` callbacks only when the DPI has actually changed
* Usage of `setBounds` instead of `setSize` and `setPosition` in order to
simultaneously set the dimensions and the coordinates of a `BrowserWindow`
* Using the original dimensions of every `BrowserWindow` and relying on
a subsequent resize after the heightChangeListener has provided the actual
height of the window
bpasero added a commit to microsoft/vscode that referenced this issue Apr 3, 2018
* electron@2.0.0-beta-1

* Update distro

* Update electron.d.ts to 2.0.0-beta.1

* Disable asar as it causes a native crash

* Adopt Module._resolveLookupPaths ASAR patch

* electron 2.x - restore inspector URL in extension host

* electron 2.x - adopt context menu callback for onHide

* electron 2.x - remove workaround for electron/electron#10442

* electron 2.x - update node.d.ts

* electron 2.x - update node.d.ts to 8.9.x

* electron 2.x - keep node.d.ts changes to a minimum

* electron 2.x - remove workaround for electron/electron#10862

* electron 2.x - bump to 2.0.0-beta2

* bump to 2.0.0-beta.3

* Context menu: selecting "Rename" does not put focs into rename box (fix #45601)

* quality "exploration" for easier testing

* empty commit

* push a workaround for #45700

* Certain themes show UI artifacts over activity bar icons (fixes #45700)

* better fix for #45700

* bump to 2.0.0-beta.4

* another fix to prevent flickering for #45700

* avoid remote access in index.js

* bump distro commit

* electron 2.x - do not use --debug anymore

* bump electron to 2.0.0-beta.5

* electron 2.x - add libgtk-3-dev as build dependency for Linux 64

* electron 2.x - workaround freeze on linux on startup

* bump local storage telemetry key

* electron 2.x - do a one time backup of local storage

* enable ELECTRON_ENABLE_LOGGING on macOS at least

* 2.0.0-beta.6

* Fix ctrl+shift+e not focusing explorer on Linux

* distro - use GH electron builds for now
@sofianguy sofianguy added bug 🐞 and removed fixme/bug 🐞 labels Apr 4, 2018
@sofianguy

This comment has been minimized.

Copy link
Member

@sofianguy sofianguy commented Aug 22, 2018

@MarshallOfSound is this still happening on later versions of Electron?

@alexdevero

This comment has been minimized.

Copy link

@alexdevero alexdevero commented Aug 23, 2018

Yes it is. Tested with electron 2.0.8. UI elements for electron window (top nav) are still scaled differently than dev tools.
scaling

@aabuhijleh

This comment has been minimized.

Copy link

@aabuhijleh aabuhijleh commented Jul 4, 2019

A multi-screen setup with different scale factors still causes issues as of electron@5.0.6 on Windows 10

I used screen.getAllDisplays() and the results are really wildly inconsistent if scale factors are different.

Monitor setup (2) is the main display

monitor_setup

First case: both displays have the same scale factor --> everything is okay

boththesame

Second case: external display has a different scale factor -->

  • height and width of the display need to be multiplied by that display's scale factor to be correct.
  • x-position is correct.
  • y-position makes absolutely no sense.

oneisdifferent

My use case: I've implemented desktop sharing via WebRTC and then I'm highlighting the display shared by the user (via a native window) based on screen.bounds result. In the second case, if the user chooses to share the second screen this is the result:

result1

Any suggestions are really appreciated.

@aabuhijleh

This comment has been minimized.

Copy link

@aabuhijleh aabuhijleh commented Jul 8, 2019

@CapOM thanks for the info. So this is apparently a chromium issue. I couldn't tell what's going wrong exactly but I tried playing with Windows API EnumDisplayMonitors and the results were more consistent.

In the "Second case" the second monitor returns:

left: 1920
top: 0
right: 3456
bottom: 864

if we convert that to an "Electron rect":

x: 1920
y: 0
width: (3456 - 1920) = 1536
height: (864 - 0) = 864

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
5.0.x
Unsorted Issues
10 participants
You can’t perform that action at this time.