816709 - (metro-hidpi) enable HiDPI for content and chrome in metrofx

Status: RESOLVED FIXED

(Core Graveyard :: Widget: WinRT, defect)

Description

[Asa Dotzler [:asa]]

Windows 8 Metro automatically scales rendering based on pixel density of the device. See more info at the "Different pixel densities" section of http://blogs.msdn.com/b/b8/archive/2012/03/21/scaling-to-different-screens.aspx

We need to support the 140% and 180% scaling factors without our fonts getting all blurry. This feels similar to the Mac HiDPI with pixel doubling on retina problem we faced in bug 674373.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Hopefully the main thing we need to do here is just to disable

// Disable auto-configuration of devPixelsPerPx until we're ready to turn
// it on.
pref("layout.css.devPixelsPerPx", "1.0");

in all.js for Metro.

Comment 2

[Jonathan Kew [:jfkthame]]

Is it possible for a Win8Metro machine to be configured with multiple screens that have differing pixel densities and correspondingly different scaling factors? Or does it apply a single scaling factor uniformly across all displays, regardless of any differences between them?

Comment 3

[Matt Brubeck (:mbrubeck)]

It's m(In reply to Jonathan Kew (:jfkthame) from comment #2)
> Is it possible for a Win8Metro machine to be configured with multiple
> screens that have differing pixel densities and correspondingly different
> scaling factors?

It's my understanding that Metro apps can only display on the primary screen.  (Additional screens can only display "desktop" applications.)

Comment 4

[Asa Dotzler [:asa]]

(In reply to Matt Brubeck (:mbrubeck) from comment #3)
> It's m(In reply to Jonathan Kew (:jfkthame) from comment #2)
> > Is it possible for a Win8Metro machine to be configured with multiple
> > screens that have differing pixel densities and correspondingly different
> > scaling factors?
> 
> It's my understanding that Metro apps can only display on the primary
> screen.  (Additional screens can only display "desktop" applications.)

Metro apps can appear on either screen and can switch between screens.

Comment 5

[Josh Tumath]

(In reply to Asa Dotzler [:asa] from comment #0)
> We need to support the 140% and 180% scaling factors without our fonts
> getting all blurry. This feels similar to the Mac HiDPI with pixel doubling
> on retina problem we faced in bug 674373.

You probably know this already, but I thought I'd be worth pointing out that only the chrome needs to be scaled to 140% and 180%. In IE10, the browser content is scaled to 150% and 200%.

Comment 6

[Jonathan Kew [:jfkthame]]

Do we want to "force" a particular scaling, based on the screen dimensions and DPI, or do we want to make the Metro browser default to following the desktop environment's scale factor (as set in the Display control panel, "Make text and other items larger or smaller"?

I'm going to assume the latter for now, unless someone wants to argue that Desktop and Metro scaling should -not- be related.

Comment 7

[Josh Tumath]

(In reply to Jonathan Kew (:jfkthame) from comment #6)
> I'm going to assume the latter for now, unless someone wants to argue that
> Desktop and Metro scaling should -not- be related.

I have already explained in comment 5 that they are not related. The desktop uses 100%, 125%, 150% and 200% scaling by default, whereas Metro uses 100%, 140% and 180%.

Comment 8

[Jonathan Kew [:jfkthame]]

Attachment: patch, support GetDefaultScaleInternal in metro widgets, and enable auto dpi handling

It looks like we'd get a long way here simply by enabling auto-configuration of devPixelsPerPx, as per comment 1, and implementing the GetDefaultScaleInternal method on MetroWidget. This can be a simple copy of the implementation in widget/windows/nsWindow.cpp, assuming we want to respect the same scaling pref from the Display control panel.

With this patch, the Metro browser seems to scale pretty nicely on my Acer Aspire. Tryserver build in progress at https://tbpl.mozilla.org/?tree=Try&rev=5099bc0f0e93 for anyone who'd like to try it.

Note that this patch as currently written will -also- affect the desktop browser, as it sets devPixelsPerPx to -1.0 regardless of the environment. I think that's a change we need to make for the sake of hidpi desktop users anyway (see bug 844604), but it is possible that there will be side-effects we need to address.

Comment 9

[Jonathan Kew [:jfkthame]]

(In reply to Josh Tumath from comment #7)
> (In reply to Jonathan Kew (:jfkthame) from comment #6)
> > I'm going to assume the latter for now, unless someone wants to argue that
> > Desktop and Metro scaling should -not- be related.
> 
> I have already explained in comment 5 that they are not related. The desktop
> uses 100%, 125%, 150% and 200% scaling by default, whereas Metro uses 100%,
> 140% and 180%.

Comment 5 was not really clear to me (sorry if I'm just being dense)... it seems to be saying we might want to use different scale factors for chrome vs content (I say "might" because it's not clear to me that we'd need to slavishly follow IE10's in that regard), which is a different matter from the question of whether desktop and metro environments should use similar scaling.

On playing around a bit more, I see that while the patch above makes the browser (whether in desktop or metro) respect the user's scaling preference from the Display control panel, there's -also- an option to "Make everything on your screen bigger" in the Metro's "PC settings". If I use that option, the Firefox display is automatically scaled by Windows, and looks rather blurry.

Comment 11

[Asa Dotzler [:asa]]

(In reply to Jonathan Kew (:jfkthame) from comment #6)
> Do we want to "force" a particular scaling, based on the screen dimensions
> and DPI, or do we want to make the Metro browser default to following the
> desktop environment's scale factor (as set in the Display control panel,
> "Make text and other items larger or smaller"?

Yes, we want to force the scaling. We do not want to follow the Desktop settings. We want what happens to other Metro apps to happen to Firefox, which is that our content and chrome scale along with the rest of Metro and that's "forced" at  particular DPIs.

Please read the section titled *Different pixel densities* here http://blogs.msdn.com/b/b8/archive/2012/03/21/scaling-to-different-screens.aspx for how this is supposed to work. 

We're not looking for a more flexible solution, or one that gives users more fine grained control. We simply want Firefox to behave as the rest of the Metro apps behave here.

Comment 12

[Josh Tumath]

(In reply to Jonathan Kew (:jfkthame) from comment #9)
> On playing around a bit more, I see that while the patch above makes the
> browser (whether in desktop or metro) respect the user's scaling preference
> from the Display control panel, there's -also- an option to "Make everything
> on your screen bigger" in the Metro's "PC settings". If I use that option,
> the Firefox display is automatically scaled by Windows, and looks rather
> blurry.

Indeed. That's what I thought this patch would address. (Sorry, I'm only an undergraduate student. I don't know if your patch actually does address that. I'm just trying to help based on what I understand about the platform. :) )

Normally, you would read the WinRT API called Windows.Graphics.Display.DisplayProperties.resolutionScale, which shows you whether the user has turned on the "Make everything bigger" option. It would give a value of 100, 140 or 180 and you would use that to scale the chrome appropriately. It's important to use this because some users choose to have completely different Metro and desktop APIs. IE10 uses the WinRT API to scale the chrome and the desktop API setting to scale the Web page content.

Comment 14

[Jim Mathies [:jimm]]

Comment on attachment 719013 [details] [diff] [review]
patch, support GetDefaultScaleInternal in metro widgets, and enable auto dpi handling

Review of attachment 719013 [details] [diff] [review]:
-----------------------------------------------------------------

::: modules/libpref/src/init/all.js
@@ -2462,5 @@
>  pref("ui.elantech_gesture_hacks.enabled", -1);
>  
> -// Disable auto-configuration of devPixelsPerPx until we're ready to turn
> -// it on.
> -pref("layout.css.devPixelsPerPx", "1.0");

There are other bugs related to desktop, so lets make this a metro only change.

::: widget/windows/winrt/MetroWidget.cpp
@@ +948,5 @@
> +    return 1.0;
> +
> +  // LOGPIXELSY returns the number of logical pixels per inch. This is based
> +  // on font DPI settings rather than the actual screen DPI.
> +  double pixelsPerInch = ::GetDeviceCaps(dc, LOGPIXELSY);

Let's use the dpi setting winrt provides for us instead. 

http://mxr.mozilla.org/mozilla-central/source/widget/windows/winrt/FrameworkView.cpp#236

Comment 15

[Jim Mathies [:jimm]]

Hmm, we seem to be using both dpi and the default scale in graphics code - 

http://mxr.mozilla.org/mozilla-central/source/gfx/src/nsDeviceContext.cpp#323

I'm not sure what each of these values is supposed to return. It looks like they both want dpi, but setting that up doesn't work right.

Comment 16

[Jim Mathies [:jimm]]

Attachment: ds: 1.0, dpi: 134

This is with widget returning the dpi the system provides from GetDPI and DefaultScaleInternal return 1. The size of the text looks right but text looks a bit blurry.

Comment 17

[Jim Mathies [:jimm]]

(In reply to Jim Mathies [:jimm] from comment #16)
> Created attachment 719436 [details]
> ds: 1.0, dpi: 134
> 
> This is with widget returning the dpi the system provides from GetDPI and
> DefaultScaleInternal return 1. The size of the text looks right but text
> looks a bit blurry.

The prefs for this are:

pref("layout.css.devPixelsPerPx", "-1.0");
pref("layout.css.dpi", "0");

Comment 18

[Jonathan Kew [:jfkthame]]

Here's a tryserver build that attempts to get DefaultScaleFactor from Windows.Graphics.Display - I'd be curious how that compares? (Unfortunately, I'm currently having trouble getting my local debug build to run as a metro app, which is making it harder to investigate what's going on inside...)

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-f1845d6c20a7/try-win32/

Comment 19

[Jim Mathies [:jimm]]

https://hg.mozilla.org/try/rev/e9c1aca2eaeb
http://msdn.microsoft.com/en-us/library/windows/apps/windows.graphics.display.resolutionscale.aspx

That might work assuming our concept of scale matches with theirs.

Comment 20

[Jim Mathies [:jimm]]

Attachment: try build

This appears to display about the same.

Comment 21

[Asa Dotzler [:asa]]

(In reply to Jonathan Kew (:jfkthame) from comment #18)
> Here's a tryserver build that attempts to get DefaultScaleFactor from
> Windows.Graphics.Display - I'd be curious how that compares? (Unfortunately,
> I'm currently having trouble getting my local debug build to run as a metro
> app, which is making it harder to investigate what's going on inside...)
> 
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-
> f1845d6c20a7/try-win32/

I installed this try build and see no obvious improvement in content or chrome display. Things are still blurry.

Comment 22

[Jonathan Kew [:jfkthame]]

Attachment: hidpi support in metro

Here's a patch that comes a lot closer to supporting hidpi display in Metro. The approach taken here is similar to what we're doing on OS X, preferring to work with 'true' device-pixel coordinates within Gecko, and convert to the host system's 'logical' coordinate system only when interacting with platform APIs that expect this (e.g. for window sizing or touch and mouse events). This doesn't yet support dynamic changes properly (i.e. toggling the Metro 'Ease of Access' option to 'Make everything [...] bigger' while Firefox is running), but it does seem to display crisply at both 'normal' and 'bigger' scales, except that some bitmapped UI assets end up blurry due to scaling, of course.

Comment 23

[Jonathan Kew [:jfkthame]]

Tryserver build with the patch from comment 22:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-9b7b6365009b/try-win32/

Asa, does this improve the rendering you see?

Comment 24

[Asa Dotzler [:asa]]

(In reply to Jonathan Kew (:jfkthame) from comment #23)
> Tryserver build with the patch from comment 22:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-
> 9b7b6365009b/try-win32/
> 
> Asa, does this improve the rendering you see?

Yes. This is a huge improvement. r=asa ;-)

Comment 25

[Jonathan Kew [:jfkthame]]

Attachment: HiDPI support for Win8 Metro

Here's a cleaned-up patch that I think should be about ready to go. This provides scaling based on the ResolutionScale as reported by Graphics.Display. I'm aware of a couple of issues with this, but I think we should get this landed as a basis and have followup bugs for these: (a) dynamic changes (toggling Metro's 'Make everything bigger' setting) while Firefox is open are not handled properly; and (b) it appears that on my 13-inch hidpi screen, IE is -also- paying attention to the Desktop environment's scale setting, and does not simply follow the scale factors from Metro's ResolutionScale. We may want to do something similar. More details in a followup.

Comment 26

[Jonathan Kew [:jfkthame]]

BTW, note that this is dependent on the default-pref change that just landed in bug 844604. If we end up having to revert that for Win desktop, we'd need to force it to automatic at least for Metro in order for any of this to work.

Comment 27

[Jim Mathies [:jimm]]

Comment on attachment 724427 [details] [diff] [review]
HiDPI support for Win8 Metro

+    ComPtr<IDisplayPropertiesStatics> dispProps;
+    if (SUCCEEDED(GetActivationFactory(HStringReference(RuntimeClass_Windows_Graphics_Display_DisplayProperties).Get(),
+                                       dispProps.GetAddressOf()))) {

Seems like we should cache this. If you don't want to try that here please file a follow up in the widget winrt component. 

Welcome to the wonderful world of winrt com apis btw. :)

+  static int32_t LogToPhys(FLOAT aValue);
+  static nsIntPoint LogToPhys(const Point& aPt);
+  static nsIntRect LogToPhys(const Rect& aRect);
+  static FLOAT PhysToLog(int32_t aValue);
+  static Point PhysToLog(const nsIntPoint& aPt);

Please add a nice big header here explaining what they do.

Comment 28

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/81b7759d33e9

(In reply to Jim Mathies [:jimm] from comment #27)
> Seems like we should cache this. If you don't want to try that here please
> file a follow up in the widget winrt component. 

I filed bug 851029 for this - as we already have a couple other users of that interface, we should convert them all to use a single shared/cached pointer, so I thought it's better to deal with that separately.

Comment 29

[Jonathan Kew [:jfkthame]]

And following up on the issues mentioned in comment 25, filed bug 850692 about possible adjustments to the scaling factors we use, and bug 851197 about handling dynamic changes to the ResolutionScale property.

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/81b7759d33e9