Fix visual artifacts when using QQ Wubi IME

src/interactivity/win32/windowproc.cpp

@@ -280,6 +280,19 @@ static constexpr TsfDataProvider s_tsfDataProvider;
         {
             gci.Flags |= CONSOLE_IGNORE_NEXT_MOUSE_INPUT;
         }
+
+        // NOTE: Initializing a TSF on WM_ACTIVATE (or earlier) ensures that we don't get a
+        // redundant, implicit IMM32 TSF client, which would unnecessarily bloat the process.
+        // (It is implicitly created during DefWindowProcW(WM_ACTIVATE).)
+        if (!g.tsf)
+        {
+            // The TSF implementation doesn't care per-se if it's STA or MTA,
+            // but it does require it to be initialized. STA because we're the UI thread.
+            CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED);
+            g.tsf = TSF::Handle::Create();
+            g.tsf.AssociateFocus(const_cast<TsfDataProvider*>(&s_tsfDataProvider));
+        }
+
         goto CallDefWin;
         break;
     }
@@ -294,12 +307,6 @@ static constexpr TsfDataProvider s_tsfDataProvider;
             renderer->AllowCursorVisibility(Render::InhibitionSource::Host, true);
         }
 
-        if (!g.tsf)
-        {
-            g.tsf = TSF::Handle::Create();
-            g.tsf.AssociateFocus(const_cast<TsfDataProvider*>(&s_tsfDataProvider));
-        }
-
         // set the text area to have focus for accessibility consumers
         if (_pUiaProvider)
         {

src/renderer/base/renderer.cpp

@@ -490,7 +490,19 @@ try
     // As we leave the scope, EndPaint will be called (declared above)
     return S_OK;
 }
-CATCH_RETURN()
+catch (...)
+{
+    // I found it useful during renderer development when exceptions aren't always silently caught and retried.
+    // Sometimes, the error goes away on the retry, but not for good reason. Catching such errors may be useful.
+#ifndef NDEBUG
+    if (IsDebuggerPresent())
+    {
+        __debugbreak();
+    }
+#endif
+
+    RETURN_CAUGHT_EXCEPTION();
+}
 
 // NOTE: You must be holding the console lock when calling this function.
 void Renderer::SynchronizedOutputChanged() noexcept
@@ -1027,6 +1039,8 @@ void Renderer::_PaintBufferOutput(_In_ IRenderEngine* const pEngine)
     // relative to the entire buffer.
     const auto compositionRow = _compositionCache ? _compositionCache->absoluteOrigin.y : -1;
     const auto& activeComposition = _pData->GetActiveComposition();
+    auto& buffer = _pData->GetTextBuffer();
+    auto& scratchRow = buffer.GetScratchpadRow();
 
     // This is effectively the number of cells on the visible screen that need to be redrawn.
     // The origin is always 0, 0 because it represents the screen itself, not the underlying buffer.
@@ -1056,8 +1070,6 @@ void Renderer::_PaintBufferOutput(_In_ IRenderEngine* const pEngine)
         // we need to walk through line-by-line and repaint onto the screen.
         const auto redraw = Viewport::Intersect(dirty, _viewport);
 
-        // Retrieve the text buffer so we can read information out of it.
-        auto& buffer = _pData->GetTextBuffer();
         // Now walk through each row of text that we need to redraw.
         for (auto row = redraw.Top(); row < redraw.BottomExclusive(); row++)
         {
@@ -1069,17 +1081,16 @@ void Renderer::_PaintBufferOutput(_In_ IRenderEngine* const pEngine)
             // Draw the active composition.
             // We have to use some tricks here with const_cast, because the code after it relies on TextBufferCellIterator,
             // which isn't compatible with the scratchpad row. This forces us to back up and modify the actual row `r`.
-            ROW* rowBackup = nullptr;
-            if (row == compositionRow)
-            {
-                rowBackup = _PaintBufferOutputComposition(buffer, r, activeComposition);
-            }
             const auto restore = wil::scope_exit([&] {
-                if (rowBackup)
+                if (row == compositionRow)
                 {
-                    const_cast<ROW&>(r).CopyFrom(*rowBackup);
+                    const_cast<ROW&>(r).CopyFrom(scratchRow);
                 }
             });
+            if (row == compositionRow)
+            {
+                _PaintBufferOutputComposition(r, scratchRow, activeComposition);
+            }
 
             // Convert the screen coordinates of the line to an equivalent
             // range of buffer cells, taking line rendition into account.
@@ -1112,9 +1123,8 @@ void Renderer::_PaintBufferOutput(_In_ IRenderEngine* const pEngine)
     }
 }
 
-ROW* Renderer::_PaintBufferOutputComposition(TextBuffer& buffer, const ROW& r, const Composition& activeComposition)
+void Renderer::_PaintBufferOutputComposition(const ROW& r, ROW& scratch, const Composition& activeComposition) const
 {
-    auto& scratch = buffer.GetScratchpadRow();
     scratch.CopyFrom(r);
 
     // *Overwrite* the original text with the active composition...
@@ -1142,7 +1152,7 @@ ROW* Renderer::_PaintBufferOutputComposition(TextBuffer& buffer, const ROW& r, c
                 attr.SetForeground(_compositionCache->baseAttribute.GetForeground());
             }
 
-            state.text = text.substr(off, len);
+            state.text = til::safe_slice_len(text, off, len);
             state.columnBegin = state.columnEnd;
             const_cast<ROW&>(r).ReplaceText(state);
             const_cast<ROW&>(r).ReplaceAttributes(state.columnBegin, state.columnEnd, attr);
@@ -1209,8 +1219,6 @@ ROW* Renderer::_PaintBufferOutputComposition(TextBuffer& buffer, const ROW& r, c
             i = spanEnd;
         }
     }
-
-    return &scratch;
 }
 
 static bool _IsAllSpaces(const std::wstring_view v)

src/renderer/base/renderer.hpp

@@ -121,7 +121,7 @@ namespace Microsoft::Console::Render
         void _scheduleRenditionBlink();
         [[nodiscard]] HRESULT _PaintBackground(_In_ IRenderEngine* const pEngine);
         void _PaintBufferOutput(_In_ IRenderEngine* const pEngine);
-        ROW* _PaintBufferOutputComposition(TextBuffer& buffer, const ROW& r, const Composition& activeComposition);
+        void _PaintBufferOutputComposition(const ROW& r, ROW& scratch, const Composition& activeComposition) const;
         void _PaintBufferOutputHelper(_In_ IRenderEngine* const pEngine, TextBufferCellIterator it, const til::point target);
         void _PaintBufferOutputGridLineHelper(_In_ IRenderEngine* const pEngine, const TextAttribute textAttribute, const size_t cchLine, const til::point coordTarget);
         bool _isHoveredHyperlink(const TextAttribute& textAttribute) const noexcept;

src/tsf/Implementation.cpp

@@ -42,9 +42,15 @@
 //   while this method is called. The keyboard layout will be adjusted when the
 //   calling thread gets focus. This flag must be used with TF_TMAE_NOACTIVATETIP.
 // - TF_TMAE_CONSOLE: A text service is activated for console usage.
-//   Some IMEs are known to use this as a hint. Particularly a Korean IME can benefit
-//   from this, because Korean relies on "recomposing" previously finished compositions.
-//   That can't work in a terminal, since we submit composed text to the shell immediately.
+//   TSF uses this flag in CInputContextAdapter (adapts TSF3 TIPs to TSF1 or IMM clients),
+//   flags the client as IMM-style, and disables "single character composition" for Korean IMEs.
+//   The latter would cause every syllable to be an "interim character": not marked as composing,
+//   yet still being rewritten by the IME. Since we send any non-composing text to the shell and
+//   can't unsend it, this would break the IME. We could fix that by recognizing `fInterimChar`
+//   and only finalizing the text in front of it of course, and conhost v1 did just that
+//   (see _IsInterimSelection/_MakeInterimString). But it also set TF_TMAE_CONSOLE, so there
+//   were no interim characters in the first place (lol).
+//   To test this, type "gksks" in a Korean IME: 하난 is correct, 하나ㄴ is broken.
 //
 // ...with the exception of, for the following reason:
 // - TF_TMAE_UIELEMENTENABLEDONLY: This flag tells TSF that the caller wants to render its
@@ -647,6 +653,11 @@
                 }
             }
 
+            // Since we can't un-finalize finalized text, we only finalize text that's at the start of the document.
+            // In other words, don't put text that's in the middle between two active compositions into the finalized string.
+            const auto isActiveComposition = composing || activeCompositionEncountered;
+            auto& target = isActiveComposition ? activeComposition : finalizedString;
+
             size_t totalLen = 0;
             for (;;)
             {
@@ -658,17 +669,7 @@
                 ULONG len = bufCap;
                 THROW_IF_FAILED(range->GetText(ec, TF_TF_MOVESTART, buf, len, &len));
 
-                // Since we can't un-finalize finalized text, we only finalize text that's at the start of the document.
-                // In other words, don't put text that's in the middle between two active compositions into the finalized string.
-                if (!composing && !activeCompositionEncountered)
-                {
-                    finalizedString.append(buf, len);
-                }
-                else
-                {
-                    activeComposition.append(buf, len);
-                }
-
+                target.append(buf, len);
                 totalLen += len;
 
                 if (len < bufCap)
@@ -677,8 +678,10 @@
                 }
             }
 
-            const auto attr = _textAttributeFromAtom(atom);
-            activeCompositionRanges.emplace_back(totalLen, attr);
+            if (isActiveComposition)
+            {
+                activeCompositionRanges.emplace_back(totalLen, _textAttributeFromAtom(atom));
+            }
 
             activeCompositionEncountered |= composing;
         }