From 87266e6c13961192b644eb587cf4bc0a3aafdef5 Mon Sep 17 00:00:00 2001 From: Aleksandr Neychev Date: Wed, 12 Aug 2026 18:12:08 +0500 Subject: [PATCH] fix bug - popup windows show hiding logic --- .../Services/CapsLockHotkeyServiceTests.cs | 29 ++ CursorLang.Agent/Interop/GdiNative.cs | 68 ++-- CursorLang.Agent/Interop/WindowNative.cs | 73 ++-- .../Services/CapsLockHotkeyService.cs | 21 +- CursorLang.Agent/Windows/NativePopupWindow.cs | 365 +++++++++++------- CursorLang.Agent/Windows/NativeWindow.cs | 9 +- .../Services/KeyboardLayoutServiceTests.cs | 89 +---- .../LayoutNotificationCoordinatorTests.cs | 1 + .../Threading/MessageTimerTests.cs | 129 +++++++ CursorLang.Core/Interop/PopupWindowNative.cs | 37 -- .../Services/KeyboardLayoutService.cs | 64 +-- CursorLang.Core/Threading/MessageLoop.cs | 28 -- .../Interop/NativeWrappersTests.cs | 32 -- 13 files changed, 503 insertions(+), 442 deletions(-) create mode 100644 CursorLang.Core.Tests/Threading/MessageTimerTests.cs diff --git a/CursorLang.Agent.Tests/Services/CapsLockHotkeyServiceTests.cs b/CursorLang.Agent.Tests/Services/CapsLockHotkeyServiceTests.cs index 5e34671..8d9660b 100644 --- a/CursorLang.Agent.Tests/Services/CapsLockHotkeyServiceTests.cs +++ b/CursorLang.Agent.Tests/Services/CapsLockHotkeyServiceTests.cs @@ -232,6 +232,32 @@ public sealed class CapsLockHotkeyServiceTests }); } + /// + /// A hold is never announced for a key that has already been let go. + /// + /// + /// The countdown started by the press can still be delivered just after the + /// release: Windows does not withdraw a WM_TIMER it has already posted. Taken at + /// face value it turned a tap into a hold — the popup came up showing the layout + /// the tap was about to change away from, and the switch followed it. + /// + /// The tick is driven straight in here rather than waited for, because the point is + /// the one ordering a real clock will not reproduce on demand. + /// + [Fact] + public void A_hold_is_not_announced_after_the_key_has_been_released() + { + using var harness = Harness.Create(holdMilliseconds: 10_000); + + harness.Press(); + harness.Release(); + + harness.ForceHoldTick(); + Pump.Drain(); + + Assert.Equal(["tap"], harness.Events); + } + /// /// The service together with its settings and the list of events that happened. /// @@ -271,6 +297,9 @@ public sealed class CapsLockHotkeyServiceTests internal void Release() => Pump.Run(() => Service.HandleKeyEvent(CapsLock, isKeyDown: false)); + /// Delivers the hold countdown by hand, the way a late WM_TIMER does. + internal void ForceHoldTick() => Pump.Run(Service.HandleHoldElapsed); + public void Dispose() => Pump.Run(Service.Dispose); } } diff --git a/CursorLang.Agent/Interop/GdiNative.cs b/CursorLang.Agent/Interop/GdiNative.cs index c068ed4..8df054a 100644 --- a/CursorLang.Agent/Interop/GdiNative.cs +++ b/CursorLang.Agent/Interop/GdiNative.cs @@ -5,7 +5,7 @@ using CursorLang.Core.Interop; namespace CursorLang.Agent.Interop; /// -/// Plain GDI: a font, a brush, a rounded region and text on a device context. +/// Plain GDI: a font, text, and an off-screen bitmap to draw them into. /// /// /// GDI rather than GDI+ on purpose. System.Drawing.Common would make the drawing @@ -85,11 +85,41 @@ internal static class GdiNative int iCharSet, int iOutPrecision, int iClipPrecision, int iQuality, int iPitchAndFamily, string pszFaceName); - [DllImport("gdi32.dll")] - internal static extern IntPtr CreateSolidBrush(uint color); + /// + /// A 32-bit surface to draw the popup into before anyone can see it. + /// + /// + /// Top-down — a negative height — so that the first row of + /// is the top row of the picture and the alpha fixing up afterwards can walk the + /// memory straight through. + /// + internal static IntPtr CreateSurface(IntPtr deviceContext, int width, int height, out IntPtr bits) + { + var header = new BitmapInfoHeader + { + biSize = Marshal.SizeOf(), + biWidth = width, + biHeight = -height, + biPlanes = 1, + biBitCount = 32, + biCompression = BI_RGB, + }; + + return CreateDIBSection(deviceContext, ref header, DIB_RGB_COLORS, out bits, IntPtr.Zero, 0); + } + + private const uint BI_RGB = 0; + private const uint DIB_RGB_COLORS = 0; [DllImport("gdi32.dll")] - internal static extern IntPtr CreateRoundRectRgn(int x1, int y1, int x2, int y2, int w, int h); + internal static extern IntPtr CreateCompatibleDC(IntPtr hdc); + + [DllImport("gdi32.dll")] + internal static extern bool DeleteDC(IntPtr hdc); + + [DllImport("gdi32.dll")] + private static extern IntPtr CreateDIBSection(IntPtr hdc, ref BitmapInfoHeader header, uint usage, + out IntPtr bits, IntPtr section, uint offset); [DllImport("gdi32.dll")] internal static extern IntPtr SelectObject(IntPtr hdc, IntPtr h); @@ -103,9 +133,6 @@ internal static class GdiNative [DllImport("gdi32.dll")] internal static extern uint SetTextColor(IntPtr hdc, uint color); - [DllImport("user32.dll")] - internal static extern int FillRect(IntPtr hdc, ref PopupWindowNative.Rect lprc, IntPtr hbr); - [DllImport("user32.dll", CharSet = CharSet.Unicode, EntryPoint = "DrawTextW")] internal static extern int DrawText(IntPtr hdc, string lpchText, int cchText, ref PopupWindowNative.Rect lprc, uint format); @@ -116,22 +143,19 @@ internal static class GdiNative [DllImport("user32.dll")] internal static extern int ReleaseDC(IntPtr hWnd, IntPtr hDC); - [DllImport("user32.dll")] - internal static extern IntPtr BeginPaint(IntPtr hWnd, out PaintStruct lpPaint); - - [DllImport("user32.dll")] - internal static extern bool EndPaint(IntPtr hWnd, ref PaintStruct lpPaint); - [StructLayout(LayoutKind.Sequential)] - internal struct PaintStruct + private struct BitmapInfoHeader { - public IntPtr hdc; - public bool fErase; - public PopupWindowNative.Rect rcPaint; - public bool fRestore; - public bool fIncUpdate; - - [MarshalAs(UnmanagedType.ByValArray, SizeConst = 32)] - public byte[] rgbReserved; + public int biSize; + public int biWidth; + public int biHeight; + public short biPlanes; + public short biBitCount; + public uint biCompression; + public uint biSizeImage; + public int biXPelsPerMeter; + public int biYPelsPerMeter; + public uint biClrUsed; + public uint biClrImportant; } } diff --git a/CursorLang.Agent/Interop/WindowNative.cs b/CursorLang.Agent/Interop/WindowNative.cs index 7b62fde..a7ad755 100644 --- a/CursorLang.Agent/Interop/WindowNative.cs +++ b/CursorLang.Agent/Interop/WindowNative.cs @@ -13,7 +13,6 @@ internal static class WindowNative internal delegate IntPtr WindowProc(IntPtr hWnd, uint message, IntPtr wParam, IntPtr lParam); internal const int WS_POPUP = unchecked((int)0x80000000); - internal const int WS_DISABLED = 0x08000000; internal const int WS_EX_LAYERED = 0x00080000; internal const int WS_EX_TOOLWINDOW = 0x00000080; @@ -25,19 +24,14 @@ internal static class WindowNative internal const int SW_SHOWNOACTIVATE = 4; internal const uint WM_DESTROY = 0x0002; - internal const uint WM_PAINT = 0x000F; internal const uint WM_CLOSE = 0x0010; internal const uint WM_QUIT = 0x0012; internal const uint WM_NULL = 0x0000; - internal const uint WM_DISPLAYCHANGE = 0x007E; - internal const uint WM_DPICHANGED = 0x02E0; internal const uint WM_ENDSESSION = 0x0016; /// WM_APP and up belong to the application; the tray takes WM_APP + 1. internal const uint WM_APP = 0x8000; - private const uint LWA_ALPHA = 0x00000002; - /// /// Registers a window class. A class already there is not an error: the name is /// unique per window kind, and a second agent in the same process would meet its @@ -80,9 +74,55 @@ internal static class WindowNative return window; } - /// The whole opacity of a layered window, 0 to 255. - internal static void SetAlpha(IntPtr window, byte alpha) => - SetLayeredWindowAttributes(window, 0, alpha, LWA_ALPHA); + /// + /// Puts a finished picture into a layered window, together with where it goes and + /// how see-through it is. + /// + /// + /// One call replaces moving the window, resizing it, painting it and setting its + /// opacity, and it works while the window is still hidden. That is the point: the + /// content is ready before anyone can see the window, so it can never be shown + /// holding the picture of the previous time. + /// + internal static bool SetContent( + IntPtr window, PopupWindowNative.Point at, Size size, IntPtr sourceDc, byte alpha) + { + var source = new PopupWindowNative.Point { X = 0, Y = 0 }; + var blend = new BlendFunction + { + BlendOp = AC_SRC_OVER, + SourceConstantAlpha = alpha, + AlphaFormat = AC_SRC_ALPHA, + }; + + return UpdateLayeredWindow( + window, IntPtr.Zero, ref at, ref size, sourceDc, ref source, 0, ref blend, ULW_ALPHA); + } + + private const byte AC_SRC_OVER = 0; + private const byte AC_SRC_ALPHA = 1; + private const uint ULW_ALPHA = 0x00000002; + + [StructLayout(LayoutKind.Sequential)] + private struct BlendFunction + { + public byte BlendOp; + public byte BlendFlags; + public byte SourceConstantAlpha; + public byte AlphaFormat; + } + + [StructLayout(LayoutKind.Sequential)] + internal struct Size + { + public int Width; + public int Height; + } + + [DllImport("user32.dll", SetLastError = true)] + private static extern bool UpdateLayeredWindow(IntPtr hWnd, IntPtr hdcDst, + ref PopupWindowNative.Point pptDst, ref Size psize, IntPtr hdcSrc, + ref PopupWindowNative.Point pptSrc, uint crKey, ref BlendFunction pblend, uint dwFlags); [DllImport("user32.dll", CharSet = CharSet.Unicode, SetLastError = true)] private static extern ushort RegisterClassEx(ref WindowClass lpwcx); @@ -104,27 +144,12 @@ internal static class WindowNative [DllImport("user32.dll")] internal static extern bool IsWindowVisible(IntPtr hWnd); - [DllImport("user32.dll")] - internal static extern bool InvalidateRect(IntPtr hWnd, IntPtr lpRect, bool bErase); - - [DllImport("user32.dll")] - internal static extern bool UpdateWindow(IntPtr hWnd); - - [DllImport("user32.dll")] - internal static extern bool SetWindowRgn(IntPtr hWnd, IntPtr hRgn, bool bRedraw); - - [DllImport("user32.dll")] - internal static extern bool GetClientRect(IntPtr hWnd, out PopupWindowNative.Rect lpRect); - [DllImport("user32.dll", CharSet = CharSet.Unicode, EntryPoint = "PostMessageW")] internal static extern bool PostMessage(IntPtr hWnd, uint message, IntPtr wParam, IntPtr lParam); [DllImport("user32.dll")] internal static extern void PostQuitMessage(int nExitCode); - [DllImport("user32.dll")] - private static extern bool SetLayeredWindowAttributes(IntPtr hWnd, uint crKey, byte bAlpha, uint dwFlags); - [DllImport("kernel32.dll", CharSet = CharSet.Unicode)] private static extern IntPtr GetModuleHandle(string? lpModuleName); diff --git a/CursorLang.Agent/Services/CapsLockHotkeyService.cs b/CursorLang.Agent/Services/CapsLockHotkeyService.cs index 78bc18a..0f03585 100644 --- a/CursorLang.Agent/Services/CapsLockHotkeyService.cs +++ b/CursorLang.Agent/Services/CapsLockHotkeyService.cs @@ -115,9 +115,28 @@ public sealed class CapsLockHotkeyService : ICapsLockHotkeyService, IDisposable return true; } - private void OnHoldTimerTick(object? sender, EventArgs e) + private void OnHoldTimerTick(object? sender, EventArgs e) => HandleHoldElapsed(); + + /// + /// The hold countdown has run out. + /// + /// + /// The key being down is checked rather than assumed. A countdown started on the + /// press can still be delivered just after the release — Windows does not withdraw a + /// WM_TIMER it has already posted — and announcing a hold then would put the popup on + /// screen showing the layout the tap is about to change away from. + /// + /// The tests reach this directly: that ordering is the whole point and a real clock + /// will not reproduce it on demand. + /// + internal void HandleHoldElapsed() { _holdTimer.Stop(); + if (!_isPressed) + { + return; + } + _isHolding = true; HoldStarted?.Invoke(this, EventArgs.Empty); } diff --git a/CursorLang.Agent/Windows/NativePopupWindow.cs b/CursorLang.Agent/Windows/NativePopupWindow.cs index a1dcceb..1a7d1ce 100644 --- a/CursorLang.Agent/Windows/NativePopupWindow.cs +++ b/CursorLang.Agent/Windows/NativePopupWindow.cs @@ -1,4 +1,5 @@ using System.Drawing; +using System.Runtime.InteropServices; using CursorLang.Agent.Interop; using CursorLang.Agent.Services; using CursorLang.Core.Interop; @@ -14,15 +15,18 @@ namespace CursorLang.Agent.Windows; /// A like-for-like replacement of the WPF popup this once was: a rounded rectangle of /// radius 4 with 10×4 padding, the fill and the text colour from the settings, the whole /// thing at the opacity from the settings, the name in Segoe UI SemiBold at the size -/// from the settings. Measured against it side by side, the two agreed to the pixel in -/// position and width. +/// from the settings. /// -/// The opacity comes from SetLayeredWindowAttributes and the rounded corners from -/// a window region, which is the cheaper of the two ways of doing it: the text keeps -/// ClearType, and no GDI+ or Direct2D is needed. The price is that a region is a binary -/// mask — the corners are cut without antialiasing. At radius 4 that is a stepped arc of -/// some fourteen pixels per corner, visible at eight times magnification and not at -/// natural size, which is why this way was kept. +/// The picture is drawn into an off-screen bitmap and handed to the window whole, by +/// UpdateLayeredWindow. Painting on demand instead — a WM_PAINT after the +/// window is shown — is what the first version did, and it had the popup appear holding +/// the picture of the previous show: hiding a window does not throw its content away, +/// and the content is always the other layout. Here there is nothing to be stale, +/// because the window is never shown before its picture is in place. +/// +/// It also does away with two devices the painted version needed: the corners came from +/// a window region, which cuts without antialiasing, and the opacity from +/// SetLayeredWindowAttributes. Both are now just pixels in the bitmap. /// /// Responsible only for showing the popup, its size and its place on screen: when to /// take it down is decided by . @@ -38,16 +42,9 @@ internal sealed class NativePopupWindow : NativeWindow, ILayoutPopupWindow private readonly AppSettings _settings; - private string _text = string.Empty; private IntPtr _font; - private IntPtr _background; - private double _fontSize; private double _fontScale; - private uint _backgroundColorRef; - private byte? _alpha; - private Size _regionSize; - private int _regionRadius = -1; internal NativePopupWindow(AppSettings settings) : base( @@ -71,8 +68,6 @@ internal sealed class NativePopupWindow : NativeWindow, ILayoutPopupWindow return; } - _text = text; - bool atFixedPoint = _settings.PlacementMode == PopupPlacementMode.FixedPoint; PopupWindowNative.Rect work = default; PopupWindowNative.Rect anchor = default; @@ -90,7 +85,7 @@ internal sealed class NativePopupWindow : NativeWindow, ILayoutPopupWindow EnsureFont(scale); - Size measured = MeasureText(); + Size measured = MeasureText(text); int width = measured.Width + (2 * PopupLayout.ToPixels(PaddingX, scale)); int height = measured.Height + (2 * PopupLayout.ToPixels(PaddingY, scale)); @@ -99,21 +94,15 @@ internal sealed class NativePopupWindow : NativeWindow, ILayoutPopupWindow work, _settings.ScreenPosition, PopupLayout.ToPixels(_settings.ScreenMargin, scale), width, height) : PopupLayout.NearAnchor(anchor, AnchorSideForMode(), OffsetForMode(scale), width, height); - PopupWindowNative.SetBounds(Handle, position.X, position.Y, width, height); - - ApplyRegion(width, height, PopupLayout.ToPixels(CornerRadius, scale)); - ApplyOpacity(); + if (!Draw(text, position, width, height, PopupLayout.ToPixels(CornerRadius, scale))) + { + return; + } if (!WindowNative.IsWindowVisible(Handle)) { WindowNative.ShowWindow(Handle, WindowNative.SW_SHOWNOACTIVATE); } - - // The repaint is forced rather than left to the queue: a layout switch is - // followed at once by the user looking at the popup, and a WM_PAINT waiting its - // turn behind a slow message is exactly how stale text gets on screen - WindowNative.InvalidateRect(Handle, IntPtr.Zero, false); - WindowNative.UpdateWindow(Handle); } public void Hide() @@ -130,27 +119,205 @@ internal sealed class NativePopupWindow : NativeWindow, ILayoutPopupWindow public override void Dispose() { ReleaseFont(); - - if (_background != IntPtr.Zero) - { - GdiNative.DeleteObject(_background); - _background = IntPtr.Zero; - } - base.Dispose(); } - protected override bool OnMessage(uint message, IntPtr wParam, IntPtr lParam, out IntPtr result) + /// + /// Draws the popup off screen and hands the finished picture to the window. + /// + /// + /// The bitmap is thrown away afterwards rather than kept: it is a few tens of + /// kilobytes for the length of one call, the popup is shown rarely, and a cached one + /// would have to be rebuilt on every change of size, colour or scale anyway. + /// + private bool Draw(string text, PopupWindowNative.Point at, int width, int height, int radius) { - result = IntPtr.Zero; - - if (message != WindowNative.WM_PAINT) + IntPtr screen = GdiNative.GetDC(IntPtr.Zero); + if (screen == IntPtr.Zero) { return false; } - Paint(); - return true; + IntPtr memory = IntPtr.Zero; + IntPtr surface = IntPtr.Zero; + + try + { + memory = GdiNative.CreateCompatibleDC(screen); + if (memory == IntPtr.Zero) + { + return false; + } + + surface = GdiNative.CreateSurface(memory, width, height, out IntPtr bits); + if (surface == IntPtr.Zero) + { + return false; + } + + GdiNative.SelectObject(memory, surface); + + Fill(bits, width, height); + DrawText(memory, text, width, height); + + // GDI writes nothing into the alpha channel, so the letters it just drew are + // sitting at zero alpha and would come out invisible. The inside of the + // popup is opaque anyway, so the whole surface is simply declared so — and + // the corners are rounded off afterwards, which is the only place alpha + // varies + MakeOpaque(bits, width, height); + RoundTheCorners(bits, width, height, radius); + + var size = new WindowNative.Size { Width = width, Height = height }; + var alpha = (byte)Math.Clamp(Math.Round(_settings.Opacity * 255), 0, 255); + + return WindowNative.SetContent(Handle, at, size, memory, alpha); + } + finally + { + // The context goes first: a bitmap still selected into one cannot be + // deleted, and this way that holds however the method was left + if (memory != IntPtr.Zero) + { + GdiNative.DeleteDC(memory); + } + + if (surface != IntPtr.Zero) + { + GdiNative.DeleteObject(surface); + } + + GdiNative.ReleaseDC(IntPtr.Zero, screen); + } + } + + private void Fill(IntPtr bits, int width, int height) + { + Color background = _settings.BackgroundColor; + + // Straight into the bitmap rather than through a brush: the pixels have to be + // written anyway to carry an alpha channel GDI would not touch + int packed = (255 << 24) | (background.R << 16) | (background.G << 8) | background.B; + var row = new int[width]; + Array.Fill(row, packed); + + for (var y = 0; y < height; y++) + { + Marshal.Copy(row, 0, bits + (y * width * 4), width); + } + } + + private void DrawText(IntPtr deviceContext, string text, int width, int height) + { + if (_font == IntPtr.Zero || text.Length == 0) + { + return; + } + + var bounds = new PopupWindowNative.Rect { Left = 0, Top = 0, Right = width, Bottom = height }; + + IntPtr previousFont = GdiNative.SelectObject(deviceContext, _font); + GdiNative.SetBkMode(deviceContext, GdiNative.TRANSPARENT); + GdiNative.SetTextColor(deviceContext, GdiNative.ToColorRef(_settings.ForegroundColor)); + + GdiNative.DrawText(deviceContext, text, text.Length, ref bounds, + GdiNative.DT_SINGLELINE | GdiNative.DT_CENTER | GdiNative.DT_VCENTER | + GdiNative.DT_NOPREFIX | GdiNative.DT_NOCLIP); + + GdiNative.SelectObject(deviceContext, previousFont); + } + + private static void MakeOpaque(IntPtr bits, int width, int height) + { + var row = new int[width]; + + for (var y = 0; y < height; y++) + { + IntPtr line = bits + (y * width * 4); + Marshal.Copy(line, row, 0, width); + + for (var x = 0; x < width; x++) + { + row[x] = (int)((uint)row[x] | 0xFF000000); + } + + Marshal.Copy(row, 0, line, width); + } + } + + /// + /// Cuts the four corners to a radius, fading the edge rather than stepping it. + /// + /// + /// The painted version cut them with a window region, which is a yes-or-no mask and + /// left a visible staircase at 200% scale. Here the corner pixels carry a partial + /// alpha worked out from how far the pixel centre is past the arc, which is what + /// antialiasing amounts to. The colours are premultiplied to match, as + /// UpdateLayeredWindow expects. + /// + private static void RoundTheCorners(IntPtr bits, int width, int height, int radius) + { + if (radius <= 0) + { + return; + } + + radius = Math.Min(radius, Math.Min(width, height) / 2); + + var row = new int[width]; + + for (var y = 0; y < height; y++) + { + bool nearTop = y < radius; + bool nearBottom = y >= height - radius; + if (!nearTop && !nearBottom) + { + continue; + } + + IntPtr line = bits + (y * width * 4); + Marshal.Copy(line, row, 0, width); + + double centreY = nearTop ? radius - 0.5 : height - radius - 0.5; + + for (var x = 0; x < width; x++) + { + bool nearLeft = x < radius; + bool nearRight = x >= width - radius; + if (!nearLeft && !nearRight) + { + continue; + } + + double centreX = nearLeft ? radius - 0.5 : width - radius - 0.5; + double distance = Math.Sqrt( + ((x - centreX) * (x - centreX)) + ((y - centreY) * (y - centreY))); + + // One pixel of softness across the arc: fully inside, fully outside, + // and a ramp in between + double coverage = Math.Clamp(radius - distance + 0.5, 0, 1); + if (coverage >= 1) + { + continue; + } + + row[x] = Premultiply(row[x], coverage); + } + + Marshal.Copy(row, 0, line, width); + } + } + + private static int Premultiply(int pixel, double coverage) + { + var value = (uint)pixel; + var alpha = (uint)Math.Round(((value >> 24) & 0xFF) * coverage); + + uint red = (uint)Math.Round(((value >> 16) & 0xFF) * coverage); + uint green = (uint)Math.Round(((value >> 8) & 0xFF) * coverage); + uint blue = (uint)Math.Round((value & 0xFF) * coverage); + + return (int)((alpha << 24) | (red << 16) | (green << 8) | blue); } // The anchor point: the caret in the input field or the mouse cursor. The cursor @@ -174,42 +341,7 @@ internal sealed class NativePopupWindow : NativeWindow, ILayoutPopupWindow _settings.PlacementMode == PopupPlacementMode.AtCaret ? _settings.CaretOffset : _settings.CursorOffset, scale); - private void Paint() - { - IntPtr deviceContext = GdiNative.BeginPaint(Handle, out GdiNative.PaintStruct paint); - if (deviceContext == IntPtr.Zero) - { - return; - } - - try - { - WindowNative.GetClientRect(Handle, out PopupWindowNative.Rect client); - - GdiNative.FillRect(deviceContext, ref client, EnsureBackground()); - - if (_font == IntPtr.Zero || _text.Length == 0) - { - return; - } - - IntPtr previousFont = GdiNative.SelectObject(deviceContext, _font); - GdiNative.SetBkMode(deviceContext, GdiNative.TRANSPARENT); - GdiNative.SetTextColor(deviceContext, GdiNative.ToColorRef(_settings.ForegroundColor)); - - GdiNative.DrawText(deviceContext, _text, _text.Length, ref client, - GdiNative.DT_SINGLELINE | GdiNative.DT_CENTER | GdiNative.DT_VCENTER | - GdiNative.DT_NOPREFIX | GdiNative.DT_NOCLIP); - - GdiNative.SelectObject(deviceContext, previousFont); - } - finally - { - GdiNative.EndPaint(Handle, ref paint); - } - } - - private Size MeasureText() + private Size MeasureText(string text) { IntPtr deviceContext = GdiNative.GetDC(Handle); if (deviceContext == IntPtr.Zero) @@ -220,10 +352,10 @@ internal sealed class NativePopupWindow : NativeWindow, ILayoutPopupWindow try { IntPtr previousFont = GdiNative.SelectObject(deviceContext, _font); - Size size = GdiNative.MeasureText(deviceContext, _text); + Size measured = GdiNative.MeasureText(deviceContext, text); GdiNative.SelectObject(deviceContext, previousFont); - return size; + return measured; } finally { @@ -231,13 +363,13 @@ internal sealed class NativePopupWindow : NativeWindow, ILayoutPopupWindow } } - // The font is rebuilt only when the size in the settings or the monitor changes: - // creating one costs a trip to the font mapper, and the popup is shown often + // The font is rebuilt only when the size in the settings or the monitor scale + // changes: it is the one expensive thing a show does private void EnsureFont(double scale) { if (_font != IntPtr.Zero && - Math.Abs(_fontSize - _settings.FontSize) < 0.001 && - Math.Abs(_fontScale - scale) < 0.001) + Math.Abs(_fontSize - _settings.FontSize) < 0.01 && + Math.Abs(_fontScale - scale) < 0.01) { return; } @@ -251,71 +383,10 @@ internal sealed class NativePopupWindow : NativeWindow, ILayoutPopupWindow private void ReleaseFont() { - if (_font == IntPtr.Zero) + if (_font != IntPtr.Zero) { - return; + GdiNative.DeleteObject(_font); + _font = IntPtr.Zero; } - - GdiNative.DeleteObject(_font); - _font = IntPtr.Zero; - } - - private IntPtr EnsureBackground() - { - uint colorRef = GdiNative.ToColorRef(_settings.BackgroundColor); - if (_background != IntPtr.Zero && _backgroundColorRef == colorRef) - { - return _background; - } - - if (_background != IntPtr.Zero) - { - GdiNative.DeleteObject(_background); - } - - _backgroundColorRef = colorRef; - _background = GdiNative.CreateSolidBrush(colorRef); - - return _background; - } - - // A region is in window coordinates, so a resize invalidates it. Reapplying it - // every show would do no harm, but the system frees the old region each time and - // the popup is shown far more often than it changes size - private void ApplyRegion(int width, int height, int radius) - { - if (_regionSize.Width == width && _regionSize.Height == height && _regionRadius == radius) - { - return; - } - - // The right and bottom edges of CreateRoundRectRgn are exclusive, and the - // ellipse the corners are cut with is twice the radius across - IntPtr region = GdiNative.CreateRoundRectRgn(0, 0, width + 1, height + 1, radius * 2, radius * 2); - if (region == IntPtr.Zero) - { - return; - } - - if (!WindowNative.SetWindowRgn(Handle, region, false)) - { - GdiNative.DeleteObject(region); - return; - } - - _regionSize = new Size(width, height); - _regionRadius = radius; - } - - private void ApplyOpacity() - { - var alpha = (byte)Math.Clamp(Math.Round(_settings.Opacity * 255), 0, 255); - if (alpha == _alpha) - { - return; - } - - _alpha = alpha; - WindowNative.SetAlpha(Handle, alpha); } } diff --git a/CursorLang.Agent/Windows/NativeWindow.cs b/CursorLang.Agent/Windows/NativeWindow.cs index 036e6e7..fac5360 100644 --- a/CursorLang.Agent/Windows/NativeWindow.cs +++ b/CursorLang.Agent/Windows/NativeWindow.cs @@ -67,9 +67,14 @@ internal abstract class NativeWindow : IDisposable /// /// A message for this window. Returning false passes it to - /// DefWindowProcW, which is what the vast majority of messages want. + /// DefWindowProcW, which is what the vast majority of messages want — and + /// what all of them want for a window whose content is set from the outside. /// - protected abstract bool OnMessage(uint message, IntPtr wParam, IntPtr lParam, out IntPtr result); + protected virtual bool OnMessage(uint message, IntPtr wParam, IntPtr lParam, out IntPtr result) + { + result = IntPtr.Zero; + return false; + } private static IntPtr StaticWindowProc(IntPtr hWnd, uint message, IntPtr wParam, IntPtr lParam) { diff --git a/CursorLang.Core.Tests/Services/KeyboardLayoutServiceTests.cs b/CursorLang.Core.Tests/Services/KeyboardLayoutServiceTests.cs index 16bd0c8..357ef21 100644 --- a/CursorLang.Core.Tests/Services/KeyboardLayoutServiceTests.cs +++ b/CursorLang.Core.Tests/Services/KeyboardLayoutServiceTests.cs @@ -17,7 +17,6 @@ public sealed class KeyboardLayoutServiceTests private const int English = 0x0409; private const int Russian = 0x0419; - private const int German = 0x0407; [Fact] public void The_current_layout_is_taken_from_the_foreground_window() @@ -52,7 +51,6 @@ public sealed class KeyboardLayoutServiceTests Pump.Run(service.Start); world.LocaleId = Russian; Pump.Run(service.Poll); - Pump.Run(service.Poll); LayoutChangedEventArgs change = Assert.Single(world.Changes); Assert.Equal(LayoutChangeReason.UserSwitched, change.Reason); @@ -70,7 +68,6 @@ public sealed class KeyboardLayoutServiceTests world.ForegroundWindow = SecondWindow; world.LocaleId = Russian; Pump.Run(service.Poll); - Pump.Run(service.Poll); LayoutChangedEventArgs change = Assert.Single(world.Changes); Assert.Equal(LayoutChangeReason.ApplicationSwitched, change.Reason); @@ -125,91 +122,10 @@ public sealed class KeyboardLayoutServiceTests // And the layout was not remembered: the change is noticed once a window is back world.ForegroundWindow = FirstWindow; Pump.Run(service.Poll); - Pump.Run(service.Poll); Assert.Single(world.Changes); } - /// - /// A value the switch is only passing through never reaches the subscribers. - /// - /// - /// A layout change is rarely a single step: the Windows language switcher takes the - /// focus while it is up, and applications with a rendering engine of their own move - /// a helper thread first. Reporting what polling caught in between meant the popup - /// coming up with one layout and turning into another in front of the user. - /// - [Fact] - public void A_layout_the_switch_only_passes_through_is_never_reported() - { - using var world = new World { LocaleId = English }; - KeyboardLayoutService service = world.CreateService(); - - Pump.Run(service.Start); - - world.LocaleId = German; - Pump.Run(service.Poll); - - Assert.Empty(world.Changes); - - world.LocaleId = Russian; - Pump.Run(service.Poll); - Pump.Run(service.Poll); - - LayoutChangedEventArgs change = Assert.Single(world.Changes); - Assert.Equal("RU", change.Layout.ShortName); - } - - // A switch that goes out and comes back is not a switch at all - [Fact] - public void A_layout_that_returns_to_where_it_was_yields_no_events() - { - using var world = new World { LocaleId = English }; - KeyboardLayoutService service = world.CreateService(); - - Pump.Run(service.Start); - - world.LocaleId = Russian; - Pump.Run(service.Poll); - - world.LocaleId = English; - Pump.Run(service.Poll); - Pump.Run(service.Poll); - - Assert.Empty(world.Changes); - } - - /// - /// A window the switch passes through does not make it an application switch. - /// - /// - /// The Windows language switcher is a window of its own and holds the foreground - /// while it is up. Judging by the window seen on the previous tick would call that - /// an application switch and swallow the popup, so the judgement is made against - /// the window that was there before the whole transition started. - /// - [Fact] - public void A_window_the_switch_passes_through_is_not_an_application_switch() - { - using var world = new World { LocaleId = English }; - KeyboardLayoutService service = world.CreateService(); - - Pump.Run(service.Start); - - world.ForegroundWindow = SecondWindow; - world.LocaleId = German; - Pump.Run(service.Poll); - - world.ForegroundWindow = FirstWindow; - world.LocaleId = Russian; - Pump.Run(service.Poll); - Pump.Run(service.Poll); - - LayoutChangedEventArgs change = Assert.Single(world.Changes); - Assert.Equal(LayoutChangeReason.UserSwitched, change.Reason); - Assert.Equal("RU", change.Layout.ShortName); - } - [Fact] public void One_change_yields_exactly_one_event() { @@ -221,7 +137,6 @@ public sealed class KeyboardLayoutServiceTests world.LocaleId = Russian; Pump.Run(service.Poll); Pump.Run(service.Poll); - Pump.Run(service.Poll); Assert.Single(world.Changes); } @@ -344,11 +259,9 @@ public sealed class KeyboardLayoutServiceTests internal KeyboardLayoutService CreateService(TimeSpan? pollInterval = null) { - TimeSpan interval = pollInterval ?? TimeSpan.FromHours(1); var options = new KeyboardLayoutOptions { - PollInterval = interval, - SettleInterval = interval, + PollInterval = pollInterval ?? TimeSpan.FromHours(1), }; _service = Pump.Run(() => new KeyboardLayoutService( diff --git a/CursorLang.Core.Tests/Services/LayoutNotificationCoordinatorTests.cs b/CursorLang.Core.Tests/Services/LayoutNotificationCoordinatorTests.cs index e0db2d3..307a8b4 100644 --- a/CursorLang.Core.Tests/Services/LayoutNotificationCoordinatorTests.cs +++ b/CursorLang.Core.Tests/Services/LayoutNotificationCoordinatorTests.cs @@ -10,6 +10,7 @@ namespace CursorLang.Core.Tests.Services; public sealed class LayoutNotificationCoordinatorTests { private static readonly KeyboardLayout Russian = KeyboardLayout.FromLocaleId(0x0419); + private static readonly KeyboardLayout English = KeyboardLayout.FromLocaleId(0x0409); [Fact] public void Starting_turns_on_the_layout_watch() diff --git a/CursorLang.Core.Tests/Threading/MessageTimerTests.cs b/CursorLang.Core.Tests/Threading/MessageTimerTests.cs new file mode 100644 index 0000000..27081fc --- /dev/null +++ b/CursorLang.Core.Tests/Threading/MessageTimerTests.cs @@ -0,0 +1,129 @@ +using CursorLang.Core.Threading; +using CursorLang.Tests.Shared; + +namespace CursorLang.Core.Tests.Threading; + +/// +/// The timer the agent has instead of a dispatcher timer. +/// +/// +/// It ticks on the message loop, so everything here runs on the pump thread — a timer +/// started on one thread and awaited on another would never be seen to fire. +/// +public sealed class MessageTimerTests +{ + [Fact] + public void A_started_timer_ticks() + { + int ticks = 0; + + Pump.Run(() => + { + using var timer = new MessageTimer(); + timer.Interval = TimeSpan.FromMilliseconds(15); + timer.Tick += (_, _) => ticks++; + timer.Start(); + + Pump.WaitFor(() => ticks > 0, "the timer ticked"); + }); + } + + [Fact] + public void A_timer_keeps_ticking_until_it_is_stopped() + { + var ticks = 0; + + Pump.Run(() => + { + using var timer = new MessageTimer(); + timer.Interval = TimeSpan.FromMilliseconds(15); + timer.Tick += (_, _) => ticks++; + timer.Start(); + + Pump.WaitFor(() => ticks >= 3, "the timer ticked more than once"); + }); + } + + // A stopped countdown does not go off, however long the loop runs afterwards + [Fact] + public void A_stopped_timer_does_not_tick() + { + var ticks = 0; + + Pump.Run(() => + { + using var timer = new MessageTimer(); + timer.Interval = TimeSpan.FromMilliseconds(10); + timer.Tick += (_, _) => ticks++; + timer.Start(); + timer.Stop(); + + Pump.Pause(TimeSpan.FromMilliseconds(80)); + }); + + Assert.Equal(0, ticks); + } + + // Restarting means from zero, so a countdown kept short by repeated restarts + // never reaches its end + [Fact] + public void Restarting_begins_the_countdown_again() + { + var ticks = 0; + + Pump.Run(() => + { + using var timer = new MessageTimer(); + timer.Interval = TimeSpan.FromMilliseconds(60); + timer.Tick += (_, _) => ticks++; + + for (var i = 0; i < 6; i++) + { + timer.Start(); + Pump.Pause(TimeSpan.FromMilliseconds(20)); + } + + Assert.Equal(0, ticks); + + Pump.WaitFor(() => ticks > 0, "left alone, the timer reached its end"); + }); + } + + [Fact] + public void Closing_the_timer_ends_the_ticking() + { + var ticks = 0; + + Pump.Run(() => + { + var timer = new MessageTimer { Interval = TimeSpan.FromMilliseconds(15) }; + timer.Tick += (_, _) => ticks++; + timer.Start(); + + Pump.WaitFor(() => ticks > 0, "the timer ticked"); + timer.Dispose(); + + int seen = ticks; + Pump.Pause(TimeSpan.FromMilliseconds(80)); + + Assert.Equal(seen, ticks); + }); + } + + [Fact] + public void A_timer_that_was_never_started_says_so() + { + Pump.Run(() => + { + using var timer = new MessageTimer(); + + Assert.False(timer.IsRunning); + + timer.Start(); + Assert.True(timer.IsRunning); + + timer.Stop(); + Assert.False(timer.IsRunning); + }); + } +} diff --git a/CursorLang.Core/Interop/PopupWindowNative.cs b/CursorLang.Core/Interop/PopupWindowNative.cs index c09bbcb..0259e9e 100644 --- a/CursorLang.Core/Interop/PopupWindowNative.cs +++ b/CursorLang.Core/Interop/PopupWindowNative.cs @@ -21,12 +21,6 @@ internal static class PopupWindowNative [DllImport("user32.dll")] private static extern IntPtr GetForegroundWindow(); - [DllImport("user32.dll", SetLastError = true)] - private static extern int GetWindowLong(IntPtr hWnd, int nIndex); - - [DllImport("user32.dll")] - private static extern int SetWindowLong(IntPtr hWnd, int nIndex, int dwNewLong); - [DllImport("user32.dll")] private static extern bool SetWindowPos(IntPtr hWnd, IntPtr hWndInsertAfter, int X, int Y, int cx, int cy, uint uFlags); @@ -61,12 +55,6 @@ internal static class PopupWindowNative public uint dwFlags; } - private const int GWL_EXSTYLE = -20; - // The window does not take focus away from the active application - private const int WS_EX_NOACTIVATE = 0x08000000; - // And does not show up in Alt+Tab - private const int WS_EX_TOOLWINDOW = 0x00000080; - private const uint SWP_NOSIZE = 0x0001; private const uint SWP_NOZORDER = 0x0004; private const uint SWP_NOACTIVATE = 0x0010; @@ -80,16 +68,6 @@ internal static class PopupWindowNative return cursor; } - /// - /// The popup shows up on top of other applications, so it must neither - /// activate itself nor steal input focus from the active window. - /// - internal static void MakePassive(IntPtr hWnd) - { - int exStyle = GetWindowLong(hWnd, GWL_EXSTYLE); - SetWindowLong(hWnd, GWL_EXSTYLE, exStyle | WS_EX_NOACTIVATE | WS_EX_TOOLWINDOW); - } - /// /// Moves the window to a screen point without changing its size or z-order. /// The coordinates are physical pixels: monitors have different scaling, while @@ -101,21 +79,6 @@ internal static class PopupWindowNative SetWindowPos(hWnd, IntPtr.Zero, x, y, 0, 0, SWP_NOSIZE | SWP_NOZORDER | SWP_NOACTIVATE); } - /// - /// Sets the window position and size in physical pixels. - /// - /// - /// The size is set this way rather than through Width/Height: on the first show the - /// window is still subject to the system minimum window size (SM_CXMIN×SM_CYMIN) - /// and the popup comes out noticeably larger than its text. By the time this is - /// called the window is already shown and has become a popup window, which that - /// restriction does not apply to. - /// - internal static void SetBounds(IntPtr hWnd, int x, int y, int width, int height) - { - SetWindowPos(hWnd, IntPtr.Zero, x, y, width, height, SWP_NOZORDER | SWP_NOACTIVATE); - } - /// The scale of the monitor the point is on (1.0 at 96 DPI). internal static double GetScaleAt(Point point) => GetScaleOf(MonitorFromPoint(point, MONITOR_DEFAULTTONEAREST)); diff --git a/CursorLang.Core/Services/KeyboardLayoutService.cs b/CursorLang.Core/Services/KeyboardLayoutService.cs index d51ac25..be062df 100644 --- a/CursorLang.Core/Services/KeyboardLayoutService.cs +++ b/CursorLang.Core/Services/KeyboardLayoutService.cs @@ -11,16 +11,6 @@ public sealed class KeyboardLayoutOptions { /// How often to check the layout of the active window. public TimeSpan PollInterval { get; init; } = TimeSpan.FromMilliseconds(150); - - /// - /// How long to wait for a layout that has just changed to stop changing. - /// - /// - /// Short on purpose: it is added to the delay before the popup shows, and it only - /// applies while a switch is in flight. See the settling in - /// . - /// - public TimeSpan SettleInterval { get; init; } = TimeSpan.FromMilliseconds(40); } /// @@ -36,18 +26,6 @@ public sealed class KeyboardLayoutOptions /// /// The timer ticks on the message loop of whatever thread starts it, the same as a /// dispatcher timer did: the agent has a plain Win32 loop and no dispatcher to offer. -/// -/// A switch is not reported the moment it is first seen but once the value has stopped -/// moving. A layout change is rarely a single step: the language switcher of Windows -/// takes the focus while it is up, and applications with a rendering engine of their -/// own change the layout of a helper thread before that of the input window. Polling -/// catches those in-between values, and reporting them meant the popup appearing with -/// one layout and turning into another in front of the user. -/// -/// The price is that the popup comes up one short tick later. While a switch is in -/// flight the timer runs at rather -/// than at the polling interval, so that tick is a few tens of milliseconds and not -/// another whole poll. /// public sealed class KeyboardLayoutService : IKeyboardLayoutService, IDisposable { @@ -55,11 +33,9 @@ public sealed class KeyboardLayoutService : IKeyboardLayoutService, IDisposable private readonly Func _getForegroundWindow; private readonly Func _getActiveLocaleId; private readonly Action _requestNextLayout; - private readonly KeyboardLayoutOptions _options; private int _lastLocaleId = -1; private IntPtr _lastForegroundWindow; - private int _settlingLocaleId = -1; public KeyboardLayoutService(KeyboardLayoutOptions options) : this( @@ -83,7 +59,6 @@ public sealed class KeyboardLayoutService : IKeyboardLayoutService, IDisposable _getForegroundWindow = getForegroundWindow; _getActiveLocaleId = getActiveLocaleId; _requestNextLayout = requestNextLayout; - _options = options; _pollTimer = new MessageTimer { Interval = options.PollInterval }; _pollTimer.Tick += OnTick; @@ -97,8 +72,6 @@ public sealed class KeyboardLayoutService : IKeyboardLayoutService, IDisposable { _lastForegroundWindow = _getForegroundWindow(); _lastLocaleId = _getActiveLocaleId(); - _settlingLocaleId = -1; - _pollTimer.Interval = _options.PollInterval; _pollTimer.Start(); } @@ -125,28 +98,16 @@ public sealed class KeyboardLayoutService : IKeyboardLayoutService, IDisposable return; } + bool appSwitched = foreground != _lastForegroundWindow; + _lastForegroundWindow = foreground; + int localeId = _getActiveLocaleId(); if (localeId == _lastLocaleId) { - _lastForegroundWindow = foreground; - Settle(inFlight: false); - return; } - if (localeId != _settlingLocaleId) - { - _settlingLocaleId = localeId; - Settle(inFlight: true); - - return; - } - - bool appSwitched = foreground != _lastForegroundWindow; - _lastLocaleId = localeId; - _lastForegroundWindow = foreground; - Settle(inFlight: false); // Moving to another application with a layout of its own is not the same as // the user switching the layout, and the subscribers are free to react to @@ -157,23 +118,4 @@ public sealed class KeyboardLayoutService : IKeyboardLayoutService, IDisposable LayoutChanged?.Invoke(this, new LayoutChangedEventArgs(KeyboardLayout.FromLocaleId(localeId), reason)); } - - // While a switch is in flight the next look comes sooner: that wait is added to the - // delay before the popup, and a whole polling interval there would be felt - private void Settle(bool inFlight) - { - if (!inFlight) - { - _settlingLocaleId = -1; - } - - TimeSpan wanted = inFlight ? _options.SettleInterval : _options.PollInterval; - if (_pollTimer.Interval == wanted || !_pollTimer.IsRunning) - { - return; - } - - _pollTimer.Interval = wanted; - _pollTimer.Start(); - } } diff --git a/CursorLang.Core/Threading/MessageLoop.cs b/CursorLang.Core/Threading/MessageLoop.cs index d24dd39..7694e62 100644 --- a/CursorLang.Core/Threading/MessageLoop.cs +++ b/CursorLang.Core/Threading/MessageLoop.cs @@ -38,38 +38,10 @@ public static class MessageLoop /// Asks the loop on this thread to finish. public static void Quit(int exitCode = 0) => PostQuitMessage(exitCode); - /// - /// Runs everything already waiting in the queue and returns. - /// - /// - /// For the tests and for the rare place that has to let a posted message through - /// without giving up control for good. - /// - public static void DrainQueue() - { - while (PeekMessage(out Message message, IntPtr.Zero, 0, 0, PM_REMOVE)) - { - if (message.message == WM_QUIT) - { - PostQuitMessage((int)message.wParam); - return; - } - - TranslateMessage(ref message); - DispatchMessage(ref message); - } - } - - private const uint WM_QUIT = 0x0012; - private const uint PM_REMOVE = 0x0001; [DllImport("user32.dll", CharSet = CharSet.Unicode, EntryPoint = "GetMessageW")] private static extern int GetMessage(out Message lpMsg, IntPtr hWnd, uint filterMin, uint filterMax); - [DllImport("user32.dll", CharSet = CharSet.Unicode, EntryPoint = "PeekMessageW")] - private static extern bool PeekMessage( - out Message lpMsg, IntPtr hWnd, uint filterMin, uint filterMax, uint removeMsg); - [DllImport("user32.dll")] private static extern bool TranslateMessage(ref Message lpMsg); diff --git a/CursorLang.Settings.Tests/Interop/NativeWrappersTests.cs b/CursorLang.Settings.Tests/Interop/NativeWrappersTests.cs index 88bb759..1da9e54 100644 --- a/CursorLang.Settings.Tests/Interop/NativeWrappersTests.cs +++ b/CursorLang.Settings.Tests/Interop/NativeWrappersTests.cs @@ -94,23 +94,6 @@ public sealed class NativeWrappersTests }); } - [Fact] - public void The_window_bounds_are_set_as_a_whole() - { - Sta.Run(() => - { - using var window = new HandleWindow(); - - PopupWindowNative.SetBounds(window.Handle, 60, 70, 320, 240); - - PopupWindowNative.Rect bounds = WindowPlacementNative.TryGetBounds(window.Handle)!.Value; - Assert.Equal(60, bounds.Left); - Assert.Equal(70, bounds.Top); - Assert.Equal(380, bounds.Right); - Assert.Equal(310, bounds.Bottom); - }); - } - [Fact] public void The_work_area_is_found_by_the_rectangle_of_the_window() { @@ -139,21 +122,6 @@ public sealed class NativeWrappersTests Assert.True(work.Value.Right > work.Value.Left); } - [Fact] - public void A_window_becomes_invisible_to_the_focus_and_the_switcher() - { - Sta.Run(() => - { - using var window = new HandleWindow(); - - PopupWindowNative.MakePassive(window.Handle); - - // Checked through the same wrapper: the style has to stick and not - // be reset by a repeated call - PopupWindowNative.MakePassive(window.Handle); - }); - } - [Fact] public void The_layout_of_the_foreground_window_is_read() {