-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pass through soft fonts over conpty #13965
Conversation
This is so clever! I'm excited to review it. Thanks! |
src/renderer/vt/paint.cpp
Outdated
// Write the actual text string. If we're using a soft font, the character | ||
// set should have already been selected, so we just need to map our internal | ||
// representation back to ASCII (handled by the _WriteTerminalDrcs method). | ||
if (_usingSoftFont) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (_usingSoftFont) | |
if (_usingSoftFont) [[unlikely]] |
@lhecker is this the correct usage of that annotation in C++?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that would work. likely/unlikely aren't particularly impactful in MSVC right now, but it does influence the inalienability of functions quite a bit, which can help make our binaries more compact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just tried this, and it does make a bit of a difference. Both of the _WriteTerminalXXX
calls are inline, so there is quite a lot of code in the two branches of that condition. With the unlikely
attribute added, part of the first branch gets pulled out of the main body of code, so there is a slightly shorter jump to the else branch.
I don't know if that's really worth the effort, though. I think if you wanted to make a significant difference with these attributes, a good place to put them might be in the RETURN_IF_FAILED
macros. That should remove all that diagnostic logging from the main path of execution which is probably quite a chunk of code.
I'm not an optimization expert though, so I'll leave it to you guys to decide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only saw Leonard's reply after I posted mine. I've applied the suggested change now.
Apply [[unlikely]] attribute to less used branch. Co-authored-by: Dustin L. Howett <dustin@howett.net>
Hello @DHowett! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
🎉 Handy links: |
This PR introduces a mechanism for passing through downloadable soft
fonts to the conpty client, so that we can support DRCS (Dynamically
Redefinable Character Sets) in Windows Terminal.
Soft fonts were first implemented in conhost (with the GDI renderer) in
PR #10011, and were implemented in the DX renderer in PR #13362.
The way this works is by passing through the
DECDLD
sequencecontaining the font definition, but with the character set ID patched to
use a hardcoded value (this is to make sure it's not going to override
the default character set). At the same time we send through an
SCS
sequence to map this character set into the G1 table so we can easily
activate it.
We still need to process the
DECDLD
sequence locally, though, sincethe initial character set mapping take place on the host side. This gets
the DRCS characters into our buffer as PUA Unicode characters. Then when
the VT engine needs to output these characters, it masks them with
7F
to map them back to ASCII, and outputs an
SO
control to activate thesoft font in the conpty client.
Validation Steps Performed
I've manually tested with a number of soft fonts and applications that
make use of soft fonts. But if you're testing with the VT320 fonts from
the vt100.net collection, note that you'll need to enable the ISO-2022
coding system first, since they use 8-bit C1 controls.