The absolute disgrace of a fix called 65b71ff failed to account for `std::move`
being unsafe to use for overlapping ranges. While `std::move` works for trivial
types (it happens to delegate to `memmove`), we need to dynamically switch
between that and `std::move_backward` to be correct.
Without this fix the LRU refresh is incorrect and might lead to crashes.
## Validation Steps Performed
I'm working on a new, pure D2D renderer inside AtlasEngine, which uses
the iterators contained in `_r.cellGlyphMapping` to draw text.
I noticed the bug, because scrolling up caused the text to be garbled
and with this fix applied it works as expected.
Fixes MSFT:38775539
Might also fix MSFT:38614563
Looking at this code should be pretty clear what's going on. On exit, the XAML root is already nulled out. But here, we're just yolo'ing and assuming it exists (why wouldn't it). So yea. This is like weirdly high percent of crashes internally, but that's not from real users. Real users, I suspect hit this as like .3% of our crashes. Not zero, but _low_.
* [x] tested manually
<hr>
May also be related to...
* MSFT:40602905
* MSFT:40602904
* MSFT:40412800
* MSFT:35213459 <---has links
Fixes MSFT:40853556
There's a small race here. The renderer thread in ConPTY might notice the terminal is gone, call CloseOutput, and release the vt renderer, and then the window proc fires and decides to minimize/restore the window, triggering an A/V.
I'm 100% confident that this has NEVER happened to a real user. But the test labs hit it so much that it makes up ~26% of our crashes.
I haven't tested this cause again, _it doesn't hit in the wild_
Rename `til::form_wchars` to `til::to_ulong` and
allow it to work with narrow `char`s.
This change will be used in #13429.
## Validation Steps Performed
* Loads `sc(...)` key bindings as expected ✅
* The change is thankfully fairly trivial if viewn with whitespace suppressed
We shouldn't add URLs into our binaries that we can't directly control.
This commit fixes the issue for URLs recently introduced in #13510.
Closes#13541
## Validation Steps Performed
This change is trivial enough that I simply opened the new redirects
in my browser, ensuring that they open the expected websites.
`debugGlyphGenerationPerformance` used to only test the performance of
text segmentation/parsing, so I renamed it to `debugTextParsingPerformance`.
The new `debugGlyphGenerationPerformance` actually clears the glyph atlas now.
Additionally this fixes a bug with `debugGeneralPerformance`:
If a `DXGI_SWAP_CHAIN_FLAG_FRAME_LATENCY_WAITABLE_OBJECT` is requested,
it needs to be used. Since `debugGeneralPerformance` is for testing without
V-Sync, we need to ensure that the waitable object is properly disabled.
## Summary of the Pull Request
Skips whitespace removal if pasted string is multiline
## PR Checklist
* [x] Closes#12387
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
## Validation Steps Performed
Tests passed
The last command in multiline paste now executes
Does two things:
* the first two commits: shakes up the way we reference MUX in our projects so we can actually just
```xml
<PropertyGroup Label="NuGet Dependencies">
<TerminalMUX>true</TerminalMUX>
</PropertyGroup>
```
Like every other dependency we have
* the last commit: update to MUX.2.7.3
This is the 1.14 PR, which should be appropriately cherry-picked through to `release-1.15` and `main`
(cherry picked from commit a277b56f6a)
Repurpose `Feature_AtlasEngine` to enable the atlas engine by default in Dev and Preview builds.
Introduce `Feature_ConhostAtlasEngine` to solely control atlas engine inclusion in conhost.
Closes#13745
We have a number of theories why #12607 is happening, one of which is that
some GPU drivers somehow rely on Win32 messages or similar which we process
on the main thread. If we then try to acquire the console lock on the main
thread, while the GPU-driver thread itself is holding that lock, we've got
ourselves a deadlock. This PR makes this less likely by running the repeat
offender `UpdatePatternLocations` on a background thread instead.
We have a number of other locations which acquire the console lock on the
main thread and a thorough bug fix must be done in a different way.
## Validation Steps Performed
* After pasting an URL it gets underlined on hover ✅
Currently "Inverse Cursor" is actually simply bitwise inversed.
It works fine, except when it does not, namely in the middle of the spectrum:
Anything close enough to dark grey (index 8, RGB(128, 128, 128) in the
classic palette) will for obvious reasons become almost the same dark grey
again after the inversion.
The issue is addressed by additionally `XOR`ing the inverted color with
RGB(63, 63, 63). This distorts the result enough to avoid collisions
in the middle. Ultimately this restores the behavior that was in
Windows Console since the Middle Ages (and still exists in ConhostV1.dll).
## PR Checklist
* [x] Closes#3647
* [x] CLA signed
* [x] Tests added/passed
* [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #3647
## Validation Steps Performed
1. Open OpenConsole.
2. Properties, Terminal, Cursor Colors, choose Inverse Color.
3. Optionally set the RGB value of the 8th color in the standard palette to RGB(128, 128, 128) on the Colors tab, but the default one will do too.
4. Type `color 80` to see black text on dark grey background.
5. Make sure the inverted cursor is visible.
6. Repeat in WT with both default and experimental renderers.
## Summary of the Pull Request
Changes the way `_addCommandsForArg` determines if the delimiter was at the beginning of the argument so that it accounts for the fact that match includes the last character of the string before it.
## PR Checklist
* [x] Closes#13277
## Validation Steps Performed
`wt -p "u"; nt -p "u"` does not cause an error
This commit builds directly on the changes made in #13677 and fixes:
* TSF resetting to AlphaNumeric ("ASCII") input mode when pressing enter
* Vietnamese IME not composing a new word after pressing whitespace, etc.
Closes#11479Closes#13398
## Validation Steps Performed
* Japanese IME (Full-Width Katakana)
Typing "saitama" produces "サイタマ" ✅
* Korean IME
Typing "gksrmf" produces "한글" ✅
* Vietnamese IME
Typing "xin chaof" produces "xin chào" ✅
* Emoji Picker (Win+.)
✅
Adds a variable `_isBackgroundLight` that is updated when the background
color is changed. When it is `true`, the BEL indicator flash will darken
the screen instead of brightening.
`_isColorLight(bg)` returns `true` if the average of `r`, `g`, and `b`
is >127
I was unsure of an appropriate way to change the color of the
`CompositionLight` based on the background, so I changed it to always be
gray and adjusted the intensity values of the original animation to have
roughly the same visual effect as the white.
## Validation Steps Performed
* Tested the two flashes on the default color schemes and some custom
background colors to see if they look consistent
* Used tracepoints and visual to check that the right animation is used
(including multiple tabs, split windows with different themes, and
changing settings while window is open)
References #9270Closes#13450
This pull request reintroduces aliases for `VkKeyScanW`,
`MapVirtualKeyW` and `GetKeyState` that redirect through ConIoSrv on
OneCore devices.
We made an assertion in PR !7096375 that those APIs were hosted in an
extension APIset that was present on all OneCore devices. It turned out
that this was _incorrect_: that APIset extension is only hosted on
OneCoreUAP and above.
This would not be a problem save for NanoServer. NanoServer is built on
top of OneCore.
As Nano is a container host OS, it is primarily interfaced vith via
ConPTY... which exercises the VkKeyScanW/MapVirtualKeyW codepaths quite
a bit. Those APIs started returning invalid data, which caused us to
convert all incoming keyboard events into numpad events. This didn't
prove to be an issue for CMD or PowerShell (weirdly,) but it did prove
to be an issue for Redis. Unfortunately, Redis is exactly the sort of
thing you might want to run in a container.
Reintroducing these aliases was complicated because we took the
opportunity to remove all of IInputServices (!7105348), which was a
wrapper around some code that would choose Win32 or OneCore depending on
the runtime environment.
I made the choice (with the help of Leonard Hecker) to reimplement these
functions in a different way: always call the delay-loaded version, and
then on OneCore editions check the return value and error code to ssee
if we hit a delay load failure. It incurs a minor cost, but all of the
delay loads are in-proc and do not require us to make a syscall, so that
cost is negligible.
Part of this new implementation requires us to change _all conhost
internal callers_ to use "OneCoreSafe" versions of those APIs. We can't
redirect the user32 versions out of the way and usurp their import
symbols, so this commit also introduces some warning defines. If you use
VkKeyScanW (and friends), you _should_ get a linker error. Assuming
HostAndPropsheetIncludes has been included. It very well may not have
been included.
Fixes MSFT-40435912
Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev 949e8dfc07f122520c6a74412329a6f7e77d19c5
wyhash was chosen based on the results found in `smhasher`, were it proved
itself as an algorithm with little flaws and fairly high output quality.
While I have a personal preference for xxhash (XXH3 specifically), wyhash is a
better fit for this project as its source code is multiple magnitudes smaller,
simplifying the review and integration into the header-only `hash.h` file.
For use with hashmaps the hash quality doesn't actually matter much for
optimal performance and instead the binary size usually matters more.
But even in that scenario wyhash is fairly close to FNV1a (aka "FNV64").
The result is that this new hash algorithm will only have little impact on
hashmap performance if used over the standard FNV1a as used in the STL,
while simultaneously offering a vastly better hash quality.
This partially solves #13124.
## Validation Steps Performed
* Added test cases ✅
Other settings model classes call `JsonUtils::SetValueForKey` with the
private `_##value` member as the value. Since `_##value` is an optional,
this prevented writing out unset, optional fields. The new `Themes` class
deviated from this and this commit brings it back in line with the others.
Closes#13544
## Validation Steps Performed
* Create a `{ "name": "test" }` theme
* Save settings via the UI
* Optional/Defaulted fields aren't written ✅
## Summary of the Pull Request
VC++ v14 Descktop Framework package is required and not installed automatically when installing package manually.
## PR Checklist
* [X] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
While working on #13398 I felt that `TSFInputControl` wasn't up to sniff.
This commit is a minor cleanup of the class:
* default member initializers
* Simplified use of STL classes which already perform boundary checks
* Correctly check text buffer emptiness in `_SendAndClearText`
* Track selection range as mandated by the API
## Validation Steps Performed
* Japanese IME (Full-Width Katakana)
Typing "saitama" produces "サイタマ" ✅
* Korean IME
Typing "gksrmf" produces "한글" ✅
* Vietnamese IME
Typing "xin chaof" continues to produce broken "xin xinchaof"
(It's supposed to produce "xin chào")
* Emoji Picker (Win+.)
✅
## Summary of the Pull Request
- We only ever have 1 color picker now, instead of each tab having its own
- `TerminalPage` constructs this color picker (upon first request for it)
- `TerminalPage` attaches the color picker to the tab that requested for it
- `TerminalTab` detaches the color picker when it is done with it, so that `TerminalPage` can attach it to another tab later on
## References
#5907
## Validation Steps Performed
User-end behaviour is the same
We must use 65535 as `MAX_PARAMETER_VALUE` in order for us to properly parse
win32-input-mode sequences, which transmit UTF-16 characters as parameters.
Closes#12977
## Validation Steps Performed
* Call `SendInput` with 🙁 (`L'\xD83D'`, `L'\xDE41'`)
* 🙁 appears on the input line ✅
## Summary of the Pull Request
In #13560 we added a member to `Pane` that lets it know if it was spawned as a default terminal session, but did not propagate that value when the pane gets split or when the pane closes. This commit fixes that.
## Validation Steps Performed
A session spawned by a def term invocation remembers it even as it goes through splits
## Summary of the Pull Request
In general, when a selection marker is shown, we should scroll to it. The `selectAll` action adds a selection marker, but we don't scroll to it. This PR makes it such that we do do that.
Epic: #4993Closes#13485
Updates the schema such that the scroll mark settings are defined as profile settings instead of global settings (because they're actually profile settings).
Separately (but still relevant), I've also updated the release notes.
Closes#13583
This fixes an issue were overwriting parts of a row would only trigger
that specific portion of the row to be redrawn. This isn't just
problematic for combining characters, but also for things like
the new `TestDbcsBisectWriteCells` test introduced in #13626.
Benchmarks showed no impact on performance whatsoever.
## Validation Steps Performed
* Pick this commit into #13626
* Run the `TestDbcsBisectWriteCells` test and break before OpenConsole exits
* A correct "QいかなZYXWVUTに" output is visible ✅
This commit makes AtlasEngine recognize Powerline glyphs as box drawing ones.
The extra pixel offsets when determining the `scale` caused weird artifacts
and thus were removed. It seems like this causes no noticeable regressions.
Closes#13029
## Validation Steps Performed
* Run all values of `wchar_t` through `isInInversionList`
and ensure it produces the expected value ✅
* Powerline glyphs are correctly scaled with Cascadia Code PL ✅
07d58a8 contains a regression where the settings' `Themes()` property is
accessed without checking whether it's a `nullptr`. This can happen because
the invalid settings modal is shown with a empty settings model object.
This commit fixes the issue by deferring the update of `_settings` until
after we ensured that the `_settings` object is valid (besides warnings).
Closes#13543
## Validation Steps Performed
* Replace any string value with `123`
* Application doesn't crash ✅
## Summary of the Pull Request
- When 'discard changes' is hit, we re-initialize our list of color scheme view models but forgot to tell xaml about it, this commit fixes that.
- Make sure to exit rename mode when 'update settings' gets called
## References
color schemes mvvm added in #13179
## PR Checklist
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
## Validation Steps Performed
Hitting discard changes doesn't cause an inconsistency with the currently selected scheme anymore
## Summary of the Pull Request
Adds a new mode to `CloseOnExit`: `Automatic`. In this mode, if a process handed off by defterm terminates for whatever reason, we always close (i.e. we treat the mode as `Always`), but for processes launched by Terminal we terminate as with the `Graceful` behaviour.
## PR Checklist
* [x] Closes#13325
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
## Detailed Description of the Pull Request / Additional comments
- Adds a new enum value to `CloseOnExit`
- Adds a new function to `Pane`: `FinalizeConfigurationGivenDefault`: this is a function that should be called when the pane is created via default terminal handoff, and can contain any special configurations we should set given that the pane was created via handoff
## Validation Steps Performed
Curiously, at least on Windows 10 (and rarely on Windows 11), if you minimize the Terminal by clicking on the taskbar, then alt-tab to try and restore the window, the Taskbar will decide to call `SwitchToWindow` on the invisible, owned ConPTY window instead of the main window. When that happens, ConPTY'll get a `WM_SIZE(SIZE_RESTORED, lParam=0)`. The main window will NOT get a `SwitchToWindow` called. If ConPTY doesn't actually inform the hosting process about this, then the main HWND might stay hidden.
* Refer to #13158 where we disabled this.
* Closes#13589
* Closes#13248
* Tested manually on a Windows 10 VM.
* Confirmed that opening tabs while maximized/snapped doesn't restore down.
* `[Native]::ShowWindow([Native]::GetConsoleWindow(), 6)` still works
## Summary of the Pull Request
Adds support for the `tab.showCloseButton` property to themes. This accepts three values:
* `"always"` (default): The close button acts like it does today.
* `"hover"`: The close button is always visible on the active tab. On inactive tabs, the close button only appears on mouse over.
* `"never"`: The close button is never visible. You can't close the tab with middle-click, but you can still use keyboard shortcuts to close the tab.
## References
* See #3327
* ⚠️ targets #13178⚠️
## PR Checklist
* [x] Closes#3335
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated - YUP
## Detailed Description of the Pull Request / Additional comments
See the following two properties in WInUI that we're leveraging here.
* [`TabViewCloseButtonOverlayMode.OnPointerOver`](https://docs.microsoft.com/en-us/windows/winui/api/microsoft.ui.xaml.controls.tabviewclosebuttonoverlaymode?view=winui-2.7&viewFallbackFrom=winui-2.2)
* [`TabViewItem.IsClosable`](https://docs.microsoft.com/en-us/windows/winui/api/microsoft.ui.xaml.controls.tabviewitem.isclosable?view=winui-2.2#microsoft-ui-xaml-controls-tabviewitem-isclosable)
One is a tabview-level property, the other is a per-tab-item property, hence why this code is a little wacky.
## Validation Steps Performed
gifs below
This commit contains 3 improvements for glyph rendering:
* Scale block element and box drawing characters to fit the cell size
"perfectly" without leaving pixel gaps between cells.
* Use `IDWriteTextLayout::GetOverhangMetrics` to determine whether glyphs
are outside the given layout box and if they are, offset their position
to fit them back in. If that still fails to fit, we downscale them.
* Always scale up glyphs that are more than 2 cells wide
This ensures that long ligatures that mimic box drawing characters like
"===" under Cascadia Code are upscaled just like regular box drawings.
Unfortunately this results in ligature-heavy text (like Myanmar) to get an
"uneven" appearance because some ligatures can suddenly appear too large.
It's difficult to come up with a good heuristic here.
Closes#12512
## Validation Steps Performed
* Print UTF-8-demo.txt
* Block characters don't leave gaps ✅
* Print a lorem-ipsum in Myanmar
* Glyphs aren't cut off anymore ✅
* Print a long "===" ligature under Cascadia Code
* The ligature is as wide as the number of cells used ✅
## Summary of the Pull Request
Implements the MVVM style for the Color Schemes editor
## PR Checklist
* [x] Closes #xxx
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] I work here
## Detailed Description of the Pull Request / Additional comments
Introduces:
- `ColorSchemesPageViewModel`: The view model responsible for the entire color schemes page. Handles what the current scheme is, adding/deleting/renaming schemes
- `ColorSchemeViewModel`: A view model class for individual color schemes
## Validation Steps Performed
Manually tested:
- Edit a color scheme
- Add new color scheme
- Rename a color scheme
- Delete a color scheme
This is an experiment, as discussed in https://github.com/microsoft/terminal/issues/11790#issuecomment-1179143049. We don't know what for sure causes these crashes, but it seems that blindly throwing, so that it gets picked up by Watson, is probably not the move. Instead, we're just gonna do our fallback, REGARDLESS of what the exception was.
See #11790, MSFT:38542548, MSFT:38572983, MSFT:38542574 et. al.
Refer to https://docs.microsoft.com/en-us/windows/win32/rstmgr/guidelines-for-applications
The OS will send us a WM_QUERYENDSESSION when it's preparing an
update for our app. It will then send us a WM_ENDSESSION, which gives
us a small timeout (~30s) to actually shut down gracefully. After
that timeout, it will send us a WM_CLOSE. If we still don't close
after the WM_CLOSE, it'll force-kill us (causing a crash which will be
bucketed to moapphang).
We will manually start a quit, so that we can persist the state. If we refuse to
gracefully shut down, the OS will crash us to focefully terminate us. We
choose to quit here, rather than just close, to skip over any warning dialogs
(e.g. "Are you sure you want to close all tabs?") which might prevent a WM_CLOSE
from cleanly closing the window.
This will cause a appHost._RequestQuitAll, which will notify the
monarch to collect up all the window state and save it.
This "crash" caused by the OS force killing us constitutes 80% of all our crashes. 80%. See MSFT:38947155, MSFT:38877540, MSFT:21058878, MSFT:31710054, MSFT:39764652, MSFT:26883776.
Closes#13569
It also fixes the issue where if you've got Terminal Dev running (outside VS), and you try to Deploy, you have to make sure to close the "Are you sure you want to close all tabs" dialog before the deployment can proceed. A deploy in VS sends the same sequence of messages as a real update.
This commit implements the remaining 5 of 8 grid lines:
left/top/right/bottom (COMMON_LVB) borders and double underline
`AtlasEngine::_resolveFontMetrics` was partially refactored to use `float`s
instead of `double`s, because that's what the remaining code uses as well.
It also helps the new, slightly more complex double underline calculation.
## Validation Steps Performed
* Print characters with the `COMMON_LVB_GRID_HORIZONTAL`, `GRID_LVERTICAL`,
`GRID_RVERTICAL` and `UNDERSCORE` attributes via `WriteConsoleOutputW`
* All 4 grid lines are visible ✅
* Grid lines correctly scale according to the `lineWidth` ✅
* Print a double underline with `printf "\033[21mtest\033[0m"`
* A double underline is fully visible ✅
Cleans up a couple local test failures.
* [x] Closes#13474: So, I clearly hadn't ran the local tests at the end of the themes PR. We needed a sensible fallback to SOME theme, even if there wasn't one provided in the user json. This is only really hit in the tests (that don't also include `defaults.json`.
* [x] Closes#13323: Meh, the ordering of the keys in this test doesn't matter. Ordering is a map implementation detail. This is fine.
* [x] Ran tests locally
We recently figured that we can drop support for Windows 7. Coincidentally
AtlasEngine never actually supported Windows 7 properly, because it called
`ResizeBuffers` with `DXGI_SWAP_CHAIN_FLAG_FRAME_LATENCY_WAITABLE_OBJECT`
no matter whether the swap chain was created with it enabled.
The new minimally supported version is Windows 8.1.
Leonard Hecker
