934762 - Defect - Metro renders squished and mouse/touch events are offset in dual monitor setup when one is HighDPI

Status: RESOLVED FIXED

(Core Graveyard :: Widget: WinRT, defect, P2)

Description

[Rodrigo Silveira [:rsilveira] [:rodms]]

Attachment: HighDPIDualMonitor.png

STR:
1. Get a device with HighDPI screen (a surface PRO will do)
2. Connect an external monitor with regular DPI (I'm using a 1080p). 
3. Set the external monitor as the "main display" in screen resolution settings in windows.
4. Start metro in the HighDPI monitor.

Actual
5. Weep. Metro renders squished on the top left (see screenshot). Mouse or touching location does not correspond to what you see.

Expected
5. Smile. Metro renders beautifully and respects my clicks/touches.

Comment 1

[Rodrigo Silveira [:rsilveira] [:rodms]]

Attachment: HighDPIDualMonitorBug927172 .png

The fix in Bug 927172 makes it render in full screen but with smaller text/tiles - see screenshot. Touch/click is still off though.

That's because DisplayProperties[1] is returning a 96 dpi with 100% scaling in this case, instead of the 140% scale needed for the surface pro to render nicely. This affects our logical to physical factor calculations at [2] and scale at [3]. Now windows is being inconsistent and sending touch positions as if it was scaled at 140% and things don't match.

According to [1] we should be using DisplayInformation[4] for windows 8.1 and up since DisplayProperties is being deprecated. I'm not sure if we have the same problem in windows 8.0, I'm using 8.1. Hopefully this would fix it, at least for 8.1.

Any other suggestions?

[1] http://msdn.microsoft.com/en-us/library/windows/apps/windows.graphics.display.displayproperties.aspx
[2] https://mxr.mozilla.org/mozilla-central/source/widget/windows/winrt/MetroUtils.cpp#37
[3] https://mxr.mozilla.org/mozilla-central/source/widget/windows/winrt/MetroWidget.cpp#1284
[4] http://msdn.microsoft.com/en-us/library/windows/apps/windows.graphics.display.displayinformation.aspx

Comment 2

[Jim Mathies [:jimm]]

> [2]
> https://mxr.mozilla.org/mozilla-central/source/widget/windows/winrt/
> MetroUtils.cpp#37

I think we should get this hooked up.

I wonder if a widget->Invalidate call in the DisplayContentsInvalidated callback might help solve some of our display problems as well.

Comment 3

[Jim Mathies [:jimm]]

(In reply to Jim Mathies [:jimm] from comment #2)
> I think we should get this hooked up.
> 
> I wonder if a widget->Invalidate call in the DisplayContentsInvalidated
> callback might help solve some of our display problems as well.

re: DisplayInformation callbacks.

Comment 4

[Rodrigo Silveira [:rsilveira] [:rodms]]

Attachment: 934762.patch

Using DisplayInformation(DI) solved the issue, it renders larger and touch/mouse pointers are in sync. This patch replaces DisplayProperties(DP) with DI (see comment 1). The proper solution would try DI and fall back to DP to support both win8.0 and win8.1.

I was only able to get everything compiling fine when using Visual Studio 2013 because DI requires windows 8.1 SDK. I downloaded the SDK[1] but wasn't able to get start-msvc11 to point to it. Support for VS2013 is tracked by bug 914596. 

Before landing a fix for this bug, we need to get the 8.1SDK in our build machines which might also require VS2013. Assuming this can take a while, I think we should [release28] this.

[1] http://msdn.microsoft.com/en-US/windows/desktop/bg162891

Comment 5

[Jim Mathies [:jimm]]

(In reply to Rodrigo Silveira [:rsilveira] from comment #4)
> Created attachment 8334386 [details] [diff] [review]
> 934762.patch
> 
> Using DisplayInformation(DI) solved the issue, it renders larger and
> touch/mouse pointers are in sync. This patch replaces DisplayProperties(DP)
> with DI (see comment 1). The proper solution would try DI and fall back to
> DP to support both win8.0 and win8.1.

yep!

> Before landing a fix for this bug, we need to get the 8.1SDK in our build
> machines which might also require VS2013. Assuming this can take a while, I
> think we should [release28] this.
> 
> [1] http://msdn.microsoft.com/en-US/windows/desktop/bg162891

Hmm, updating our build tool chain isn't going to happen in such a small time window. Updating the sdk will also take some time.

What we usually do in these cases is define the interface manually in our own header with an ifdef around it to protect against errors when users build with the newer sdk. This way we can skip minor sdk releases. Doing this with win32 / com apis is pretty straight forward, lets take a look at doing the same for this winrt api.

Comment 6

[Rodrigo Silveira [:rsilveira] [:rodms]]

Attachment: 934762.patch

Copied over DisplayInformation related interfaces from SDK 8.1. Try DisplayInformation first and fallback to DisplayProperties.

I tested that it builds fine with VS2012 and SDK 8.0. I don't have any 8.0 machines to test it though. 

Pushed to try at https://tbpl.mozilla.org/?tree=Try&rev=7921ca1e8ed9

Comment 7

[Jim Mathies [:jimm]]

Comment on attachment 8335820 [details] [diff] [review]
934762.patch

Looks good!

Comment 8

[Jim Mathies [:jimm]]

(In reply to Rodrigo Silveira [:rsilveira] from comment #6)
> Created attachment 8335820 [details] [diff] [review]
> 934762.patch
> 
> Copied over DisplayInformation related interfaces from SDK 8.1. Try
> DisplayInformation first and fallback to DisplayProperties.
> 
> I tested that it builds fine with VS2012 and SDK 8.0. I don't have any 8.0
> machines to test it though. 
> 
> Pushed to try at https://tbpl.mozilla.org/?tree=Try&rev=7921ca1e8ed9

Did you test building with the 8.1 sdk to insure there weren't conflicts?

Comment 9

[Rodrigo Silveira [:rsilveira] [:rodms]]

Yes, tested on both 8.0 and 8.1. Try only had a known intermittent issue.

https://hg.mozilla.org/integration/fx-team/rev/3fd476d927e4

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3fd476d927e4