Closed Bug 1023546 Opened 9 years ago Closed 9 years ago

DevTools - Support HDPI resolutions for Windows

Categories

(DevTools :: Framework, defect)

defect
Not set
normal

Tracking

(firefox40 fixed)

RESOLVED FIXED
Firefox 40
Tracking Status
firefox40 --- fixed

People

(Reporter: ntim, Assigned: ntim)

References

Details

(Whiteboard: [polish-backlog][difficulty=easy])

Attachments

(1 file, 1 obsolete file)

With bug 837188, 2x resolution is pretty much done.

It'd be nice if 125%/150%/180% dpi settings were also supported on Windows.

Now there are 3 solutions possible :
(1) Use SVG assets everywhere
(2) Downscale 2x icons for those dpi settings
(3) Include assets of each of those dpi settings

Solution (2) sounds best to me, as it requires not much work.
OS: Windows 8.1 → All
Hardware: x86_64 → All
What would you go for to fix this bug ?
Flags: needinfo?(bgrinstead)
(In reply to Tim Nguyen [:ntim] from comment #0)
> With bug 837188, 2x resolution is pretty much done.
> 
> It'd be nice if 125%/150%/180% dpi settings were also supported on Windows.
> 
> Now there are 3 solutions possible :
> (1) Use SVG assets everywhere
> (2) Downscale 2x icons for those dpi settings
> (3) Include assets of each of those dpi settings
> 
> Solution (2) sounds best to me, as it requires not much work.

Does (2) require adding new images for each dpi setting or just modifying the existing 2x images?
Flags: needinfo?(bgrinstead)
What systems or scenarios exist where these dpi settings are common?
(In reply to Brian Grinstead [:bgrins] from comment #2)
> (In reply to Tim Nguyen [:ntim] from comment #0)
> > With bug 837188, 2x resolution is pretty much done.
> > 
> > It'd be nice if 125%/150%/180% dpi settings were also supported on Windows.
> > 
> > Now there are 3 solutions possible :
> > (1) Use SVG assets everywhere
> > (2) Downscale 2x icons for those dpi settings
> > (3) Include assets of each of those dpi settings
> > 
> > Solution (2) sounds best to me, as it requires not much work.
> 
> Does (2) require adding new images for each dpi setting or just modifying
> the existing 2x images?

(2) just requires updating CSS code.
(In reply to Jeff Griffiths (:canuckistani) from comment #3)
> What systems or scenarios exist where these dpi settings are common?

Windows has DPI settings from 125% to 200%, and currently, those dpi settings use upscaled 1x graphics.
(In reply to Tim Nguyen [:ntim] from comment #5)
> (In reply to Jeff Griffiths (:canuckistani) from comment #3)
> Windows has DPI settings from 125% to 200%, and currently, those dpi
> settings use upscaled 1x graphics.

Ah, got it. If this is just css changes eg really shallow, looks good. IMO the usage of these settings on Windows systems is probably not high enough to worry too much about. If we had data that told me significant numbers of web developers used our tools with these oddball dpi settings, I'd get more interested in custom assets.
Attached patch Patch (obsolete) — Splinter Review
Mass change from 2dppx to 1.25dppx. (Please also ask people in the future to use 1.25 instead of 2 too).

I only tested this on retina OSX, and it doesn't regress anything. But I'll need to test this on Windows tomorrow to see if it really works.
Assignee: nobody → ntim.bugs
Status: NEW → ASSIGNED
Attachment #8598285 - Flags: review?(bgrinstead)
(In reply to Tim Nguyen [:ntim] from comment #7)
> Created attachment 8598285 [details] [diff] [review]
> Patch
> 
> Mass change from 2dppx to 1.25dppx.
This is the approach taken on the Firefox Desktop side FYI.
Whiteboard: [devedition-40][difficulty=easy]
(In reply to Tim Nguyen [:ntim] from comment #8)
> This is the approach taken on the Firefox Desktop side FYI.

A quick search seems to indicate there is still more 2dppx usage: https://dxr.mozilla.org/mozilla-central/search?q=2dppx&redirect=true vs https://dxr.mozilla.org/mozilla-central/search?q=1.25dppx&redirect=true.

I'm not necessarily against this, but I'm going to ask around to see if someone else who knows about this can advise.
(In reply to Tim Nguyen [:ntim] from comment #10)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=6d7ac80a379a
This try push is just for faster testing (which is why there are not tests being ran).

(In reply to Brian Grinstead [:bgrins] from comment #9)
> (In reply to Tim Nguyen [:ntim] from comment #8)
> > This is the approach taken on the Firefox Desktop side FYI.
> 
> A quick search seems to indicate there is still more 2dppx usage:
> https://dxr.mozilla.org/mozilla-central/search?q=2dppx&redirect=true vs
> https://dxr.mozilla.org/mozilla-central/search?q=1.25dppx&redirect=true.

If you remove the devtools results and the OSX files, the difference is slight. Also, newer code being written is using 1.25dppx : see attachment 8598235 [details] [diff] [review] for example
(In reply to Brian Grinstead [:bgrins] from comment #9)
> (In reply to Tim Nguyen [:ntim] from comment #8)
> > This is the approach taken on the Firefox Desktop side FYI.
> 
> A quick search seems to indicate there is still more 2dppx usage:
> https://dxr.mozilla.org/mozilla-central/search?q=2dppx&redirect=true vs
> https://dxr.mozilla.org/mozilla-central/search?q=1.25dppx&redirect=true.

Not all icons have been switched over yet, but this is the plan until everything is converted to SVG (longer term).
Comment on attachment 8598285 [details] [diff] [review]
Patch

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

OK, sounds good.  Can you add the couple of instances in browser/devtools to this patch?
Attachment #8598285 - Flags: review?(bgrinstead) → feedback+
Attached patch PatchSplinter Review
Tested, and it looks much better on Windows.
Attachment #8598285 - Attachment is obsolete: true
Attachment #8598656 - Flags: review?(bgrinstead)
Attachment #8598656 - Flags: review?(bgrinstead) → review+
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/d925799e0a8a
Keywords: checkin-needed
Whiteboard: [devedition-40][difficulty=easy] → [devedition-40][difficulty=easy][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/d925799e0a8a
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [devedition-40][difficulty=easy][fixed-in-fx-team] → [devedition-40][difficulty=easy]
Target Milestone: --- → Firefox 40
Depends on: 1167892
Whiteboard: [devedition-40][difficulty=easy] → [polish-backlog][difficulty=easy]
Depends on: 1201829
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.