Fix icons of PropertyGrid control are not scaled well on 100% secondary monitor - #14828
Fix icons of PropertyGrid control are not scaled well on 100% secondary monitor#14828ricardobossan wants to merge 1 commit into
Conversation
…ry monitor Fixes dotnet#8268 ## Proposed changes `PropertyGrid`'s toolbar icons (sort-alphabetically / sort-by-category / property-pages buttons) didn't rescale correctly after a PerMonitorV2 DPI change. Root-caused to two independent, stacked defects in the toolbar rebuild path (`PropertyGrid.SetupToolbar`/`CreatePushButton`): - `SetupToolbar()` rebuilt the toolbar's `ImageList` at the new DPI but never updated `ToolStrip.ImageScalingSize` (what actually drives rendered icon size), and the `ImageList.ImageSize` assignment itself was gated on `ScaleHelper.IsScalingRequired`, which only reflects the _process's_ startup DPI rather than whether the app is PerMonitorV2-aware. Fixed by always setting `ImageScalingSize` right after the `ImageList` is rebuilt; before any button is created, since a `ToolStripItem`'s size is computed once and never revisited later; and switching to `ScaleHelper.IsScalingRequirementMet`. - `ToolStripItem`/`ToolStripButton` bake their default `Margin` and minimum button width from `ScaleHelper.InitialSystemDpi` (again the process's startup DPI) at construction time, so a button (re)created after a runtime DPI change still inherits sizing baked in for whatever DPI the process happened to start at. Fixed by having `CreatePushButton` explicitly re-apply `Margin` and `DeviceDpi` using the grid's current DPI; `DeviceDpi` routes through `ToolStripButton`'s own already-correct `DeviceDpi` setter override, which recomputes the minimum width but was never being invoked for ordinary toolbar buttons. Together these explain the issue's reported asymmetry: sizing baked in for a _higher_ DPI and later applied at a _lower_ one is very visibly wrong (oversized icons/buttons); the reverse just leaves a little extra room, so it's easy to miss. ## Customer Impact - Apps using `PerMonitorV2` render `PropertyGrid` toolbar icons at the wrong size after the app moves between monitors with different DPI, until the process restarts. Visual/readability issue only; the grid remains fully usable. ## Regression? - Yes, relative to .NET Framework; not a regression within .NET Core/5+ (reproduces on .NET 6/7/8 alike per the issue). ## Risk - All changes are scoped to `PropertyGrid`'s own toolbar-button construction/rebuild path; none affect `SystemAware`/`DpiUnaware` apps or any other control. The `DeviceDpi`/`IsScalingRequirementMet` changes invoke existing, already-correct framework code paths rather than introducing new logic. <!-- end TELL-MODE-->> ### Before ### After ## Test environment(s) <!-- Remove any that don't apply-->> 11.0.100-preview.5.26302.115
|
LGTM! Could you please check the following:
|
| _toolStrip.ImageList = LargeButtons ? _largeButtonImages : _normalButtonImages; | ||
|
|
||
| // Covers LargeButtons only (_largeButtonImages isn't ready earlier); a no-op otherwise. | ||
| _toolStrip.ImageScalingSize = LargeButtons ? _largeButtonImages!.ImageSize : _normalButtonImages!.ImageSize; |
There was a problem hiding this comment.
Suggestion (readability + DRY): since _toolStrip.ImageList was just assigned on the line above, and ToolStrip.ImageList's getter simply returns the backing field, you can derive the scaling size directly from it and drop the duplicated LargeButtons ternary:
_toolStrip.ImageScalingSize = _toolStrip.ImageList!.ImageSize;This is runtime-equivalent to the current line, but keeps ImageScalingSize tied to the exact ImageList that was set, removing the risk that the two LargeButtons ? _largeButtonImages : _normalButtonImages expressions diverge in a future edit. The ! is still needed since ImageList is typed ImageList? (both branches are guaranteed non-null here, so it won't throw).
| // ImageScalingSize (drives ToolStripItem.PreferredImageSize) before CreatePushButton runs below. | ||
| if (!LargeButtons) | ||
| { | ||
| _toolStrip.ImageScalingSize = _normalButtonImages.ImageSize; |
There was a problem hiding this comment.
This early ImageScalingSize assignment looks redundant — the later unconditional set (_toolStrip.ImageScalingSize = LargeButtons ? ... : _normalButtonImages!.ImageSize;) already runs before the newly created buttons are added to the ToolStrip and laid out, and for the !LargeButtons case both assign the identical value (_normalButtonImages.ImageSize).
The stated rationale ("set before CreatePushButton runs, since a ToolStripItem's size is computed once at creation") doesn't seem to hold here: PropertyGridToolStripButton doesn't set the ToolStrip as its Owner at construction (it only stores the grid in _owningPropertyGrid), so during CreatePushButton the button's Owner is null. Since ToolStripItem.PreferredImageSize reads Owner.ImageScalingSize dynamically and returns Size.Empty when Owner is null, the effective size is bound at layout time — which happens after _toolStrip.Items.Add(...), i.e. after the later assignment has already set the correct value.
Could you double-check whether dropping this if (!LargeButtons) block (keeping only the later assignment) still fixes the issue with a "start 100% → move to 225%" repro? If so, this block and its comment can likely be removed. If there is a layout-cache subtlety that makes it necessary, it'd be great to capture that in the comment instead.
Fixes #8268
Proposed changes
PropertyGrid's toolbar icons (sort-alphabetically / sort-by-category / property-pages buttons) didn't rescale correctly after a PerMonitorV2 DPI change. Root-caused to two independent, stacked defects in the toolbar rebuild path (PropertyGrid.SetupToolbar/CreatePushButton):SetupToolbar()rebuilt the toolbar'sImageListat the new DPI but never updatedToolStrip.ImageScalingSize(what actually drives rendered icon size), and theImageList.ImageSizeassignment itself was gated onScaleHelper.IsScalingRequired, which only reflects the process's startup DPI rather than whether the app is PerMonitorV2-aware. Fixed by always settingImageScalingSizeright after theImageListis rebuilt; before any button is created, since aToolStripItem's size is computed once and never revisited later; and switching toScaleHelper.IsScalingRequirementMet.ToolStripItem/ToolStripButtonbake their defaultMarginand minimum button width fromScaleHelper.InitialSystemDpi(again the process's startup DPI) at construction time, so a button (re)created after a runtime DPI change still inherits sizing baked in for whatever DPI the process happened to start at. Fixed by havingCreatePushButtonexplicitly re-applyMarginandDeviceDpiusing the grid's current DPI;DeviceDpiroutes throughToolStripButton's own already-correctDeviceDpisetter override, which recomputes the minimum width but was never being invoked for ordinary toolbar buttons.Together these explain the issue's reported asymmetry: sizing baked in for a higher DPI and later applied at a lower one is very visibly wrong (oversized icons/buttons); the reverse just leaves a little extra room, so it's easy to miss.
Customer Impact
PerMonitorV2renderPropertyGridtoolbar icons at the wrong size after the app moves between monitors with different DPI, until the process restarts. Visual/readability issue only; the grid remains fully usable.Regression?
Risk
PropertyGrid's own toolbar-button construction/rebuild path; none affectSystemAware/DpiUnawareapps or any other control. TheDeviceDpi/IsScalingRequirementMetchanges invoke existing, already-correct framework code paths rather than introducing new logic.Before
After
Test environment
11.0.100-preview.5.26302.115
Microsoft Reviewers: Open in CodeFlow