maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Merge branch 'master' into dev/cazamor/spec-keyboard-selection

Browse source
This commit is contained in:
Carlos Zamora 2020-06-23 11:43:48 -07:00
parent f71275c8c0 ff23be04fb
commit 81c5a7c810
20 changed files with 972 additions and 219 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
doc/specs/#2046 - Command Palette.md
View file

@ -1,7 +1,7 @@
---
author: Mike Griese @zadjii-msft
created on: 2019-08-01
last updated: 2020-06-10
last updated: 2020-06-16
issue id: 2046
---
@ -522,6 +522,13 @@ default. These are largely the actions that are bound by default.
]
```
## Addenda
This spec also has a follow-up spec which introduces further changes upon this
original draft. Please also refer to:
* June 2020: Unified keybindings and commands, and synthesized action names.
## Future considerations
* Commands will provide an easy point for allowing an extension to add its

608
doc/specs/#2046 - Unified keybindings and commands, and synthesized action names.md Normal file
View file

@ -0,0 +1,608 @@
---
author: Mike Griese @zadjii-msft
created on: 2020-06-15
last updated: 2020-06-19
issue id: 2046
---
# Command Palette, Addendum 1 - Unified keybindings and commands, and synthesized action names
## Abstract
This document is intended to serve as an addition to the [Command Palette Spec].
While that spec is complete in it's own right, subsequent discussion revealed
additional ways to improve the functionality and usability of the command
palette. This document builds largely on the topics already introduced in the
original spec, so readers should first familiarize themselves with that
document.
One point of note from the original document was that the original specification
was entirely too verbose when defining both keybindings and commands for
actions. Consider, for instance, a user that wants to bind the action "duplicate
the current pane". In that spec, they need to add both a keybinding and a
command:
```json
{
"keybindings": [
{ "keys": [ "ctrl+alt+t" ], "command": { "action": "splitPane", "split":"auto", "splitMode": "duplicate" } },
],
"commands": [
{ "name": "Duplicate Pane", "action": { "action": "splitPane", "split":"auto", "splitMode": "duplicate" }, "icon": null },
]
}
```
These two entries are practically the same, except for two key differentiators:
* the keybinding has a `keys` property, indicating which key chord activates the
action.
* The command has a `name` property, indicating what name to display for the
command in the Command Palette.
What if the user didn't have to duplicate this action? What if the user could
just add this action once, in their `keybindings` or `commands`, and have it
work both as a keybinding AND a command?
## Solution Design
This spec will outline two primary changes to keybindings and commands.
1. Unify keybindings and commands, so both `keybindings` and `commands` can
specify either actions bound to keys, and/or actions bound to entries in the
Command Palette.
2. Propose a mechanism by which actions do not _require_ a `name` to appear in
the Command Palette.
These proposals are two atomic units - either could be approved or rejected
independently of one another. They're presented together here in the same doc
because together, they present a compelling story.
### Proposal 1: Unify Keybindings and Commands
As noted above, keybindings and commands have nearly the exact same syntax, save
for a couple properties. To make things easier for the user, I'm proposing
treating everything in _both_ the `keybindings` _and_ the `commands` arrays as
**BOTH** a keybinding and a command.
Furthermore, as a change from the previous spec, we'll be using `bindings` from
here on as the unified `keybindings` and `commands` lists. This is considering
that we'll currently be using `bindings` for both commands and keybindings, but
we'll potentially also have mouse & touch bindings in this array in the future.
We'll "deprecate" the existing `keybindings` property, and begin to exclusively
use `bindings` as the new property name. For compatibility reasons, we'll
continue to parse `keybindings` in the same way we parse `bindings`. We'll
simply layer `bindings` on top of the legacy `keybindings`.
* Anything entry that has a `keys` value will be added to the keybindings.
Pressing that keybinding will activate the action defined in `command`.
* Anything with a `name`<sup>[1]</sup> will be added as an entry (using that
name) to the Command Palette's Action Mode.
###### Caveats
* **Nested commands** (commands with other nested commands). If a command has
nested commands in the `commands` property, AND a `keys` property, then
pressing that keybinding should open the Command Palette directly to that
level of nesting of commands.
* **"Iterable" commands** (with an `iterateOn` property): These are commands
that are expanded into one command per profile. These cannot really be bound
as keybindings - which action should be bound to the key? They can't all be
bound to the same key. If a KeyBinding/Command json blob has a valid
`iterateOn` property, then we'll ignore it as a keybinding. This includes any
commands that are nested as children of this command - we won't be able to
know which of the expanded children will be the one to bind the keys to.
<sup>[1]</sup>: This requirement will be relaxed given **Proposal 2**, below,
but ignored for the remainder of this section, for illustrative purposes.
#### Example
Consider the following settings:
```json
"bindings": [
{ "name": "Duplicate Tab", "command": "duplicateTab", "keys": "ctrl+alt+a" },
{ "command": "nextTab", "keys": "ctrl+alt+b" },
{
"icon": "...",
"name": { "key": "NewTabWithProfileRootCommandName" },
"commands": [
{
"iterateOn": "profiles",
"icon": "${profile.icon}",
"name": { "key": "NewTabWithProfileCommandName" },
"command": { "action": "newTab", "profile": "${profile.name}" }
}
]
},
{
"icon": "...",
"name": "Connect to ssh...",
"commands": [
{
"keys": "ctrl+alt+c",
"icon": "...",
"name": "first.com",
"command": { "action": "newTab", "commandline": "ssh me@first.com" }
},
{
"keys": "ctrl+alt+d",
"icon": "...",
"name": "second.com",
"command": { "action": "newTab", "commandline": "ssh me@second.com" }
}
]
}
{
"keys": "ctrl+alt+e",
"icon": "...",
"name": { "key": "SplitPaneWithProfileRootCommandName" },
"commands": [
{
"iterateOn": "profiles",
"icon": "${profile.icon}",
"name": { "key": "SplitPaneWithProfileCommandName" },
"commands": [
{
"keys": "ctrl+alt+f",
"icon": "...",
"name": { "key": "SplitPaneName" },
"command": { "action": "splitPane", "profile": "${profile.name}", "split": "automatic" }
},
{
"icon": "...",
"name": { "key": "SplitPaneVerticalName" },
"command": { "action": "splitPane", "profile": "${profile.name}", "split": "vertical" }
},
{
"icon": "...",
"name": { "key": "SplitPaneHorizontalName" },
"command": { "action": "splitPane", "profile": "${profile.name}", "split": "horizontal" }
}
]
}
]
}
]
```
This will generate a tree of commands as follows:
```
<Command Palette>
├─ Duplicate tab { ctrl+alt+a }
├─ New Tab With Profile...
│ ├─ Profile 1
│ ├─ Profile 2
│ └─ Profile 3
├─ Connect to ssh...
│ ├─ first.com { ctrl+alt+c }
│ └─ second.com { ctrl+alt+d }
└─ New Pane... { ctrl+alt+e }
├─ Profile 1...
| ├─ Split Automatically
| ├─ Split Vertically
| └─ Split Horizontally
├─ Profile 2...
| ├─ Split Automatically
| ├─ Split Vertically
| └─ Split Horizontally
└─ Profile 3...
├─ Split Automatically
├─ Split Vertically
└─ Split Horizontally
```
Note also the keybindings in the above example:
* <kbd>ctrl+alt+a</kbd>: This key chord is bound to the "Duplicate tab"
(`duplicateTab`) action, which is also bound to the command with the same
name.
* <kbd>ctrl+alt+b</kbd>: This key chord is bound to the `nextTab` action, which
doesn't have an associated command.
* <kbd>ctrl+alt+c</kbd>: This key chord is bound to the "Connect to
ssh../first.com" action, which will open a new tab with the `commandline`
`"ssh me@first.com"`. When the user presses this keybinding, the action will
be executed immediately, without the Command Palette appearing.
* <kbd>ctrl+alt+d</kbd>: This is the same as the above, but with the "Connect to
ssh../second.com" action.
* <kbd>ctrl+alt+e</kbd>: This key chord is bound to opening the Command Palette
to the "New Pane..." command's menu. When the user presses this keybinding,
they'll be prompted with this command's sub-commands:
```
Profile 1...
Profile 2...
Profile 3...
```
* <kbd>ctrl+alt+f</kbd>: This key will _not_ be bound to any action. The parent
action is iterable, which means that the `SplitPaneName` command is going to
get turned into one command for each and every profile, and therefore cannot
be bound to just a single action.
### Proposal 2: Automatically synthesize action names
Previously, all Commands were required to have a `name`. This name was used as
the text for the action in the Action Mode of the Command Palette. However, this
is a little tedious for users who already have lots of keys bound. They'll need
to go through and add names to each of their existing keybindings to ensure that
the actions appear in the palette. Could we instead synthesize the names for the
commands ourselves? This would enable users to automatically get each of their
existing keybindings to appear in the palette without any extra work.
To support this, the following changes will be made:
* `ActionAndArgs` will get a `GenerateName()` method defined. This will create a
string describing the `ShortcutAction` and it's associated `ActionArgs`.
- Not EVERY action _needs_ to define a result for `GenerateName`. Actions that
don't _won't_ be automatically added to the Command Palette.
- Each of the strings used in `GenerateName` will need to come from our
resources, so they can be localized appropriately.
* When we're parsing commands, if a command doesn't have a `name`, we'll instead
attempt to use `GenerateName` to create the unique string for the action
associated with this command. If the command does have a `name` set, we'll use
that string instead, allowing the user to override the default name.
- If a command has it's name set to `null`, then we'll ignore the command
entirely, not just use the generated name.
[**Appendix 1**](#appendix-1-name-generation-samples-for-ShortcutActions) below
shows a complete sample of the strings that will be generated for each of the existing
`ShortcutActions`, and many of the actions that have been proposed, but not yet
implemented.
These strings should be human-friendly versions of the actions and their
associated args. For some of these actions, with very few arguments, the strings
can be relatively simple. Take for example, `CopyText`:
JSON | Generated String
-- | --
`{ "action":"copyText" }` | "Copy text"
`{ "action":"copyText", "singleLine": true }` | "Copy text as a single line"
`{ "action":"copyText", "singleLine": false, "copyFormatting": false }` | "Copy text without formatting"
`{ "action":"copyText", "singleLine": true, "copyFormatting": true }` | "Copy text as a single line without formatting"
CopyText is a bit of a simplistic case however, with very few args or
permutations of argument values. For things like `newTab`, `splitPane`, where
there are many possible arguments and values, it will be acceptable to simply
append `", property:value"` strings to the generated names for each of the set
values.
For example:
JSON | Generated String
-- | --
`{ "action":"newTab", "profile": "Hello" }` | "Open a new tab, profile:Hello"
`{ "action":"newTab", "profile": "Hello", "directory":"C:\\", "commandline": "wsl.exe", title": "Foo" }` | "Open a new tab, profile:Hello, directory:C:\\, commandline:wsl.exe, title:Foo"
This is being chosen in favor of something that might be more human-friendly,
like "Open a new tab with profile {profile name} in {directory} with
{commandline} and a title of {title}". This string would be much harder to
synthesize, especially considering localization concerns.
#### Remove the resource key notation
Since we'll be using localized names for each of the actions in `GenerateName`,
we no longer _need_ to provide the `{ "name":{ "key": "SomeResourceKey" } }`
syntax introduced in the original spec. This functionality was used to allow us
to define localizable names for the default commands.
However, I think we should keep this functionality, to allow us additional
flexibility when defining default commands.
### Complete Defaults
Considering both of the above proposals, the default keybindings and commands
will be defined as follows:
* The current default keybindings will be untouched. These actions will
automatically be added to the Command Palette, using their names generated
from `GenerateName`.
- **TODO: FOR DISCUSSION**: Should we manually set the names for the default
"New Tab, profile index: 0" keybindings to `null`? This seems like a not
terribly helpful name for the Command Palette, especially considering the
iterable commands listed below.
* We'll add a few new commands:
- A nested, iterable command for "Open new tab with
profile..."/"Profile:{profile name}"
- A nested, iterable command for "Select color scheme..."/"{scheme name}"
- A nested, iterable command for "New Pane..."/"Profile:{profile
name}..."/["Automatic", "Horizontal", "Vertical"]
> 👉 NOTE: These default nested commands can be removed by the user defining
> `{ "name": "Open new tab with profile...", "action":null }` (et al) in their
> settings.
- If we so chose, in the future we can add further commands that we think are
helpful to `defaults.json`, without needing to give them keys. For example,
we could add
```json
{ "command": { "action": "copy", "singleLine": true } }
```
to `bindings`, to add a "copy text as a single line" command, without
necessarily binding it to a keystroke.
These changes to the `defaults.json` are represented in json as the following:
```json
"bindings": [
{
"icon": null,
"name": { "key": "NewTabWithProfileRootCommandName" },
"commands": [
{
"iterateOn": "profiles",
"icon": "${profile.icon}",
"name": "${profile.name}",
"command": { "action": "newTab", "profile": "${profile.name}" }
}
]
},
{
"icon": null,
"name": { "key": "SelectColorSchemeRootCommandName" },
"commands": [
{
"iterateOn": "schemes",
"icon": null,
"name": "${scheme.name}",
"command": { "action": "selectColorScheme", "scheme": "${scheme.name}" }
}
]
},
{
"icon": null,
"name": { "key": "SplitPaneWithProfileRootCommandName" },
"commands": [
{
"iterateOn": "profiles",
"icon": "${profile.icon}",
"name": { "key": "SplitPaneWithProfileCommandName" },
"commands": [
{
"icon": null,
"name": { "key": "SplitPaneName" },
"command": { "action": "splitPane", "profile": "${profile.name}", "split": "automatic" }
},
{
"icon": null,
"name": { "key": "SplitPaneVerticalName" },
"command": { "action": "splitPane", "profile": "${profile.name}", "split": "vertical" }
},
{
"icon": null,
"name": { "key": "SplitPaneHorizontalName" },
"command": { "action": "splitPane", "profile": "${profile.name}", "split": "horizontal" }
}
]
}
]
}
]
```
A complete diagram of what the default Command Palette will look like given the
default keybindings and these changes is given in [**Appendix
2**](#appendix-2-complete-default-command-palette).
## Concerns
**DISCUSSION**: "New tab with index {index}". How does this play with
the new tab dropdown customizations in [#5888]? In recent iterations of that
spec, we changed the meaning of `{ "action": "newTab", "index": 1 }` to mean
"open the first entry in the new tab menu". If that's a profile, then we'll open
a new tab with it. If it's an action, we'll perform that action. If it's a
nested menu, then we'll open the menu to that entry.
Additionally, how exactly does that play with something like `{ "action":
"newTab", "index": 1, "commandline": "wsl.exe" }`? This is really a discussion
for that spec, but is an issue highlighted by this spec. If the first entry is
anything other than a `profile`, then the `commandline` parameter doesn't really
mean anything anymore. I'm tempted to revert this particular portion of the new
tab menu customization spec over this.
We could instead add an `index` to `openNewTabDropdown`, and have that string
instead be "Open new tab dropdown, index:1". That would help disambiguate the
two.
Following discussion, it was decided that this was in fact the cleanest
solution, when accounting for both the needs of the new tab dropdown and the
command palette. The [#5888] spec has been updated to reflect this.
## Future considerations
* Some of these command names are starting to get _very_ long. Perhaps we need a
netting to display Command Palette entries on two lines (or multiple, as
necessary).
* When displaying the entries of a nested command to the user, should we display
a small label showing the name of the previous command? My gut says _yes_. In
the Proposal 1 example, pressing `ctrl+alt+e` to jump to "Split Pane..."
should probably show a small label that displays "Split Pane..." above the
list of nested commands.
* It wouldn't be totally impossible to allow keys to be bound to an iterable
command, and then simply have the key work as "open the command palette with
only the commands generated by this iterable command". This is left as a
future option, as it might require some additional technical plumbing.
## Appendix 1: Name generation samples for `ShortcutAction`s
### Current `ShortcutActions`
* `CopyText`
- "Copy text"
- "Copy text as a single line"
- "Copy text without formatting"
- "Copy text as a single line without formatting"
* `PasteText`
- "Paste text"
* `OpenNewTabDropdown`
- "Open new tab dropdown"
* `DuplicateTab`
- "Duplicate tab"
* `NewTab`
- "Open a new tab, profile:{profile name}, directory:{directory}, commandline:{commandline}, title:{title}"
* `NewWindow`
- "Open a new window"
- "Open a new window, profile:{profile name}, directory:{directory}, commandline:{commandline}, title:{title}"
* `CloseWindow`
- "Close window"
* `CloseTab`
- "Close tab"
* `ClosePane`
- "Close pane"
* `NextTab`
- "Switch to the next tab"
* `PrevTab`
- "Switch to the previous tab"
* `SplitPane`
- "Open a new pane, profile:{profile name}, split direction:{direction}, split size:{X%/Y chars}, resize parents, directory:{directory}, commandline:{commandline}, title:{title}"
- "Duplicate the current pane, split direction:{direction}, split size:{X%/Y chars}, resize parents, directory:{directory}, commandline:{commandline}, title:{title}"
* `SwitchToTab`
- "Switch to tab {index}"
* `AdjustFontSize`
- "Increase the font size"
- "Decrease the font size"
* `ResetFontSize`
- "Reset the font size"
* `ScrollUp`
- "Scroll up a line"
- "Scroll up {amount} lines"
* `ScrollDown`
- "Scroll down a line"
- "Scroll down {amount} lines"
* `ScrollUpPage`
- "Scroll up a page"
- "Scroll up {amount} pages"
* `ScrollDownPage`
- "Scroll down a page"
- "Scroll down {amount} pages"
* `ResizePane`
- "Resize pane {direction}"
- "Resize pane {direction} {percent}%"
* `MoveFocus`
- "Move focus {direction}"
* `Find`
- "Toggle the search box"
* `ToggleFullscreen`
- "Toggle fullscreen mode"
* `OpenSettings`
- "Open settings"
- "Open settings file"
- "Open default settings file"
* `ToggleCommandPalette`
- "Toggle the Command Palette"
- "Toggle the Command Palette in commandline mode"
### Other yet unimplemented actions:
* `SwitchColorScheme`
- "Select color scheme {name}"
* `ToggleRetroEffect`
- "Toggle the retro terminal effect"
* `ExecuteCommandline`
- "Run a wt commandline: {cmdline}"
* `ExecuteActions`
- OPINION: THIS ONE SHOULDN'T HAVE A NAME. We're not including any of these by
default. The user knows what they're putting in the settings by adding this
action, let them name it.
- Alternatively: "Run actions: {action.ToName() for action in actions}"
* `SendInput`
- OPINION: THIS ONE SHOULDN'T HAVE A NAME. We're not including any of these by
default. The user knows what they're putting in the settings by adding this
action, let them name it.
* `ToggleMarkMode`
- "Toggle Mark Mode"
* `NextTab`
- "Switch to the next most-recent tab"
* `SetTabColor`
- "Set the color of the current tab to {#color}"
* It would be _really_ cool if we could display a sample of the color
inline, but that's left as a future consideration.
- "Set the color for this tab..."
* this command isn't nested, but hitting enter immediately does something
with the UI, so that's _fine_
* `RenameTab`
- "Rename this tab to {name}"
- "Rename this tab..."
* this command isn't nested, but hitting enter immediately does something
with the UI, so that's _fine_
## Appendix 2: Complete Default Command Palette
This diagram shows what the default value of the Command Palette would be. This
assumes that the user has 3 profiles, "Profile 1", "Profile 2", and "Profile 3",
as well as 3 schemes: "Scheme 1", "Scheme 2", and "Scheme 3".
```
<Command Palette>
├─ Close Window
├─ Toggle fullscreen mode
├─ Open new tab dropdown
├─ Open settings
├─ Open default settings file
├─ Toggle the search box
├─ New Tab
├─ New Tab, profile index: 0
├─ New Tab, profile index: 1
├─ New Tab, profile index: 2
├─ New Tab, profile index: 3
├─ New Tab, profile index: 4
├─ New Tab, profile index: 5
├─ New Tab, profile index: 6
├─ New Tab, profile index: 7
├─ New Tab, profile index: 8
├─ Duplicate tab
├─ Switch to the next tab
├─ Switch to the previous tab
├─ Switch to tab 0
├─ Switch to tab 1
├─ Switch to tab 2
├─ Switch to tab 3
├─ Switch to tab 4
├─ Switch to tab 5
├─ Switch to tab 6
├─ Switch to tab 7
├─ Switch to tab 8
├─ Close pane
├─ Open a new pane, split: horizontal
├─ Open a new pane, split: vertical
├─ Duplicate the current pane
├─ Resize pane down
├─ Resize pane left
├─ Resize pane right
├─ Resize pane up
├─ Move focus down
├─ Move focus left
├─ Move focus right
├─ Move focus up
├─ Copy Text
├─ Paste Text
├─ Scroll down a line
├─ Scroll down a page
├─ Scroll up a line
├─ Scroll up a page
├─ Increase the font size
├─ Decrease the font size
├─ Reset the font size
├─ New Tab With Profile...
│ ├─ Profile 1
│ ├─ Profile 2
│ └─ Profile 3
├─ Select Color Scheme...
│ ├─ Scheme 1
│ ├─ Scheme 2
│ └─ Scheme 3
└─ New Pane...
├─ Profile 1...
| ├─ Split Automatically
| ├─ Split Vertically
| └─ Split Horizontally
├─ Profile 2...
| ├─ Split Automatically
| ├─ Split Vertically
| └─ Split Horizontally
└─ Profile 3...
├─ Split Automatically
├─ Split Vertically
└─ Split Horizontally
```
<!-- Footnotes -->
[Command Palette Spec]: https://github.com/microsoft/terminal/blob/master/doc/specs/%232046%20-%20Command%20Palette.md

4
samples/ReadConsoleInputStream/ReadConsoleInputStream.cs
View file

@ -37,7 +37,7 @@ namespace Samples.Terminal
internal ReadConsoleInputStream(HFILE handle,
BlockingCollection<Kernel32.INPUT_RECORD> nonKeyEvents)
{
Debug.Assert(handle.IsInvalid == false, "handle.IsInvalid == false");
Debug.Assert(!handle.IsInvalid, "handle.IsInvalid == false");
_handle = handle.DangerousGetHandle();
_nonKeyEvents = nonKeyEvents;
@ -111,7 +111,7 @@ namespace Samples.Terminal
if (record.EventType == Kernel32.EVENT_TYPE.KEY_EVENT)
{
// skip key up events - if not, every key will be duped in the stream
if (record.Event.KeyEvent.bKeyDown == false) continue;
if (!record.Event.KeyEvent.bKeyDown) continue;
// pack ucs-2/utf-16le/unicode chars into position in our byte[] buffer.
var glyph = (ushort) record.Event.KeyEvent.uChar;

9
src/cascadia/TerminalApp/Tab.cpp
View file

@ -45,6 +45,15 @@ namespace winrt::TerminalApp::implementation
void Tab::_MakeTabViewItem()
{
_tabViewItem = ::winrt::MUX::Controls::TabViewItem{};
_tabViewItem.DoubleTapped([weakThis = get_weak()](auto&& /*s*/, auto&& /*e*/) {
if (auto tab{ weakThis.get() })
{
tab->_inRename = true;
tab->_UpdateTabHeader();
}
});
_UpdateTitle();
}

4
src/cascadia/TerminalApp/WslDistroGenerator.cpp
View file

@ -76,7 +76,7 @@ std::vector<TerminalApp::Profile> WslDistroGenerator::GenerateProfiles()
THROW_HR(ERROR_UNHANDLED_EXCEPTION);
}
DWORD exitCode;
if (GetExitCodeProcess(pi.hProcess, &exitCode) == false)
if (!GetExitCodeProcess(pi.hProcess, &exitCode))
{
THROW_HR(E_INVALIDARG);
}
@ -116,7 +116,7 @@ std::vector<TerminalApp::Profile> WslDistroGenerator::GenerateProfiles()
continue;
}
size_t firstChar = distName.find_first_of(L"( ");
const size_t firstChar = distName.find_first_of(L"( ");
// Some localizations don't have a space between the name and "(Default)"
// https://github.com/microsoft/terminal/issues/1168#issuecomment-500187109
if (firstChar < distName.size())

43
src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp
View file

@ -403,7 +403,7 @@ int NonClientIslandWindow::_GetResizeHandleHeight() const noexcept
// window frame.
[[nodiscard]] LRESULT NonClientIslandWindow::_OnNcCalcSize(const WPARAM wParam, const LPARAM lParam) noexcept
{
if (wParam == false)
if (!wParam)
{
return 0;
}
@ -417,7 +417,7 @@ int NonClientIslandWindow::_GetResizeHandleHeight() const noexcept
const auto originalSize = params->rgrc[0];
// apply the default frame
auto ret = DefWindowProc(_window.get(), WM_NCCALCSIZE, wParam, lParam);
const auto ret = DefWindowProc(_window.get(), WM_NCCALCSIZE, wParam, lParam);
if (ret != 0)
{
return ret;
@ -783,44 +783,20 @@ void NonClientIslandWindow::_UpdateFrameMargins() const noexcept
[[nodiscard]] LRESULT NonClientIslandWindow::_OnNcCreate(WPARAM wParam, LPARAM lParam) noexcept
{
const auto ret = IslandWindow::_OnNcCreate(wParam, lParam);
if (ret == FALSE)
if (!ret)
{
return ret;
return FALSE;
}
// Set the frame's theme before it is rendered (WM_NCPAINT) so that it is
// rendered with the correct theme.
_UpdateFrameTheme();
// This is a hack to make the window borders dark instead of light.
// It must be done before WM_NCPAINT so that the borders are rendered with
// the correct theme.
// For more information, see GH#6620.
LOG_IF_FAILED(TerminalTrySetDarkTheme(_window.get(), true));
return TRUE;
}
// Method Description:
// - Updates the window frame's theme depending on the application theme (light
// or dark). This doesn't invalidate the old frame so it will not be
// rerendered until the user resizes or focuses/unfocuses the window.
// Return Value:
// - <none>
void NonClientIslandWindow::_UpdateFrameTheme() const
{
bool isDarkMode;
switch (_theme)
{
case ElementTheme::Light:
isDarkMode = false;
break;
case ElementTheme::Dark:
isDarkMode = true;
break;
default:
isDarkMode = Application::Current().RequestedTheme() == ApplicationTheme::Dark;
break;
}
LOG_IF_FAILED(TerminalTrySetDarkTheme(_window.get(), isDarkMode));
}
// Method Description:
// - Called when the app wants to change its theme. We'll update the frame
// theme to match the new theme.
@ -833,7 +809,6 @@ void NonClientIslandWindow::OnApplicationThemeChanged(const ElementTheme& reques
IslandWindow::OnApplicationThemeChanged(requestedTheme);
_theme = requestedTheme;
_UpdateFrameTheme();
}
// Method Description:

1
src/cascadia/WindowsTerminal/NonClientIslandWindow.h
View file

@ -85,7 +85,6 @@ private:
void _UpdateFrameMargins() const noexcept;
void _UpdateMaximizedState();
void _UpdateIslandPosition(const UINT windowWidth, const UINT windowHeight);
void _UpdateFrameTheme() const;
void _OpenSystemMenu(const int mouseX, const int mouseY) const noexcept;
};

2
src/host/CursorBlinker.cpp
View file

@ -218,7 +218,7 @@ void CursorBlinker::KillCaretTimer()
// A failure to delete the timer with the LastError being ERROR_IO_PENDING means that the timer is
// currently in use and will get cleaned up when released. Delete should not be called again.
// We treat that case as a success.
if (bRet == false && GetLastError() != ERROR_IO_PENDING)
if (!bRet && GetLastError() != ERROR_IO_PENDING)
{
LOG_LAST_ERROR();
}

3
src/inc/LibraryIncludes.h
View file

@ -84,7 +84,10 @@
// Dynamic Bitset (optional dependency on LibPopCnt for perf at bit counting)
// Variable-size compressed-storage header-only bit flag storage library.
#pragma warning(push)
#pragma warning(disable:4702) // unreachable code
#include <dynamic_bitset.hpp>
#pragma warning(pop)
// {fmt}, a C++20-compatible formatting library
#include <fmt/format.h>

24
src/inc/til/bitmap.h
View file

@ -81,16 +81,20 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
const ptrdiff_t _end;
til::rectangle _run;
// Update _run to contain the next rectangle of consecutively set bits within this bitmap.
// _calculateArea may be called repeatedly to yield all those rectangles.
void _calculateArea()
{
// Backup the position as the next one.
_nextPos = _pos;
// The following logic first finds the next set bit in this bitmap and the next unset bit past that.
// The area in between those positions are thus all set bits and will end up being the next _run.
// Seek forward until we find an on bit.
while (_nextPos < _end && !_values[_nextPos])
{
++_nextPos;
}
// dynamic_bitset allows you to quickly find the next set bit using find_next(prev),
// where "prev" is the position _past_ which should be searched (i.e. excluding position "prev").
// If _pos is still 0, we thus need to use the counterpart find_first().
const auto nextPos = _pos == 0 ? _values.find_first() : _values.find_next(_pos - 1);
// If no next set bit can be found, npos is returned, which is SIZE_T_MAX.
// saturated_cast can ensure that this will be converted to PTRDIFF_T_MAX (which is greater than _end).
_nextPos = base::saturated_cast<ptrdiff_t>(nextPos);
// If we haven't reached the end yet...
if (_nextPos < _end)
@ -118,8 +122,10 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
}
else
{
// If we reached the end, set the pos because the run is empty.
_pos = _nextPos;
// If we reached the end _nextPos may be >= _end (potentially even PTRDIFF_T_MAX).
// ---> Mark the end of the iterator by updating the state with _end.
_pos = _end;
_nextPos = _end;
_run = til::rectangle{};
}
}

2
src/interactivity/onecore/ConIoSrvComm.cpp
View file

@ -100,7 +100,7 @@ ConIoSrvComm::~ConIoSrvComm()
// Initialize the server port name.
Ret = RtlCreateUnicodeString(&PortName, CIS_ALPC_PORT_NAME);
if (Ret == FALSE)
if (!Ret)
{
return STATUS_NO_MEMORY;
}

52
src/renderer/dx/CustomTextLayout.cpp
View file

@ -20,14 +20,12 @@ using namespace Microsoft::Console::Render;
// - analyzer - DirectWrite text analyzer from the factory that has been cached at a level above this layout (expensive to create)
// - format - The DirectWrite format object representing the size and other text properties to be applied (by default) to a layout
// - font - The DirectWrite font face to use while calculating layout (by default, will fallback if necessary)
// - clusters - From the backing buffer, the text to be displayed clustered by the columns it should consume.
// - width - The count of pixels available per column (the expected pixel width of every column)
// - boxEffect - Box drawing scaling effects that are cached for the base font across layouts.
CustomTextLayout::CustomTextLayout(gsl::not_null<IDWriteFactory1*> const factory,
gsl::not_null<IDWriteTextAnalyzer1*> const analyzer,
gsl::not_null<IDWriteTextFormat*> const format,
gsl::not_null<IDWriteFontFace1*> const font,
std::basic_string_view<Cluster> const clusters,
size_t const width,
IBoxDrawingEffect* const boxEffect) :
_factory{ factory.get() },
@ -47,8 +45,37 @@ CustomTextLayout::CustomTextLayout(gsl::not_null<IDWriteFactory1*> const factory
// Fetch the locale name out once now from the format
_localeName.resize(gsl::narrow_cast<size_t>(format->GetLocaleNameLength()) + 1); // +1 for null
THROW_IF_FAILED(format->GetLocaleName(_localeName.data(), gsl::narrow<UINT32>(_localeName.size())));
}
_textClusterColumns.reserve(clusters.size());
//Routine Description:
// - Resets this custom text layout to the freshly allocated state in terms of text analysis.
// Arguments:
// - <none>, modifies internal state
// Return Value:
// - S_OK or suitable memory management issue
[[nodiscard]] HRESULT STDMETHODCALLTYPE CustomTextLayout::Reset() noexcept
try
{
_runs.clear();
_breakpoints.clear();
_runIndex = 0;
_isEntireTextSimple = false;
_textClusterColumns.clear();
_text.clear();
return S_OK;
}
CATCH_RETURN()
// Routine Description:
// - Appends text to this layout for analysis/processing.
// Arguments:
// - clusters - From the backing buffer, the text to be displayed clustered by the columns it should consume.
// Return Value:
// - S_OK or suitable memory management issue.
[[nodiscard]] HRESULT STDMETHODCALLTYPE CustomTextLayout::AppendClusters(const std::basic_string_view<::Microsoft::Console::Render::Cluster> clusters)
try
{
_textClusterColumns.reserve(_textClusterColumns.size() + clusters.size());
for (const auto& cluster : clusters)
{
@ -64,7 +91,10 @@ CustomTextLayout::CustomTextLayout(gsl::not_null<IDWriteFactory1*> const factory
_text += text;
}
return S_OK;
}
CATCH_RETURN()
// Routine Description:
// - Figures out how many columns this layout should take. This will use the analyze step only.
@ -1827,21 +1857,13 @@ void CustomTextLayout::_SplitCurrentRun(const UINT32 splitPosition)
// - <none>
void CustomTextLayout::_OrderRuns()
{
const size_t totalRuns = _runs.size();
std::vector<LinkedRun> runs;
runs.resize(totalRuns);
UINT32 nextRunIndex = 0;
for (UINT32 i = 0; i < totalRuns; ++i)
std::sort(_runs.begin(), _runs.end(), [](auto& a, auto& b) { return a.textStart < b.textStart; });
for (UINT32 i = 0; i < _runs.size() - 1; ++i)
{
runs.at(i) = _runs.at(nextRunIndex);
runs.at(i).nextRunIndex = i + 1;
nextRunIndex = _runs.at(nextRunIndex).nextRunIndex;
til::at(_runs, i).nextRunIndex = i + 1;
}
runs.back().nextRunIndex = 0;
_runs.swap(runs);
_runs.back().nextRunIndex = 0;
}
#pragma endregion

5
src/renderer/dx/CustomTextLayout.h
View file

@ -24,10 +24,13 @@ namespace Microsoft::Console::Render
gsl::not_null<IDWriteTextAnalyzer1*> const analyzer,
gsl::not_null<IDWriteTextFormat*> const format,
gsl::not_null<IDWriteFontFace1*> const font,
const std::basic_string_view<::Microsoft::Console::Render::Cluster> clusters,
size_t const width,
IBoxDrawingEffect* const boxEffect);
[[nodiscard]] HRESULT STDMETHODCALLTYPE AppendClusters(const std::basic_string_view<::Microsoft::Console::Render::Cluster> clusters);
[[nodiscard]] HRESULT STDMETHODCALLTYPE Reset() noexcept;
[[nodiscard]] HRESULT STDMETHODCALLTYPE GetColumns(_Out_ UINT32* columns);
// IDWriteTextLayout methods (but we don't actually want to implement them all, so just this one matching the existing interface)

62
src/renderer/dx/CustomTextRenderer.cpp
View file

@ -409,6 +409,40 @@ CATCH_RETURN()
::Microsoft::WRL::ComPtr<ID2D1DeviceContext> d2dContext;
RETURN_IF_FAILED(drawingContext->renderTarget->QueryInterface(d2dContext.GetAddressOf()));
// Determine clip rectangle
D2D1_RECT_F clipRect;
clipRect.top = origin.y;
clipRect.bottom = clipRect.top + drawingContext->cellSize.height;
clipRect.left = 0;
clipRect.right = drawingContext->targetSize.width;
// If we already have a clip rectangle, check if it different than the previous one.
if (_clipRect.has_value())
{
const auto storedVal = _clipRect.value();
// If it is different, pop off the old one and push the new one on.
if (storedVal.top != clipRect.top || storedVal.bottom != clipRect.bottom ||
storedVal.left != clipRect.left || storedVal.right != clipRect.right)
{
d2dContext->PopAxisAlignedClip();
// Clip all drawing in this glyph run to where we expect.
// We need the AntialiasMode here to be Aliased to ensure
// that background boxes line up with each other and don't leave behind
// stray colors.
// See GH#3626 for more details.
d2dContext->PushAxisAlignedClip(clipRect, D2D1_ANTIALIAS_MODE_ALIASED);
_clipRect = clipRect;
}
}
// If we have no clip rectangle, it's easy. Push it on and go.
else
{
// See above for aliased flag explanation.
d2dContext->PushAxisAlignedClip(clipRect, D2D1_ANTIALIAS_MODE_ALIASED);
_clipRect = clipRect;
}
// Draw the background
// The rectangle needs to be deduced based on the origin and the BidiDirection
const auto advancesSpan = gsl::make_span(glyphRun->glyphAdvances, glyphRun->glyphCount);
@ -425,18 +459,6 @@ CATCH_RETURN()
}
rect.right = rect.left + totalSpan;
// Clip all drawing in this glyph run to where we expect.
// We need the AntialiasMode here to be Aliased to ensure
// that background boxes line up with each other and don't leave behind
// stray colors.
// See GH#3626 for more details.
d2dContext->PushAxisAlignedClip(rect, D2D1_ANTIALIAS_MODE_ALIASED);
// Ensure we pop it on the way out
auto popclip = wil::scope_exit([&d2dContext]() noexcept {
d2dContext->PopAxisAlignedClip();
});
d2dContext->FillRectangle(rect, drawingContext->backgroundBrush);
RETURN_IF_FAILED(_drawCursor(d2dContext.Get(), rect, *drawingContext, true));
@ -635,6 +657,22 @@ CATCH_RETURN()
}
#pragma endregion
[[nodiscard]] HRESULT CustomTextRenderer::EndClip(void* clientDrawingContext) noexcept
try
{
DrawingContext* drawingContext = static_cast<DrawingContext*>(clientDrawingContext);
RETURN_HR_IF(E_INVALIDARG, !drawingContext);
if (_clipRect.has_value())
{
drawingContext->renderTarget->PopAxisAlignedClip();
_clipRect = std::nullopt;
}
return S_OK;
}
CATCH_RETURN()
[[nodiscard]] HRESULT CustomTextRenderer::_DrawBasicGlyphRun(DrawingContext* clientDrawingContext,
D2D1_POINT_2F baselineOrigin,
DWRITE_MEASURING_MODE measuringMode,

27
src/renderer/dx/CustomTextRenderer.h
View file

@ -18,18 +18,20 @@ namespace Microsoft::Console::Render
IDWriteFactory* dwriteFactory,
const DWRITE_LINE_SPACING spacing,
const D2D_SIZE_F cellSize,
const D2D_SIZE_F targetSize,
const std::optional<CursorOptions>& cursorInfo,
const D2D1_DRAW_TEXT_OPTIONS options = D2D1_DRAW_TEXT_OPTIONS_NONE) noexcept
const D2D1_DRAW_TEXT_OPTIONS options = D2D1_DRAW_TEXT_OPTIONS_NONE) noexcept :
renderTarget(renderTarget),
foregroundBrush(foregroundBrush),
backgroundBrush(backgroundBrush),
forceGrayscaleAA(forceGrayscaleAA),
dwriteFactory(dwriteFactory),
spacing(spacing),
cellSize(cellSize),
targetSize(targetSize),
cursorInfo(cursorInfo),
options(options)
{
this->renderTarget = renderTarget;
this->foregroundBrush = foregroundBrush;
this->backgroundBrush = backgroundBrush;
this->forceGrayscaleAA = forceGrayscaleAA;
this->dwriteFactory = dwriteFactory;
this->spacing = spacing;
this->cellSize = cellSize;
this->cursorInfo = cursorInfo;
this->options = options;
}
ID2D1RenderTarget* renderTarget;
@ -39,6 +41,7 @@ namespace Microsoft::Console::Render
IDWriteFactory* dwriteFactory;
DWRITE_LINE_SPACING spacing;
D2D_SIZE_F cellSize;
D2D_SIZE_F targetSize;
std::optional<CursorOptions> cursorInfo;
D2D1_DRAW_TEXT_OPTIONS options;
};
@ -98,6 +101,8 @@ namespace Microsoft::Console::Render
BOOL isRightToLeft,
IUnknown* clientDrawingEffect) noexcept override;
[[nodiscard]] HRESULT STDMETHODCALLTYPE EndClip(void* clientDrawingContext) noexcept;
private:
[[nodiscard]] HRESULT _FillRectangle(void* clientDrawingContext,
IUnknown* clientDrawingEffect,
@ -128,5 +133,7 @@ namespace Microsoft::Console::Render
DWRITE_MEASURING_MODE measuringMode,
_In_ const DWRITE_GLYPH_RUN* glyphRun,
_In_opt_ const DWRITE_GLYPH_RUN_DESCRIPTION* glyphRunDescription) noexcept;
std::optional<D2D1_RECT_F> _clipRect;
};
}

297
src/renderer/dx/DxRenderer.cpp
View file

@ -83,6 +83,7 @@ DxEngine::DxEngine() :
_glyphCell{},
_boxDrawingEffect{},
_haveDeviceResources{ false },
_swapChainDesc{ 0 },
_swapChainFrameLatencyWaitableObject{ INVALID_HANDLE_VALUE },
_recreateDeviceRequested{ false },
_retroTerminalEffects{ false },
@ -96,7 +97,9 @@ DxEngine::DxEngine() :
_scale{ 1.0f },
_prevScale{ 1.0f },
_chainMode{ SwapChainMode::ForComposition },
_customRenderer{ ::Microsoft::WRL::Make<CustomTextRenderer>() }
_customLayout{},
_customRenderer{ ::Microsoft::WRL::Make<CustomTextRenderer>() },
_drawingContext{}
{
const auto was = _tracelogCount.fetch_add(1);
if (0 == was)
@ -425,22 +428,30 @@ try
_displaySizePixels = _GetClientSize();
// Get the other device types so we have deeper access to more functionality
// in our pipeline than by just walking straight from the D3D device.
RETURN_IF_FAILED(_d3dDevice.As(&_dxgiDevice));
RETURN_IF_FAILED(_d2dFactory->CreateDevice(_dxgiDevice.Get(), _d2dDevice.ReleaseAndGetAddressOf()));
// Create a device context out of it (supercedes render targets)
RETURN_IF_FAILED(_d2dDevice->CreateDeviceContext(D2D1_DEVICE_CONTEXT_OPTIONS_NONE, &_d2dDeviceContext));
if (createSwapChain)
{
_swapChainFlags = 0;
_swapChainDesc = { 0 };
_swapChainDesc.Flags = 0;
// requires DXGI 1.3 which was introduced in Windows 8.1
WI_SetFlagIf(_swapChainFlags, DXGI_SWAP_CHAIN_FLAG_FRAME_LATENCY_WAITABLE_OBJECT, IsWindows8Point1OrGreater());
WI_SetFlagIf(_swapChainDesc.Flags, DXGI_SWAP_CHAIN_FLAG_FRAME_LATENCY_WAITABLE_OBJECT, IsWindows8Point1OrGreater());
DXGI_SWAP_CHAIN_DESC1 SwapChainDesc = { 0 };
SwapChainDesc.Format = DXGI_FORMAT_B8G8R8A8_UNORM;
SwapChainDesc.BufferUsage = DXGI_USAGE_RENDER_TARGET_OUTPUT;
SwapChainDesc.SwapEffect = DXGI_SWAP_EFFECT_FLIP_SEQUENTIAL;
SwapChainDesc.BufferCount = 2;
SwapChainDesc.SampleDesc.Count = 1;
SwapChainDesc.AlphaMode = DXGI_ALPHA_MODE_UNSPECIFIED;
SwapChainDesc.Scaling = DXGI_SCALING_NONE;
SwapChainDesc.Flags = _swapChainFlags;
_swapChainDesc.Format = DXGI_FORMAT_B8G8R8A8_UNORM;
_swapChainDesc.BufferUsage = DXGI_USAGE_RENDER_TARGET_OUTPUT;
_swapChainDesc.SwapEffect = DXGI_SWAP_EFFECT_FLIP_SEQUENTIAL;
_swapChainDesc.BufferCount = 2;
_swapChainDesc.SampleDesc.Count = 1;
_swapChainDesc.AlphaMode = DXGI_ALPHA_MODE_UNSPECIFIED;
_swapChainDesc.Scaling = DXGI_SCALING_NONE;
switch (_chainMode)
{
@ -450,23 +461,23 @@ try
RECT rect = { 0 };
RETURN_IF_WIN32_BOOL_FALSE(GetClientRect(_hwndTarget, &rect));
SwapChainDesc.Width = rect.right - rect.left;
SwapChainDesc.Height = rect.bottom - rect.top;
_swapChainDesc.Width = rect.right - rect.left;
_swapChainDesc.Height = rect.bottom - rect.top;
// We can't do alpha for HWNDs. Set to ignore. It will fail otherwise.
SwapChainDesc.AlphaMode = DXGI_ALPHA_MODE_IGNORE;
_swapChainDesc.AlphaMode = DXGI_ALPHA_MODE_IGNORE;
const auto createSwapChainResult = _dxgiFactory2->CreateSwapChainForHwnd(_d3dDevice.Get(),
_hwndTarget,
&SwapChainDesc,
&_swapChainDesc,
nullptr,
nullptr,
&_dxgiSwapChain);
if (FAILED(createSwapChainResult))
{
SwapChainDesc.Scaling = DXGI_SCALING_STRETCH;
_swapChainDesc.Scaling = DXGI_SCALING_STRETCH;
RETURN_IF_FAILED(_dxgiFactory2->CreateSwapChainForHwnd(_d3dDevice.Get(),
_hwndTarget,
&SwapChainDesc,
&_swapChainDesc,
nullptr,
nullptr,
&_dxgiSwapChain));
@ -477,16 +488,16 @@ try
case SwapChainMode::ForComposition:
{
// Use the given target size for compositions.
SwapChainDesc.Width = _displaySizePixels.width<UINT>();
SwapChainDesc.Height = _displaySizePixels.height<UINT>();
_swapChainDesc.Width = _displaySizePixels.width<UINT>();
_swapChainDesc.Height = _displaySizePixels.height<UINT>();
// We're doing advanced composition pretty much for the purpose of pretty alpha, so turn it on.
SwapChainDesc.AlphaMode = DXGI_ALPHA_MODE_PREMULTIPLIED;
_swapChainDesc.AlphaMode = DXGI_ALPHA_MODE_PREMULTIPLIED;
// It's 100% required to use scaling mode stretch for composition. There is no other choice.
SwapChainDesc.Scaling = DXGI_SCALING_STRETCH;
_swapChainDesc.Scaling = DXGI_SCALING_STRETCH;
RETURN_IF_FAILED(_dxgiFactory2->CreateSwapChainForComposition(_d3dDevice.Get(),
&SwapChainDesc,
&_swapChainDesc,
nullptr,
&_dxgiSwapChain));
break;
@ -532,7 +543,7 @@ try
if (_isPainting)
{
// TODO: MSFT: 21169176 - remove this or restore the "try a few times to render" code... I think
_d2dRenderTarget->BeginDraw();
_d2dDeviceContext->BeginDraw();
}
freeOnFail.release(); // don't need to release if we made it to the bottom and everything was good.
@ -552,35 +563,59 @@ try
}
CATCH_RETURN();
static constexpr D2D1_ALPHA_MODE _dxgiAlphaToD2d1Alpha(DXGI_ALPHA_MODE mode) noexcept
{
switch (mode)
{
case DXGI_ALPHA_MODE_PREMULTIPLIED:
return D2D1_ALPHA_MODE_PREMULTIPLIED;
case DXGI_ALPHA_MODE_STRAIGHT:
return D2D1_ALPHA_MODE_STRAIGHT;
case DXGI_ALPHA_MODE_IGNORE:
return D2D1_ALPHA_MODE_IGNORE;
case DXGI_ALPHA_MODE_FORCE_DWORD:
return D2D1_ALPHA_MODE_FORCE_DWORD;
default:
case DXGI_ALPHA_MODE_UNSPECIFIED:
return D2D1_ALPHA_MODE_UNKNOWN;
}
}
[[nodiscard]] HRESULT DxEngine::_PrepareRenderTarget() noexcept
{
try
{
// Pull surface out of swap chain.
RETURN_IF_FAILED(_dxgiSwapChain->GetBuffer(0, IID_PPV_ARGS(&_dxgiSurface)));
const D2D1_RENDER_TARGET_PROPERTIES props =
D2D1::RenderTargetProperties(
D2D1_RENDER_TARGET_TYPE_DEFAULT,
D2D1::PixelFormat(DXGI_FORMAT_UNKNOWN, D2D1_ALPHA_MODE_PREMULTIPLIED),
0.0f,
0.0f);
// Make a bitmap and bind it to the swap chain surface
const auto bitmapProperties = D2D1::BitmapProperties1(
D2D1_BITMAP_OPTIONS_TARGET | D2D1_BITMAP_OPTIONS_CANNOT_DRAW,
D2D1::PixelFormat(_swapChainDesc.Format, _dxgiAlphaToD2d1Alpha(_swapChainDesc.AlphaMode)));
RETURN_IF_FAILED(_d2dFactory->CreateDxgiSurfaceRenderTarget(_dxgiSurface.Get(),
&props,
&_d2dRenderTarget));
RETURN_IF_FAILED(_d2dDeviceContext->CreateBitmapFromDxgiSurface(_dxgiSurface.Get(), bitmapProperties, &_d2dBitmap));
// Assign that bitmap as the target of the D2D device context. Draw commands hit the context
// and are backed by the bitmap which is bound to the swap chain which goes on to be presented.
// (The foot bone connected to the leg bone,
// The leg bone connected to the knee bone,
// The knee bone connected to the thigh bone
// ... and so on)
_d2dDeviceContext->SetTarget(_d2dBitmap.Get());
// We need the AntialiasMode for non-text object to be Aliased to ensure
// that background boxes line up with each other and don't leave behind
// stray colors.
// See GH#3626 for more details.
_d2dRenderTarget->SetAntialiasMode(D2D1_ANTIALIAS_MODE_ALIASED);
_d2dRenderTarget->SetTextAntialiasMode(_antialiasingMode);
_d2dDeviceContext->SetAntialiasMode(D2D1_ANTIALIAS_MODE_ALIASED);
_d2dDeviceContext->SetTextAntialiasMode(_antialiasingMode);
RETURN_IF_FAILED(_d2dRenderTarget->CreateSolidColorBrush(D2D1::ColorF(D2D1::ColorF::DarkRed),
&_d2dBrushBackground));
RETURN_IF_FAILED(_d2dDeviceContext->CreateSolidColorBrush(D2D1::ColorF(D2D1::ColorF::DarkRed),
&_d2dBrushBackground));
RETURN_IF_FAILED(_d2dRenderTarget->CreateSolidColorBrush(D2D1::ColorF(D2D1::ColorF::White),
&_d2dBrushForeground));
RETURN_IF_FAILED(_d2dDeviceContext->CreateSolidColorBrush(D2D1::ColorF(D2D1::ColorF::White),
&_d2dBrushForeground));
const D2D1_STROKE_STYLE_PROPERTIES strokeStyleProperties{
D2D1_CAP_STYLE_SQUARE, // startCap
@ -628,17 +663,22 @@ void DxEngine::_ReleaseDeviceResources() noexcept
_d2dBrushForeground.Reset();
_d2dBrushBackground.Reset();
if (nullptr != _d2dRenderTarget.Get() && _isPainting)
_d2dBitmap.Reset();
if (nullptr != _d2dDeviceContext.Get() && _isPainting)
{
_d2dRenderTarget->EndDraw();
_d2dDeviceContext->EndDraw();
}
_d2dRenderTarget.Reset();
_d2dDeviceContext.Reset();
_dxgiSurface.Reset();
_dxgiSwapChain.Reset();
_swapChainFrameLatencyWaitableObject.reset();
_d2dDevice.Reset();
_dxgiDevice.Reset();
if (nullptr != _d3dDeviceContext.Get())
{
// To ensure the swap chain goes away we must unbind any views from the
@ -654,6 +694,44 @@ void DxEngine::_ReleaseDeviceResources() noexcept
CATCH_LOG();
}
// Routine Description:
// - Calculates whether or not we should force grayscale AA based on the
// current renderer state.
// Arguments:
// - <none> - Uses internal state of _antialiasingMode, _defaultTextBackgroundOpacity,
// _backgroundColor, and _defaultBackgroundColor.
// Return Value:
// - True if we must render this text in grayscale AA as cleartype simply won't work. False otherwise.
[[nodiscard]] bool DxEngine::_ShouldForceGrayscaleAA() noexcept
{
// GH#5098: If we're rendering with cleartype text, we need to always
// render onto an opaque background. If our background's opacity is
// 1.0f, that's great, we can use that. Otherwise, we need to force the
// text renderer to render this text in grayscale. In
// UpdateDrawingBrushes, we'll set the backgroundColor's a channel to
// 1.0 if we're in cleartype mode and the background's opacity is 1.0.
// Otherwise, at this point, the _backgroundColor's alpha is <1.0.
//
// Currently, only text with the default background color uses an alpha
// of 0, every other background uses 1.0
//
// DANGER: Layers slow us down. Only do this in the specific case where
// someone has chosen the slower ClearType antialiasing (versus the faster
// grayscale antialiasing)
const bool usingCleartype = _antialiasingMode == D2D1_TEXT_ANTIALIAS_MODE_CLEARTYPE;
const bool usingTransparency = _defaultTextBackgroundOpacity != 1.0f;
// Another way of naming "bgIsDefault" is "bgHasTransparency"
const auto bgIsDefault = (_backgroundColor.a == _defaultBackgroundColor.a) &&
(_backgroundColor.r == _defaultBackgroundColor.r) &&
(_backgroundColor.g == _defaultBackgroundColor.g) &&
(_backgroundColor.b == _defaultBackgroundColor.b);
const bool forceGrayscaleAA = usingCleartype &&
usingTransparency &&
bgIsDefault;
return forceGrayscaleAA;
}
// Routine Description:
// - Helper to create a DirectWrite text layout object
// out of a string.
@ -696,6 +774,7 @@ CATCH_RETURN()
try
{
_sizeTarget = Pixels;
_invalidMap.resize(_sizeTarget / _glyphCell, true);
return S_OK;
}
@ -1007,10 +1086,11 @@ try
// Now let go of a few of the device resources that get in the way of resizing buffers in the swap chain
_dxgiSurface.Reset();
_d2dRenderTarget.Reset();
_d2dDeviceContext->SetTarget(nullptr);
_d2dBitmap.Reset();
// Change the buffer size and recreate the render target (and surface)
RETURN_IF_FAILED(_dxgiSwapChain->ResizeBuffers(2, clientSize.width<UINT>(), clientSize.height<UINT>(), DXGI_FORMAT_B8G8R8A8_UNORM, _swapChainFlags));
RETURN_IF_FAILED(_dxgiSwapChain->ResizeBuffers(2, clientSize.width<UINT>(), clientSize.height<UINT>(), _swapChainDesc.Format, _swapChainDesc.Flags));
RETURN_IF_FAILED(_PrepareRenderTarget());
// OK we made it past the parts that can cause errors. We can release our failure handler.
@ -1023,8 +1103,26 @@ try
_firstFrame = true;
}
_d2dRenderTarget->BeginDraw();
_d2dDeviceContext->BeginDraw();
_isPainting = true;
{
// Get the baseline for this font as that's where we draw from
DWRITE_LINE_SPACING spacing;
RETURN_IF_FAILED(_dwriteTextFormat->GetLineSpacing(&spacing.method, &spacing.height, &spacing.baseline));
// Assemble the drawing context information
_drawingContext = std::make_unique<DrawingContext>(_d2dDeviceContext.Get(),
_d2dBrushForeground.Get(),
_d2dBrushBackground.Get(),
_ShouldForceGrayscaleAA(),
_dwriteFactory.Get(),
spacing,
_glyphCell,
_d2dDeviceContext->GetSize(),
std::nullopt,
D2D1_DRAW_TEXT_OPTIONS_ENABLE_COLOR_FONT);
}
}
return S_OK;
@ -1048,7 +1146,10 @@ try
{
_isPainting = false;
hr = _d2dRenderTarget->EndDraw();
// If there's still a clip hanging around, remove it. We're all done.
LOG_IF_FAILED(_customRenderer->EndClip(_drawingContext.get()));
hr = _d2dDeviceContext->EndDraw();
if (SUCCEEDED(hr))
{
@ -1279,14 +1380,14 @@ try
// If the entire thing is invalid, just use one big clear operation.
if (_invalidMap.all())
{
_d2dRenderTarget->Clear(nothing);
_d2dDeviceContext->Clear(nothing);
}
else
{
// Runs are counts of cells.
// Use a transform by the size of one cell to convert cells-to-pixels
// as we clear.
_d2dRenderTarget->SetTransform(D2D1::Matrix3x2F::Scale(_glyphCell));
_d2dDeviceContext->SetTransform(D2D1::Matrix3x2F::Scale(_glyphCell));
for (const auto rect : _invalidMap.runs())
{
// Use aliased.
@ -1294,11 +1395,11 @@ try
// the edges are cut nice and sharp (not blended by anti-aliasing).
// For performance reasons, it takes a lot less work to not
// do anti-alias blending.
_d2dRenderTarget->PushAxisAlignedClip(rect, D2D1_ANTIALIAS_MODE_ALIASED);
_d2dRenderTarget->Clear(nothing);
_d2dRenderTarget->PopAxisAlignedClip();
_d2dDeviceContext->PushAxisAlignedClip(rect, D2D1_ANTIALIAS_MODE_ALIASED);
_d2dDeviceContext->Clear(nothing);
_d2dDeviceContext->PopAxisAlignedClip();
}
_d2dRenderTarget->SetTransform(D2D1::Matrix3x2F::Identity());
_d2dDeviceContext->SetTransform(D2D1::Matrix3x2F::Identity());
}
return S_OK;
@ -1323,56 +1424,11 @@ try
const D2D1_POINT_2F origin = til::point{ coord } * _glyphCell;
// Create the text layout
CustomTextLayout layout(_dwriteFactory.Get(),
_dwriteTextAnalyzer.Get(),
_dwriteTextFormat.Get(),
_dwriteFontFace.Get(),
clusters,
_glyphCell.width(),
_boxDrawingEffect.Get());
// Get the baseline for this font as that's where we draw from
DWRITE_LINE_SPACING spacing;
RETURN_IF_FAILED(_dwriteTextFormat->GetLineSpacing(&spacing.method, &spacing.height, &spacing.baseline));
// GH#5098: If we're rendering with cleartype text, we need to always
// render onto an opaque background. If our background's opacity is
// 1.0f, that's great, we can use that. Otherwise, we need to force the
// text renderer to render this text in grayscale. In
// UpdateDrawingBrushes, we'll set the backgroundColor's a channel to
// 1.0 if we're in cleartype mode and the background's opacity is 1.0.
// Otherwise, at this point, the _backgroundColor's alpha is <1.0.
//
// Currently, only text with the default background color uses an alpha
// of 0, every other background uses 1.0
//
// DANGER: Layers slow us down. Only do this in the specific case where
// someone has chosen the slower ClearType antialiasing (versus the faster
// grayscale antialiasing)
const bool usingCleartype = _antialiasingMode == D2D1_TEXT_ANTIALIAS_MODE_CLEARTYPE;
const bool usingTransparency = _defaultTextBackgroundOpacity != 1.0f;
// Another way of naming "bgIsDefault" is "bgHasTransparency"
const auto bgIsDefault = (_backgroundColor.a == _defaultBackgroundColor.a) &&
(_backgroundColor.r == _defaultBackgroundColor.r) &&
(_backgroundColor.g == _defaultBackgroundColor.g) &&
(_backgroundColor.b == _defaultBackgroundColor.b);
const bool forceGrayscaleAA = usingCleartype &&
usingTransparency &&
bgIsDefault;
// Assemble the drawing context information
DrawingContext context(_d2dRenderTarget.Get(),
_d2dBrushForeground.Get(),
_d2dBrushBackground.Get(),
forceGrayscaleAA,
_dwriteFactory.Get(),
spacing,
_glyphCell,
_frameInfo.cursorInfo,
D2D1_DRAW_TEXT_OPTIONS_ENABLE_COLOR_FONT);
RETURN_IF_FAILED(_customLayout->Reset());
RETURN_IF_FAILED(_customLayout->AppendClusters(clusters));
// Layout then render the text
RETURN_IF_FAILED(layout.Draw(&context, _customRenderer.Get(), origin.x, origin.y));
RETURN_IF_FAILED(_customLayout->Draw(_drawingContext.get(), _customRenderer.Get(), origin.x, origin.y));
return S_OK;
}
@ -1415,7 +1471,7 @@ try
end = start;
end.x += font.width();
_d2dRenderTarget->DrawLine(start, end, _d2dBrushForeground.Get(), 1.0f, _strokeStyle.Get());
_d2dDeviceContext->DrawLine(start, end, _d2dBrushForeground.Get(), 1.0f, _strokeStyle.Get());
}
if (lines & GridLines::Left)
@ -1423,7 +1479,7 @@ try
end = start;
end.y += font.height();
_d2dRenderTarget->DrawLine(start, end, _d2dBrushForeground.Get(), 1.0f, _strokeStyle.Get());
_d2dDeviceContext->DrawLine(start, end, _d2dBrushForeground.Get(), 1.0f, _strokeStyle.Get());
}
// NOTE: Watch out for inclusive/exclusive rectangles here.
@ -1441,7 +1497,7 @@ try
end = start;
end.x += font.width() - 1.f;
_d2dRenderTarget->DrawLine(start, end, _d2dBrushForeground.Get(), 1.0f, _strokeStyle.Get());
_d2dDeviceContext->DrawLine(start, end, _d2dBrushForeground.Get(), 1.0f, _strokeStyle.Get());
}
start = { target.x + font.width() - 0.5f, target.y + 0.5f };
@ -1451,7 +1507,7 @@ try
end = start;
end.y += font.height() - 1.f;
_d2dRenderTarget->DrawLine(start, end, _d2dBrushForeground.Get(), 1.0f, _strokeStyle.Get());
_d2dDeviceContext->DrawLine(start, end, _d2dBrushForeground.Get(), 1.0f, _strokeStyle.Get());
}
// Move to the next character in this run.
@ -1471,6 +1527,9 @@ CATCH_RETURN()
[[nodiscard]] HRESULT DxEngine::PaintSelection(const SMALL_RECT rect) noexcept
try
{
// If a clip rectangle is in place from drawing the text layer, remove it here.
LOG_IF_FAILED(_customRenderer->EndClip(_drawingContext.get()));
const auto existingColor = _d2dBrushForeground->GetColor();
_d2dBrushForeground->SetColor(_selectionBackground);
@ -1478,7 +1537,7 @@ try
const D2D1_RECT_F draw = til::rectangle{ Viewport::FromExclusive(rect).ToInclusive() }.scale_up(_glyphCell);
_d2dRenderTarget->FillRectangle(draw, _d2dBrushForeground.Get());
_d2dDeviceContext->FillRectangle(draw, _d2dBrushForeground.Get());
return S_OK;
}
@ -1592,6 +1651,17 @@ CATCH_RETURN()
}*/
}
// If we have a drawing context, it may be choosing its antialiasing based
// on the colors. Update it if it exists.
// We only need to do this here because this is called all the time on painting frames
// and will update it in a timely fashion. Changing the AA mode or opacity do affect
// it, but we will always hit updating the drawing brushes so we don't
// need to update this in those locations.
if (_drawingContext)
{
_drawingContext->forceGrayscaleAA = _ShouldForceGrayscaleAA();
}
return S_OK;
}
@ -1617,6 +1687,9 @@ try
// Calculate and cache the box effect for the base font. Scale is 1.0f because the base font is exactly the scale we want already.
RETURN_IF_FAILED(CustomTextLayout::s_CalculateBoxEffect(_dwriteTextFormat.Get(), _glyphCell.width(), _dwriteFontFace.Get(), 1.0f, &_boxDrawingEffect));
// Prepare the text layout
_customLayout = WRL::Make<CustomTextLayout>(_dwriteFactory.Get(), _dwriteTextAnalyzer.Get(), _dwriteTextFormat.Get(), _dwriteFontFace.Get(), _glyphCell.width(), _boxDrawingEffect.Get());
return S_OK;
}
CATCH_RETURN();
@ -1744,17 +1817,11 @@ try
const Cluster cluster(glyph, 0); // columns don't matter, we're doing analysis not layout.
// Create the text layout
CustomTextLayout layout(_dwriteFactory.Get(),
_dwriteTextAnalyzer.Get(),
_dwriteTextFormat.Get(),
_dwriteFontFace.Get(),
{ &cluster, 1 },
_glyphCell.width(),
_boxDrawingEffect.Get());
RETURN_IF_FAILED(_customLayout->Reset());
RETURN_IF_FAILED(_customLayout->AppendClusters({ &cluster, 1 }));
UINT32 columns = 0;
RETURN_IF_FAILED(layout.GetColumns(&columns));
RETURN_IF_FAILED(_customLayout->GetColumns(&columns));
*pResult = columns != 1;
@ -2245,6 +2312,6 @@ CATCH_LOG()
// - S_OK
[[nodiscard]] HRESULT DxEngine::PrepareRenderInfo(const RenderFrameInfo& info) noexcept
{
_frameInfo = info;
_drawingContext->cursorInfo = info.cursorInfo;
return S_OK;
}

25
src/renderer/dx/DxRenderer.hpp
View file

@ -13,6 +13,7 @@
#include <d3d11.h>
#include <d2d1.h>
#include <d2d1_1.h>
#include <d2d1helper.h>
#include <dwrite.h>
#include <dwrite_1.h>
@ -22,6 +23,7 @@
#include <wrl.h>
#include <wrl/client.h>
#include "CustomTextLayout.h"
#include "CustomTextRenderer.h"
#include "../../types/inc/Viewport.hpp"
@ -166,11 +168,13 @@ namespace Microsoft::Console::Render
static std::atomic<size_t> _tracelogCount;
// Device-Independent Resources
::Microsoft::WRL::ComPtr<ID2D1Factory> _d2dFactory;
::Microsoft::WRL::ComPtr<ID2D1Factory1> _d2dFactory;
::Microsoft::WRL::ComPtr<IDWriteFactory1> _dwriteFactory;
::Microsoft::WRL::ComPtr<IDWriteTextFormat> _dwriteTextFormat;
::Microsoft::WRL::ComPtr<IDWriteFontFace1> _dwriteFontFace;
::Microsoft::WRL::ComPtr<IDWriteTextAnalyzer1> _dwriteTextAnalyzer;
::Microsoft::WRL::ComPtr<CustomTextLayout> _customLayout;
::Microsoft::WRL::ComPtr<CustomTextRenderer> _customRenderer;
::Microsoft::WRL::ComPtr<ID2D1StrokeStyle> _strokeStyle;
@ -179,14 +183,21 @@ namespace Microsoft::Console::Render
bool _haveDeviceResources;
::Microsoft::WRL::ComPtr<ID3D11Device> _d3dDevice;
::Microsoft::WRL::ComPtr<ID3D11DeviceContext> _d3dDeviceContext;
::Microsoft::WRL::ComPtr<IDXGIFactory2> _dxgiFactory2;
::Microsoft::WRL::ComPtr<IDXGISurface> _dxgiSurface;
::Microsoft::WRL::ComPtr<ID2D1RenderTarget> _d2dRenderTarget;
::Microsoft::WRL::ComPtr<ID2D1Device> _d2dDevice;
::Microsoft::WRL::ComPtr<ID2D1DeviceContext> _d2dDeviceContext;
::Microsoft::WRL::ComPtr<ID2D1Bitmap1> _d2dBitmap;
::Microsoft::WRL::ComPtr<ID2D1SolidColorBrush> _d2dBrushForeground;
::Microsoft::WRL::ComPtr<ID2D1SolidColorBrush> _d2dBrushBackground;
UINT _swapChainFlags;
::Microsoft::WRL::ComPtr<IDXGIFactory2> _dxgiFactory2;
::Microsoft::WRL::ComPtr<IDXGIDevice> _dxgiDevice;
::Microsoft::WRL::ComPtr<IDXGISurface> _dxgiSurface;
DXGI_SWAP_CHAIN_DESC1 _swapChainDesc;
::Microsoft::WRL::ComPtr<IDXGISwapChain1> _dxgiSwapChain;
wil::unique_handle _swapChainFrameLatencyWaitableObject;
std::unique_ptr<DrawingContext> _drawingContext;
// Terminal effects resources.
bool _retroTerminalEffects;
@ -207,8 +218,6 @@ namespace Microsoft::Console::Render
float _defaultTextBackgroundOpacity;
RenderFrameInfo _frameInfo;
// DirectX constant buffers need to be a multiple of 16; align to pad the size.
__declspec(align(16)) struct
{
@ -225,6 +234,8 @@ namespace Microsoft::Console::Render
void _ReleaseDeviceResources() noexcept;
bool _ShouldForceGrayscaleAA() noexcept;
[[nodiscard]] HRESULT _CreateTextLayout(
_In_reads_(StringLength) PCWCHAR String,
_In_ size_t StringLength,

8
src/tools/U8U16Test/U8U16Test.cpp
View file

@ -303,7 +303,7 @@ void u16state::reset() noexcept
}
// *** convert the code point to UTF-16 ***
if (codePoint != unicodeReplacementChar || discardInvalids == false)
if (codePoint != unicodeReplacementChar || !discardInvalids)
{
if (codePoint < 0x00010000u)
{
@ -471,7 +471,7 @@ void u16state::reset() noexcept
}
// *** convert the code point to UTF-16 ***
if (codePoint != unicodeReplacementChar || discardInvalids == false)
if (codePoint != unicodeReplacementChar || !discardInvalids)
{
if (codePoint < 0x00010000u)
{
@ -562,7 +562,7 @@ void u16state::reset() noexcept
}
// *** convert the code point to UTF-8 ***
if (codePoint != unicodeReplacementChar || discardInvalids == false)
if (codePoint != unicodeReplacementChar || !discardInvalids)
{
// the outcome of performance tests is that subsequent calls of push_back
// perform much better than appending a single initializer_list
@ -664,7 +664,7 @@ void u16state::reset() noexcept
}
// *** convert the code point to UTF-8 ***
if (codePoint != unicodeReplacementChar || discardInvalids == false)
if (codePoint != unicodeReplacementChar || !discardInvalids)
{
// the outcome of further performance tests is that using pointers
// perform even better than subsequent calls of push_back

2
src/tools/U8U16Test/main.cpp
View file

@ -538,7 +538,7 @@ ptrdiff_t RandomIndex(ptrdiff_t length)
{
static bool generatorInitialized{ false };
static std::default_random_engine generator;
if (generatorInitialized == false)
if (!generatorInitialized)
{
generator.seed(static_cast<unsigned>(std::chrono::system_clock::now().time_since_epoch().count()));
generatorInitialized = true;

4
src/tools/pixels/main.cpp
View file

@ -222,9 +222,7 @@ int __cdecl wmain(int /*argc*/, WCHAR* /*argv*/[])
CONSOLE_SCREEN_BUFFER_INFOEX csbiex = { 0 };
csbiex.cbSize = sizeof(csbiex);
BOOL b = GetConsoleScreenBufferInfoEx(hOut, &csbiex);
if (b == FALSE)
if (!GetConsoleScreenBufferInfoEx(hOut, &csbiex))
{
wcout << GetLastError() << endl;
}