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

Improved dpi scaling and support for logical coordinates #305

Open
lhak opened this Issue Jan 8, 2019 · 6 comments

Comments

Projects
None yet
4 participants
@lhak
Copy link

lhak commented Jan 8, 2019

This issue is created as requested for PR #279 to discuss the proposed improvements and new APIs.

Current status of dpi scaling

Here is some of the insight I have gained into the current dpi scaling implementation by going throught the current codebase. Please correct me if I got something wrong:

  • The Control class contains a DeviceDpi property. This returns the system dpi by default and will be updated during handle creation to the monitor dpi

  • The Scale() method in the Control class provides a generic way to scale the size and position of a control with a specified factor

  • ContainerControls support an AutoScaleMode property. If this is set to dpi, it will call the Scale() method of its children during the layout pass. The scaling factor is calculated as (system) dpi divided by the AutoScaleDimensions property (e.g. 96dpi). If AutoScaleMode is set to font, the factor will be (font size at system dpi)/AutoScaleDimensions

  • The Form control can receive a DPI_CHANGED message during dpi changes. It will update its deviceDpi value, create a scaled copy of its font and then call the Scale() method of its children.

  • Each control (except the toplevel control) can receive a DPI_CHANGED_BEFORE_PARENT (and DPI_CHANGED_AFTER_PARENT) if per monitor dpi awareness v2 is enabled. This will update the deviceDpi value and create a scaled copy of its font (only is one is directly associated with it). It will also call the RescaleConstantsForDpiChange() method that can be implemented by controls.

  • Fonts specified in point units will always be drawn with the system dpi scaling factor applied.

  • The inbox controls have different levels of dpi awareness support. Some only check for the system dpi value in their constructor, while a few implement the RescaleConstantsForDpiChange() method to rescale to a new dpi value dynamically.

Issues with the current implementation

The biggest flaw is that the code (and the APIs) are based on physical pixels instead of logical coordinates/effective pixels. Thus, each control and application needs to do the coordinate transformation itself, which is challenging and prone to errors. Unfortunately, there is not really a low-level visual positioning and rendering layer where this transformation could be done. Instead, a lot of code directly calls into the win32 (pixel based) functions. Therefore, improvements of the current implementation seems to be the most reasonable approach. The next big issues I have encountered is that there is currently no way to know to which dpi a control is actually scaled to. There is the DeviceDpi property, but this one is for example completely unrelated to the scaling initiated by the AutoScaleMode pass. Setting one of the properties of a control (e.g. the width) can have a completely different result depending on where it is done in the code.

Some additional issues:

  • The current code assumes in my places that the initial dpi is always the system dpi. This is certainly not always the case. In fact, the system dpi value might not correspond to any of the monitor dpi values. Therefore, the initial scaling of the control positions and fonts can be wrong.
  • A lot of the per monitor dpi support is limited to the v2 version, even though the v1 version for older Windows versions also supports the required functionality.

Improvements of the general scaling mechanism

The PR adds a new code path that can be enabled for a single control with the LogicalDpiScaling property or by default with the Application.SetLogicalDpiScalingDefault() method. The basic idea is that each control keeps track on the dpi value it is currently scaled to and rescales itself if necessary (and ignores AutoScale changes):

  • A new field lastScaleDpi is added to the Control class that defaults to 96dpi as a baseline. Its value can be retrieved with the new CurrentDpi property.
  • During handle creation (or if its parent control changes), the new UpdateDpiScalingFactor() method of the control will retrieve the new dpi value and compare it with lastScaleDpi. If it has changed, it will scale its associated font if present, call its Scale() method and finally update the lastScaleDpi value. It will also call the RescaleConstantsForDpiChange() method to take advantage of already implemented dpi support in the controls.
  • As an optimization, the ContainerControl() overrides the UpdateDpiScalingFactor() method and provides the retrieved dpi value to all of its child controls. This basically allows to scale all child controls in one go with layout suspended.
  • The DPI_CHANGED message handling in the Form control is slightly updated to give the controls a chance to update their lastScaleDpi value. Additionally, if per monitor v1 mode is enabled, it will raise the DpiChangedBeforeParent and DpiChangedAfterParent events manually to emulate the v2 functionality (update the deviceDpi value, scale font and call the RescaleConstantsForDpiChange() method)

These changes should especially improve the initial positioning of the controls and sizes of the fonts. Additionally, per monitor v1 support is now basically on-par with the v2 version (except for its fundamental limitations). Because the dpi value for each control is always known, new APIs can be added that transform between logical and pixel coordinates in a reliable way.

New APIs for logical coordinates

The new APIs in the Control class are supposed to be used in applications and new controls. The final goal is that they can always stay in the logical coordinate space and do not have to worry about dpi scaling. The positioning APIs should also be usable with existing controls.

Positioning
  • New properties LogicalBounds, LogicalLocation,LogicalWidth, LogicalHeight, LogicalSize, LogicalMaximumSize, LogicalMinimumSize, LogicalMargin and LogicalPadding provide the transformation to- and from their pixel based counterparts.

  • GetPreferredLogicalSize() can be implemented by controls to enable auto sizing

Drawing
  • PaintEventArgs provides a ApplyDpiScalingTransformation() that uses the PageScale property of the Graphics class to setup a render transformation. Thus, drawing can now be done with logical coordinates.

  • FontWithLogicalSize provides a way to set a font to a control which will then be rescaled to the correct dpi value and passed to the Font property. The unscaled version is kept for text drawing with the render transformation applied. Unfortunately, the Graphics class seems to ignore the PageScale factor if a Font in point units is rendered. Thus, a copy of the font with its size converted to pixel units is created in this case.

  • InvalideLogicalRect() as the counterpart to Invalidate()

Other
  • MouseEventArgs can return the position in logical coordinates
Bitmaps (not in the PR)

Unlike vector graphics and font rendering, bitmaps are inherently pixel based. It would be great if there would be APIs for the application to provide bitmaps in different sizes and the best fit for the current dpi would automatically be picked.

Summary

I hope the proposed changes provide valuable improvements of the current dpi scaling mechanism that can be easily integrated into existing applications. In addition, they are compatible with the already present dpi support code in the various controls. The added APIs can be leveraged to write new code in a dpi independent way.

@merriemcgaw

This comment has been minimized.

Copy link
Member

merriemcgaw commented Jan 11, 2019

This is a large feature request and not fully in scope for the 3.0 release. However we are actively investigating ways we can improve Per Monitor support for all of our controls. I added the API suggestion tag because we will need to also discuss options around adding to the API and whether that's a good option or not. Thanks @lhak for kicking off the discussions.

@wjk

This comment has been minimized.

Copy link

wjk commented Jan 11, 2019

@lhak Given what @merriemcgaw said regarding how this change is not likely to be merged any time soon, I have a question for you. Is there any way I can glue your changes on top of an otherwise-unmodified copy of winforms (via subclassing, extension methods, etc.)?

I have had to jury-rig a custom ToolStripRenderer to draw correctly under high-DPI scenarios in the past, and it was not easy. More importantly, what the user sees at 200% DPI is not an exact replica of what the user sees at 100% DPI. I am very interested in improving both the UI and my development experience for that code, and I think your changes would be helpful in doing so. Any assistance would be appreciated. Thanks!

@merriemcgaw

This comment has been minimized.

Copy link
Member

merriemcgaw commented Jan 14, 2019

@wjk you should work with @KlausLoeffelmann , he's been working on the ToolStrip and I'm pretty sure he ported some fixes into .NET Core rather recently (or it's in the works :)).
@lhak , one thing I'd like to understand is what kind of testing you've done and you would propose around this new feature. We're looking at perhaps creating an all new setting somewhere higher in the stack (form or application level perhaps) that flags going down a new code path similar to what you're describing. Testing such a complex space has always posed challenges because every HDPI fix in one area has ripple effects throughout the codebase. Before we go far down any path I'd like to plan out our testing strategy so we don't inadvertently regress things. HDPI testing is absolutely a bear to automate.

@merriemcgaw merriemcgaw self-assigned this Jan 14, 2019

@KlausLoeffelmann

This comment has been minimized.

Copy link
Member

KlausLoeffelmann commented Jan 15, 2019

Yes, the Toolstrip is in the works - actually, it is ready, but for easier testing, we should have the HighDpiMode API extension in, before we PR the toolstrip.

Also, without having taken a look at the correlating PR yet - if we really want to approach a big change like the proposed, let's think about having appropriate analyzers to guarantee consistency in the changes. Tests alone would not be enough for me to let me sleep OK at night.

Thoughts?

@lhak

This comment has been minimized.

Copy link

lhak commented Jan 19, 2019

@wjk The toolstrip controls (without the upcoming changes) all seem to be system dpi aware, so they should scale in (many) static scenarios. However, I have seen in the code that some values are not scaled (e.g. the number of dots in the grip will change, but not their sizes). Are these the kind of differences you have mentioned? The changes in the PR will not magically fix this, but they should make it easier to write code that achieves what you are asking for (i.e. the rendering at different dpi values basically looks the same, just sharper).

@wjk

This comment has been minimized.

Copy link

wjk commented Jan 19, 2019

@lhak Please see the attached project. At 100% scaling/96 DPI, the custom ToolStripRenderer draws exactly as I desire it to. If you run this code at any higher scaling value, while most of the custom drawing is correct, things like rounded corners and arrow glyphs are incorrectly scaled. My goal with this code is to have the custom drawing look identical at 100% scaling and higher, even if the code used to generate it is different.

In another version of this code (not attached, sorry) I attempted to use the DpiScaleConstant() function (in Windows7Renderer.cs) to scale all manually specified coordinates, margins, and thicknesses, and it worked somewhat, but the end result is not completely identical (some lines, for example, were thicker than they should be). If you can give me any recommendations on how to better accomplish this goal, I would be most appreciative. Thanks!

Windows7Renderer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment