Handle things above U+FFFF in GDI renderer

[alabuzhev]

Why not let Windows draw surrogate pairs? It can do that.

Basically, the comment says everything:

terminal/src/renderer/gdi/paint.cpp

Lines 346 to 347 in c90de69

	// Our GDI renderer hasn't and isn't going to handle things above U+FFFF or sequences.
	// So replace anything complicated with a replacement character for drawing purposes.

However, handling things above U+FFFF doesn't really require extra effort.

Proposed technical implementation details (optional)

• Put all characters to the output buffer
• Set the first width to cluster width and the rest to 0
• Sit back and relax while Windows does the rest

diff --git a/src/renderer/gdi/paint.cpp b/src/renderer/gdi/paint.cpp
index 9295d3d6..e436b156 100644
--- a/src/renderer/gdi/paint.cpp
+++ b/src/renderer/gdi/paint.cpp
@@ -328,11 +328,13 @@ using namespace Microsoft::Console::Render;
 
         const auto pPolyTextLine = &_pPolyText[_cPolyText];
 
-        auto& polyString = _polyStrings.emplace_back(cchLine, UNICODE_NULL);
+        auto& polyString = _polyStrings.emplace_back();
+        polyString.reserve(cchLine);
 
         COORD const coordFontSize = _GetFontSize();
 
-        auto& polyWidth = _polyWidths.emplace_back(cchLine, 0);
+        auto& polyWidth = _polyWidths.emplace_back();
+        polyWidth.reserve(cchLine);
 
         // Sum up the total widths the entire line/run is expected to take while
         // copying the pixel widths into a structure to direct GDI how many pixels to use per character.
@@ -343,11 +345,11 @@ using namespace Microsoft::Console::Render;
         {
             const auto& cluster = til::at(clusters, i);
 
-            // Our GDI renderer hasn't and isn't going to handle things above U+FFFF or sequences.
-            // So replace anything complicated with a replacement character for drawing purposes.
-            polyString[i] = cluster.GetTextAsSingle();
-            polyWidth[i] = gsl::narrow<int>(cluster.GetColumns()) * coordFontSize.X;
-            cchCharWidths += polyWidth[i];
+            const auto text = cluster.GetText();
+            polyString += text;
+            polyWidth.push_back(gsl::narrow<int>(cluster.GetColumns()) * coordFontSize.X);
+            cchCharWidths += polyWidth.back();
+            polyWidth.append(text.size() - 1, 0);
         }
 
         // Detect and convert for raster font...
@@ -396,7 +398,7 @@ using namespace Microsoft::Console::Render;
         const auto bottomOffset = _currentLineRendition == LineRendition::DoubleHeightTop ? halfHeight : 0;
 
         pPolyTextLine->lpstr = polyString.data();
-        pPolyTextLine->n = gsl::narrow<UINT>(clusters.size());
+        pPolyTextLine->n = gsl::narrow<UINT>(polyString.size());
         pPolyTextLine->x = ptDraw.x;
         pPolyTextLine->y = ptDraw.y;
         pPolyTextLine->uiFlags = ETO_OPAQUE | ETO_CLIPPED;
@@ -436,10 +438,23 @@ using namespace Microsoft::Console::Render;
 
     if (_cPolyText > 0)
     {
+#if 1
+        assert(_cPolyText == 1);
+        for (size_t i = 0; i != _cPolyText; ++i)
+        {
+            const auto& t = _pPolyText[i];
+            if (!ExtTextOutW(_hdcMemoryContext, t.x, t.y, t.uiFlags, &t.rcl, t.lpstr, t.n, t.pdx))
+            {
+                hr = E_FAIL;
+                break;
+            }
+        }
+#else
         if (!PolyTextOutW(_hdcMemoryContext, _pPolyText, (UINT)_cPolyText))
         {
             hr = E_FAIL;
         }
+#endif
 
         _polyStrings.clear();
         _polyWidths.clear();

• This patch also includes PolyTextOutW -> ExtTextOutW replacement, discussed in Missing glyph substitution #10472 - it's needed to show these glyphs in the first place.

Testing

@echo off
chcp 65001

echo 𠜎𠝹𠱓𠱸𠲖𠳏𠳕𠴕𠵼𠵿𠸎
echo 👨👩👧👦

Save this as a UTF-8 cmd file and run.

Before the change

After the change

[lhecker]

This looks awesome! But it's also a bit scary for me personally. Your other issue is quite easy for me to understand, but the changes you suggest here would add surrogate support, despite us not properly supporting surrogate pairs anywhere else yet... I'm not 100% sure if your change would work in all corner cases.

But granted, I'm relatively new in these parts of our codebase and other maintainers of this project are a better fit to review the changes you suggest.

[DHowett]

Surrogates definitely need a little more love on the text buffer side, for sure. Right now, they (perhaps puzzlingly) only work for double-width glyphs because technically they're being stored directly int eh buffer.

This complicates a number of things:

1. The clipboard code doesn't know what to do if you manage to select half of one.
   • you end up being able to select partial surrogate pair wide glyphs because the codepoint width estimator is asked about the individual code units and probably says "heck, i don't know"
2. single-width surrogates will be rendered either broken or by removing an entire column from the line (something we measured as two columns (see 1.x above) takes up one, everything after it is offset)

As opposed to #10472 (which I would love to see a pull request for! for attribution's sake 😄) I'm more hesitant on this one. It will make things look like they're working more correctly than they actually are, and that may get us into more compliance trouble than simply rejecting them would.

[DHowett]

(We're working on surrogate pairs et al over in #8000)