Skip to content

DarkMode (b) DarkModeButtonRenderer #13408

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
Create a ButtonDarkModeRenderer class similar to the existing ButtonRenderer in System.Windows.Forms, but specifically designed for dark mode rendering. The class should follow the same architectural patterns as the provided ButtonRenderer while implementing Windows 11 dark mode visual styling.

## Visual Styling Details

### Button Shapes and Corners
1. All buttons should have rounded corners with a radius of approximately 4-5 pixels
2. The corners must be properly anti-aliased
3. For the rounded corners, implement transparency at the very edges so the parent background shows through correctly - this is crucial as the button's rectangular client area includes the corners, but visually they should appear rounded
4. Use alpha blending at the rounded edges to create a smooth transition between the button and its background

### Color Schemes
1. Normal Button (non-default):
- Background: Dark gray (#2B2B2B) when not hovered
- Hover state: Slightly lighter gray (#3B3B3B) - this change should be clearly noticeable
- Pressed state: Even lighter gray (#4B4B4B) with a subtle inward appearance
- Disabled state: Very dark gray (#252525) with reduced opacity text (around 40% opacity)

2. Default Button:
- Background: Accent color (purple tone similar to #6B2FBF when not hovered)
- Hover state: Slightly lighter accent color (#7C3FD0)
- Pressed state: Darker accent color (#5B1FAF) with subtle inward appearance
- Disabled state: Desaturated accent color with reduced opacity text

3. Text Colors:
- Normal state: Light gray (#E0E0E0) for standard buttons, white (#FFFFFF) for default buttons
- Disabled state: Reduced opacity version of normal text color (around 40% opacity)

### Border Styles
1. None border style:
- No visible border, matching Windows 11 dark mode
- Only the fill color and rounded corners are visible

2. Single border style:
- Hair-thin (1px) border with a color that provides sufficient contrast
- For normal buttons: Medium gray (#555555)
- For default buttons: Slightly darker version of the accent color

3. 3D border style:
- Slightly thicker border (1-2px)
- For normal buttons: Top/left slightly lighter (#555555), bottom/right slightly darker (#222222)
- For default buttons: Similar effect but with accent color variations
- On hover: Subtly enhance the contrast between the edges
- On press: Invert the light/dark edges to create an inset appearance

### Focus Indication
1. When a button has focus, draw a 1px dotted outline 2px inside the button's edge
2. The focus indicator should follow the rounded corners
3. For normal buttons: Use a light gray (#AAAAAA) for the focus indicator
4. For default buttons: Use white (#FFFFFF) or very light purple for the focus indicator
5. Ensure the focus indicator doesn't interfere with text or image rendering

### State Transitions
1. All state changes (hover, press, etc.) should have a subtle fade/transition effect if possible
2. The hover and pressed state visual changes should be more pronounced than in the standard renderer
3. Implement a slight scale-down effect (approximately 0.5-1% reduction) when buttons are pressed

## Functional Requirements

1. Maintain complete feature parity with ButtonRenderer:
- Support for text rendering with all the same formatting options
- Support for image rendering with the same positioning options
- Support for combined text and image rendering
- Proper handling of all PushButtonState values

2. Use SystemColors appropriately:
- Rely on the dark mode SystemColors when Application.IsDarkModeEnabled is true
- If specific colors need adjustments beyond SystemColors, implement these within the renderer

3. Transparency and background handling:
- Properly support the IsBackgroundPartiallyTransparent method
- Correctly implement the DrawParentBackground method to handle transparent areas
- Ensure transparent corners are properly rendered showing the parent background

4. Implement all public methods present in ButtonRenderer with identical signatures and parameters

5. Thread safety:
- Maintain the same thread-safety approach as ButtonRenderer with ThreadStatic renderer instances

## Implementation Notes

1. Follow the same pattern as ButtonRenderer, creating a static class with identical method signatures
2. Do not add any additional features beyond what is specified
3. Maintain backward compatibility with existing button rendering behavior
4. Handle fallback for systems where visual styles are not enabled or supported
5. Provide appropriate XML documentation following the same style as the original ButtonRenderer
6. Prioritize rendering performance, especially for the alpha-blended rounded corners

The class should integrate seamlessly with the existing WinForms infrastructure while providing modern Windows 11 dark mode visuals.
5 changes: 3 additions & 2 deletions Winforms.sln
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,10 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Copilot", "Copilot", "{02EA
.github\copilot-instructions.md = .github\copilot-instructions.md
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Rendering", "Rendering", "{D619FF8C-D99A-48AB-B16B-2F0E819B46D5}"
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "GDI", "GDI", "{D619FF8C-D99A-48AB-B16B-2F0E819B46D5}"
ProjectSection(SolutionItems) = preProject
.github\copilot\GDIPlus-copilot-instructions.md = .github\copilot\GDIPlus-copilot-instructions.md
.github\copilot\GDI\DarkModeButtonRendererCodeGenerationInstructions.md = .github\copilot\GDI\DarkModeButtonRendererCodeGenerationInstructions.md
.github\copilot\GDI\GDIPlus-copilot-instructions.md = .github\copilot\GDI\GDIPlus-copilot-instructions.md
EndProjectSection
EndProject
Global
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,11 @@ public AutoSizeMode AutoSizeMode

protected override AccessibleObject CreateAccessibilityInstance() => new ButtonAccessibleObject(this);

internal override ButtonBaseAdapter CreateFlatAdapter() => new ButtonFlatAdapter(this);
internal override ButtonBaseAdapter CreateFlatAdapter() => DarkModeAdapterFactory.CreateFlatAdapter(this);

internal override ButtonBaseAdapter CreatePopupAdapter() => new ButtonPopupAdapter(this);
internal override ButtonBaseAdapter CreatePopupAdapter() => DarkModeAdapterFactory.CreatePopupAdapter(this);

internal override ButtonBaseAdapter CreateStandardAdapter() => new ButtonStandardAdapter(this);
internal override ButtonBaseAdapter CreateStandardAdapter() => DarkModeAdapterFactory.CreateStandardAdapter(this);

internal override Size GetPreferredSizeCore(Size proposedConstraints)
{
Expand Down Expand Up @@ -121,6 +121,7 @@ protected override CreateParams CreateParams
else
{
cp.Style |= PInvoke.BS_PUSHBUTTON;

if (IsDefault)
{
cp.Style |= PInvoke.BS_DEFPUSHBUTTON;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,44 @@ protected internal bool IsDefault
/// <summary>
/// Gets or sets the flat style appearance of the button control.
/// </summary>
/// <remarks>
/// <para>
/// The <see cref="FlatStyle"/> property determines how the button is rendered. The following values are supported:
/// </para>
/// <list type="bullet">
/// <item>
/// <term><see cref="FlatStyle.Standard"/></term>
/// <description>
/// The default style. The button is not wrapping the system button. It is rendered using the StandardButton adapter.
/// VisualStyleRenderer from the OS is used for certain parts, which may have issues in high-resolution scenarios.
Copy link
Member

Choose a reason for hiding this comment

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

Cref for VisualStyleRenderer?

/// Dark mode works to some extent, but improvements are needed.
Copy link
Member

Choose a reason for hiding this comment

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

What improvements are needed? As public documentation it would be better to say something like.

Suggested change
/// Dark mode works to some extent, but improvements are needed.
/// Partially supports dark mode.

And then make sure we have tracking issues around what we might be able to address.

Copy link
Member Author

Choose a reason for hiding this comment

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

/// </description>
/// </item>
/// <item>
/// <term><see cref="FlatStyle.Popup"/></term>
/// <description>
/// The button is fully owner-drawn. No rendering is delegated to the OS, not even VisualStyleRenderer.
/// This style works well in dark mode and is fully controlled by the application.
/// 3D effects are expected but may not be rendered; consider revisiting for meaningful styling.
Copy link
Member

Choose a reason for hiding this comment

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

What does this mean for the user? Does it really work well if 3d effects don't render? I don't really understand this, but I would guess you might be saying something like:

"Generally, works well in dark mode, with the exception of ..."

Copy link
Member Author

Choose a reason for hiding this comment

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

Note, that this is describing a status quo, which has been changing over the time up to a point where the docs often no longer reflect the presents. The docs are still stating things like certain Borderstyle effects (not only for buttons), which are no longer true in general. For example, many controls have a BorderStyle 3D, which no longer resemble any 3D effect. But then we have control like the RichTextBox, which still does that in the same way as under Windows 95.

That was the original motivation behind the Visual Styles, which became then strongly connected to the dark mode changes, and the reason, I would argue to revisit the VisualStyles approach, should it come to it (resources, mandate).

/// </description>
/// </item>
/// <item>
/// <term><see cref="FlatStyle.Flat"/></term>
/// <description>
/// The button is fully owner-drawn, with no OS calls or VisualStyleRenderer usage.
/// This fits modern design language and works well in dark mode.
/// </description>
/// </item>
/// <item>
/// <term><see cref="FlatStyle.System"/></term>
/// <description>
/// The button wraps the system button and is not owner-drawn.
/// No <c>OnPaint</c>, <c>OnPaintBackground</c>, or adapter is involved.
Copy link
Member

Choose a reason for hiding this comment

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

Would be better as a cref for the events.

/// In dark mode, this style is used as a fallback for Standard-style buttons.
Copy link
Member

Choose a reason for hiding this comment

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

What are standard style buttons? Is there a cref that could be used here? Or another link that users can follow to understand this from docs?

/// </description>
/// </item>
/// </list>
/// </remarks>
[SRCategory(nameof(SR.CatAppearance))]
[DefaultValue(FlatStyle.Standard)]
[Localizable(true)]
Expand All @@ -356,6 +394,7 @@ public FlatStyle FlatStyle
_flatStyle = value;
LayoutTransaction.DoLayoutIf(AutoSize, ParentInternal, this, PropertyNames.FlatStyle);
Invalidate();

UpdateOwnerDraw();
}
}
Expand Down Expand Up @@ -409,12 +448,18 @@ public Image? Image
StopAnimate();

_image = value;

if (_image is not null)
{
ImageIndex = ImageList.Indexer.DefaultIndex;
ImageList = null;
}

// If we have an Image, for some flat styles we need to change the rendering approach from
// being a wrapper around the Win32 control to being owner-drawn. The Win32 control does not
// support images in the same flexible way as we need it.
UpdateOwnerDraw();

LayoutTransaction.DoLayoutIf(AutoSize, ParentInternal, this, PropertyNames.Image);
Animate();
Invalidate();
Expand Down Expand Up @@ -620,7 +665,14 @@ internal virtual Rectangle OverChangeRectangle
}
}

private protected virtual bool OwnerDraw => FlatStyle != FlatStyle.System;
/// <summary>
/// OwnerDraw ultimately determines, if we're wrapping the respective Win32 control
Copy link
Member

Choose a reason for hiding this comment

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

Give some thought to whether there is a more descriptive name for this. If you can do describe this in a briefer <summary> with the details to <remarks> than maybe OwnerDraw is the best name.

/// (Button, CheckBox, RadioButton - OwnerDraw == false) or not. When we're not OwnerDraw,
/// both Light- and DarkMode are (and can be) rendered by the System, but then there is
/// no image rendering, and no OnPaint. This is the original behavior of the Win32 controls.
/// </summary>
private protected virtual bool OwnerDraw =>
FlatStyle != FlatStyle.System;

bool? ICommandBindingTargetProvider.PreviousEnabledStatus { get; set; }

Expand Down Expand Up @@ -960,15 +1012,20 @@ public override Size GetPreferredSize(Size proposedSize)

internal override Size GetPreferredSizeCore(Size proposedConstraints)
{
Size preferedSize = Adapter.GetPreferredSizeCore(proposedConstraints);
return LayoutUtils.UnionSizes(preferedSize + Padding.Size, MinimumSize);
Size preferredSize = Adapter.GetPreferredSizeCore(proposedConstraints);
return LayoutUtils.UnionSizes(preferredSize + Padding.Size, MinimumSize);
}

/// <summary>
/// Returns an adapter for Rendering one of the FlatStyles. Note, that we always render
/// buttons ourselves, except when the User explicitly requests FlatStyle.System rendering!
Copy link
Member

Choose a reason for hiding this comment

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

Please avoid ! in comments. Also, should this be called FlatStyleAdapter? Lastly, notes and implementation details should be part of <remarks>.

/// </summary>
internal ButtonBaseAdapter Adapter
{
get
{
if (_adapter is null || FlatStyle != _cachedAdapterType)
if (_adapter is null
|| FlatStyle != _cachedAdapterType)
{
switch (FlatStyle)
{
Expand All @@ -980,7 +1037,6 @@ internal ButtonBaseAdapter Adapter
break;
case FlatStyle.Flat:
_adapter = CreateFlatAdapter();
;
break;
default:
Debug.Fail($"Unsupported FlatStyle: \"{FlatStyle}\"");
Expand Down Expand Up @@ -1238,7 +1294,11 @@ private void SetFlag(int flag, bool value)

private bool ShouldSerializeImage() => _image is not null;

private void UpdateOwnerDraw()
// Indicates whether this control uses owner drawing, enabling UserPaint and determining
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this comment be part of OwnerDraw?

Copy link
Member Author

Choose a reason for hiding this comment

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

There should be sufficient OwnerDraw/IsOwnerDraw comments throughout the Control source. We have a lot of spots, where controls do in details different but overall similar things in slightly different approaches with different names. Unifying them altogether would be a good refactoring effort, IMO. (See Label and ButtonBase, for example). For ButtonBase, I added comments at the relevants spots.

// if we wrap the native Win32 control (OwnerDraw == false) or render it ourselves.
// Also needed to detect a Dark Mode opt-out for FlatStyle.Standard when system painting
// cannot be forced.
private protected void UpdateOwnerDraw()
{
if (OwnerDraw != GetStyle(ControlStyles.UserPaint))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,12 @@ internal abstract partial class ButtonBaseAdapter
/// <summary>
/// Returns a darkened color according to the required color contrast ratio.
/// </summary>
private protected static Color GetContrastingBorderColor(Color buttonBorderShadowColor) => Color.FromArgb(
buttonBorderShadowColor.A,
(int)(buttonBorderShadowColor.R * 0.8f),
(int)(buttonBorderShadowColor.G * 0.8f),
(int)(buttonBorderShadowColor.B * 0.8f));
private protected static Color GetContrastingBorderColor(Color buttonBorderShadowColor) =>
Color.FromArgb(
buttonBorderShadowColor.A,
(int)(buttonBorderShadowColor.R * 0.8f),
(int)(buttonBorderShadowColor.G * 0.8f),
(int)(buttonBorderShadowColor.B * 0.8f));

internal void Paint(PaintEventArgs e)
{
Expand All @@ -48,6 +49,7 @@ internal void Paint(PaintEventArgs e)
internal virtual Size GetPreferredSizeCore(Size proposedSize)
{
LayoutOptions? options = default;

using (var screen = GdiCache.GetScreenHdc())
using (PaintEventArgs e = new(screen, default))
{
Expand Down Expand Up @@ -277,11 +279,10 @@ private void Draw3DBorderRaised(IDeviceContext deviceContext, ref Rectangle boun
hdc.DrawLine(topLeftInsetPen, p2, p3); // Left (up-down)

// Bottom + right inset

using CreatePenScope bottomRightInsetPen = new(
disabledHighContrast
? colors.WindowDisabled
: stockColor ? SystemColors.ControlDark : colors.ButtonShadow);
? colors.WindowDisabled
: stockColor ? SystemColors.ControlDark : colors.ButtonShadow);

p1.Offset(0, -1); // Need to paint last pixel too.
hdc.DrawLine(bottomRightInsetPen, p3, p4); // Bottom (left-right)
Expand Down Expand Up @@ -342,6 +343,7 @@ internal static void DrawFlatBorderWithSize(
g.FillRectangle(brush, right);
g.FillRectangle(brush, top);
g.FillRectangle(brush, bottom);

return;
}

Expand Down Expand Up @@ -648,10 +650,12 @@ private ColorOptions CommonRender(IDeviceContext deviceContext) =>
internal static ColorOptions PaintFlatRender(Graphics g, Color foreColor, Color backColor, bool enabled) =>
CommonRender(g, foreColor, backColor, enabled);

protected ColorOptions PaintFlatRender(IDeviceContext deviceContext) => CommonRender(deviceContext);
protected ColorOptions PaintFlatRender(IDeviceContext deviceContext)
=> CommonRender(deviceContext);

internal static ColorOptions PaintPopupRender(Graphics g, Color foreColor, Color backColor, bool enabled) =>
CommonRender(g, foreColor, backColor, enabled);

protected ColorOptions PaintPopupRender(IDeviceContext deviceContext) => CommonRender(deviceContext);
protected ColorOptions PaintPopupRender(IDeviceContext deviceContext)
=> CommonRender(deviceContext);
}
Original file line number Diff line number Diff line change
Expand Up @@ -223,23 +223,15 @@ internal override void PaintOver(PaintEventArgs e, CheckState state)

Rectangle r = Control.ClientRectangle;

Color backColor;
if (!Control.FlatAppearance.MouseOverBackColor.IsEmpty)
{
backColor = Control.FlatAppearance.MouseOverBackColor;
}
else if (!Control.FlatAppearance.CheckedBackColor.IsEmpty)
{
backColor = state is CheckState.Checked or CheckState.Indeterminate
? Control.FlatAppearance.CheckedBackColor.MixColor(colors.LowButtonFace)
: colors.LowButtonFace;
}
else
{
backColor = state is CheckState.Indeterminate
? colors.ButtonFace.MixColor(colors.LowButtonFace)
: colors.LowButtonFace;
}
Color backColor = !Control.FlatAppearance.MouseOverBackColor.IsEmpty
Copy link
Member

Choose a reason for hiding this comment

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

Did anything actually change here? This particular change seems marginally worse than the original code.

Copy link
Member Author

Choose a reason for hiding this comment

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

But an Analyzer suggestion, which I took, and then it forced me to go all the way.
I'll leave it like this.

Copy link
Member

Choose a reason for hiding this comment

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

Consider not doing this sort of thing in the future when under time constraints. It requires significantly more time in reviews to try to validate that style only changes are not changing behavior. For reference, I've easily spent at least two hours reviewing just the isolated style changes in these PRs.

? Control.FlatAppearance.MouseOverBackColor
: !Control.FlatAppearance.CheckedBackColor.IsEmpty
? state is CheckState.Checked or CheckState.Indeterminate
? Control.FlatAppearance.CheckedBackColor.MixColor(colors.LowButtonFace)
: colors.LowButtonFace
: state is CheckState.Indeterminate
? colors.ButtonFace.MixColor(colors.LowButtonFace)
: colors.LowButtonFace;

PaintBackground(e, r, IsHighContrastHighlighted() ? SystemColors.Highlight : backColor);

Expand All @@ -249,11 +241,14 @@ internal override void PaintOver(PaintEventArgs e, CheckState state)
}

PaintImage(e, layout);

PaintField(
e,
layout,
colors,
IsHighContrastHighlighted() ? SystemColors.HighlightText : colors.WindowText,
IsHighContrastHighlighted()
? SystemColors.HighlightText
: colors.WindowText,
drawFocus: false);

if (Control.Focused && Control.ShowFocusCues)
Expand Down
Loading