Bug 816709 (metro-hidpi)

enable HiDPI for content and chrome in metrofx

RESOLVED FIXED in mozilla22

Status

()

RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: asa, Assigned: jfkthame)

Tracking

(Blocks: 1 bug)

Trunk
mozilla22
x86_64
Windows 8.1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

6 years ago
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.
(Reporter)

Updated

6 years ago
Blocks: 816814
No longer blocks: 816814
Depends on: 816814
Blocks: 816814
No longer depends on: 816814
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.

Updated

6 years ago
Whiteboard: [LOE:?]
(Assignee)

Comment 2

6 years ago
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?
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.)
(Reporter)

Comment 4

6 years ago
(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.

Updated

6 years ago
Whiteboard: [LOE:?] → [metro-mvp?][LOE:?]

Updated

6 years ago
OS: Windows 8 → Windows 8 Metro
(Reporter)

Updated

6 years ago
Whiteboard: [metro-mvp?][LOE:?] → [metro-mvp][LOE:?]
Whiteboard: [metro-mvp][LOE:?] → [metro-mvp][LOE:3]

Updated

6 years ago
Depends on: 797828
(Reporter)

Updated

6 years ago
Depends on: 833192
(Reporter)

Updated

6 years ago
Assignee: nobody → asa
No longer depends on: 833192
Summary: support HiDPI for content and chrome in Windows 8 Metro → Epic - HiDPI for content and chrome
Whiteboard: [metro-mvp][LOE:3] → feature=epic
(Reporter)

Updated

6 years ago
Depends on: 833195
(Reporter)

Updated

6 years ago
Depends on: 833192
(Reporter)

Updated

6 years ago
No longer blocks: 816814

Comment 5

6 years ago
(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%.

Updated

6 years ago
No longer depends on: 797828
(Assignee)

Comment 6

6 years ago
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

6 years ago
(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%.
(Assignee)

Comment 8

6 years ago
Created attachment 719013 [details] [diff] [review]
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.
(Assignee)

Comment 9

6 years ago
(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.

Updated

6 years ago
Blocks: 833192
No longer depends on: 833192, 833195
Summary: Epic - HiDPI for content and chrome → enable HiDPI for content and chrome in metrofx
Whiteboard: feature=epic
Version: unspecified → Trunk

Updated

6 years ago
Duplicate of this bug: 844853

Updated

6 years ago
Assignee: asa → nobody
(Reporter)

Comment 11

6 years ago
(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

6 years ago
(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.

Updated

6 years ago
Alias: metro-hidpi

Updated

6 years ago
Duplicate of this bug: 797828

Comment 14

6 years ago
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

Updated

6 years ago
Assignee: nobody → jmathies

Comment 15

6 years ago
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

6 years ago
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.

Comment 17

6 years ago
(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");
(Assignee)

Comment 18

6 years ago
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/jkew@mozilla.com-f1845d6c20a7/try-win32/

Comment 19

6 years ago
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.
Assignee: jmathies → jfkthame

Comment 20

6 years ago
Created attachment 719444 [details]
try build

This appears to display about the same.
(Reporter)

Comment 21

6 years ago
(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/jkew@mozilla.com-
> f1845d6c20a7/try-win32/

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

Comment 22

6 years ago
Created attachment 723238 [details] [diff] [review]
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.
(Assignee)

Updated

6 years ago
Attachment #719013 - Attachment is obsolete: true
(Assignee)

Comment 23

6 years ago
Tryserver build with the patch from comment 22:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-9b7b6365009b/try-win32/

Asa, does this improve the rendering you see?
(Reporter)

Comment 24

6 years ago
(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/jkew@mozilla.com-
> 9b7b6365009b/try-win32/
> 
> Asa, does this improve the rendering you see?

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

Comment 25

6 years ago
Created attachment 724427 [details] [diff] [review]
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.
Attachment #724427 - Flags: review?(jmathies)
(Assignee)

Updated

6 years ago
Attachment #723238 - Attachment is obsolete: true
(Assignee)

Comment 26

6 years ago
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.
Depends on: 844604
(Assignee)

Updated

6 years ago
Component: Graphics: Text → Widget: WinRT

Comment 27

6 years ago
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.
Attachment #724427 - Flags: review?(jmathies) → review+
(Assignee)

Updated

6 years ago
Blocks: 850692
(Assignee)

Comment 28

6 years ago
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.
Target Milestone: --- → mozilla22
(Assignee)

Updated

6 years ago
Blocks: 851197
(Assignee)

Comment 29

6 years ago
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.
https://hg.mozilla.org/mozilla-central/rev/81b7759d33e9
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Updated

6 years ago
Depends on: 854203
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.