fix(controls): Fix flickering issue when hovering over the top-right close button#1653
fix(controls): Fix flickering issue when hovering over the top-right close button#1653Koichi-Kobayashi wants to merge 6 commits intolepoco:mainfrom
Conversation
src/Wpf.Ui/Interop/PInvoke.cs
Outdated
| } | ||
|
|
||
| [StructLayout(LayoutKind.Sequential)] | ||
| internal struct POINT |
There was a problem hiding this comment.
Can we use the one generated from cswin32?
https://github.com/microsoft/CsWin32
src/Wpf.Ui/Interop/PInvoke.cs
Outdated
| DllImport("USER32.dll", ExactSpelling = true, EntryPoint = "GetCursorPos", SetLastError = true), | ||
| DefaultDllImportSearchPaths(DllImportSearchPath.System32) | ||
| ] | ||
| internal static extern bool GetCursorPos(out POINT lpPoint); |
There was a problem hiding this comment.
Can we use the one generated from cswin32?
https://github.com/microsoft/CsWin32
There was a problem hiding this comment.
Thanks for the suggestion. I replaced the manual interop with the CsWin32-generated GetCursorPos API by adding it to NativeMethods.txt, removed the custom GetCursorPos/POINT declarations from Interop/PInvoke.cs, and updated call sites accordingly.
| hit |= 0b1000u; // bottom | ||
| #pragma warning restore | ||
|
|
||
| if (hit == 0b0110u) |
There was a problem hiding this comment.
A lot of magic numbers here. Does CsWin32 generate a flags enum, perhaps? Or can we at least use constants?
There was a problem hiding this comment.
Good point, thank you. I removed the magic bitmask values and introduced a named [Flags] enum (BorderHitEdges), then updated the hit-test logic to use named combinations for better readability and maintainability.
| { | ||
| UIElement? headerLeftUIElement = Header as UIElement; | ||
| UIElement? headerCenterUIElement = CenterContent as UIElement; | ||
| UIElement? headerRightUiElement = TrailingContent as UIElement; |
There was a problem hiding this comment.
"Right" should now be "Trailing"
There was a problem hiding this comment.
Agreed, thanks. I renamed headerRightUiElement to headerTrailingUiElement to align terminology with TrailingContent.
…al POINT interop Replaced manual GetCursorPos/POINT declarations with CsWin32-generated APIs by updating NativeMethods and title bar call sites.
…flags Introduce BorderHitEdges flag enum for border hit-testing and replace binary literals with named combinations to improve readability and maintainability.
…ement Align variable naming with TrailingContent terminology for consistency in title bar hit-testing logic.
Nuklon
left a comment
There was a problem hiding this comment.
It mostly works OK, but it's not 100% yet. In WinUI: if you're at the minimize/maximize/close button block at the top with the resize cursor, the buttons are no longer shown as mouseover.
There's another bug if you're very close to the right side of the "X" (of the close button), the resize cursor gets rendered even though you're like 20px+ away from the end. This is due to DPI mismatch in your calculation (not everyone uses 100% display scaling 😊).
You can use the code below to address it (use in TitleBar.cs), as I don't have write access to your repo.
private IntPtr HwndSourceHook(IntPtr hwnd, int msg, IntPtr wParam, IntPtr lParam, ref bool handled)
{
var message = (uint)msg;
// Invalidate cached border size on DPI change message
if (message == PInvoke.WM_DPICHANGED)
{
InvalidateBorderCache();
}
if (
message
is not (
PInvoke.WM_NCHITTEST
or PInvoke.WM_NCMOUSELEAVE
or PInvoke.WM_NCLBUTTONDOWN
or PInvoke.WM_NCLBUTTONUP
)
)
{
return IntPtr.Zero;
}
bool isMouseOverHeaderContent = false;
bool isMouseOverButtons = false;
IntPtr htResult = (IntPtr)PInvoke.HTNOWHERE;
// For WM_NCHITTEST, perform resize detection first, and skip button hit testing if top-left or top-right corner resize detection succeeds
if (message == PInvoke.WM_NCHITTEST)
{
if (TrailingContent is UIElement || Header is UIElement || CenterContent is UIElement)
{
UIElement? headerLeftUIElement = Header as UIElement;
UIElement? headerCenterUIElement = CenterContent as UIElement;
UIElement? headerTrailingUiElement = TrailingContent as UIElement;
isMouseOverHeaderContent =
(headerLeftUIElement is not null
&& headerLeftUIElement != _titleBlock
&& TitleBarButton.IsMouseOverNonClient(headerLeftUIElement, lParam)) || (headerCenterUIElement is not null
&& TitleBarButton.IsMouseOverNonClient(headerCenterUIElement, lParam)) || (headerTrailingUiElement is not null
&& TitleBarButton.IsMouseOverNonClient(headerTrailingUiElement, lParam));
}
TitleBarButton? rightmostButton = null;
double rightmostRightEdge = double.MinValue;
foreach (TitleBarButton button in _buttons)
{
if (button is null)
{
continue;
}
try
{
if (PresentationSource.FromVisual(button) is not null)
{
double buttonRightEdge = button.PointToScreen(new Point(button.RenderSize.Width, 0)).X;
if (buttonRightEdge > rightmostRightEdge)
{
rightmostRightEdge = buttonRightEdge;
rightmostButton = button;
}
}
}
catch
{
// Ignore visual transform errors and keep searching.
}
if (TitleBarButton.IsMouseOverNonClient(button, lParam))
{
isMouseOverButtons = true;
}
}
htResult = GetWindowBorderHitTestResult(hwnd, lParam);
// Resize zones always take priority over buttons, matching native Windows behavior.
// The resize strip occupies the outermost few pixels of each edge; GetWindowBorderHitTestResult
// operates in physical pixels throughout, so the zone is correctly positioned at any DPI.
if (htResult != (IntPtr)PInvoke.HTNOWHERE)
{
RemoveButtonHovers();
handled = true;
return htResult;
}
if (rightmostButton is not null
&& Windows.Win32.PInvoke.GetCursorPos(out System.Drawing.Point cursorPoint))
{
Point cursorPosition = new(cursorPoint.X, cursorPoint.Y);
try
{
Point rightmostTopLeft = rightmostButton.PointToScreen(new Point(0, 0));
double rightEdge = rightmostButton.PointToScreen(new Point(rightmostButton.RenderSize.Width, 0)).X;
double leftEdge = rightEdge - 1;
double bottomEdge = rightmostButton.PointToScreen(new Point(0, rightmostButton.RenderSize.Height)).Y;
if (
cursorPosition.X >= leftEdge
&& cursorPosition.X <= rightEdge
&& cursorPosition.Y >= rightmostTopLeft.Y
&& cursorPosition.Y <= bottomEdge
)
{
RemoveButtonHovers();
handled = true;
return (IntPtr)PInvoke.HTRIGHT;
}
}
catch
{
// Ignore transform errors and fall back to default hit testing.
}
}
if (isMouseOverButtons)
{
htResult = (IntPtr)PInvoke.HTNOWHERE;
}
}
else if (message == PInvoke.WM_NCLBUTTONDOWN)
{
// For WM_NCLBUTTONDOWN, also skip button hit testing if within top-left or top-right corner resize area
// This ensures resize handling works correctly
foreach (TitleBarButton button in _buttons)
{
if (button is null)
{
continue;
}
if (TitleBarButton.IsMouseOverNonClient(button, lParam))
{
isMouseOverButtons = true;
break;
}
}
htResult = GetWindowBorderHitTestResult(hwnd, lParam);
if (htResult != (IntPtr)PInvoke.HTNOWHERE)
{
// If within resize area, skip button hit testing
// and let Windows handle the default resize processing
handled = false;
return IntPtr.Zero;
}
}
foreach (TitleBarButton button in _buttons)
{
if (!button.ReactToHwndHook(message, lParam, out IntPtr returnIntPtr))
{
continue;
}
// Fix for when sometimes, button hover backgrounds aren't cleared correctly, causing multiple buttons to appear as if hovered.
foreach (TitleBarButton anotherButton in _buttons)
{
if (anotherButton == button)
{
continue;
}
if (anotherButton.IsHovered && button.IsHovered)
{
anotherButton.RemoveHover();
}
}
handled = true;
return returnIntPtr;
}
var e = new HwndProcEventArgs(hwnd, msg, wParam, lParam, isMouseOverHeaderContent);
WndProcInvoked?.Invoke(this, e);
if (e.ReturnValue != null)
{
handled = e.Handled;
return e.ReturnValue ?? IntPtr.Zero;
}
switch (message)
{
case PInvoke.WM_NCHITTEST when CloseWindowByDoubleClickOnIcon && TitleBarButton.IsMouseOverNonClient(_icon, lParam):
// Ideally, clicking on the icon should open the system menu, but when the system menu is opened manually, double-clicking on the icon does not close the window
handled = true;
return (IntPtr)PInvoke.HTSYSMENU;
case PInvoke.WM_NCHITTEST when htResult != (IntPtr)PInvoke.HTNOWHERE:
handled = true;
return htResult;
case PInvoke.WM_NCHITTEST when TitleBarButton.IsMouseOverNonClient(this, lParam) && !isMouseOverHeaderContent:
handled = true;
return (IntPtr)PInvoke.HTCAPTION;
default:
return IntPtr.Zero;
}
}
private void RemoveButtonHovers()
{
foreach (TitleBarButton button in _buttons)
{
button?.RemoveHover();
}
}
Pull request type
What is the current behavior?
This pull request revisits and consolidates the changes from previous pull requests (#1647 and #1604).
It fixes an issue where the close button in the upper-right corner could flicker and become unresponsive when the mouse cursor was nearby, and restores the ability to resize the window from that corner.
Issue Number: #1647
What is the new behavior?
Other information
N/A