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

Add support for logical coordinates/effective pixels APIs and improve dpi scaling #279

Closed
wants to merge 49 commits into from

Conversation

lhak
Copy link

@lhak lhak commented Dec 31, 2018

The coordinate system of Windows Forms is currently based on physical pixels. Therefore, when running with display scaling (>100%), all positioning values need to be transformed accordingly. This conversion currently has to be done by the developer of the control or the application and is often challenging. There is some additional code for an initial scaling pass to the system dpi (ContainerControl AutoScaleMode) and dynamic rescaling after dpi changes. However, I have found the current implementation to be very unreliable. Making a winforms application dpi aware was a very frustating experience and the result is still not perfect. In contrast, modern frameworks like UWP/WPF use logical coordinates (effective pixels) to be mostly independent from the scale factor. Also, many machines (basically all notebooks) from the last few years run with >100% scaling and so I think the user can expect all supported applications to work now without being blurry or having broken rendering (per monitor dpi scaling was according to the documentation introduced with Windows 8.1, so 5 years ago and static dpi scaling has even existed for even longer). Hence, I think it would be great if Windows Forms could also support effective pixels (and have better scaling support) especially if it now runs on .net core and applications based on it will exist for the foreseeable future.

Unfortunately, the code for rendering and positioning of elements in winforms is scattered around the codebase. So it does not seem to be realistic to apply the coordinate transformation at a low level without rewriting a huge amount of code. So I have investigated the possibility to improve the current dpi scaling behavior and provide additional APIs in logical units on top of the current physical pixel based code.


Here is the list of changes:

  • A new boolean property (LogicalDpiScaling) is added to the Control class to enable the new scaling behavior (there is also a static method SetLogicalDpiScalingDefault in the Application class to set this property by default for all controls):

    • The controls keep track of the dpi value they are scaled to. At instantiation, they are assumed to be at 96dpi to establish a predictable baseline. The initial autoscale dpi pass by the ContainerControl is ignored. Instead, at handle creation (or if their parent control changes), the controls scale themselves to the target dpi. Their Font property is also adjusted accordingly (the same code currently only runs for dynamic dpi changes). These changes together are probably the most significant ones because they improve rendering when the dpi at launch is not equal to the system dpi (which the current code often assumes).

    • The controls will be rescaled when dynamic dpi changes happen. The necessary factor is calculated by comparing the new dpi value with the old one of the control.

    • Support for per monitor dpi awareness v1 (for Windows 8.1 and older Windows 10 versions) has been extended to be basically on-par with the v2 implementation in the Control class. This includes manually raising the DpiChangedBeforeParent and DpiChangedAfterParent events.

  • New properties for logical units are added to the Control class: LogicalBounds, LogicalLocation, LogicalSize, LogicalMargin, LogicalPadding, LogicalMaximumSize and LogicalMinimumSize that map to the current pixel unit based properties.

  • A new method InvalidateLogicalRect() is added

  • A new method GetPreferredLogicalSize() is added to allow controls to return their preferred size in logical units

  • A new property (FontWithLogicalSize) is added that sets the Font property to a correctly scaled version of the supplied font. An unscaled version of the font is also kept for scaled painting of controls (see below)

  • The MouseEventArgs class now also provides the position in logical units.

  • The PaintEventArgs contains the clip rectangle in logical units. Additionally, a new method (ApplyLogicalToDeviceScaling()) has been added that sets up a render transformation to the graphics object to allow drawing with logical coordinates.

  • A new form is added to the WinFormsControls test application that uses the new APIs.

Compatibility:

By default, there should not be any change in application behavior. Enabling the new scaling code path with existing controls and application code should also work with few exceptions if the following aspects are taken into account:

  • All container controls should have their AutoScaleMode set to dpi
  • If fonts are set in the application, the FontWithLogicalSize property should be used to generate a correctly scaled copy of the font
  • The default fonts returned by the SystemFonts class should not be used as they have weird sizes if one of the advanced dpi settings ("fix blurry apps") is enabled

Even if the new code path is turned on, the currently present dpi related functions and callbacks will still use the current scaling behavior (system dpi at launch, then switch to the monitor dpi value), so all dpi related code in existing controls should still work correctly.

Next steps:

So far, none of the controls have been touched. There are a a few regressions (mostly side effects of scaling certain fonts correctly) that have to be fixed. Additionally, some controls explicitly check for per monitor v2 support and this code can be updated to also check for per monitor v1 awareness.

@lhak lhak requested a review from a team as a code owner December 31, 2018 11:25
@wjk
Copy link
Contributor

wjk commented Dec 31, 2018

@lhak I would like to question your assumptions regarding this PR.

As I understand it, best practice is to always use AutoScaleMode.Font, not Dpi. If I set AutoScaleMode to Dpi, then if the font changes, the controls will not resize to match. Even though this rarely happens on Windows, I sometimes target ReactOS, an open-source operating system that due to copyright doesn't have the Segoe UI font available. It can instead use literally any font under the sun, and as such I have to load (and possibly change) the proper font at runtime using SystemFonts.MessageBoxFont.

Also, I have read how-to guides in the past that discuss why using AutoScaleMode.Font is required for proper autoscaling code (source).

While I understand that DPI scaling in WinForms is a difficult task that definitely needs some TLC, I honestly do not believe that this PR is the way it should be done. (Of course, the .NET team has the final say, and they may well disagree with me.) Thanks nonetheless for your contribution!

… scaling issues in PropertyGrid control and improve dpi scaling of its toolbar icons.
@lhak
Copy link
Author

lhak commented Jan 2, 2019

@wjk There is actually not much difference between setting the AutoScaleMode to font and dpi in the current codebase. The initial autoscale pass of the container control will compute the scaling factor as (system dpi)/(AutoScaleDimensions) for dpi and (system dpi scaled font size)/(AutoScaleDimensions). AutoScaleDimensions will in most cases be 96dpi or the font size at 96dpi, respectively. Therefore, the resulting scaling factor will be basically the same. The only benefit you get when setting the AutoScaleMode to font is if you set a custom font in your application but have designed it for another font (size). I have updated the PR to fix an issue where forms designed with AutoScaleMode=Font (like in the test app) were double scaled if the new scaling code was enabled.

@zsd4yr
Copy link
Contributor

zsd4yr commented Jan 2, 2019

Hey @lhak firstly thanks for putting in the work here. High-dots-per-inch scenarios are certainly an area we are interested in looking at and improving.

For a PR of this size, would you please create and link a corresponding issue through which we can discuss the more theoretical ask and api-suggestion? There are certainly some thoughts among the team (as well as in the greater community) about the best way to move forward here, and I'd like to capture everyone's thoughts in an issue before moving on with API review

…changes. Add a separate field to keep track of the font scaling. This avoids an issue where the font is scaled twice.
… update the Font property if new scaling path is enabled.
@merriemcgaw
Copy link
Member

We will be taking another look at this in Core 3.1, but it's not on our current roadmap. We really appreciate the work you put in here, and will want to incorporate some of your ideas in our final solution. Please stay tuned so we can get your input on our eventual approach.

@merriemcgaw merriemcgaw closed this Apr 1, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Feb 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants