winforms icon indicating copy to clipboard operation
winforms copied to clipboard

Update the OwnerDraw flag when BackgroundImage changes

Open LeafShi1 opened this issue 4 months ago • 6 comments

Fixes #13823

Root Cause

The OwnerDraw property of the Button control incorrectly returns false when BackgroundImage is set during early initialization. This is due to the OwnerDraw logic checking BackgroundImage == null before the control's handle is created, which causes a premature fallback to the system renderer. As a result, even though BackgroundImage is later assigned, it is ignored during rendering in FlatStyle.Standard mode.

Proposed changes

  • Invoking UpdateOwnerDraw in BackgroundImage property of Button.cs to update the OwnerDraw flag to ensure correct visual behavior in Standard mode
  • Place the DrawButtonBorder method after PaintBackgroundImage.
  • Recalculate the background image drawing area.
  • Remove the border rounding logic for flat mode to ensure that the border completely surrounds the button (in classic mode, flat mode buttons also do not have rounded corners).

Customer Impact

  • The BackgroundImage of Button can be displayed when the FlatStyle is Standard in DrakMode

Regression?

  • Yes

Risk

  • Minimal

Screenshots

Before

The BackgroundImage of Button is not displayed when the FlatStyle is Standard in DrakMode image

After

The BackgroundImage of Button displayed when the FlatStyle is Standard in DrakMode image

Test methodology

  • Manually

Test environment(s)

  • .net 10.0.0-rc.1.25418.105
Microsoft Reviewers: Open in CodeFlow

LeafShi1 avatar Aug 20 '25 07:08 LeafShi1

Codecov Report

:x: Patch coverage is 0% with 42 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 77.11923%. Comparing base (5dcf17d) to head (a6d78ba). :warning: Report is 5 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #13824         +/-   ##
===================================================
- Coverage   77.12069%   77.11923%   -0.00146%     
===================================================
  Files           3273        3274          +1     
  Lines         644919      645070        +151     
  Branches       47693       47703         +10     
===================================================
+ Hits          497366      497473        +107     
- Misses        143849      143939         +90     
+ Partials        3704        3658         -46     
Flag Coverage Δ
Debug 77.11923% <0.00000%> (-0.00146%) :arrow_down:
integration 18.85980% <0.00000%> (-0.12540%) :arrow_down:
production 51.94586% <0.00000%> (-0.00925%) :arrow_down:
test 97.41286% <ø> (+0.00029%) :arrow_up:
unit 49.42084% <0.00000%> (+0.01500%) :arrow_up:

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

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Aug 20 '25 08:08 codecov[bot]

@Epica3055, how is tbis related to the changes for RC2?

KlausLoeffelmann avatar Sep 04 '25 00:09 KlausLoeffelmann

@Epica3055, how is this related to the changes for RC2?

This fix is unrelated to RC2. The original issue is https://github.com/dotnet/winforms/issues/13720, but there are two pull requests (PRs) addressing this issue.

  1. #13809 (already in RC2). After testing, this only addresses the issue where the background image doesn't display when the button's FlatStyle is Flat & Popup.
  2. This pull request provides a supplementary fix for the issue where background images do not display in Dark Mode when the button's FlatStyle is set to Standard.

LeafShi1 avatar Sep 05 '25 07:09 LeafShi1

Makes sense. Please make sure, we would get the other edge case also taken into account. @merriemcgaw - this would make very much sense to also take in RC2, if tactics agree. Argument for late discovery: All edge cases, which CTI found out after implementing recent fixes. All completely scope to dark mode AND to the respective narrow area, and without ANY chance to regress the light mode render path.

@LeafShi1, @Epica3055: If we would get green light for RC2, this would need to be back ported. All: Before we backport this, let's take @Olina-Zhang another look at it, to see, if we would find more edge cases in this area, so we do not need to go another time to tactics to ask for backporting a fix!

Thanks!

KlausLoeffelmann avatar Sep 07 '25 21:09 KlausLoeffelmann

@KlausLoeffelmann @merriemcgaw Based on the test results of this PR, I need to confirm should OwnerDraw be enabled in Dark Mode with FlatStyle = Standard?

If OwnerDraw should not be enabled

  • The BackColor, ForeColor, and background image settings for the Button control do not take effect. This mean that the current issue is not a bug, but rather a By Design issue. Please confirm this.
  • A further question is: For other controls (such as Label, GroupBox), color property settings currently take effect in DarkMode. If OwnerDraw is not enabled for Button, this will lead to inconsistent UI styles. Should OwnerDraw be disabled for these controls as well to maintain consistency?
  • Also, it's important to note that in .NET 9.0, color property settings for Button do take effect, so this is currently a regression.

If we decide to enable OwnerDraw to restore the effects of these properties, some existing drawing logic (such as the code at https://github.com/dotnet/winforms/blob/9634900e037acfc7feb588e5361a6ad79693f76b/src/System.Windows.Forms/System/Windows/Forms/Controls/Buttons/RadioButton.cs#L174-L185) may need to be adjusted.

LeafShi1 avatar Sep 08 '25 05:09 LeafShi1

@KlausLoeffelmann do you mean to approve this?

merriemcgaw avatar Nov 20 '25 23:11 merriemcgaw