Move all blink handling into Renderer #19330

lhecker · 2025-09-08T19:39:36Z

Note that this PR is not quite finished (~90%?). Missing:

Some polish in both conhost & particularly WT integration (e.g. caching the blink interval value)
Propagating the user blink setting on the WT side

This PR moves the cursor blinker and VT blink rendition timer into Renderer. To do so, this PR introduces a generic timer system with which you can schedule arbitrary timer jobs. Thanks to this, this PR removes a crapton of code, particularly throughout conhost, and improves throughput by another ~10%. On my PC it can now churn through >400MB/s while rendering at 240FPS. Fun fact: Processing 100kB of text and rendering it in conhost now takes less time than MSCTF (TSF) needs to process 1 keyboard character. When I look at our shell code.

Validation Steps Performed

TBD

src/interactivity/win32/AccessibilityNotifier.cpp

src/renderer/base/renderer.cpp

Goal: Remove `CursorBlinker`. Problem: Spooky action at a distance via `Cursor::HasMoved`. Solution: Moved all the a11y event raising into `_stream.cpp` and pray for the best. Goal: Prevent node.js from tanking conhost performance via MSAA (WHY). Problem: `ServiceLocator`. Solution: Unserviced the locator. Debounced event raising. Performance increased by >10x. Problem 2: Lots of files changed. This PR is a prerequisite for #19330 ## Validation Steps Performed Ran NVDA with and without UIA enabled and with different delays. ✅

carlos-zamora · 2025-09-23T19:11:23Z

src/cascadia/TerminalControl/HwndTerminal.hpp

 __declspec(dllexport) HRESULT _stdcall TerminalCalculateResize(_In_ void* terminal, _In_ til::CoordType width, _In_ til::CoordType height, _Out_ til::size* dimensions);
 __declspec(dllexport) void _stdcall TerminalDpiChanged(void* terminal, int newDpi);
 __declspec(dllexport) void _stdcall TerminalUserScroll(void* terminal, int viewTop);
-__declspec(dllexport) void _stdcall TerminalClearSelection(void* terminal);


@DHowett Curious. Since VS is a consumer here, do we just notify them of these changes? Is there anything special we have to be aware of with the partnership?

carlos-zamora · 2025-09-23T19:13:43Z

src/cascadia/TerminalControl/HwndTerminal.cpp

-void _stdcall TerminalClearSelection(void* terminal)
-try
-{
-    const auto publicTerminal = static_cast<HwndTerminal*>(terminal);
-    const auto lock = publicTerminal->_terminal->LockForWriting();
-    publicTerminal->_ClearSelection();
-}
-CATCH_LOG()
-


Is there a reason we're removing this one? Doesn't seem related to the cursor blink changes, and it's removing an API, right?

So, the good news about all of the Terminal* APIs inside HwndTerminal.cpp is that they are all internal. They are a contract between the WPF control and the TermControl. We can change them practically at-will. :)

carlos-zamora · 2025-09-23T19:15:41Z

src/cascadia/TerminalControl/HwndTerminal.cpp


-void _stdcall TerminalSetCursorVisible(void* terminal, const bool visible)
-try
+void HwndTerminal::_setFocused(bool focused) noexcept


Big fan of this. It's nice that it's all in one function 😊

carlos-zamora · 2025-09-23T19:48:32Z

src/renderer/base/renderer.cpp

-#define FOREACH_ENGINE(var)   \
-    for (auto var : _engines) \
-        if (!var)             \
-            break;            \
-        else


Huh. We don't need the if (!var) break anymore?

carlos-zamora · 2025-09-23T20:04:57Z

src/renderer/base/renderer.cpp

+    else if (!IsTimerRunning(_cursorBlinker))
+    {
+        const auto actual = GetTimerInterval(_cursorBlinker);
+        auto expected = _pData->GetBlinkInterval();


Suggested change

auto expected = _pData->GetBlinkInterval();

const auto expected = _pData->GetBlinkInterval();

const?

carlos-zamora · 2025-09-23T21:20:20Z

src/host/renderData.cpp

+
+bool RenderData::IsUiaDataInitialized() const noexcept
+{
+    return true;


(Double checking) In terminalrenderdata.cpp, we have this:

terminal/src/cascadia/TerminalCore/terminalrenderdata.cpp

Lines 226 to 234 in 1b712b2

bool Terminal::IsUiaDataInitialized() const noexcept

{

// GH#11135: Windows Terminal needs to create and return an automation peer

// when a screen reader requests it. However, the terminal might not be fully

// initialized yet. So we use this to check if any crucial components of

// UiaData are not yet initialized.

_assertLocked();

return !!_mainBuffer;

}

We don't need to check the buffer here?

carlos-zamora · 2025-09-23T21:22:25Z

src/host/output.cpp

-    // initialize cursor GH#4102 - Typically, the cursor is set to on by the
-    // cursor blinker. Unfortunately, in conpty mode, there is no cursor
-    // blinker. So, in conpty mode, we need to leave the cursor on always. The
-    // cursor can still be set to hidden, and whether the cursor should be
-    // blinking will still be passed through to the terminal, but internally,
-    // the cursor should always be on.
-    //
-    // In particular, some applications make use of a calling
-    // `SetConsoleScreenBuffer` and `SetCursorPosition` without printing any
-    // text in between these calls. If we initialize the cursor to Off in conpty
-    // mode, then the cursor will remain off until they print text. This can
-    // lead to alignment problems in the terminal, because we won't move the
-    // terminal's cursor in this _exact_ scenario.
-    screenInfo.GetTextBuffer().GetCursor().SetIsOn(gci.IsInVtIoMode());
-


Hmm... Do we need to worry about this scenario still?

carlos-zamora · 2025-09-23T21:23:49Z

src/terminal/adapter/adaptDispatch.cpp

        // We trigger a scroll rather than a redraw, since that's more efficient,
        // but we need to turn the cursor off before doing so; otherwise, a ghost
        // cursor can be left behind in the previous position.


update the comment?

github-advanced-security bot found potential problems Sep 8, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

lhecker force-pushed the dev/lhecker/cursor-blinker branch from 4de989d to 8d17e85 Compare September 8, 2025 19:44

lhecker mentioned this pull request Sep 12, 2025

Rewrite the MSAA/UIA integration into conhost #19344

Merged

lhecker changed the base branch from main to dev/lhecker/BEGONE-FOUL-SPIRIT September 14, 2025 11:16

lhecker force-pushed the dev/lhecker/cursor-blinker branch from 8d17e85 to 8390d60 Compare September 14, 2025 11:26

lhecker force-pushed the dev/lhecker/cursor-blinker branch from 8390d60 to 5742c64 Compare September 22, 2025 22:59

lhecker changed the base branch from dev/lhecker/BEGONE-FOUL-SPIRIT to main September 22, 2025 22:59

lhecker marked this pull request as ready for review September 22, 2025 22:59

Move all blink handling into Renderer

1c23430

lhecker force-pushed the dev/lhecker/cursor-blinker branch from 5742c64 to 1c23430 Compare September 23, 2025 13:55

lhecker mentioned this pull request Sep 23, 2025

WPF terminal control in a tabbed interface deadlocks when rapidly opening new tabs #19373

Open

Fix tests?

1b712b2

carlos-zamora reviewed Sep 23, 2025

View reviewed changes

Fix tests

1c07227

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Move all blink handling into Renderer #19330

Move all blink handling into Renderer #19330

Uh oh!

lhecker commented Sep 8, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

carlos-zamora Sep 23, 2025

Uh oh!

DHowett Sep 24, 2025

Uh oh!

carlos-zamora Sep 23, 2025

Uh oh!

DHowett Sep 24, 2025

Uh oh!

carlos-zamora Sep 23, 2025

Uh oh!

carlos-zamora Sep 23, 2025

Uh oh!

carlos-zamora Sep 23, 2025

Uh oh!

carlos-zamora Sep 23, 2025

Uh oh!

carlos-zamora Sep 23, 2025

Uh oh!

carlos-zamora Sep 23, 2025

Uh oh!

Uh oh!

	auto expected = _pData->GetBlinkInterval();
	const auto expected = _pData->GetBlinkInterval();

	bool Terminal::IsUiaDataInitialized() const noexcept
	{
	// GH#11135: Windows Terminal needs to create and return an automation peer
	// when a screen reader requests it. However, the terminal might not be fully
	// initialized yet. So we use this to check if any crucial components of
	// UiaData are not yet initialized.
	_assertLocked();
	return !!_mainBuffer;
	}

Move all blink handling into Renderer #19330

Are you sure you want to change the base?

Move all blink handling into Renderer #19330

Uh oh!

Conversation

lhecker commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Validation Steps Performed

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lhecker commented Sep 8, 2025 •

edited

Loading