Add support for downloadable soft fonts #10011

j4james · 2021-05-01T17:09:15Z

This PR adds conhost support for downloadable soft fonts - also known as
dynamically redefinable character sets (DRCS) - using the DECDLD
escape sequence.

These fonts are typically designed to work on a specific terminal model,
and each model tends to have a different character cell size. So in
order to support as many models as possible, the code attempts to detect
the original target size of the font, and then scale the glyphs to fit
our current cell size.

Once a font has been downloaded to the terminal, it can be designated in
the same way you would a standard character set, using an SCS escape
sequence. The identification string for the set is defined by the
DECDLD sequence. Internally we map the characters in this set to code
points U+EF20 to U+EF7F in the Unicode private use are (PUA).

Then in the renderer, any characters in that range are split off into
separate runs, which get painted with a special font. The font itself is
dynamically generated as an in-memory resource, constructed from the
downloaded character bitmaps which have been scaled to the appropriate
size.

If no soft fonts are in use, then no mapping of the PUA code points will
take place, so this shouldn't interfere with anyone using those code
points for something else, as along as they aren't also trying to use
soft fonts. I also tried to pick a PUA range that hadn't already been
snatched up by Nerd Fonts, but if we do receive reports of a conflict,
it's easy enough to change.

Validation Steps Performed

I added an adapter test that runs through a bunch of parameter
variations for the DECDLD sequence, to make sure we're correctly
detecting the font sizes for most of the known DEC terminal models.

I've also tested manually on a wide range of existing fonts, of varying
dimensions, and from multiple sources, and made sure they all worked
reasonably well.

Closes #9164

.github/actions/spelling/dictionary/apis.txt

src/renderer/base/lib/base.vcxproj.filters

crutkas · 2021-05-03T21:32:45Z

Hey @j4james can you shoot me/kayla an email? [email protected]

DHowett · 2021-05-05T00:20:37Z

SO excited to read this.

src/terminal/adapter/FontBuffer.cpp

DHowett

30/49 -- have yet to get into the meat of it but I am setting a checkpoint here.

This is really excellent work. I'm so stoked about it :D

src/renderer/base/lib/base.vcxproj.filters

DHowett · 2021-05-21T21:43:28Z

src/renderer/gdi/gdirenderer.hpp

@@ -95,6 +100,7 @@ namespace Microsoft::Console::Render
        HFONT _hfont;
        HFONT _hfontItalic;
        TEXTMETRICW _tmFontMetrics;
+        FontResource _softFont;


How much does this grow the GdiEngine by? I'm not terribly concerned about it, but it's just a thing to keep in mind.

I think it's 80 bytes in the x64 build. There's a std::vector (24 bytes), two til::size fields (16 bytes each), a size_t and two resource handles (8 each). I'm guessing the x86 build is around half that. Obviously there's going to be more memory involved once you actually create a soft font, but that'll be freed again if you execute an RIS reset.

github-actions · 2021-05-22T12:01:46Z

@check-spelling-bot Report

Unrecognized words, please review:

AAAAA
AAAAAAAAAAAAA
BBBBB
BBBBBBBB
Gravell
It'd
ok'd
pws
qos
RIS
sxs
sys
We'd
ZZZZZ

Previously acknowledged words that are now absent

apos bc ci conpixels cplinfo crt cw cx DECDLD df dh dw eb EK ev exeuwp fd Fx gb gh Gravell's hc hh hk hu Kd KF KJ KU KX lk nc Nx oa OI Oj Oo oq Ou Ov pb pv pw px py qi QJ qo ri Rl rmdir ru smalllogo splashscreen storelogo sx sy sz td tf tl tt uk ul vf vk wx xa xy Yw yx yy YZ Zc zd zh ZM zu zy zz

Some files were were automatically ignored

These sample patterns would exclude them:

^src/host/ft_uia/run\.bat$
^src/host/runft\.bat$
^src/host/runut\.bat$
^src/terminal/adapter/ut_adapter/run\.bat$
^src/terminal/parser/delfuzzpayload\.bat$
^src/terminal/parser/ft_fuzzer/run\.bat$
^src/terminal/parser/ft_fuzzwrapper/run\.bat$
^src/terminal/parser/ut_parser/run\.bat$
^src/tools/integrity/packageuwp/ConsoleUWP\.appxSources$
^src/tools/lnkd/lnkd\.bat$
^src/tools/pixels/pixels\.bat$
^src/tools/texttests/fira\.txt$

You should consider excluding directory paths (e.g. (?:^|/)vendor/), filenames (e.g. (?:^|/)yarn\.lock$), or file extensions (e.g. \.gz$)

You should consider adding them to:

.github/actions/spelling/excludes.txt

File matching is via Perl regular expressions.

To check these files, more of their words need to be in the dictionary than not. You can use patterns.txt to exclude portions, add items to the dictionary (e.g. by adding them to allow.txt), or fix typos.

To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands

... in a clone of the [email protected]:j4james/terminal.git repository
on the feature-decdld branch:

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spelling/expect/alphabet.txt
.github/actions/spelling/expect/expect.txt
.github/actions/spelling/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spelling/expect/3f82613a3da742a6ab996f7684be161ba77ad002.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
(cat '.github/actions/spelling/excludes.txt' - <<EOF
$should_exclude_patterns
EOF
) |grep .|
sort -f |
uniq > '.github/actions/spelling/excludes.txt.temp' &&
mv '.github/actions/spelling/excludes.txt.temp' '.github/actions/spelling/excludes.txt'
}

comment_json=$(mktemp)
curl -L -s -S \
  --header "Content-Type: application/json" \
  "https://api.github.com/repos/microsoft/terminal/issues/comments/846398325" > "$comment_json"
comment_body=$(mktemp)
jq -r .body < "$comment_json" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")
  

patch_add=$(perl -e '$/=undef;
$_=<>;
s{<details>.*}{}s;
s{^#.*}{};
s{\n##.*}{};
s{(?:^|\n)\s*\*}{}g;
s{\s+}{ }g;
print' < "$comment_body")
  

should_exclude_patterns=$(perl -e '$/=undef;
$_=<>;
exit unless s{(?:You should consider excluding directory paths|You should consider adding them to).*}{}s;
s{.*These sample patterns would exclude them:}{}s;
s{.*\`\`\`([^`]*)\`\`\`.*}{$1}m;
print' < "$comment_body" | grep . || true)

update_files
rm $comment_body
git add -u

src/renderer/base/FontResource.cpp

src/terminal/adapter/FontBuffer.cpp

lhecker · 2021-05-22T14:22:46Z

While this PR contains a lot of extremely helpful comments, I'm having (or had) a hard time with the more arithmetic code (like _generateErrorGlyph or _regenerateFont) as well as the overarching design between FontResource and FontBuffer.
The way I understand it now is that the DRCS sixels are parsed into a bitmap, contained within FontBuffer. That bitmap is then passed to FontResource which maps it to the format that GDI expects it in. Is that correct?
I recently wrote this comment for til::rle. In my humble opinion this PR could greatly benefit from comments which equally lay down this "overarching design". For instance just a single large comment - not as large as mine of course 😄 - explaining the "why is the font stored as a bitmap instead of X and how does the bitmap look like if we receive the hypothetical sixel data xyz, etc." would make more detailed comments redundant. A lot of the code contained within this PR might be forever too hard to explain with mere comments anyways, simply due to the fact that the problem being solved is just hard. Maybe that's just me though - I'm not as knowledgeable as you in this field after all. But assuming that I'm not alone in feeling this is a naturally "hard" problem being solved here, such "design comments" would IMHO greatly improve the speed at which newcomers could get started modifying your code in the future.

P.S.: I'm sorry that I can only give superficial comments. I'm still trying to wrap my head around all this, as it's my first time really looking at the render part of this project.

j4james · 2021-05-23T00:11:20Z

The way I understand it now is that the DRCS sixels are parsed into a bitmap, contained within FontBuffer. That bitmap is then passed to FontResource which maps it to the format that GDI expects it in. Is that correct?

More or less. Architecturally, this is just "separation of concerns". The adapter is responsible for parsing the sixel data, but shouldn't need to know anything about how it's going to be rendered. So the FontBuffer just stores the parsed glyph bitmaps in the simplest generic structure possible: essentially a list of scanlines.

The renderers are then responsible for taking that data and actually rendering it. The GDI renderer currently uses an in-memory font resource for this (managed by the FontResource class), but in previous iterations I had it StretchBlting bitmaps, and the DX renderer might take yet another approach. The adapter doing the parsing doesn't need to know any of those details, though - hence the separation.

In my humble opinion this PR could greatly benefit from comments which equally lay down this "overarching design".

You're probably right, but realistically I think it's unlikely I'll have the enthusiasm to produce any more documentation for this code. I'm generally not a big comment writer at the best of time, and this PR has already exceeded my typical comment quota by a few orders of magnitude. That said, I'm happy to try answer specific questions if there's something in particular that's not clear.

miniksa

Very awesome. I have to trust to some degree on the algorithms but they make general sense to me. Very intriguing scaling the bits into the font size we need and forming a memory-FNT so GDI will load it. We'll have to figure out the best strategy one day for getting DirectWrite to see that (as well as how to get ConPTY to pass this out), but a lot of the "figuring it out" to read the data is already done here which is a big tough part.

I'm also curious... now that we have some amount of sixel parsing... how we could support sixel graphics by just reserving areas of the text buffer and pointing at some bitmap space to the renderers... HMMMMMM. (That could also be a way to make DirectWrite do it... just set up a bitmap on the PUA region and blit the bitmap characters into a ID2D1Bitmap and then compose that over where DirectWrite would have been asked to place characters...)

Anyway... intriguing and very cool. Thank you very much, @j4james, for continuing to push the boundaries of what conhost can accept in the VT world.

src/renderer/base/renderer.cpp

miniksa · 2021-05-24T16:54:15Z

src/renderer/base/FontResource.cpp

+    }
+}
+
+FontResource::operator HFONT()


This has me thinking about how we possibly do this for the DxRenderer.

I imagine that we will either have to...

Form all this data into an alternate stream that looks like a TTF/OTF font instead of this old Windows Font format that GDI seems to understand on AddFontMemoryResourceEx and then feed it into IDWriteInMemoryFontFileLoader::CreateInMemoryFontFileReference per the documentation at Custom Font Sets.

Make the DxRenderer load this HFONT into a GDI HDC and then call IDWriteGdiInterop::CreateFontFaceFromHdc and use the resulting IDWriteFontFace normally.

I bet the second one is easier but is a little gross for using GDI inside the DX renderer. The first one is probably difficult because I think TTF/OTF always specifies fonts as vector outlines, so I'm not sure how we'd live-specify these. Probably a lot more math...

Just ideas.

I was kind of hoping we could just register the font the same way we are in GDI with AddFontMemResourceEx, and the given font name would then be available for use in DX, but maybe that's nonsense. And that's still dependent on a GDI API so in that respect it's no better than option 2.

Otherwise if we have to create a TTF or OTF font, I was wondering whether there might still be a way to do that with a raster format. I came across something called embedded bitmap data in the OpenType format, which sounded promising (see https://docs.microsoft.com/en-us/typography/opentype/otspec150/eblc). But again I don't know anything about the subject, so maybe that's not what I think it is.

And as a last resort, there's always the possibility of creating a texture with all the characters, and then blitting the appropriate sections of the texture for each run of characters.

Yeah what you're saying with AddFontMemResourceEx would be the precursor to doing all the other CreateFontFaceFromHdc stuff... so it's just a part of number 2.

I think @DHowett mentioned to me the other day that DirectWrite doesn't look at the embedded bitmap data in TTF/OTF... or something like that. We'd have to likely talk to the DirectWrite team about it...

The last resort might honestly be the easiest one, unfortunately.

there's always the possibility of creating a texture with all the characters, and then blitting the appropriate sections of the texture for each run of characters

The last resort might honestly be the easiest one

lol

(For James' edification--though I suspect he is entirely aware--between Michael's comment in May and Mike's yesterday it became obvious that an atlas-based renderer would be terrifically effective for us, and we've put some time into prototyping one that will eventually ship.)

If you're interested you can check out https://github.com/microsoft/terminal/compare/dev/lhecker/atlas-engine for this. I know the branch is called "atlas-engine", but it actually contains a mishmash of various branches I built over time. The final PR I'm submitting will only contain the AtlasEngine project. (Before that I'll further refine the code in my spare time over the coming days.)
If anyone wants to try it out: It's only integrated in Host.EXE and can be enabled by setting UseDx to 2 under HKCU:\Console (UseDx == 1 is the current DxEngine).

j4james · 2021-05-24T22:36:20Z

as well as how to get ConPTY to pass this out

The one really hacky way that occurred to me was to pass through the PUA characters that have already been translated as is, and have WT just assume they always map to the active soft font (obviously wouldn't work for any other clients though). Otherwise we would need to reimplement most of the charset mapping framework from conhost, and then conpty would need to wrap runs of the PUA characters in a pair of SCS sequences. I think it's doable, but it's going to be a lot of hassle.

I'm also curious... now that we have some amount of sixel parsing... how we could support sixel graphics by just reserving areas of the text buffer and pointing at some bitmap space to the renderers... HMMMMMM.

I actually have a basic conhost sixel graphics implementation that I've been experimenting with for a while, and the way it currently works is by associating an image segment (when needed) with each row in the buffer, and then the renderer just does a StretchDIBits call as an additional pass when painting a row.

I had initially considered storing a flag in the text attributes to track which cells are covered by image segments, and I figured that could potentially be used by conpty when decided what parts of the buffer it was safe to forward. It turned out that wasn't necessary for the conhost side of thing, but there's no reason why we couldn't still track that information for conpty.

As with soft fonts, though, it might be doable, but it's likely to be complicated. So I can't help thinking that the time taken on that would be better spent trying to implement a direct pass-through version of conpty. Admittedly that's not going to be easy either, and maybe isn't even possible, but it would solve so many problems if it could be made to work.

miniksa · 2021-05-26T16:45:24Z

as well as how to get ConPTY to pass this out

The one really hacky way that occurred to me was to pass through the PUA characters that have already been translated as is, and have WT just assume they always map to the active soft font (obviously wouldn't work for any other clients though). Otherwise we would need to reimplement most of the charset mapping framework from conhost, and then conpty would need to wrap runs of the PUA characters in a pair of SCS sequences. I think it's doable, but it's going to be a lot of hassle.

I think if we did the first option (implicit assumption), that we would have to coin a VT sequence handshake between WT and ConPTY that says "I'm happy taking the implicit PUA characters in this range as a part of the active soft font."

The second option does sound a lot more involved... I just want to make sure we maintain some sort of commitment to allowing terminal competition on top of ConPTY.

I'm also curious... now that we have some amount of sixel parsing... how we could support sixel graphics by just reserving areas of the text buffer and pointing at some bitmap space to the renderers... HMMMMMM.

I actually have a basic conhost sixel graphics implementation that I've been experimenting with for a while, and the way it currently works is by associating an image segment (when needed) with each row in the buffer, and then the renderer just does a StretchDIBits call as an additional pass when painting a row.

I had initially considered storing a flag in the text attributes to track which cells are covered by image segments, and I figured that could potentially be used by conpty when decided what parts of the buffer it was safe to forward. It turned out that wasn't necessary for the conhost side of thing, but there's no reason why we couldn't still track that information for conpty.

As with soft fonts, though, it might be doable, but it's likely to be complicated. So I can't help thinking that the time taken on that would be better spent trying to implement a direct pass-through version of conpty. Admittedly that's not going to be easy either, and maybe isn't even possible, but it would solve so many problems if it could be made to work.

True... pass through might solve a lot of this. I think @zadjii-msft has spent more time thinking about that in particular than I have, though.

HBelusca · 2021-07-05T19:04:44Z

Note: this could be interesting also for e.g. NTVDM in windowed mode (using window console) emulating custom VGA fonts.

HBelusca · 2021-07-05T19:06:11Z

Out of curiosity @j4james , have you looked at how other terminals (e.g. putty/mintty, ...) handle the sixel rendering?

j4james · 2021-07-05T21:38:41Z

Out of curiosity @j4james , have you looked at how other terminals (e.g. putty/mintty, ...) handle the sixel rendering?

I've tested about a dozen or so sixel terminals. The only ones that are any good, in my opinion, are the commercial terminal emulators. I expect sixel terminals to make a reasonable effort to actually emulate the original sixel devices, but that doesn't seem to be considered important by many others in the open source community. 🤷‍♂️

HBelusca · 2021-07-05T21:46:27Z

Out of curiosity @j4james , have you looked at how other terminals (e.g. putty/mintty, ...) handle the sixel rendering?

I've tested about a dozen or so sixel terminals. The only ones that are any good, in my opinion, are the commercial terminal emulators. I expect sixel terminals to make a reasonable effort to actually emulate the original sixel devices, but that doesn't seem to be considered important by many others in the open source community. 🤷‍♂️

Thanks for your reply. Actually my question was more targeting your comment concerning your idea of implementation (not the question of the emulation capability), compared to what other terminals do:

[...] and the way it currently works is by associating an image segment (when needed) with each row in the buffer, and then the renderer just does a StretchDIBits call as an additional pass when painting a row.

I had initially considered storing a flag in the text attributes to track which cells are covered by image segments, and I figured that could potentially be used by conpty when decided what parts of the buffer it was safe to forward. It turned out that wasn't necessary for the conhost side of thing, but there's no reason why we couldn't still track that information for conpty.

j4james · 2021-07-05T22:34:22Z

Thanks for your reply. Actually my question was more targeting your comment concerning your idea of implementation (not the question of the emulation capability), compared to what other terminals do:

Ah. Sorry I misunderstood you. But no, I haven't look at other implementations. For one thing, the requirements for a real sixel emulator are quite different from the pseudo-sixel protocol that most terminals implement, so I wouldn't expect there's much to learn from their architecture. Also, the issues I was discussing above were mainly concerning how we propagate sixel over conpty, which is not something other terminals have to deal with.

zadjii-msft

You know what, I read most of this before I went on leave and forgot to come back to it. That's my bad.

Most of this is really straightforward, until FontBuffer.cpp. That one looks generally valid. I'm not totally sure of it's correctness, since a lot of it is magic, but I'm sure the person who's most concerned with the correctness of this kinda thing is you, so I'm not worried haha.

Very much looking forward to where this leads us 😉

zadjii-msft · 2021-07-08T17:20:17Z

src/terminal/adapter/charsets.hpp

+    // We're reserving 96 characters (U+EF20 to U+EF7F) from the Unicode
+    // Private Use Area for our dynamically redefinable characters sets.
+    static constexpr auto DRCS_BASE_CHAR = L'\uEF20';
+    static constexpr auto Drcs94 = CharSet<DRCS_BASE_CHAR, 95>{ { DRCS_BASE_CHAR, '\x20' } };


95

is that a typo?

No, it's confusing but intentional. The 94-character sets are meant to map codepoints 0x21 to 0x7E, while the 96-character sets map 0x20 to 0x7F. But it adds a bunch of complexity to make the start point variable, so we just make the 94-character sets have an extra character at the start which is always a space - that's essentially the same as not remapping that codepoint.

FYI, I've now added a comment explaining the reasoning behind the 95 (near the top of the header where they're first used).

zadjii-msft · 2021-07-08T17:49:21Z

src/terminal/adapter/FontBuffer.cpp

+}
+
+auto FontBuffer::_generateErrorGlyph() -> std::array<uint16_t, MAX_HEIGHT>
+{


holy shit this whole function is magic

okay I mean, the whole PR is magic, but this specifically is extra magic

zadjii-msft · 2021-07-08T17:50:25Z

src/terminal/adapter/FontBuffer.cpp

+    }
+}
+
+auto FontBuffer::_generateErrorGlyph() -> std::array<uint16_t, MAX_HEIGHT>


I suppose this should probably just be

Suggested change

auto FontBuffer::_generateErrorGlyph() -> std::array<uint16_t, MAX_HEIGHT>

std::array<uint16_t, MAX_HEIGHT> FontBuffer::_generateErrorGlyph()

right? I've never seen the other notation before, and I think in this case, they're the same, yea?

Yeah. I suspect there might have been a reason for it in an earlier draft of the code that became unnecessary as things evolved. I'll see if I can remove it.

FYI, I've updated this now. I think the reason I was using it was to avoid the class scope prefix on the MAX_HEIGHT constant. But considering that saved all of 4 characters, it really doesn't seem like it was worth the confusion.

zadjii-msft · 2021-08-04T14:21:08Z

src/renderer/base/FontResource.cpp

+    }
+}
+
+FontResource::operator HFONT()


there's always the possibility of creating a texture with all the characters, and then blitting the appropriate sections of the texture for each run of characters

The last resort might honestly be the easiest one

lol

zadjii-msft · 2021-08-04T14:49:59Z

src/terminal/adapter/FontBuffer.cpp

+    // to produce a scaled down version of reasonable quality.
+    constexpr std::array<uint32_t, MAX_WIDTH + 1> widthMasks = {
+        // clang-format off
+        0, 1, 3, 32771, 8457, 9481, 9545, 9673, 42441, 26061,


these might make more sense as hex or binary constants, so we can see the actual bits, but also ¯\_(ツ)_/¯

I did think that originally, but it didn't really make things much clearer, so in the end I decided to go with whatever took the least amount of space. If there's a strong preference for this, I don't mind changing it to binary though.

zadjii-msft · 2021-08-04T14:56:45Z

(oh this needs some merge conflict resolution)

ghost · 2021-08-05T13:21:35Z

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

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 (`@msftbot`) and give me an instruction to get started! Learn more here.

DHowett · 2021-08-05T17:02:22Z

@msftbot make sure @DHowett signs off

ghost · 2021-08-05T17:02:30Z

Hello @DHowett!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

I'll only merge this pull request if it's approved by @DHowett

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

DHowett · 2021-08-05T17:14:34Z

src/renderer/base/renderer.cpp

+    TriggerRedrawAll();
+}
+
+bool Renderer::_IsSoftFontChar(const std::wstring_view v) const


This is going to sound like the rantings of a crazy person, but we learned that MSVC has some unfortunate code gen when confronted with a string_view copy. References proved to be slightly better, and since we're doing this per character in the hot path it might be wise to take every win we can get.

@lhecker knows more, but I do not recall whether he's available right now.

At the same time, perhaps this is trivially inlinable and doesn't contribute its fcall overhead?

Hmmm... this looks like a no-win situation. The current code is inlined - in both places it's used - but the generated assembly is possibly not ideal. It looks to me like it's doing some unnecessary copying. If I change it to a reference, though, the generated assembly is better, but the second call (inside the inner loop) is no longer inlined. Maybe there's a way to get the best of both worlds, but it's not immediately obvious to me. I'll do a bit more experimenting, but you may need the experts for this.

This may be a terrible idea...

#define Renderer__Paamayim_Nekudotayim__IsSoftFontChar(v) (v.size() == 1 && v.front() >= _firstSoftFontChar && v.front() <= _lastSoftFontChar)

(yes, i have made myself sad)

OK I've just tried that, and it does actually produce the best results. I just can't bring myself to use a define for this. Please let there be a better solution.

I'm not sure if this classifies as better, but I can get the equivalent assembly that the #define produces by turning the _IsSoftFontChar method into a static, and passing in the _firstSoftFontChar and _lastSoftFontChar fields as additional parameters. I don't like it, but I dislike it less than the #define.

Just so everyone knows what kind of difference we're talking about here, this is a snippet of the assembly from the first part of the code (the rest is identical).

Original implementation:

00007FF6EF447638 movups xmm0,xmmword ptr [r13] 00007FF6EF44763D movaps xmmword ptr [rbp-80h],xmm0 00007FF6EF447641 psrldq xmm0,8 00007FF6EF447646 movq rax,xmm0 00007FF6EF44764B cmp rax,1 00007FF6EF44764F jne Microsoft::Console::Render::Renderer::_PaintBufferOutputHelper+36Fh (07FF6EF44766Fh) 00007FF6EF447651 mov rax,qword ptr [rbp-80h] 00007FF6EF447655 movzx ecx,word ptr [rax] 00007FF6EF447658 cmp rcx,qword ptr [r14+0B0h]

Static function or #define:

00007FF69B127638 cmp qword ptr [r13+8],1 00007FF69B12763D jne Microsoft::Console::Render::Renderer::_PaintBufferOutputHelper+35Dh (07FF69B12765Dh) 00007FF69B12763F mov rax,qword ptr [r13] 00007FF69B127643 movzx ecx,word ptr [rax] 00007FF69B127646 cmp rcx,qword ptr [r14+0B0h]

A couple more potential optimisations.

If we make the _firstSoftFontChar a static constexpr, the final memory comparison in the code above becomes a register comparison (albeit with an additional mov), which I'm assuming would be slightly more efficient.

00007FF6DFAF7627 mov eax,0EF20h 00007FF6DFAF762C cmp cx,ax

If the conditions are reordered so the string size is compared last, we're more likely to fall through on the first test in typical usage (most characters would be less than 0xEF20). However, this is assuming the size will always be at least 1, otherwise we could potentially be reading past the end of the string. I don't know enough about the buffer storage to know if that's possible.

Anyway, let me know if you want me to make any of these changes, or whether I should just leave it for the experts to optimise in a followup PR.

Let's go with the middle ground version, which I've decided to be the static one. Then we can optimize from there 😄

DHowett · 2021-08-05T17:17:59Z

src/renderer/gdi/paint.cpp

@@ -347,6 +350,7 @@ using namespace Microsoft::Console::Render;

            const auto text = cluster.GetText();
            polyString += text;
+            polyString.back() &= softFontCharMask;


I wonder -- this may impact surrogate pairs negatively. We don't fully support them here, but they do somewhat operate acceptably. I was wondering whether somebody setting the soft font and then emitting a surrogate pair would be a rare/freak/unlikely occurrence, but then I remembered that they can be paired with normal text (right?)

I was a little worried about this too, but I've double checked the logic and I think it's OK. The renderer splits soft font runs from regular font runs, so when it's in a soft font run there should never be a surrogate pair, so the cluster text will always be a single wchar_t. And when it's in a regular font run the mask has all bits sets, so although it may only be masking half of the surrogate pair, it's essentially a noop, so it doesn't matter.

I've also run some tests mixing soft fonts with emojis, and they seem to work fine.

Absolutely awesome. Thanks.

DHowett

Alrihgt. I've tried to read and understand the FontBuffer and FontResource classes, in addition to everything else. I trust it -- though I couldn't articulate its finer points. Only have questions about the string_view and the surrogate pair change.

This is excellent, excellent work that we let sit around with one review for far too long. Let's get those answered and ship it.

Sorry for the delay, and thank you.

DHowett · 2021-08-05T17:23:40Z

src/terminal/adapter/adaptDispatch.cpp

+    if (!_fontBuffer)
+    {
+        _fontBuffer = std::make_unique<FontBuffer>();
+    }


~~Do we need to delay setting this member until the font has been totally constructed? We could make one here, move it into the lambda, and only move it into final storage when it is finalized.~~

~~Or, is it not a concern that somebody might begin a DRCS, fail, and start a new one? I suppose that we always set the state bits, so... maybe it doesn't matter.~~ 😄

More tightly considered: it is not possible to emit sixel data into a font that did not pass EraseControl/Attributes/StartChar setup, so we will never be appending sixel data into a font that already has some sixel data.

~~Is it possible to emit some sixel data, bail out, and then later start a "new" DRCS exchange and inherit the previous sixel data?~~

You can safely ignore this entire line of questioning! The first thing we do in SetEraseControl is mark the matrix as invalid, and it is legal to add glyphs to an existing font. Teach me to read bottom up eh.

DHowett · 2021-08-05T17:28:39Z

src/renderer/base/FontResource.cpp

+    }
+}
+
+FontResource::operator HFONT()


(For James' edification--though I suspect he is entirely aware--between Michael's comment in May and Mike's yesterday it became obvious that an atlas-based renderer would be terrifically effective for us, and we've put some time into prototyping one that will eventually ship.)

## Summary of the Pull Request This PR adds support for downloadable soft fonts in the DirectX renderer, potentially enabling them to be used in Windows Terminal. ## References Soft fonts were first implemented in conhost (with the GDI renderer) in PR #10011. ## PR Checklist * [ ] Closes #xxx * [x] CLA signed. * [ ] Tests added/passed * [ ] Documentation updated. * [ ] 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 ## Detailed Description of the Pull Request / Additional comments The way the DirectX implementation works is by building up a bitmap containing all of the glyphs, and then drawing an appropriate subsection of that bitmap for each character that needs to be rendered. The current text color is applied with a color matrix effect, and the glyphs are automatically scaled up to the current font size with a scaling effect. By default the scaling uses a high quality cubic interpolation, which gives it a smoother antialiased effect. But if the *Text antialiasing* option is configured as *Aliased*, we use a simpler nearest-neighbor interpolation, which more closely matches the rendering of the original GDI implementation. ## Validation Steps Performed I've manually tested the renderer in conhost with the `UseDx` registry entry. I've also tested in Windows Terminal using the experimental passthrough mode.

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` sequence containing 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, since the 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 the soft 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.

PhMajerus · 2024-07-04T14:58:15Z

I don't know if you're still using PUA for soft fonts, but if you do, I have a question.

If I understand correctly, you need to somehow encode the characters from the soft font in a UTF-16 string, so they must fit in a WCHAR and you use PUA U+EF20 to U+EF7F for this.
From my understanding, this could collide with PUA used within organisations or for personal use by Terminal users.

What about using the Unicode Tags block U+E0020 to U+E007F instead ?
While this could still collide with some existing use, that block is supposed to be an invisible version of the printable ASCII range, which maps very well to the soft font concept.
It would ensure PUAs are left available for any private use, and only means that if an app outputs Tags, they would be visible instead of hidden.

My rationale is that I don't see any legitimate use for Tags in text output designed for the Terminal, while I definitely see PUA being used for internal characters or as a mean to add characters using a custom TTF instead of soft fonts.
Of course, an extra flag outside of the WCHAR would be best to avoid collisions altogether, but to me Tags seems less problematic than PUA if a separate flag isn't possible.

j4james · 2024-07-04T20:42:00Z

From my understanding, this could collide with PUA used within organisations or for personal use by Terminal users.

As I mentioned in the PR notes, if no soft fonts are in use, then no mapping of the PUA code points will take place, so this shouldn't interfere with anyone using those code points for something else, as along as they aren't also trying to use soft fonts, which seems unlikely.

And using anything other than a 16-bit char for the mapping would require significant changes to the architecture, so it isn't worth considering unless someone has a genuine problem with the existing implementation. And even then I'd be inclined to say it just isn't supported.

j4james commented May 1, 2021

View reviewed changes

.github/actions/spelling/dictionary/apis.txt Outdated Show resolved Hide resolved

j4james commented May 1, 2021

View reviewed changes

src/renderer/base/lib/base.vcxproj.filters Show resolved Hide resolved

j4james marked this pull request as ready for review May 1, 2021 20:43

lhecker reviewed May 5, 2021

View reviewed changes

src/terminal/adapter/FontBuffer.cpp Show resolved Hide resolved

j4james force-pushed the feature-decdld branch from 33450c3 to 66e1213 Compare May 6, 2021 14:22

DHowett reviewed May 21, 2021

View reviewed changes

j4james force-pushed the feature-decdld branch from 1afec1f to 24b44dc Compare May 22, 2021 13:33

lhecker reviewed May 22, 2021

View reviewed changes

src/renderer/base/FontResource.cpp Show resolved Hide resolved

src/renderer/base/FontResource.cpp Outdated Show resolved Hide resolved

src/renderer/base/FontResource.cpp Show resolved Hide resolved

src/terminal/adapter/FontBuffer.cpp Outdated Show resolved Hide resolved

miniksa approved these changes May 24, 2021

View reviewed changes

zadjii-msft mentioned this pull request Jul 9, 2021

Add support in Windows Terminal for Kitty's Graphics Protocol #8389

Closed

zadjii-msft approved these changes Aug 4, 2021

View reviewed changes

j4james added 2 commits August 5, 2021 10:26

Hook up the DECDLD sequence to the dispatch classes.

c992564

Create a class for managing the font buffer.

73c46d2

j4james added 4 commits August 5, 2021 11:01

Avoid using a 1-element array for the font bitmap.

f91ace2

Use an atomic counter instead of steady_clock for the font name.

df7edd6

Correct a repeated typo in the comments.

8869068

Get rid of the unnecessary suffix return type syntax.

cf7de8e

zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Aug 5, 2021

Add a comment explaining why 94-character sets have a size of 95.

c5084c2

j4james force-pushed the feature-decdld branch from 4e556d6 to c5084c2 Compare August 5, 2021 13:46

ghost removed the AutoMerge Marked for automatic merge by the bot when requirements are met label Aug 5, 2021

ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label Aug 5, 2021

DHowett reviewed Aug 5, 2021

View reviewed changes

DHowett requested changes Aug 5, 2021

View reviewed changes

ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Aug 5, 2021

Make _IsSoftFontChar static for better optimization.

e03524d

ghost removed the AutoMerge Marked for automatic merge by the bot when requirements are met label Aug 6, 2021

DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Aug 6, 2021

DHowett approved these changes Aug 6, 2021

View reviewed changes

ghost merged commit 90ff261 into microsoft:main Aug 6, 2021

dotnetCarpenter mentioned this pull request Aug 26, 2021

Support for downloadable "soft fonts" (DRCS) #9164

Closed

j4james deleted the feature-decdld branch September 26, 2021 16:02

WSLUser mentioned this pull request Oct 19, 2021

[VT] Add support for downloadable soft fonts contour-terminal/contour#467

Open

j4james mentioned this pull request Jun 23, 2022

Add support for soft fonts in the DX renderer #13362

Merged

6 tasks

j4james mentioned this pull request Sep 11, 2022

Pass through soft fonts over conpty #13965

Merged

This pull request was closed.

	auto FontBuffer::_generateErrorGlyph() -> std::array<uint16_t, MAX_HEIGHT>
	std::array<uint16_t, MAX_HEIGHT> FontBuffer::_generateErrorGlyph()

Add support for downloadable soft fonts #10011

Add support for downloadable soft fonts #10011

Conversation

j4james commented May 1, 2021 • edited by DHowett Loading

Validation Steps Performed

crutkas commented May 3, 2021

DHowett commented May 5, 2021

DHowett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 22, 2021 • edited Loading

@check-spelling-bot Report

Unrecognized words, please review:

lhecker commented May 22, 2021

j4james commented May 23, 2021

miniksa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lhecker Aug 5, 2021 • edited Loading

Choose a reason for hiding this comment

j4james commented May 24, 2021

miniksa commented May 26, 2021

HBelusca commented Jul 5, 2021

HBelusca commented Jul 5, 2021

j4james commented Jul 5, 2021

HBelusca commented Jul 5, 2021

j4james commented Jul 5, 2021

zadjii-msft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

j4james Aug 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zadjii-msft commented Aug 4, 2021

ghost commented Aug 5, 2021

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 (@msftbot) and give me an instruction to get started! Learn more here.

DHowett commented Aug 5, 2021

ghost commented Aug 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

j4james Aug 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DHowett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PhMajerus commented Jul 4, 2024 • edited Loading

j4james commented Jul 4, 2024

j4james commented May 1, 2021 •

edited by DHowett

Loading

github-actions bot commented May 22, 2021 •

edited

Loading

lhecker Aug 5, 2021 •

edited

Loading

j4james Aug 5, 2021 •

edited

Loading

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 (`@msftbot`) and give me an instruction to get started! Learn more here.

j4james Aug 5, 2021 •

edited

Loading

PhMajerus commented Jul 4, 2024 •

edited

Loading