I moved GetDPIScale from gfxWindowsPlatform because all callers are under widget/windows.

Comment on attachment 771752 [details] [diff] [review] patch Review of attachment 771752 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for tackling this! I haven't tested it myself, as I don't have a Win8.1 preview system set up, but it looks good overall. Before we go ahead and land this, I'd like to at least know what approach we're going to take to the "FIXME" item in nsScreenManagerWin, even if we leave the actual fix for a followup. ::: widget/windows/nsScreenManagerWin.cpp @@ +88,5 @@ > // convert coordinates from logical to device pixels for MonitorFromRect > double dpiScale = nsIWidget::DefaultScaleOverride(); > if (dpiScale <= 0.0) { > + //FIXME: need an appropriate monitor > + dpiScale = WinUtils::GetDPIScale(GetPrimaryMonitor()); I think we need to handle this somehow, otherwise there's probably a risk that in certain edge cases we might restore a window onto the wrong monitor, for example. How does Windows handle the coordinate space across multiple monitors with varying DPI scale? For global window placement, we really want to be using a coordinate system that spans all the screens in a meaningful and unambiguous way. On OS X, that's the global "Cocoa pixels" coordinate space (whereas "device pixel" coordinates are only valid within a single device), but I wonder whether Windows has an equivalent concept. Or does it treat device pixels as the primary, continuous space, and then "logical DPI" is just a scale factor that applications apply on a per-screen basis? In that case, the "logical pixels" space may have discontinuities at the screen boundaries. ::: widget/windows/nsWindowBase.h @@ +64,5 @@ > { > return (mInputContext.mIMEState.mEnabled == IMEState::PLUGIN); > } > > + void ChangedDPI(); I don't see a definition of this method; is there a new nsWindowBase.cpp file that you forgot to "hg add" to the patch?

(In reply to Jonathan Kew (:jfkthame) from comment #3) > ::: widget/windows/nsWindowBase.h > @@ +64,5 @@ > > { > > return (mInputContext.mIMEState.mEnabled == IMEState::PLUGIN); > > } > > > > + void ChangedDPI(); > > I don't see a definition of this method; is there a new nsWindowBase.cpp > file that you forgot to "hg add" to the patch? Oh, never mind - I see it's in nsWindow.cpp. Maybe it'd be good to move it to the top of the file rather than in the midst of the nsWindow methods?

(In reply to Jonathan Kew (:jfkthame) from comment #3) > Comment on attachment 771752 [details] [diff] [review] > patch > > Review of attachment 771752 [details] [diff] [review]: > ----------------------------------------------------------------- > > Thanks for tackling this! I haven't tested it myself, as I don't have a > Win8.1 preview system set up, but it looks good overall. > > Before we go ahead and land this, I'd like to at least know what approach > we're going to take to the "FIXME" item in nsScreenManagerWin, even if we > leave the actual fix for a followup. > > ::: widget/windows/nsScreenManagerWin.cpp > @@ +88,5 @@ > > // convert coordinates from logical to device pixels for MonitorFromRect > > double dpiScale = nsIWidget::DefaultScaleOverride(); > > if (dpiScale <= 0.0) { > > + //FIXME: need an appropriate monitor > > + dpiScale = WinUtils::GetDPIScale(GetPrimaryMonitor()); > > I think we need to handle this somehow, otherwise there's probably a risk > that in certain edge cases we might restore a window onto the wrong monitor, > for example. > > How does Windows handle the coordinate space across multiple monitors with > varying DPI scale? For global window placement, we really want to be using a > coordinate system that spans all the screens in a meaningful and unambiguous > way. On OS X, that's the global "Cocoa pixels" coordinate space (whereas > "device pixel" coordinates are only valid within a single device), but I > wonder whether Windows has an equivalent concept. On Windows 8.1, global screen coordinates depend on the DPI awareness of the app. For example, consider the following monitor settings: Monitor 0 (primary): (0,0)-(2560,1440), 192 dpi Monitor 1 : (2560,0)-(3584,768), 96 dpi Per-monitor DPI aware apps will see the above coordinates as-is. For DPI unaware apps, all coordinates will be scaled to 96 dpi: Monitor 0 (primary): (0,0)-(1280,720), 96 dpi Monitor 1 : (2560,0)-(3584,768), 96 dpi For system DPI aware apps, all coordinates will be scaled to the system dpi: Monitor 0 (primary): (0,0)-(2560,1440), 192 dpi Monitor 1 : (5120,0)-(7158,1536), 192 dpi > Or does it treat device > pixels as the primary, continuous space, and then "logical DPI" is just a > scale factor that applications apply on a per-screen basis? In that case, > the "logical pixels" space may have discontinuities at the screen boundaries. Yes, logical coordinates may be discontinuities as shown in the above example.

(In reply to Masatoshi Kimura [:emk] from comment #5) > On Windows 8.1, global screen coordinates depend on the DPI awareness of the > app. > For example, consider the following monitor settings: > Monitor 0 (primary): (0,0)-(2560,1440), 192 dpi > Monitor 1 : (2560,0)-(3584,768), 96 dpi > Per-monitor DPI aware apps will see the above coordinates as-is. This doesn't sit right with me. Shouldn't inactive monitors be virtualized to the DPI of the active monitor? (Screen coordinates would depend on the current DPI of the window.) That's what the word document seemed to be saying. Too bad I don't have Windows 8.1 to test.

(In reply to Greg Edwards from comment #6) > This doesn't sit right with me. Shouldn't inactive monitors be virtualized > to the DPI of the active monitor? What is "active monitor"? The word doesn't appear in the whitepaper. > (Screen coordinates would depend on the > current DPI of the window.) The window is used to identify the DPI-awareness of the owner app. > Too bad I don't have Windows 8.1 to test. I actually tested the multimonitor configuration and wrote the result here.

I'm not doubting you, it just seems like a strange design choice by Microsoft. The formula to convert to logical coordinates would be weird and depend on the monitor arrangement.

- Use a suggested size on WM_DPICHANGE. - Implement nsScreenManagerWin::ScreenForRect using logical coordinates. - Use GetDpiForMonitor only when the process is per-monitor DPI aware because GetDpiForMonitor is not virtualized regardless of the DPI awareness. Opened windows are not positioned correctly yet on secondary monitors.

Minor question -- does "True/PM" in the manifest work properly on older windows versions? i.e. 7 etc where we just want it to behave as if it was "true"?

Yes. Microsoft carefully chose the string so that it would work on down-level systems.

If the developers don't care about older systems, they can use the string "Per Monitor".

Unassigning because I couldn't sort out the comment #5 issue. If a high-dpi monitor is on the right side of a low-dpi monitor, the global logical display pixel will overlap on Windows 8.1 per-monitor dpi environments. This will lead a wrong choice of the screen for pop-ups. This is the opposite of bug 814434 on Mac OS X. For example, consider the following configuration: Primary monitor: (0,0)-(1024,768), 96 dpi Secondary monitor: (1024,0)-(3072,1536), 192 dpi The global display coordinates of the second monitor would be (512,0)-(1536,768). So ScreenForRect() is unimplementable on Windows 8.1 per-monitor dpi environments. To resolve this issue, either 1. Use physical device pixels for Move() and Resize() on Windows, or 2. Make logical-to-physical conversion more complex than the simple multiplication by the scale factor. I tried both without success and I gave up.

(In reply to Masatoshi Kimura [:emk] from comment #15) > Unassigning because I couldn't sort out the comment #5 issue. I tested Firefox 34.0.5 with the <dpiAware>True/PM</dpiAware> setting [1] and the fonts are much more readable. Is it possible to make this default? [1] http://msdn.microsoft.com/en-us/library/windows/desktop/dn469266(v=vs.85).aspx#declaring_dpi_awareness [2] http://technet.microsoft.com/en-us/library/dn528847.aspx

(In reply to mm.bugzilla from comment #19) > I tested Firefox 34.0.5 with the <dpiAware>True/PM</dpiAware> setting [1] > and the fonts are much more readable. > Is it possible to make this default? This is not as easy as just adding the manifest. 1. If two monitors have different DPI, Firefox will not change the size when the window is moved across the monitors. 2. DPI can be changed dynamically on Win8.1. Again, Firefox will not respond the DPI change until the next sign on. We will have to add a message handler for WM_DPICHANGE. But I'm stuck on the technical problem written in comment #5.

(In reply to Masatoshi Kimura [:emk] from comment #22) > This is not as easy as just adding the manifest. > 1. If two monitors have different DPI, Firefox will not change the size when > the window is moved across the monitors. Please look at the screenshots I attached. I'm using two displays, fist 1920x1080 with 12.5" and second 2560x1440 with 27". The left instance of Firefox has the patched manifest. So somehow the DPI gets adjusted right in firefox or am I missing something? > 2. DPI can be changed dynamically on Win8.1. Again, Firefox will not respond > the DPI change until the next sign on. I prefer a one time relogin to a permanent display error. > We will have to add a message handler for WM_DPICHANGE. But I'm stuck on the > technical problem written in comment #5. My uneducated guess are the LogicalToPhysicalPointForPerMonitorDPI and PhysicalToLogicalPointForPerMonitor functions, with them you may be able to get the "right" coordinates.

(In reply to mm.bugzilla from comment #23) > Please look at the screenshots I attached. I'm using two displays, fist > 1920x1080 with 12.5" and second 2560x1440 with 27". The left instance of > Firefox has the patched manifest. > So somehow the DPI gets adjusted right in firefox or am I missing something? I need to know the DPI value for each your monitor to know what is the expected behavior, but I'm assuming your first monitor has the higher DPI than the second. Firefox should scale down the window in this case when you dragged the window to the second monitor. That is, the screenshot shows exactly the lacking feature. > > 2. DPI can be changed dynamically on Win8.1. Again, Firefox will not respond > > the DPI change until the next sign on. > I prefer a one time relogin to a permanent display error. Display error will be fixed on relogin anyway. > > We will have to add a message handler for WM_DPICHANGE. But I'm stuck on the > > technical problem written in comment #5. > > My uneducated guess are the LogicalToPhysicalPointForPerMonitorDPI and > PhysicalToLogicalPointForPerMonitor functions, with them you may be able to > get the "right" coordinates. The problem is an architectural difference between OS X and Windows. On OS X. Screen coordinates are managed using "virtual" coordinates adjusted to the same DPI on all display. On Windows, it is managed by the physical display pixels. Since High-DPI support for Firefox was introduced on OS X first, Firefox assumes the virtual screen coordinates model. It is really hard (if at all possible) to accommodate the model on Windows.

(In reply to Masatoshi Kimura [:emk] from comment #24) > I need to know the DPI value for each your monitor to know what is the > expected behavior [...] Display: physical dpi (windows dpi [1]) First display: 176dpi (120) Second display: 108dpi (96) [1] GetDpiForMonitor --> https://emoacht.wordpress.com/2013/10/

attachment8438052 [details] [diff] [review] rebase

Hi all I'm having an issue. If I create a single window the size of all monitors and position it at top left most point there are no issues: http://i.imgur.com/qlWmwZo.png But if I create a window per monitor the non-primary monitor is bigger then it should be: http://i.imgur.com/48fHFxP.png Does anyone know why? Heres more info my dilemma: http://stackoverflow.com/questions/31499545/understanding-multi-monitor-dpi/31500103#31500103 http://imgur.com/qlWmwZo,48fHFxP

Actually the why is the non-dpi awareness as explored in that stackoverflow topic, is there a chance that we can land a fix to this bug soon? :)

I'm working around the dilemma by two methods: 1) prevent rescale when moved to another monitor by making the window i open on other monitors resized so its big enough that it reaches and covers at least 50% of primary monitor - i dont know why but this makes it prevent rescaling 2) im figuring out the scale factor then drawing things that much smaller/bigger: http://stackoverflow.com/questions/31508754/in-win81-non-dpi-aware-process-figure-out-scale-factor?noredirect=1#comment50988819_31508754

We need this fixed. https://www.reddit.com/r/firefox/comments/3fk0xw/is_firefox_multimonitor_dpiaware/

It sounds like we need to define our own DPI-independent screen coordinate space for Windows, and have a complex mapping between that and Windows' internal screen coordinates. I guess it's a bit tricky but not too hard. Is it hard for some reason I can't see?

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #31) > It sounds like we need to define our own DPI-independent screen coordinate > space for Windows, and have a complex mapping between that and Windows' > internal screen coordinates. I guess it's a bit tricky but not too hard. Is > it hard for some reason I can't see? There are two patches provided above, they do change the approach and implement runtime change of scaling per window. I've rebased the main patch on top of a more recent version in comment 26. I did find a couple issues though, e.g. context menus aren't sized correctly.

dolske, trying to get this on the fx team's radar. If the front end team considers this bug a priority the platform team might be able to rearrange some schedules to find some developer time. At this point though no one has the free cycles.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #31) > It sounds like we need to define our own DPI-independent screen coordinate > space for Windows, and have a complex mapping between that and Windows' > internal screen coordinates. I guess it's a bit tricky but not too hard. Is > it hard for some reason I can't see? It was hard mainly because of my (lack of) ability. I'd appreciate it if you take this bug.

(In reply to Jim Mathies [:jimm] from comment #33) > dolske, trying to get this on the fx team's radar. If the front end team > considers this bug a priority the platform team might be able to rearrange > some schedules to find some developer time. At this point though no one has > the free cycles. I don't have any specific data to help guide prioritization (like how many users have multiple-display setups with differing DPIs), but I do think this is valuable. We improved a lot of hidpi support in Firefox 40 (by adding hidpi assets for the Windows theme), and when users encounters situations where the browser is being scaled it sure looks bad. This is basically a Windows 10 issue, no? AIUI Microsoft didn't support per-monitor DPI settings until Windows 10, previously one could only specify a single scaling factor to be used on all displays.

Per-monitor DPI was introduced in Windows 8.1.

I've been trying to make some headway here, and am currently leaning towards the opinion that on per-monitor-dpi Windows systems, we need to use the "native" device pixels as the coordinate space in which we manage window positions. There's a try build in progress at https://treeherder.mozilla.org/#/jobs?repo=try&revision=84a132f3e7d7 that includes some (hacky) patches to make this work for menus/popups, and this seems to be a step in the right direction. It still has problems with window positioning (e.g. when restoring saved window positions or putting up an alert on a secondary display) as much of that code hasn't yet been touched. Hope to have a more complete patch available for testing fairly soon...

Here's my current WIP on this bug.... it's not nearly review-ready yet, but shows some progress at least.

:kip, I understand you have a multi-monitor/mixed-DPI configuration; would you be willing to try including the WIP patch above in your build, and see how things go? A couple of issues that I know are still broken: - When dragging a window between monitors with different DPI, the menu bar is mis-rendered (we leave too much or too little height for it). You can hide and re-show the menu bar to fix this, or maximize/restore the window. - When a window is opened on a secondary display (either restoring a session, or opening an alert, etc) the positioning is often wrong. Things that *should* work OK include menus (both the menubar and in-content), as well as fullscreen mode (F11) on both primary and secondary displays. And content should be rendered at the native resolution, which is particularly noticeable in the form of crisper text.

Yes, my home machine has this setup -- I'd be glad to try this out tonight.

I just wanted to share my finds when working with multi monitor before landing this patch. I found that if you move the window to the other monitor it gets magnified, but if you drag the corner of that window to resize it (dont move it) so that a majority of that window is covering the primary monitor, then it will not be magnified. I thought that was interesting. For right now though I put my window to the other window and I drew on a canvas and scaled it. Did this in my addon NativeShot: https://addons.mozilla.org/en-US/firefox/addon/nativeshot/ Just found it interesting, would be nice to take that work around out :)

I have a mixed-DPI Windows machine as my main working platform. I'm glad to test this as well. ni? myself so that I remember to test once I have time.

I did some test on Windows 10, and what I found the following issues: 1. The titlebar in hidpi monitor is smaller than normal (for e.g. Library, Browser Console, or the browser window with "browser.tabs.drawInTitlebar" set to false). 2. Scrollbar are narrow than normal in hidpi monitor. 3. The scroll wheel is only effective when the pointer is in some areas (it is an existing issue even when we do not use per-monitor DPI). 4. On hidpi monitor, the fullscreen transition animation failed to cover the whole screen (which is probably something I should figure out why). 5. When a window is opened on the boundary of two different dpi monitors, the window may be larger than normal. To reproduce this issue, open "View Certificates" in Options / Advanced / Certificates, and drag the window to the left of some boundary, then choose any certificate and click "View". The certificate window opened across the boundary, which is larger than it should be. 6. There's a chance that drag window across the dpi boundary back and forth could incorrectly make the window larger. (This issue is easy to reproduce for me on Connection Settings window opened from Options / Advanced / Network.) 7. The checkboxs and radioboxs in the Connection Settings window are smaller than normal on hidpi monitor. 8. The About window is broken (larger than it should be) on hidpi monitor. That's what I found by now. Thanks for working on this, jfkthame!

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #43) > I did some test on Windows 10, and what I found the following issues: Thanks for the notes. A couple comments: > 1. The titlebar in hidpi monitor is smaller than normal (for e.g. Library, > Browser Console, or the browser window with "browser.tabs.drawInTitlebar" > set to false). AFAICT, I think this is expected behavior (though I don't like it): according to [1], "Note that the non-client area of a per monitor–DPI aware application is not scaled by Windows, and will appear proportionately smaller on a high DPI display." I see a similarly small titlebar (and matching small menu when I right-click it) with an Internet Explorer window on the secondary, high-dpi display. Not sure if there's anything we can do to change this at present... > 2. Scrollbar are narrow than normal in hidpi monitor. Again, I think this may be expected.... I see the same size of scrollbars in IE11. > 3. The scroll wheel is only effective when the pointer is in some areas (it > is an existing issue even when we do not use per-monitor DPI). I'm not seeing this at the moment with my setup. How are your monitors arranged, and which is the primary screen? This is probably worth a separate bug, anyhow, if it's an existing issue. > 4. On hidpi monitor, the fullscreen transition animation failed to cover the > whole screen (which is probably something I should figure out why). Again, this seems to WFM at present, but may depend on the arrangement of the monitors. [1] https://msdn.microsoft.com/en-us/library/windows/desktop/dn469266%28v=vs.85%29.aspx

(In reply to Jonathan Kew (:jfkthame) from comment #44) > (In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #43) > > I did some test on Windows 10, and what I found the following issues: > > Thanks for the notes. A couple comments: > > > 1. The titlebar in hidpi monitor is smaller than normal (for e.g. Library, > > Browser Console, or the browser window with "browser.tabs.drawInTitlebar" > > set to false). > > AFAICT, I think this is expected behavior (though I don't like it): > according to [1], "Note that the non-client area of a per monitor–DPI aware > application is not scaled by Windows, and will appear proportionately > smaller on a high DPI display." > > I see a similarly small titlebar (and matching small menu when I right-click > it) with an Internet Explorer window on the secondary, high-dpi display. > > Not sure if there's anything we can do to change this at present... > > > 2. Scrollbar are narrow than normal in hidpi monitor. > > Again, I think this may be expected.... I see the same size of scrollbars in > IE11. Interesting. It is not what I see on some native applications on Windows 10 at least. It seems to me the calcualtor has the normal titlebar size on both monitors, and cmd has both normal titlebar size and normal scrollbar. But yes, I see the scrollbars in Edge become smaller. Microsoft themselves must haven't been completely clear with how to deal with per-monitor DPI :) > > 3. The scroll wheel is only effective when the pointer is in some areas (it > > is an existing issue even when we do not use per-monitor DPI). > > I'm not seeing this at the moment with my setup. How are your monitors > arranged, and which is the primary screen? My primary screen is not a hidpi screen, this is probably related. I'll upload an attachment for my arrangement of monitors. > This is probably worth a separate bug, anyhow, if it's an existing issue. Probably yes. > > 4. On hidpi monitor, the fullscreen transition animation failed to cover the > > whole screen (which is probably something I should figure out why). > > Again, this seems to WFM at present, but may depend on the arrangement of > the monitors.

See the attachment. All my tests are currently based on this setting. Monitor 1 is a HiDPI monitor, while monitor 2 and 3 are not. Monitor 2 is my primary monitor. Resolutions are: 1: 2880x1620, 2: 1920x1200, 3: 1200x1920

> AFAICT, I think this is expected behavior (though I don't like it): according to [1], "Note that the non-client area of a per monitor–DPI aware application is not scaled by Windows, and will appear proportionately smaller on a high DPI display." Note that this will make the scrollbars/titlebar huge on low-dpi displays when your primary display is hi-dpi.

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #45) > It seems to me the calcualtor has the normal titlebar size on both monitors, > and cmd has both normal titlebar size and normal scrollbar. I don't know about the calculator, but cmd is not per-monitor DPI aware. > > > 3. The scroll wheel is only effective when the pointer is in some areas (it > > > is an existing issue even when we do not use per-monitor DPI). AFAIK it is by design on Windows 10. Windows 10 will send a wheel message to the window which the mouse cursor points, not to the active window.

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #45) > Interesting. It is not what I see on some native applications on Windows 10 > at least. > > It seems to me the calcualtor has the normal titlebar size on both monitors, > and cmd has both normal titlebar size and normal scrollbar. AFAICS on Windows 8.1, at least (I haven't done the update to Win10 yet), common Windows applications such as Calculator have not been made per-monitor-dpi-aware. If you use Magnifier to look closely at Calculator on a secondary, high-dpi display (with the primary display being lower dpi), you can see that it is rendering low-dpi text and then being bitmap-scaled; compare the text within the window (lo-dpi) with the application name in the title bar (rendered at high dpi). It's much blockier. There's a shortage of true per-monitor-dpi applications to compare with at present....

(In reply to LordJZ from comment #47) > > AFAICT, I think this is expected behavior (though I don't like it): according to [1], "Note that the non-client area of a per monitor–DPI aware application is not scaled by Windows, and will appear proportionately smaller on a high DPI display." > > Note that this will make the scrollbars/titlebar huge on low-dpi displays > when your primary display is hi-dpi. Right; this is also what I see with IE11.

On Windows 10, it seems to me most builtin apps have been per-monitor DPI aware.

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #51) > On Windows 10, it seems to me most builtin apps have been per-monitor DPI > aware. Note that on Windows 10 some built-in apps like Calculator are not being re-scaled correctly when transitioned to a different DPI monitor. I have a small app that does position recalculation mostly correctly: (mouse pointer is a little bit off sometimes) https://github.com/LordJZ/WindowPositions/releases/tag/v1.1 It also has issues with scrollbars (not titlebar as it draws it's own titlebar).

(In reply to LordJZ from comment #52) > (In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #51) > > On Windows 10, it seems to me most builtin apps have been per-monitor DPI > > aware. > > Note that on Windows 10 some built-in apps like Calculator are not being > re-scaled correctly when transitioned to a different DPI monitor. It works... sometimes... Yes, I can confirm that Calculator has the same issue 6 I mentioned in comment 43, and it is another "likely" buggy one. But Command Prompt works mostly fine at least.

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #53) > Command Prompt works mostly fine at least. It is not PM-DPI-aware.

(In reply to LordJZ from comment #54) > (In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #53) > > Command Prompt works mostly fine at least. > > It is not PM-DPI-aware. It is PM-DPI-aware. See the screenshots (since not really related to this bug, I use imgur instead of attachments): http://i.imgur.com/GysxVJ9.png (LoDPI) http://i.imgur.com/BlCCArn.png (HiDPI)

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #55) > (In reply to LordJZ from comment #54) > > (In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #53) > > > Command Prompt works mostly fine at least. > > > > It is not PM-DPI-aware. > > It is PM-DPI-aware. No it is not. System will automatically stretch non-per-monitor DPI aware apps. > See the screenshots (since not really related to this > bug, I use imgur instead of attachments): http://i.imgur.com/GysxVJ9.png > (LoDPI) http://i.imgur.com/BlCCArn.png (HiDPI) Beware that https://i.imgur.com/GysxVJ9.png is a bit blurry.

But that window was opened in the LoDPI monitor. If it is possible that we can always use HiDPI backend and let the system scale down to LoDPI, I guess we probably should take that approach instead of implementing the "true" PM-DPI support. I heard it is how OS X handles this.

OK, yes, you are right. The manifest of cmd indicates it is DPI-aware but not PM-DPI-aware. Then it's interesting how it could have the HiDPI setting even when my primary monitor is LoDPI.

The primary monitor is always Monitor 1 (that is, the High DPI monitor on your configuration). It is not relevant what monitor the window was opened in.

All other non-PM-DPI-aware applications (including Firefox) uses LoDPI on my machine whatever monitor the window was opened in.

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #60) > All other non-PM-DPI-aware applications (including Firefox) uses LoDPI on my > machine whatever monitor the window was opened in. How did you confirm? For Firefox, probably it is just Firefox does not have High-DPI assets. So Firefox stretches images and it looks blurry. For other apps, probably they are not even system-DPI aware.

(In reply to Masatoshi Kimura [:emk] from comment #61) > (In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #60) > > All other non-PM-DPI-aware applications (including Firefox) uses LoDPI on my > > machine whatever monitor the window was opened in. > > How did you confirm? For Firefox, probably it is just Firefox does not have > High-DPI assets. So Firefox stretches images and it looks blurry. For other > apps, probably they are not even system-DPI aware. All text in Firefox is blurry in my monitor 1... This is also true for GVIM, Paint.NET, etc... When I do not use my external monitors, they work perfectly with the HiDPI laptop screen. Also, it is easy to confirm whether an app is system-DPI-aware: you just need to open the exe file in notepad, and search for "dpiAware".

I've submitted a tryserver job with updated WIP patches for this: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e4c9b3b18a7c. :xidorn, :emk, :kip, others... if you could give that build a try and see what does/doesn't work, that would be helpful. AFAICT it should be quite a bit better than before, anyhow. There are still some issues with remembering the positioning of windows such as Library, etc., when re-opening them if the parent window has been moved between displays. And it still has a problem with the About Firefox window being somewhat too tall on a secondary hi-dpi display. (And the patches currently include some hacks that will break behavior on other platforms, so don't even bother trying that for the time being.)

Updated WIP... contains various ugly hacks, not ready for review.

WIP pt 2, updates to windows native-theme code.

Try build with an improved set of patches: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d83dd1115274 I'll update the patches here after some more cleanup, but in the meantime any testing of the build above would be appreciated.

(In reply to Xidorn Quan [:xidorn] (UTC+9) from comment #43) > I did some test on Windows 10, and what I found the following issues: > > 1. The titlebar in hidpi monitor is smaller than normal (for e.g. Library, > Browser Console, or the browser window with "browser.tabs.drawInTitlebar" > set to false). Not fixed unfortunately. Not very important anyway if it is hard to fix. > 2. Scrollbar are narrow than normal in hidpi monitor. This seems to be fixed. > 3. The scroll wheel is only effective when the pointer is in some areas (it > is an existing issue even when we do not use per-monitor DPI). Later I confirmed this was a bug of some software else I installed, not ours. > 4. On hidpi monitor, the fullscreen transition animation failed to cover the > whole screen (which is probably something I should figure out why). This is magically fixed. > 5. When a window is opened on the boundary of two different dpi monitors, > the window may be larger than normal. To reproduce this issue, open "View > Certificates" in Options / Advanced / Certificates, and drag the window to > the left of some boundary, then choose any certificate and click "View". The > certificate window opened across the boundary, which is larger than it > should be. Cannot reproduce anymore, but if that window is opened in my HiDPI display, the whole window is in its low dpi state. Dragging the window to a low dpi display and back makes everything works fine then. > 6. There's a chance that drag window across the dpi boundary back and forth > could incorrectly make the window larger. (This issue is easy to reproduce > for me on Connection Settings window opened from Options / Advanced / > Network.) Cannot reproduce anymore. > 7. The checkboxs and radioboxs in the Connection Settings window are smaller > than normal on hidpi monitor. The checkboxs and radioboxs are no longer small, but when I open this window in a low dpi display, and drag to the hidpi display, the checkboxs and radioboxs become blurred as if they are scaled. > 8. The About window is broken (larger than it should be) on hidpi monitor. The About window is no longer larger, but like the certificate view dialog, it is in its low dpi state even if it is opened in a hi-dpi display. Dragging to a low dpi display and back probably corrects its size, but the three links on the bottom are overflow and invisible then.

Still some (relatively minor) issues to try and fix up here, but I'm uploading my current set of WIP patches...

drive-by comment: It would be *really* nice if we could add a type for "desktop pixels" into layout/base/Units.h and use strongly typed units throughout this code rather than plain double/int values which are going to get confusing real quick.

I've posted my current patches here, and would appreciate any testing people can do. In theory, we should be able to land everything up to the manifest change without affecting existing behavior; it's only when we add per-monitor-awareness to the manifest that things happen. A bit of this patch set has been updated to work with a new DesktopPixel type, but there's scope to extend that further and replace some more of the untyped pixel/scaling code, I think. (With these patches, I'm still seeing a problem with the About window size, as well as some other minor issues, so still hope to improve things before we're ready to actually ship this.)

I'm going to attach an updated copy of my patches here (probably not all have changed since the last set, but a number of them have). These seem to work fairly well in my testing on Win8.1; however, there are some undesired side-effects on an OS X multi-monitor setup, so I need to investigate that further before we're ready to review stuff here.

Try build with latest patches: https://treeherder.mozilla.org/#/jobs?repo=try&revision=100e1a534ba7 In addition to the usual smattering of intermittents, I expect there will be a reftest failure (in indic-clusters-1.html) due to an unrelated patch in my local queue, so that can be ignored here. Once builds are ready, testing would be very welcome -- both on multimonitor/mixed-dpi Windows (8.1 or 10) systems, and on older Windows and other platforms to check for any accidental regressions there. (In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #67) > ...if that window is opened in my HiDPI display, > the whole window is in its low dpi state. Dragging the window to a low dpi > display and back makes everything works fine then. I suspect this might be related to running with e10s enabled; could you try disabling e10s and see if that affects the behavior? Thanks. > The About window is no longer larger, but like the certificate view dialog, > it is in its low dpi state even if it is opened in a hi-dpi display. Again, does disabling e10s affect this? These two issues... > 1. The titlebar in hidpi monitor is smaller than normal (for e.g. Library, > Browser Console, or the browser window with "browser.tabs.drawInTitlebar" > set to false). > The checkboxs and radioboxs are no longer small, but when I open this window > in a low dpi display, and drag to the hidpi display, the checkboxs and > radioboxs become blurred as if they are scaled. will not be fixed, because AFAICT they're not currently fixable through the available Windows APIs. I'm hoping to discuss this with some people at MS to explore possible ways forward. (If you make the hidpi monitor as your primary display, and log out/in to Windows, the behavior will be a bit different -- e.g. instead of the titlebar being small on hidpi, it'll be correct there but oversized on lodpi. Still the same underlying problem, though.)

Immediately see a major issue: all popups are mispositioned in my hidpi monitor, including dropdowns, tooltips, and context menus from both the toolbar and the content. It seems this is only a rendering issue, because it seems the widgets correctly get hovered in the position it should be (so different from where it is displayed.) And if the window is initially open in the hidpi monitor, the main menu (from the three bar button) keeps empty. This issue is there whether e10s is enabled or not. Another issue is that, if I put the devtools into a separate window, that window doesn't have titlebar. It is not affected by e10s as well. When moving the browser window from hidpi monitor to lowdpi monitor, the toolbar buttons may be collapsed to an right arrow even if they shouldn't (which means if I resize the window larger to expand them, and then back to the original size, the buttons keep being expanded instead of collapsing again.) It happens if the window is narrow enough but not too narrow. This isn't affected by e10s. Probably we dispatch resize event in a wrong time? About the resize event, it seems a little too many resize events are dispatched. There are consistently three resize events dispatched to the content for moving across boundary in both direction. During handling the first and second events, content would get the inner window size in the new size with the previous dpi (which means it is double-sized or half-sized for a short while), and the last one is correct. It is a bit better for e10s because the content window is not directly affected by the dpi change, but the content's inner window size is still affected by the incorrectly-sized UI components. (Also there is a three pixels difference for the outer window size in different monitors, which may be because of the titlebar size change.) (In reply to Jonathan Kew (:jfkthame) from comment #100) > (In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #67) > > > ...if that window is opened in my HiDPI display, > > the whole window is in its low dpi state. Dragging the window to a low dpi > > display and back makes everything works fine then. > > I suspect this might be related to running with e10s enabled; could you try > disabling e10s and see if that affects the behavior? Thanks. > > > The About window is no longer larger, but like the certificate view dialog, > > it is in its low dpi state even if it is opened in a hi-dpi display. > > Again, does disabling e10s affect this? I do not see these two issues with the latest build, with or without e10s enabled. But if I followed the steps to open the certificate details window, when I drag it from the hidpi monitor to the lowdpi monitor, it becomes double sized with the extra area black-filled (the content is in normal size). And if I move that window back to hidpi monitor, the window size keeps doubled, and the content refill the whole window (so it's now really double sized). It seems this happens for other dialogs as well. And it is not affected by e10s.

(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #101) > Immediately see a major issue: all popups are mispositioned in my hidpi > monitor, including dropdowns, tooltips, and context menus from both the > toolbar and the content. Ugh -- yes, what a mess. Sorry about the brokenness! I made a "trivial" last-minute change to fix an issue I was seeing on OS X, but failed to re-test it properly on Windows, and of course it broke.... :( Here's a build (in progress) that should be much better: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2fcd69e54f03 > Another issue is that, if I put the devtools into a separate window, that > window doesn't have titlebar. It is not affected by e10s as well. Huh, that's very strange. Yes, I can reproduce that (with latest build, too).

Using this: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2fcd69e54f03 on primary 4K 175%, secondary 1080p 100%, Windows 10 10240 1. Scaling goes all sorts of nuts if "Disable DPI scaling" Windows compatibility option is enabled 2. When full-sized (Windows+Up Arrow or double-click the title bar) on a secondary display, a few pixels are chopped off off the top of the window. 3. Can confirm multiple redraws when window is moved to other display -- it also sometimes re-scales back to the ratio of the monitor it's been dragged from, effectively not re-scaling at all. Thanks!

4. The <select> dropdown is slightly misplaced on hi-dpi display: http://www.w3schools.com/tags/tryit.asp?filename=tryhtml_select (few pixels to the left)

(In reply to LordJZ from comment #105) > 4. The <select> dropdown is slightly misplaced on hi-dpi display: > http://www.w3schools.com/tags/tryit.asp?filename=tryhtml_select (few pixels > to the left) if enabled, that might be e10s related.

* * * [mq]: setposition-fix

(In reply to LordJZ from comment #104) > Using this: > https://treeherder.mozilla.org/#/jobs?repo=try&revision=2fcd69e54f03 on > primary 4K 175%, secondary 1080p 100%, Windows 10 10240 > > 1. Scaling goes all sorts of nuts if "Disable DPI scaling" Windows > compatibility option is enabled Hmm. I have an idea how that can be fixed, in principle, but so far I've not succeeded in doing it without some unwelcome side effects under normal use. (Is there any significant use-case for that option, given a properly DPI-aware application? I'd be happy to just disable it altogether if we could.) > 2. When full-sized (Windows+Up Arrow or double-click the title bar) on a > secondary display, a few pixels are chopped off off the top of the window. That's if your primary display is hi-dpi and the secondary is lo-dpi, right? (If they're the other way around, you see a few pixels of extra border at the top.) This seems to be related to how Windows does not scale the non-client area of the window as one would expect (hence the tiny titlebars on a secondary hi-dpi screen, or extra-large ones on a secondary lo-dpi screen). I'd consider it a relatively minor cosmetic issue -- obviously we should try to fix/workaround it if we can, but it's not a blocker. > 3. Can confirm multiple redraws when window is moved to other display Yes.... this is more noticeable with e10s enabled, I think, though it can be a bit untidy even without. > also sometimes re-scales back to the ratio of the monitor it's been dragged > from, effectively not re-scaling at all. I can confirm that occasionally the content doesn't get scaled correctly -- it seems like we've lost a DPI-change event somewhere -- though I can't reproduce this consistently. When it does happen, simply dragging the window back and forth across the monitors again should cause it to refresh properly. (In reply to LordJZ from comment #105) > 4. The <select> dropdown is slightly misplaced on hi-dpi display: > http://www.w3schools.com/tags/tryit.asp?filename=tryhtml_select (few pixels > to the left) As jimm suggested in comment 106, I think this is an e10s issue (even without per-monitor DPI support, the dropdown tends to be slightly misplaced). (In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #101) > Another issue is that, if I put the devtools into a separate window, that > window doesn't have titlebar. It is not affected by e10s as well. I'm no longer seeing this. > When moving the browser window from hidpi monitor to lowdpi monitor, the > toolbar buttons may be collapsed to an right arrow even if they shouldn't > (which means if I resize the window larger to expand them, and then back to > the original size, the buttons keep being expanded instead of collapsing > again.) It happens if the window is narrow enough but not too narrow. This > isn't affected by e10s. Probably we dispatch resize event in a wrong time? Yes, I can reproduce this sometimes; I don't have a solution yet. > But if I followed the steps to open the certificate details window, when I > drag it from the hidpi monitor to the lowdpi monitor, it becomes double > sized with the extra area black-filled (the content is in normal size).... This should be fixed now. New try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cfaeddfb508e

Using this: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cfaeddfb508e and the previous version, I have a window on 4K 175% primary display, then disable this display in settings, and the window gets moved to 1080p 100% secondary display, and the scaling goes nuts, without obvious ways to fix it -- it stays broken until I enable the primary display again.

(In reply to LordJZ from comment #114) > Using this: > https://treeherder.mozilla.org/#/jobs?repo=try&revision=cfaeddfb508e and the > previous version, > I have a window on 4K 175% primary display, then disable this display in > settings, and the window gets moved to 1080p 100% secondary display, and the > scaling goes nuts, without obvious ways to fix it -- it stays broken until I > enable the primary display again. AFAICT, Windows (at least up through Win8.1, the latest version described at [1]) does not fully support dynamic changes like that: if you disable the primary display, I think you have (implicitly) changed the system DPI (because the formerly-secondary display is now the only, hence primary, display on the system); and this is one of the changes that requires a Log Off / Log On to be properly recognized. In some cases you might find that resizing the window will help refresh its content with proper scaling, but I suspect that various things (such as menus) will remain improperly scaled until you log off and on again, at which point the system and monitor DPI values will be consistent again. [1] https://msdn.microsoft.com/en-gb/library/windows/desktop/dn469266(v=vs.85).aspx

(In reply to Jonathan Kew (:jfkthame) from comment #115) > > AFAICT, Windows (at least up through Win8.1, the latest version described at > [1]) does not fully support dynamic changes like that: if you disable the > primary display, I think you have (implicitly) changed the system DPI > (because the formerly-secondary display is now the only, hence primary, > display on the system); and this is one of the changes that requires a Log > Off / Log On to be properly recognized. > Right, in such a scenario system DPI will change on log off or reboot. However, all apps (including DWM-scaled and PM-DPI-aware like Edge or File Explorer) handle this scenario correctly. My own PM-DPI-aware apps also handle this correctly, and I didn't do anything specific for display disconnect to work. So I think this might be a bug in Firefox implementation.

AFAICS there are two basic issues that make it currently broken. First, the window content doesn't get refreshed at the new scale. Manually resizing the window fixes this (at least for me; can you confirm?), so that actual webpage content looks OK; and once it's fixed, it stays fixed. The second problem is that various UI-related things are not scaled properly; in particular, all system font sizes (e.g. text in tab titles and URL bar, menus, dialogs, etc) are wrong. And this persists until you actually log out and back in again; in particular, quitting and restarting the browser (without logging out of Windows) is NOT sufficient to fix it. Does this match your observation?

Right. I also see very similar behavior if the "Disable DPI scaling" compatibility option is enabled. These things may be related.

This should improve the behavior of a maximized window on a secondary display, such that it fits properly at the top of the screen. Try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bd5824a494ed

Experimental patch to work better under the Disable Scaling for HiDPI compatibility setting; but this may give inferior results in some cases under default settings (in particular, alert windows may be mis-sized in some situations). Try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a99e3fe99d7

Most recent try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a2791f0d4176 There are still some rough edges here -- in particular, the issue with occasionally failing to resize content properly when moving window across displays -- but I'd like to start getting patches reviewed so we can land this in Nightly sometime soon (probably hold it back from release channels until it's had more testing), and work on the remaining issues in followups if necessary.

Comment on attachment 8692968 [details] [diff] [review] patch 0.0 - Declare a DesktopPixel type in Units.h, to be used for the coordinate system used by the host system to manage the desktop space Review of attachment 8692968 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the detailed comment!

Comment on attachment 8692969 [details] [diff] [review] patch 0.1 - Switch from 'display pixels' to 'desktop pixels' terminology in widget code Review of attachment 8692969 [details] [diff] [review]: ----------------------------------------------------------------- Seems like a straightforward substitution, LGTM.

Comment on attachment 8694908 [details] [diff] [review] patch 5 - Make Windows native-theme code handle per-monitor DPI scaling when necessary Review of attachment 8694908 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/windows/nsNativeThemeWin.cpp @@ +2000,5 @@ > HANDLE theme = GetTheme(aWidgetType); > + nsresult rv = NS_OK; > + if (!theme) { > + rv = ClassicGetWidgetBorder(aContext, aFrame, aWidgetType, aResult); > + goto SCALE_AND_RETURN; Please add a conversion function instead of using goto.

Comment on attachment 8695403 [details] [diff] [review] patch 9 - Adjustment to non-client margin at top of maximized window on secondary display Review of attachment 8695403 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/windows/nsWindow.cpp @@ +2280,5 @@ > + // Any modifications here need to be tested for both high- and low-dpi > + // secondary displays, and for windows both with and without the titlebar > + // and/or menubar displayed. > + double ourScale = WinUtils::LogToPhysFactor(mWnd); > + double sysScale = WinUtils::LogToPhysFactor(WinUtils::GetPrimaryMonitor()); Is the primary monitor dpi is always equal to the system dpi? (I don't think so.) We will need a WinUtil function to obtain the system dpi using ::GetDeviceCaps() even in per-monitor dpi mode.

Comment on attachment 8695933 [details] [diff] [review] patch 4 - Update widget/windows code for per-monitor DPI support Review of attachment 8695933 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/windows/nsWindow.cpp @@ +184,5 @@ > #endif > > +#if !defined(WM_DPICHANGED) > +#define WM_DPICHANGED 0x02E0 > +#endif IIRC Win 8.1 SDK is the requirement now.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #125) > Comment on attachment 8692969 [details] [diff] [review] > patch 0.1 - Switch from 'display pixels' to 'desktop pixels' terminology in > widget code > > Review of attachment 8692969 [details] [diff] [review]: > ----------------------------------------------------------------- > > Seems like a straightforward substitution, LGTM. A bunch of recent changes in widget code (e.g. bugs 1229237, 1228125, 1225007) have conflicted with this, such that I'll need to post a fresh patch here after I get home from Mozlando and can re-test across multiple platforms and screen configurations.

CC'ing nick as well so that he's aware of the changes happening here, and can maybe avoid bitrotting your patches further.

Comment on attachment 8695403 [details] [diff] [review] patch 9 - Adjustment to non-client margin at top of maximized window on secondary display Waiting for unbitrotted patches.

(In reply to Masatoshi Kimura [:emk] from comment #127) > Is the primary monitor dpi is always equal to the system dpi? (I don't think > so.) > We will need a WinUtil function to obtain the system dpi using > ::GetDeviceCaps() even in per-monitor dpi mode. I don't think we need that; AFAICT, getting the system dpi from GetDeviceCaps will always give the same result as getting the primary monitor's dpi. Note that as far as "system dpi" is concerned, MSDN says: "On a multiple monitor system, if hdc is the desktop, GetDeviceCaps returns the capabilities of the primary monitor." I'm assuming that using the desktop HDC would be equivalent to getting "system dpi"; it's not clear to me that any other concept of "system dpi" exists for per-monitor-aware apps.

Updated/unbitrotted patch, carrying over r=kats.

Updated/unbitrotted patch, carrying over r=kats.

Updated/unbitrotted patch, carrying over r=emk.

Updated/unbitrotted patch, carrying over r=emk.

Updated/unbitrotted patch, carrying over r=emk.

Updated/unbitrotted patch, carrying over r=emk.

Updated/unbitrotted patch, carrying over r=emk.

Updated/unbitrotted patch, carrying over r=emk.

Updated/unbitrotted patch, carrying over r=emk.

Updated/unbitrotted patch, carrying over r=emk.