Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows Hi DPI support requires a settings change by the user? #10178

Closed
jasonsanjose opened this issue Dec 15, 2014 · 11 comments
Assignees
Milestone

Comments

@jasonsanjose
Copy link
Member

@jasonsanjose jasonsanjose commented Dec 15, 2014

I'm running 1.1.0-15542 on Win 8 on a Samsing Ativ Book 9 (3200 x 1800) with the display set at 200%. On the left is Brackets 1.1 out of the box, on the right is Brackets with the "Disable display scaling on high DPI settings" checkbox enabled.

Notice with the setting disabled...

  • the titlebar minimize/maximize/exit icons are teeny tiny
  • the in-browser fonts and menu font is crisper

windows hi dpi

Do we expect users to do this manually or should it be automatically detected? References:

http://www.magpcss.org/ceforum/viewtopic.php?f=6&t=12366
http://msdn.microsoft.com/en-us/library/windows/desktop/dn469266%28v=vs.85%29.aspx
https://code.google.com/p/chromiumembedded/issues/detail?id=1445

@JeffryBooher

This comment has been minimized.

Copy link
Contributor

@JeffryBooher JeffryBooher commented Dec 15, 2014

@jasonsanjose the itty-bity-icons are were never tested with HiDPI so this will need to be fixed. @larz0 we'll need new icons for Rretina, err, Hi DPI Windows displays :)

We should enable HiDPI by default by adding hi dpi aware information to the application manifest.

@pthiess Should we task out the work to be done on this card https://trello.com/c/WRhZ0xvj or a new github story card? It sounds like there are a few bugs and cleanup items before we can say Windows is 100% hi dpi; plus the card references Linux which is still yet to be supported.

@larz0

This comment has been minimized.

Copy link
Member

@larz0 larz0 commented Dec 16, 2014

Made a PR for new HiDPI icons: adobe/brackets-shell#494

@pthiess

This comment has been minimized.

Copy link
Member

@pthiess pthiess commented Jan 5, 2015

@ryanstewart We need your input on this team nomination.

@peterflynn

This comment has been minimized.

Copy link
Member

@peterflynn peterflynn commented Jan 18, 2015

In review last week we talked about splitting this into two pieces:

  1. Change the manifest only, so the setting change isn't required.
  2. Update the custom window chrome so the minimize/maximize/restore icons are drawn & position at the correct size (instead of tiny as in screenshot above).

The benefit seems big enough that it might be worth getting part 1 merged sooner -- the idea is that people would be ok living with the tiny window-size icons until part 2 lands later, in exchange for getting automatic high DPI support sooner. @JeffryBooher I think you were going to post an estimate of how much work it is do to just the manifest change?

@JeffryBooher

This comment has been minimized.

Copy link
Contributor

@JeffryBooher JeffryBooher commented Jan 18, 2015

@peterflynn sorry, I thought I mentioned that this work would take only a day to add and test during the meeting. Either we add a manifest file since there isn't one and add the setting

<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0" xmlns:asmv3="urn:schemas-microsoft-com:asm.v3" >
  <asmv3:application>
    <asmv3:windowsSettings xmlns="http://schemas.microsoft.com/SMI/2005/WindowsSettings">
      <dpiAware>true</dpiAware>
    </asmv3:windowsSettings>
  </asmv3:application>
</assembly>

or we can just call SetProcessDpiAwareness(true); on startup -- which seems easier since we don't need to worry about adding a manifest file to the repo and that change would require changing the grunt script as well...

@peterflynn

This comment has been minimized.

Copy link
Member

@peterflynn peterflynn commented Jan 19, 2015

@JeffryBooher Oh cool, ok! I'm going to nominate this for 1.2 then, since it seems relatively manageable to get that first half in...

@nethip

This comment has been minimized.

Copy link
Contributor

@nethip nethip commented Feb 5, 2015

Initial implementation done. Needs rigorous testing on all platforms. Moving this to 1.3

@jasonsanjose

This comment has been minimized.

Copy link
Member Author

@jasonsanjose jasonsanjose commented Feb 11, 2015

@nethip It looks like this landed in the 1.2 release branch in brackets-shell. Is that what you intended?

@nethip

This comment has been minimized.

Copy link
Contributor

@nethip nethip commented Feb 11, 2015

@jasonsanjose Yes indeed 😄 I am closing this bug.

@nethip nethip closed this Feb 11, 2015
@MarcelGerber

This comment has been minimized.

Copy link
Contributor

@MarcelGerber MarcelGerber commented Feb 13, 2015

@nethip @jasonsanjose Either one of you should set the milestone of this issue back to 1.2 if it gets shipped with 1.2

@nethip nethip modified the milestones: Release 1.2, Release 1.3 Feb 13, 2015
@nethip

This comment has been minimized.

Copy link
Contributor

@nethip nethip commented Feb 13, 2015

@MarcelGerber Thanks! Changed it to Release 1.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.