## Summary of the Pull Request
This includes the memory leak fixes that @lhecker and I investigated as
a part of #19710.
The `ITextRangeProvider`s (namely `UiaTextRange`s) weren't being
destroyed after they were done being used by the screen reader.
## Validation Steps Performed
In my own testing, I set a breakpoint on the destructor for
`UiaTextRangeBase`. Prior to this change, that destructor would mainly
be called when the terminal control was closed, which would result in us
leaking these objects. With this change, I've confirmed that these text
ranges are being destroyed immediately after they are done being used
(without needing to close the terminal control).
## PR Checklist
Closes#19710
Interesting changes in this update:
- better support for `REFIID,IUnknown**` in `capture`
- `LOAD_LIBRARY_SEARCH_DEFAULT_DIRS` for all SxS DLL loading
- `get_self` reading from classic COM interfaces (rather than WinRT
ones)
- better incremental builds by ignoring stale winmd files (see
microsoft/cppwinrt#1404)
- some ability to mix c++17 and c++20 static libraries
- better codegen for `consume` methods
This version of C++/WinRT is better about propagating `protected`
fields from the metadata into the C++ projections. This required
us to switch to the `I...Protected` interfaces for some things
we are _technically_ not allowed access to. We also had some
`overridable` (protected!) members of our own that needed undec-
oration.
Looks like there's a new VS version on the build agents. This just goes
through and fixes any issues they found.
There's still a COMPILER CRASH though.
I sure hope I didn't break anything!
While `til::math` was a good idea its convenience led us to use it
in the only place where it absolutely must not be used: The UI code.
So, this PR replaces all those `til::point`s, etc., with floats.
Now we use DIPs consistently throughout all layers of the UI code,
except for the UIA area (that would've required too many changes).
## Validation Steps Performed
Launch, looks good, no obvious defects, UIA positioning seems ok. ✅
While `double` is probably generally preferable for UI code,
our application is essentially a complex wrapper wrapper around
DWrite, D2D and D3D, all of which use `float` exclusively.
Of course it also uses XAML, but that one uses `float` for roughly
1/3rd of its API functions, so I'm not sure what it prefers.
Additionally, it's mostly a coincidence that we use WinUI/XAML for
Windows Terminal whereas DWrite/D2D/D3D are effectively essential.
This is demonstrated by the fact that we have a `HwndTerminal`,
while there's no alternative to e.g. D3D on Windows.
The goal of this PR is that DIP based calculations never end up
mixing `float` and `double`. This PR also changes opacity-related
values to `float` because I felt like that fits the theme.
This PR automagically finds and replaces all[^1] usages of our
TYPED_EVENT macro with `til::event`. Benefits include:
* less macro magic
* editors are more easily able to figure out the relationship between
`til::event<> Foo;`, `Foo.raise(...)`, and `bar.Foo({this,
&Bar::FooHandler})` (whereas before the relationship between
`_FooHandlers(...)` and `bar.Foo({...})`) couldn't be figured out by
vscode & sublime.
Other find & replace work that had to be done:
* I added the `til::typed_event<>` == `<IInspectable, IInspectable>`
thing from #16170, since that is goodness
* I actually fixed `til::property_changed_event`, so you can use that
for your property changed events. They're all the same anyways.
* events had to come before `WINRT_PROPERTY`s, since the latter macro
leaves us in a `private:` block
* `Pane::SetupChildCloseHandlers` I had to swap _back_, because the
script thought that was an event 🤦
* `ProfileViewModel::DeleteProfile` had to be renamed
`DeleteProfileRequested`, since there was already a `DeleteProfile`
method on it.
* WindowManager.cpp was directly wiring up it's `winrt::event`s to the
monarch & peasant. That doesn't work with `til::event`s and I'm kinda
surprised it ever did
<details>
<summary>The script in question</summary>
```py
import os
import re
def replace_in_file(file_path, file_name):
with open(file_path, 'r', encoding="utf8") as file:
content = file.read()
found_matches = False
# Define the pattern for matching
pattern = r' WINRT_CALLBACK\((\w+),\s*(.*?)\);'
event_matches = re.findall(pattern, content)
if event_matches:
found_matches = True
print(f'found events in {file_path}:')
for match in event_matches:
name = match[0]
args = match[1]
if name == "newConnection" and file_name == "ConptyConnection.cpp":
# This one is special
continue
old_declaration = 'WINRT_CALLBACK(' + name + ', ' + args + ');'
new_declaration = 'til::event<' + args + '> ' + name + ';' if name != "PropertyChanged" else 'til::property_changed_event PropertyChanged;'
print(f' {old_declaration} -> {new_declaration}')
content = content.replace(old_declaration, new_declaration)
typed_event_pattern = r' TYPED_EVENT\((\w+),\s*(.*?)\);'
typed_matches = re.findall(typed_event_pattern, content)
if typed_matches:
found_matches = True
print(f'found typed_events in {file_path}:')
for match in typed_matches:
name = match[0]
args = match[1]
if name == "newConnection" and file_name == "ConptyConnection.cpp":
# This one is special
continue
old_declaration = f'TYPED_EVENT({name}, {args});'
was_inspectable = (args == "winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable" ) or (args == "IInspectable, IInspectable" )
new_declaration = f'til::typed_event<{args}> {name};' if not was_inspectable else f"til::typed_event<> {name};"
print(f' {old_declaration} -> {new_declaration}')
content = content.replace(old_declaration, new_declaration)
handlers_pattern = r'_(\w+)Handlers\('
handler_matches = re.findall(handlers_pattern, content)
if handler_matches:
found_matches = True
print(f'found handlers in {file_path}:')
for match in handler_matches:
name = match
if name == "newConnection" and file_name == "ConptyConnection.cpp":
# This one is special
continue
old_declaration = f'_{name}Handlers('
new_declaration = f'{name}.raise('
print(f' {old_declaration} -> {new_declaration}')
content = content.replace(old_declaration, new_declaration)
if found_matches:
with open(file_path, 'w', encoding="utf8") as file:
file.write(content)
def find_and_replace(directory):
for root, dirs, files in os.walk(directory):
if 'Generated Files' in dirs:
dirs.remove('Generated Files') # Exclude the "Generated Files" directory
for file in files:
if file.endswith('.cpp') or file.endswith('.h') or file.endswith('.hpp'):
file_path = os.path.join(root, file)
try:
replace_in_file(file_path, file)
except Exception as e:
print(f"error reading {file_path}")
if file == "TermControl.cpp":
print(e)
# raise e
# Replace in files within a specific directory
directory_path = 'D:\\dev\\public\\terminal\\src'
find_and_replace(directory_path)
```
</details>
[^1]: there are other macros we use that were also using this macro,
those weren't replaced.
---------
Co-authored-by: Dustin Howett <duhowett@microsoft.com>
Co-authored-by: Leonard Hecker <lhecker@microsoft.com>
My goal is to make `IRenderData` "snapshottable", so that we
can render a frame of text without holding the console lock.
To facilitate this, this commit merges our 3 data interfaces
into one. It includes no actual changes apart from renames.
This is a follow-up of #13025 to make the members of `til::point/size/rect`
uniform and consistent without the use of `unions`. The only file that has
any changes is `src/host/getset.cpp` where an if condition was simplified.
## Validation Steps Performed
* Host unit tests ✅
* Host feature tests ✅
* ControlCore feature tests ✅
Previously this project used a great variety of types to present text buffer
coordinates: `short`, `unsigned short`, `int`, `unsigned int`, `size_t`,
`ptrdiff_t`, `COORD`/`SMALL_RECT` (aka `short`), and more.
This massive commit migrates almost all use of those types over to the
centralized types `til::point`/`size`/`rect`/`inclusive_rect` and their
underlying type `til::CoordType` (aka `int32_t`).
Due to the size of the changeset and statistics I expect it to contain bugs.
The biggest risk I see is that some code potentially, maybe implicitly, expected
arithmetic to be mod 2^16 and that this code now allows it to be mod 2^32.
Any narrowing into `short` later on would then throw exceptions.
## PR Checklist
* [x] Closes#4015
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
Casual usage of OpenConsole and Windows Terminal. ✅
#4015 requires sweeping changes in order to allow a migration of our buffer
coordinates from `int16_t` to `int32_t`. This commit reduces the size of
future commits by using type inference wherever possible, dropping the
need to manually adjust types throughout the project later.
As an added bonus this commit standardizes the alignment of cv qualifiers
to be always left of the type (e.g. `const T&` instead of `T const&`).
The migration to type inference with `auto` was mostly done
using JetBrains Resharper with some manual intervention and the
standardization of cv qualifier alignment using clang-format 14.
## References
This is preparation work for #4015.
## Validation Steps Performed
* Tests pass ✅
## Summary of the Pull Request
This change makes Windows Terminal raise a `RaiseNotificationEvent()` ([docs](https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.automation.peers.automationpeer.raisenotificationevent?view=winrt-22000)) for new text output to the buffer.
This is intended to help Narrator identify what new output appears and reduce the workload of diffing the buffer when a `TextChanged` event occurs.
## Detailed Description of the Pull Request / Additional comments
The flow of the event occurs as follows:
- `Terminal::_WriteBuffer()`
- New text is output to the text buffer. Notify the renderer that we have new text (and what that text is).
- `Renderer::TriggerNewTextNotification()`
- Cycle through all the rendering engines and tell them to notify handle the new text output.
- None of the rendering engines _except_ `UiaEngine` has it implemented, so really we're just notifying UIA.
- `UiaEngine::NotifyNewText()`
- Concatenate any new output into a string.
- When we're done painting, tell the notification system to actually notify of new events occurring and clear any stored output text. That way, we're ready for the next renderer frame.
- `InteractivityAutomationPeer::NotifyNewOutput()` --> `TermControlAutomationPeer::NotifyNewOutput`
- NOTE: these are split because of the in-proc and out-of-proc separation of the buffer.
- Actually `RaiseNotificationEvent()` for the new text output.
Additionally, we had to handle the "local echo" problem: when a key is pressed, the character is said twice (once for the keyboard event, and again for the character being written to the buffer). To accomplish this, we did the following:
- `TermControl`:
- here, we already handle keyboard events, so I added a line saying "if we have an automation peer attached, record the keyboard event in the automation peer".
- `TermControlAutomationPeer`:
- just before the notification is dispatched, check if the string of recent keyboard events match the beginning of the string of new output. If that's the case, we can assume that the common prefix was the "local echo".
This is a fairly naive heuristic, but it's been working.
Closes the following ADO bugs:
- https://dev.azure.com/microsoft/OS/_workitems/edit/36506838
- (Probably) https://dev.azure.com/microsoft/OS/_workitems/edit/38011453
## Test cases
- [x] Base case: "echo hello"
- [x] Partial line change
- [x] Scrolling (should be unaffected)
- [x] Large output
- [x] "local echo": keyboard events read input character twice
This commit makes the following changes to `til::point/size/rectangle`
for the following reasons:
* Rename `rectangle` into `rect`
This will make the naming consistent with a later `small_rect` struct
as well as the existing Win32 POINT/SIZE/RECT structs.
* Standardizes til wrappers on `int32_t` instead of `ptrdiff_t`
Provides a consistent behavior between x86 and x64, preventing accidental
errors on x86, as it's less rigorously tested than x64. Additionally it
improves interop with MIDL3 which only supports fixed width integer types.
* Standardizes til wrappers on throwing `gsl::narrow_error`
Makes the behavior of our code more consistent.
* Makes all eligible functions `constexpr`
Because why not.
* Removes implicit constructors and conversion operators
This is a complex and controversial topic. My reasons are: You can't Ctrl+F
for an implicit conversion. This breaks most non-IDE engines, like the one on
GitHub or those we have internally at MS. This is important for me as these
implicit conversion operators aren't cost free. Narrowing integers itself,
as well as the boundary checks that need to be done have a certain,
fixed overhead each time. Additionally the lack of noexcept prevents
many advanced compiler optimizations. Removing their use entirely
drops conhost's code segment size by around ~6.5%.
## References
Preliminary work for #4015.
## PR Checklist
* [x] I work here
* [x] Tests added/passed
## Validation Steps Performed
I'm mostly relying on our unit tests here. Both OpenConsole and WT appear to work fine.
FontInfoBase and it's descendents are missing noexcept annotations, which
virally forces other code to not be noexcept as well during AuditMode checks.
Apart from adding noexcept, this commit also
* Passes std::wstring_view by reference.
* Pass the FillLegacyNameBuffer argument as a simple pointer-to-array,
allowing us to fill the buffer with a single memcpy.
(gsl::span's iterators inhibit any internal STL optimizations.)
* Move operator== declarations inside the class to reduce code size.
All other changes are an effect of the virality of noexcept.
This is an offshoot from #11623.
## Validation Steps Performed
* It still compiles ✔️
## Summary of the Pull Request
As a part of the Interactivity split, `TermControlAutomationPeer` had to be split into `TermControlAutomationPeer` (TCAP) and `InteractivityAutomationPeer` (IAP). Just about all of the functions in `InterativityAutomationPeer` operate by calling the non-XAML UIA Provider then wrapping the resulting `UIATextRange` into a XAML format (a `XamlUiaTextRange` [XUTR]). As a part of that XUTR constructor, we need a reference to the parent provider.
We generally get that via `ProviderFromPeer()`, but IAP's `ProviderFromPeer()` returned null (presumably because IAP isn't in the UI tree, whereas TCAP is directly registered as the automation peer for the `TermControl`).
It looks like some screen readers didn't care (like NVDA, though there may be a chance we just didn't encounter an issue just yet), but Narrator definitely did.
The fix was to provide XUTR constructors the `ProviderFromPeer` from TCAP, _not_ IAP. To accomplish this, IAP now holds a weak reference to TCAP, and provides the `ProviderFromPeer` when needed. We can't cache this result because there is no guarantee that it won't change.
Some miscellaneous changes include:
- `TermControl::OnCreateAutomationPeer` now returns the existing auto peer instead of always creating a new one
- `TCAP::WrapArrayOfTextRangeProviders` was removed as it was unused (normally, this would be directly affected by the main `ProviderFromPeer` change here)
- `XUTR::GetEnclosingElement` is now hooked up to trace logging for debugging purposes
## References
Introduced in #10051Closes#11488
## Validation Steps Performed
✅ Narrator scan mode now works (verified with character, word, and line navigation)
✅ NVDA movement still works (verified with word and line navigation)
## Summary of the Pull Request
This forces the `TermControl` to only use `ControlCore` and `ControlInteractivity` via their WinRT projections. We want this, because WinRT projections can be used across process boundaries. In the future, `ControlCore` and `ControlInteractivity` are going to be living in a different process entirely from `TermControl`. By enforcing this boundary now, we can make sure that they will work seamlessly in the future.
## References
* Tear-out: #1256
* Megathread: #5000
* Project: https://github.com/microsoft/terminal/projects/5
## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/projects/5#card-50760270
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated
## Detailed Description of the Pull Request / Additional comments
Most all this was just converting pure c++ types to winrt types when possible. I've added a couple helper projections with `til` converters, which made most of this really easy.
The "`MouseButtonState` needs to be composed of `Int32`s instead of `bool`s" is MENTAL. I have no idea why this is, but when I had the control OOP in the sample, that would crash when trying to de-marshal the bools. BODGY.
The biggest changes are in the way the UIA stuff is hooked up. The UiaEngine needs to be attached directly to the `Renderer`, and it can't be easily projected, so it needs to live next to the `ControlCore`. But the `TermControlAutomationPeer` needed the `UiaEngine` to help implement some interfaces.
Now, there's a new layer we've introduced. `InteractivityAutomationPeer` does the `ITextProvider`, `IControlAccessibilityInfo` and the `IUiaEventDispatcher` thing. `TermControlAutomationPeer` now has a
`InteractivityAutomationPeer` stashed inside itself, so that it can ask the interactivity layer to do the real work. We still need the `TermControlAutomationPeer` though, to be able to attach to the real UI tree.
## Validation Steps Performed
The terminal behaves basically the same as before.
Most importantly, I whipped out Accessibility Insights, and the Terminal looks the same as before.
