Skip to content

Commit

Permalink
Apply my feedback to Klaus' dark mode feature
Browse files Browse the repository at this point in the history
- Move Experimental URL to DiagnosticIds
- Move special dark mode logic from CreateBrushScope to GetSysColorBrush
- Remove unnecessary logic in FindNearestColor
- Update brushes and pens by poking SystemEvents static directly
  • Loading branch information
JeremyKuhne committed Aug 11, 2024
1 parent b238485 commit df24969
Show file tree
Hide file tree
Showing 14 changed files with 106 additions and 81 deletions.
17 changes: 0 additions & 17 deletions src/System.Drawing.Common/src/System/Drawing/SystemBrushes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,6 @@ namespace System.Drawing;

public static class SystemBrushes
{
#if NET9_0_OR_GREATER
#pragma warning disable SYSLIB5002 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
private static bool s_colorSetOnLastAccess = SystemColors.UseAlternativeColorSet;
#pragma warning restore SYSLIB5002 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
#endif
private static readonly object s_systemBrushesKey = new();

public static Brush ActiveBorder => FromSystemColor(SystemColors.ActiveBorder);
Expand Down Expand Up @@ -62,18 +57,6 @@ public static Brush FromSystemColor(Color c)
throw new ArgumentException(SR.Format(SR.ColorNotSystemColor, c.ToString()));
}

#if NET9_0_OR_GREATER
#pragma warning disable SYSLIB5002 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
if (s_colorSetOnLastAccess != SystemColors.UseAlternativeColorSet)
{
s_colorSetOnLastAccess = SystemColors.UseAlternativeColorSet;

// We need to clear the SystemBrushes cache, when the ColorMode had changed.
Gdip.ThreadData.Remove(s_systemBrushesKey);
}
#pragma warning restore SYSLIB5002 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
#endif

if (!Gdip.ThreadData.TryGetValue(s_systemBrushesKey, out object? tempSystemBrushes)
|| tempSystemBrushes is not Brush[] systemBrushes)
{
Expand Down
18 changes: 0 additions & 18 deletions src/System.Drawing.Common/src/System/Drawing/SystemPens.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,6 @@ namespace System.Drawing;

public static class SystemPens
{
#if NET9_0_OR_GREATER
#pragma warning disable SYSLIB5002 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
private static bool s_colorSetOnLastAccess = SystemColors.UseAlternativeColorSet;
#pragma warning restore SYSLIB5002 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
#endif

private static readonly object s_systemPensKey = new();

public static Pen ActiveBorder => FromSystemColor(SystemColors.ActiveBorder);
Expand Down Expand Up @@ -64,18 +58,6 @@ public static Pen FromSystemColor(Color c)
throw new ArgumentException(SR.Format(SR.ColorNotSystemColor, c.ToString()));
}

#if NET9_0_OR_GREATER
#pragma warning disable SYSLIB5002 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
if (s_colorSetOnLastAccess != SystemColors.UseAlternativeColorSet)
{
s_colorSetOnLastAccess = SystemColors.UseAlternativeColorSet;

// We need to clear the SystemBrushes cache, when the ColorMode had changed.
Gdip.ThreadData.Remove(s_systemPensKey);
}
#pragma warning restore SYSLIB5002 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
#endif

if (!Gdip.ThreadData.TryGetValue(s_systemPensKey, out object? tempSystemPens)
|| tempSystemPens is not Pen[] systemPens)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ internal static class DiagnosticIDs
public const string DisposeModalDialog = "WFO2000";

// Experimental, number group 5000+
public const string ExperimentalVisualStyles = "WFO5000";
public const string ExperimentalDarkMode = "WFO5001";
public const string ExperimentalAsync = "WFO5002";
}
1 change: 1 addition & 0 deletions src/System.Windows.Forms.Primitives/src/NativeMethods.txt
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,7 @@ SelectPalette
SendDlgItemMessage
SendInput
SendMessage
SendMessageCallback
SendMessageTimeout
SetActiveWindow
SetBkColor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,22 +116,7 @@ internal static unsafe void DrawLines(this HDC hdc, HPEN hpen, ReadOnlySpan<int>
/// </remarks>
internal static Color FindNearestColor(this HDC hdc, Color color)
{
#pragma warning disable SYSLIB5002 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
if (color.IsSystemColor && SystemColors.UseAlternativeColorSet)
{
// We need to "un-system" the color for nearest color to work correctly in dark mode,
// since GetNearestColor doesn't understand dark mode system colors and internally works
// with the light mode colors.
color = Color.FromArgb(color.ToArgb());
Debug.Assert(!color.IsSystemColor, "Color should not be a system color.");
}
#pragma warning restore SYSLIB5002 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.

Color newColor = ColorTranslator.FromWin32(
(int)PInvoke.GetNearestColor(
hdc: hdc,
color: (COLORREF)(uint)ColorTranslator.ToWin32(color)).Value);

Color newColor = ColorTranslator.FromWin32((int)PInvoke.GetNearestColor(hdc, (COLORREF)(uint)ColorTranslator.ToWin32(color)).Value);
return newColor.ToArgb() == color.ToArgb() ? color : newColor;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,9 @@ internal readonly ref struct CreateBrushScope
/// </summary>
public CreateBrushScope(Color color)
{
#pragma warning disable SYSLIB5002 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
HBRUSH = color.IsSystemColor && !SystemColors.UseAlternativeColorSet
HBRUSH = color.IsSystemColor
? PInvoke.GetSysColorBrush(color)
: PInvoke.CreateSolidBrush(color);
#pragma warning restore SYSLIB5002 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.

ValidateBrushHandle();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,19 @@ internal static partial class PInvoke
/// <inheritdoc cref="GetSysColorBrush(SYS_COLOR_INDEX)"/>
public static HBRUSH GetSysColorBrush(Color systemColor)
{
#pragma warning disable SYSLIB5002 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
bool useSolidBrush = SystemColors.UseAlternativeColorSet;
#pragma warning restore SYSLIB5002

if (useSolidBrush)
{
// We don't have a real system color, so we'll just create a solid brush.
return CreateSolidBrush(systemColor);
}

Debug.Assert(systemColor.IsSystemColor);

// Extract the COLOR value
return PInvoke.GetSysColorBrush((SYS_COLOR_INDEX)(ColorTranslator.ToOle(systemColor) & 0xFF));
return GetSysColorBrush((SYS_COLOR_INDEX)(ColorTranslator.ToOle(systemColor) & 0xFF));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.InteropServices;
using System.Runtime.CompilerServices;

namespace Windows.Win32;

internal static partial class PInvoke
{
internal static unsafe BOOL SendMessageCallback<T>(
T hWnd,
MessageId Msg,
Action callback,
WPARAM wParam = default,
LPARAM lParam = default)
where T : IHandle<HWND>
{
GCHandle gcHandle = GCHandle.Alloc(callback);
BOOL result = SendMessageCallback(hWnd.Handle, Msg, wParam, lParam, &NativeCallback, (nuint)(nint)gcHandle);
GC.KeepAlive(hWnd.Wrapper);
return result;
}

[UnmanagedCallersOnly(CallConvs = [typeof(CallConvStdcall)])]
private static void NativeCallback(HWND hwnd, uint Msg, nuint dwData, LRESULT lResult)
{
GCHandle gcHandle = (GCHandle)(nint)dwData;
Action action = (Action)gcHandle.Target!;
gcHandle.Free();
action();
}
}
50 changes: 43 additions & 7 deletions src/System.Windows.Forms/src/System/Windows/Forms/Application.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ public sealed partial class Application
private static SystemColorMode? s_systemColorMode;
#pragma warning restore WFO5001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.

internal const string WinFormsExperimentalUrl = "https://aka.ms/winforms-experimental/{0}";

private const string DarkModeKeyPath = "HKEY_CURRENT_USER\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Themes\\Personalize";
private const string DarkModeKey = "AppsUseLightTheme";
private const int DarkModeNotAvailable = -1;
Expand Down Expand Up @@ -252,7 +250,7 @@ internal static bool CustomThreadExceptionHandlerAttached
/// Gets the default dark mode for the application. This is the SystemColorMode which either has been set
/// by <see cref="SetColorMode(SystemColorMode)"/> or its default value <see cref="SystemColorMode.Classic"/>.
/// </summary>
[Experimental(DiagnosticIDs.ExperimentalDarkMode, UrlFormat = WinFormsExperimentalUrl)]
[Experimental(DiagnosticIDs.ExperimentalDarkMode, UrlFormat = DiagnosticIDs.UrlFormat)]
public static SystemColorMode ColorMode =>
!s_systemColorMode.HasValue
? SystemColorMode.Classic
Expand All @@ -264,7 +262,7 @@ internal static bool CustomThreadExceptionHandlerAttached
/// Sets the default dark mode for the application.
/// </summary>
/// <param name="systemColorMode">The default dark mode to set.</param>
[Experimental(DiagnosticIDs.ExperimentalDarkMode, UrlFormat = WinFormsExperimentalUrl)]
[Experimental(DiagnosticIDs.ExperimentalDarkMode, UrlFormat = DiagnosticIDs.UrlFormat)]
public static void SetColorMode(SystemColorMode systemColorMode)
{
try
Expand All @@ -278,6 +276,11 @@ public static void SetColorMode(SystemColorMode systemColorMode)
_ => throw new ArgumentOutOfRangeException(nameof(systemColorMode))
};

if (systemColorMode == s_systemColorMode)
{
return;
}

if (GetSystemColorModeInternal() > -1)
{
s_systemColorMode = systemColorMode;
Expand All @@ -288,7 +291,40 @@ public static void SetColorMode(SystemColorMode systemColorMode)
}
finally
{
SystemColors.UseAlternativeColorSet = IsDarkModeEnabled;
bool useAlternateColorSet = SystemColors.UseAlternativeColorSet;
bool darkModeEnabled = IsDarkModeEnabled;

if (useAlternateColorSet != darkModeEnabled)
{
SystemColors.UseAlternativeColorSet = darkModeEnabled;
NotifySystemEventsOfColorChange();
}
}

static void NotifySystemEventsOfColorChange()
{
string s_systemTrackerWindow = $".NET-BroadcastEventWindow.{AppDomain.CurrentDomain.GetHashCode():x}.0";

HWND hwnd = PInvoke.FindWindow(s_systemTrackerWindow, s_systemTrackerWindow);
if (hwnd.IsNull)
{
// Haven't created the window yet, so no need to notify.
return;
}

bool complete = false;
bool success = PInvoke.SendMessageCallback(hwnd, PInvoke.WM_SYSCOLORCHANGE + MessageId.WM_REFLECT, () => complete = true);
Debug.Assert(success);
if (!success)
{
return;
}

while (!complete)
{
DoEvents();
Thread.Yield();
}
}
}

Expand All @@ -310,7 +346,7 @@ public static void SetColorMode(SystemColorMode systemColorMode)
/// SystemColorModes is not supported, if the Windows OS <c>High Contrast Mode</c> has been enabled in the system settings.
/// </para>
/// </remarks>
[Experimental(DiagnosticIDs.ExperimentalDarkMode, UrlFormat = WinFormsExperimentalUrl)]
[Experimental(DiagnosticIDs.ExperimentalDarkMode, UrlFormat = DiagnosticIDs.UrlFormat)]
public static SystemColorMode SystemColorMode =>
GetSystemColorModeInternal() == 0
? SystemColorMode.Dark
Expand Down Expand Up @@ -349,7 +385,7 @@ private static int GetSystemColorModeInternal()
/// Gets a value indicating whether the application is running in a dark system color context.
/// Note: In a high contrast mode, this will always return <see langword="false"/>.
/// </summary>
[Experimental(DiagnosticIDs.ExperimentalDarkMode, UrlFormat = WinFormsExperimentalUrl)]
[Experimental(DiagnosticIDs.ExperimentalDarkMode, UrlFormat = DiagnosticIDs.UrlFormat)]
public static bool IsDarkModeEnabled =>
!SystemInformation.HighContrast
&& (ColorMode == SystemColorMode.Dark);
Expand Down
4 changes: 1 addition & 3 deletions src/System.Windows.Forms/src/System/Windows/Forms/Control.cs
Original file line number Diff line number Diff line change
Expand Up @@ -716,8 +716,7 @@ internal HBRUSH BackColorBrush
Color color = BackColor;
HBRUSH backBrush;

#pragma warning disable WFO5001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
if (color.IsSystemColor && !Application.IsDarkModeEnabled)
if (color.IsSystemColor)
{
backBrush = PInvoke.GetSysColorBrush(color);
SetState(States.OwnCtlBrush, false);
Expand All @@ -727,7 +726,6 @@ internal HBRUSH BackColorBrush
backBrush = PInvoke.CreateSolidBrush((COLORREF)(uint)ColorTranslator.ToWin32(color));
SetState(States.OwnCtlBrush, true);
}
#pragma warning restore WFO5001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.

Debug.Assert(!backBrush.IsNull, "Failed to create brushHandle");
Properties.SetObject(s_backBrushProperty, backBrush);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,6 @@ public enum ControlStyles
/// from this setting. Note that using this settings will cause some
/// win32 control theming renderers to become inactive for a specific theme.
/// </summary>
[Experimental(DiagnosticIDs.ExperimentalDarkMode, UrlFormat = Application.WinFormsExperimentalUrl)]
[Experimental(DiagnosticIDs.ExperimentalDarkMode, UrlFormat = DiagnosticIDs.UrlFormat)]
ApplyThemingImplicitly = 0x00080000
}
Loading

0 comments on commit df24969

Please sign in to comment.