-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix Region update logic to ensure Region updates are applied correctly when handle is created #14022
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
Fix Region update logic to ensure Region updates are applied correctly when handle is created #14022
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR modifies the SetRegionInternal method to remove an optimization that prevented reapplying the same region instance when a handle exists. The change ensures that regions are always applied to window handles even when setting the same region instance, which may fix issues with region application during handle creation or recreation scenarios.
Key Changes
- Removed the
oldRegion == regioncheck from the early return condition inSetRegionInternal - The method now only checks
!IsHandleCreatedfor early return, ensuring region application always occurs when a handle exists
Comments suppressed due to low confidence (1)
src/System.Windows.Forms/System/Windows/Forms/Control.cs:2830
- Removing the
oldRegion == regioncheck may cause unnecessary calls toPInvoke.SetWindowRgnwhen the same region instance is being reapplied. Consider whether this could impact performance in scenarios whereSetRegionInternalis called frequently with the same region, especially in ActiveX scenarios (line 1824 in ActiveXImpl.cs). If this change is intentional to fix a specific bug, consider adding a comment explaining why the same region should always be reapplied.
if (!IsHandleCreated)
{
// We'll get called when OnHandleCreated runs.
return oldRegion;
}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #14022 +/- ##
===================================================
+ Coverage 77.15552% 77.15805% +0.00253%
===================================================
Files 3279 3279
Lines 645263 645317 +54
Branches 47718 47718
===================================================
+ Hits 497856 497914 +58
- Misses 143705 143715 +10
+ Partials 3702 3688 -14
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
JeremyKuhne
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a regression test, otherwise looks good.
src/test/unit/System.Windows.Forms/System/Windows/Forms/ControlTests.cs
Outdated
Show resolved
Hide resolved
JeremyKuhne
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming you validated that the new test fails without your change this looks good.
4552a8f
Yes, the test will fail without my changes. |
|
/backport to release/10.0 |
|
Started backporting to |
Fixes #14020
Root Cause
The previous logic (Changed in #12205) allowed early returns or skipped updates under certain conditions (e.g., when regions were considered equal). This prevented
PInvoke.SetWindowRgn()logic from executing, causing custom shapes like rounded corners to fail when the control handle was created or refreshed.Proposed changes
oldRegion == regionfrom methodSetRegionInternalof the Control classCustomer Impact
Regression?
Risk
Screenshots
Sample project: RegionTest.zip
Before
After
Test methodology
Test environment(s)
Microsoft Reviewers: Open in CodeFlow