Repository navigation
Go back to using implementation types in TermControl - #20721
Merged
Dustin L. Howett (DHowett) merged 9 commits intoOct 7, 2026
Merged
Dustin L. Howett (DHowett) merged 9 commits into
Dustin L. Howett (DHowett) merged 9 commits into
Conversation
| _lastSwapChainHandle = std::move(duplicatedHandle); | ||
| // Now bubble the event up to the control. | ||
| SwapChainChanged.raise(*this, winrt::box_value<uint64_t>(reinterpret_cast<uint64_t>(_lastSwapChainHandle.get()))); | ||
| SwapChainChanged.raise(*this, nullptr); |
Member
Author
There was a problem hiding this comment.
change: the recipient will just call .SwapChainHandle on us...
This comment has been minimized.
This comment has been minimized.
Dustin L. Howett (DHowett)
force-pushed
the
dev/duhowett/stop-using-projection-for-control-innards
branch
from
September 29, 2026 22:22
a752604 to
8067dca
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Dustin L. Howett (DHowett)
force-pushed
the
dev/duhowett/stop-using-projection-for-control-innards
branch
from
September 29, 2026 23:29
2ba16ab to
dd04072
Compare
This comment has been minimized.
This comment has been minimized.
Dustin L. Howett (DHowett)
force-pushed
the
dev/duhowett/stop-using-projection-for-control-innards
branch
from
September 29, 2026 23:33
dd04072 to
307abbb
Compare
Prevents us from having to use projected types just to see autorevokers.
Dustin L. Howett (DHowett)
force-pushed
the
dev/duhowett/stop-using-projection-for-control-innards
branch
from
September 29, 2026 23:45
307abbb to
b8ea704
Compare
Leonard Hecker (lhecker)
approved these changes
Oct 7, 2026
Leonard Hecker (lhecker)
left a comment
Member
There was a problem hiding this comment.
The middle comment may be worth addressing.
| _automationPeer = winrt::make<implementation::TermControlAutomationPeer>(get_strong(), padding, interactivityAutoPeer); | ||
| return _automationPeer; | ||
| } | ||
| _automationPeer = winrt::make_self<implementation::TermControlAutomationPeer>(get_strong()); |
There was a problem hiding this comment.
This reference cycle was intentional right?
Member
Author
There was a problem hiding this comment.
yes, TCAP stores it as a weak_ref actually but it needs the full realized pointer to hand to the parent class (inside xaml)
| { | ||
| return { til::math::rounding, control->_core->FontSize() }; | ||
| } | ||
| return {}; |
There was a problem hiding this comment.
Should probably """return""" UIA_E_ELEMENTNOTAVAILABLE, because other parts of the peer call GetFontSize() and would otherwise div-by-zero.
Dustin L. Howett (DHowett)
deleted the
dev/duhowett/stop-using-projection-for-control-innards
branch
October 7, 2026 22:32
Dustin L. Howett (DHowett)
added a commit
that referenced
this pull request
Oct 20, 2026
This pull request replaces all of HwndTerminal, which used to be a thin layer on top of TerminalCore.lib, with a new implementation that relies on the Interactivity and Core layers in TerminalControl.lib. We used to spend a lot of time reimplementing interactivity components for HwndTerminal or--if I'm being honest--letting it rot and never giving it the improvements the rest of the application got. Among those improvements: - Automatic scrolling finally works (as of #20710) - Selection works the same way as it does in Terminal proper (starts on the half-cell, uses an exclusive range) - Consistent double- and triple-click behavior without having to reimplement word- and line-wise expansion - Scroll wheel behavior - Shift+Click to suppress VT mouse mode - Scrolling and focus handling move down into the Win32 message loop from WPF This pull request seeks to get HwndTerminal back to where it was (plus the things we naturally get from switching over). In the future, we can more easily add: - title reporting (surprised we didn't have this) - clickable URLs - automatic copy-to-clipboard - right-click menu support - DTTERM window manipulation - shaders - Ctrl+Scroll zoom Implementation Details ---------------------- The WPF control is backed by a flat "C" API, which takes in a `void*` and does some stuff to it. We call that a `PTERM`. References to "Flat C" in this commit body describe the body of work below WPF. X-macros are used to fill out the definintions of every C API and bridge it to a properly typed C++ member function. Ugh. No more crap where we split the implementation into half C export and half C++ member. ControlCore and ControlInteractivity require something that looks like TerminalSettings. The WPF control only supports the most basic settings, in `struct TerminalTheme`, and so I've added a bridge WinRT type to adapt TerminalTheme to IXxxSettings. Most of the values it returns are our "defaults", and some of them are overridden. Core requires an `ITerminalConnection`, but the WPF API uses a pair of "write output" and "send input" callbacks. Flat C provides an ITerminalConnection that bridges those callbacks. It does not communicate connection state or anything else. Resizing is probably going to be a problem. Core requires a dispatcher queue to ensure that some operations happen on the UI thread. I have stubbed out an implementation of `IDispatcherQueue` that calls a couple function pointers with a context pointer; the WPF code hooks up to that and bridges it into the WPF dispatcher. Yes, the Core's dispatcher queue _is_ the WPF main UI thread. It has to be, otherwise we'll explode the first time we try to update the scroll bar. Prep work was done in #20713. To make the above easier, I removed all use of DispatcherTimer (see #20711). Attempts to use it will almost certainly cause the WPF control to implode. Core needed to be taught a couple tricks to start up targeting an HWND instead of a composition handle. See #20722 for more. Refs #20710 Refs #20711 Refs #20713 Refs #20722 Required #20721 Refs #20745 Related to #6999
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This reverts a large part of #10051 and moves us back to using the native
implementation-namespace types from TermControl to talk to ControlInteractivity and ControlCore.I started down this rabbit hole because I was trying to get rid of InteractivityAutomationPeer, and I learned that we introduced it as a go-between for the process-split TermControl/Interactivity.
InteractviityAutomationPeer is not required, and removing it lets us get rid of some other mild annoyances:
Bounds and padding can simply be read directly from the control.
I also undid the move from HANDLE to uint64_t which was only required for projecting HANDLEs.
The only thing we risked losing out on was automatic event revokers, but this pull request adds
winrt::auto_revokesupport directly totil::event. It is suitably terrifying.