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

RESOLVED FIXED in mozilla28

Status

defect
P2
normal
RESOLVED FIXED
6 years ago
3 months ago

People

(Reporter: rsilveira, Assigned: rsilveira)

Tracking

unspecified
mozilla28
x86_64
Windows 8.1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [block28] feature=defect c=tbd u=tbd p=5)

Attachments

(3 attachments, 1 obsolete attachment)

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.
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
Assignee: nobody → rsilveira
Status: NEW → ASSIGNED
Blocks: metrov1it18
Priority: -- → P2
QA Contact: jbecerra
Summary: Metro renders squished and mouse/touch events are offset in dual monitor setup when one is HighDPI → Defect - Metro renders squished and mouse/touch events are offset in dual monitor setup when one is HighDPI
Whiteboard: [block28] feature=defect c=tbd u=tbd p=5
Blocks: metrov1it19
No longer blocks: metrov1it18

Updated

6 years ago
Depends on: 937626
> [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.
(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.

Updated

6 years ago
Component: Pan and Zoom → Widget: WinRT
Product: Firefox for Metro → Core
Posted patch 934762.patch (obsolete) — Splinter Review
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
Attachment #8334386 - Flags: feedback?(jmathies)
(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.

Updated

6 years ago
Attachment #8334386 - Flags: feedback?(jmathies) → feedback+
Posted patch 934762.patchSplinter Review
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
Attachment #8334386 - Attachment is obsolete: true
Attachment #8335820 - Flags: review?(jmathies)
Comment on attachment 8335820 [details] [diff] [review]
934762.patch

Looks good!
Attachment #8335820 - Flags: review?(jmathies) → review+
(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?
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
https://hg.mozilla.org/mozilla-central/rev/3fd476d927e4
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28

Updated

3 months ago
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.