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.

Sign up for GitHub

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

Jump to bottom

Scale errorProvider Icon to DeviceDPI #12947

Merged
merged 11 commits into from
Mar 5, 2025
Merged

Scale errorProvider Icon to DeviceDPI #12947

merged 11 commits into from
Mar 5, 2025
+35 −17

Conversation

LeafShi1
Copy link
Member

@LeafShi1 LeafShi1 commented Feb 14, 2025 •
edited
Loading

Fixes #12939

Proposed changes

  • Scale errorProvider Icon to DeviceDPI

Customer Impact

  • The icon of errorProvider can be scaled well on HDPI screen

Regression?

  • Yes

Risk

  • Minimal

Screenshots

Before

The icon of errorProvider is not scaled well on HDPI screen with SystemAware and PerMoniterV2 modes

BeforeChange

After

The icon of errorProvider can be scaled well on HDPI screen

AfterChange

Test methodology

  • Manually

Test environment(s)

  • .net 10.0.0-preview.2.25111.15
Microsoft Reviewers: Open in CodeFlow

@LeafShi1
Scale errorProvider Icon to DeviceDPI
Loading
Loading status checks…
9d67f71
@LeafShi1 LeafShi1 requested a review from a team as a code owner February 14, 2025 03:15
@codecov Codecov
Copy link

codecov bot commented Feb 18, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 76.47059% with 4 lines in your changes missing coverage. Please review.

Project coverage is 76.10553%. Comparing base (7210b61) to head (4a02afb).
Report is 104 commits behind head on main.

Additional details and impacted files 
@@                 Coverage Diff                 @@
##                main      #12947         +/-   ##
===================================================
+ Coverage   76.07479%   76.10553%   +0.03074%     
===================================================
  Files           3255        3275         +20     
  Lines         642151      643806       +1655     
  Branches       47295       47449        +154     
===================================================
+ Hits          488515      489972       +1457     
- Misses        150083      150273        +190     
- Partials        3553        3561          +8
Flag Coverage Δ
Debug 76.10553% <76.47059%> (+0.03074%) ⬆️
integration 18.04793% <0.00000%> (-0.02791%) ⬇️
production 50.13709% <76.47059%> (+0.13727%) ⬆️
test 96.95044% <ø> (-0.01493%) ⬇️
unit 47.56830% <76.47059%> (+0.14520%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@LeafShi1
Copy link
Member Author

@Tanya-Solyanik The fix has been tested passed, please review the PR.

JeremyKuhne
JeremyKuhne requested changes Feb 20, 2025
View reviewed changes
Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the existing wrapper. In general, it is a good idea to look for other API usages in the codebase to see if there might be existing patterns that can be leveraged.

@LeafShi1 LeafShi1 added the waiting-review This item is waiting on review by one or more members of team label Feb 24, 2025
@LeafShi1 LeafShi1 force-pushed the Issue_12939_Scale_Icon_To_DeviceDPI branch from d7a20d3 to 3d08357 Compare February 27, 2025 01:48
@LeafShi1
Considering that the ErrorIcon is not displayed on the system DPI scr…
619752d 
…een for the first time, add the CurrentDpi property in the ErrorProvider class
JeremyKuhne
JeremyKuhne previously approved these changes Mar 3, 2025
View reviewed changes
Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LeafShi1 Awesome! This looks a lot cleaner. :) Thank you!

JeremyKuhne
JeremyKuhne approved these changes Mar 4, 2025
View reviewed changes
Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Message me directly when your back ports are ready, and I'll go over them.

@JeremyKuhne JeremyKuhne removed the waiting-review This item is waiting on review by one or more members of team label Mar 4, 2025
@LeafShi1
Copy link
Member Author

LeafShi1 commented Mar 5, 2025

/backport to release/9.0

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 5, 2025

Started backporting to release/9.0: https://github.com/dotnet/winforms/actions/runs/13669469329

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 5, 2025

@LeafShi1 backporting to release/9.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Scale errorProvider Icon to DeviceDPI
Applying: Scale IconRegion according to currentDpi
Applying: Add _parentControl is null judgment
Applying: Add _initialLoad
Applying: Remove unused codeline
Applying: Replcase PInvoke.GetSystemMetricsForDpi with PInvoke.GetCurrentSystemMetrics
Applying: Scale icon in IconRegion class
error: sha1 information is lacking or useless (src/System.Windows.Forms/src/System/Windows/Forms/ErrorProvider/ErrorProvider.IconRegion.cs).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0007 Scale icon in IconRegion class
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@LeafShi1 LeafShi1 merged commit cd2a51e into dotnet:main Mar 5, 2025
8 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0 Preview3 milestone Mar 5, 2025
LeafShi1 added a commit to LeafShi1/winforms that referenced this pull request Mar 5, 2025
@LeafShi1
Scale errorProvider Icon to DeviceDPI (dotnet#12947)
2837ea3 
<!-- Please read CONTRIBUTING.md before submitting a pull request -->

Fixes dotnet#12939

## Proposed changes

- Scale errorProvider Icon to DeviceDPI

<!-- We are in TELL-MODE the following section must be completed -->

## Customer Impact

- The icon of errorProvider can be scaled well on HDPI screen

## Regression?

- Yes

## Risk

- Minimal

<!-- end TELL-MODE -->

## Screenshots <!-- Remove this section if PR does not change UI -->

### Before
The icon of errorProvider is not scaled well on HDPI screen with
SystemAware and PerMoniterV2 modes

![BeforeChange](https://github.com/user-attachments/assets/bf20fcc8-bc09-44f3-b9c4-9fb1beb4751c)

### After
The icon of errorProvider can be scaled well on HDPI screen

![AfterChange](https://github.com/user-attachments/assets/c757eb70-9bee-4114-a2a5-55dc7a34fb2b)

## Test methodology <!-- How did you ensure quality? -->

- Manually

## Test environment(s) <!-- Remove any that don't apply -->

- .net 10.0.0-preview.2.25111.15

<!-- Mention language, UI scaling, or anything else that might be
relevant -->

###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/dotnet/winforms/pull/12947)
This was referenced Mar 5, 2025
Merged
Merged
@LeafShi1
Copy link
Member Author

LeafShi1 commented Mar 5, 2025

@JeremyKuhne I backported this PR to release/9.0 and release/8.0, please review them.

LeafShi1 added a commit to LeafShi1/winforms that referenced this pull request Mar 6, 2025
@LeafShi1
Scale errorProvider Icon to DeviceDPI (dotnet#12947)
dbc53c3 
<!-- Please read CONTRIBUTING.md before submitting a pull request -->

Fixes dotnet#12939

## Proposed changes

- Scale errorProvider Icon to DeviceDPI

<!-- We are in TELL-MODE the following section must be completed -->

## Customer Impact

- The icon of errorProvider can be scaled well on HDPI screen

## Regression? 

- Yes 

## Risk

- Minimal

<!-- end TELL-MODE -->


## Screenshots <!-- Remove this section if PR does not change UI -->

### Before
The icon of errorProvider is not scaled well on HDPI screen with
SystemAware and PerMoniterV2 modes


![BeforeChange](https://github.com/user-attachments/assets/bf20fcc8-bc09-44f3-b9c4-9fb1beb4751c)

### After
The icon of errorProvider can be scaled well on HDPI screen


![AfterChange](https://github.com/user-attachments/assets/c757eb70-9bee-4114-a2a5-55dc7a34fb2b)


## Test methodology <!-- How did you ensure quality? -->

- Manually

## Test environment(s) <!-- Remove any that don't apply -->

- .net 10.0.0-preview.2.25111.15


<!-- Mention language, UI scaling, or anything else that might be
relevant -->

###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/dotnet/winforms/pull/12947)
LeafShi1 added a commit that referenced this pull request Mar 7, 2025
@LeafShi1 @ricardobossan
[release/8.0] Fix 12850 error provider exception in different dpi (#1…
98f54ff 
…2915)

<!-- Please read CONTRIBUTING.md before submitting a pull request -->

Back port pr #10850 and pr #12947 to release8.0
Fixes #12850 and #12939

## Bug Description

The PR #8486 removed `new Icon(icon, 16 ,16)` from `IconRegion`
constructor, that causes the released icon object to be directly
referenced, so there is an exception `Cannot access a disposed object.
Object name: 'Icon' `.

Backport #10850 fixes the above problem, but the PR always scaled
`IconRegion` to 100% DPI, so it causes issue #12939: _The icon of
errorProvider is not scaled well on HDPI screen_, so backport #12947 to
scale the ErrorProvider's `IconRegion` according to the current device
DPI.

## Customer Impact

- ErrorProvider exception when shown a second time after being dragged
on another monitor with different DPI. This regression was reported by
the customer.

## Regression? 

- Yes, it's fine in dotnet 7. This is due to #8486

## Risk

- Low - the fix only change ErrorProvider icon's display size when it
run in HDPI screen.

## Testing

- Manual testing with the user-provided project.
- Automation regression test

###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/dotnet/winforms/pull/12915)

---------

Co-authored-by: Ricardo Bossan <ricardobossan@protonmail.com>
Co-authored-by: Ricardo Bossan (BEYONDSOFT CONSULTING INC) (from Dev Box) <v-rbossan@microsoft.com>
LeafShi1 added a commit that referenced this pull request Mar 7, 2025
@LeafShi1
[release/9.0] Scale errorProvider Icon to DeviceDPI (#13079)
02a98c7 
<!-- Please read CONTRIBUTING.md before submitting a pull request -->

BackPort PR #12947 to 9.0
Fixes #12939

## Proposed changes

- Calculate the display size of the ErrorProvide Icon according to the
DPI of the current device

<!-- We are in TELL-MODE the following section must be completed -->

## Customer Impact

- The icon of errorProvider is not scaled well on HDPI screen

## Regression?

- Yes. it's fine in dotnet 8. This is due to
#10850, the initial loaded size
of the ErrorProvider IconRegion is scaled to 100% DPI even on HDPI
screens.

## Risk

- Low – because the change only change ErrorProvider icon's display size
when it run in HDPI screen.

<!-- end TELL-MODE -->

## Testing

- Manual testing with the user-provided project
###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/dotnet/winforms/pull/13079)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tanya-Solyanik Tanya-Solyanik

@JeremyKuhne JeremyKuhne

Assignees

@LeafShi1 LeafShi1

Labels
None yet
Projects
None yet
Milestone
10.0 Preview3
Development

Successfully merging this pull request may close these issues.

[HDPI]The icon of errorProvider is not scaled well on HDPI screen with SystemAware and PerMoniterV2 modes
3 participants
@LeafShi1 @JeremyKuhne @Tanya-Solyanik