Merge pull request #1395 from contour-terminal/fix/wide-emoji-overwrites

Fixes screen destruction on sibling cells when overwriting wide characters
contour-terminal · Dec 31, 2023 · 835d52f · 835d52f
2 parents a29dd9c + cff48c7
commit 835d52f
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 25 deletions.
diff --git a/metainfo.xml b/metainfo.xml
@@ -111,6 +111,7 @@
           <li>Fixes variable fonts loading</li>
           <li>Fixes Command modifier for input mappings, such as Command+C or Command+V on on MacOS (#1379).</li>
           <li>Fixes CSIu encoding of shift modifier produced characters (#1373).</li>
+          <li>Fixes screen destruction on sibling cells when overwriting wide characters, such as Emoji, Kanji, etc.</li>
           <li>Changes VT sequence `DECSCUSR` (`CSI ? 0 SP q` and `CSI ? SP q`) to reset to user-configured cursor style (#1377).</li>
           <li>Remove `contour-latest` terminfo file. Please use `contour` terminfo instead.</li>
           <li>Adds `Command` as modifier to input mappings on MacOS to work along with `Meta` for convenience reasons (#1379).</li>

diff --git a/src/contour/helper.cpp b/src/contour/helper.cpp
@@ -423,7 +423,7 @@ void sendWheelEvent(QWheelEvent* event, TerminalSession& session)
         session.addScrollX(xDelta);
         inputLog()("Accumulate x scroll with current value {} ", session.getScrollX());
     }
-    if (std::abs(session.getScrollX()) > unbox(session.terminal().cellPixelSize().width))
+    if (std::abs(session.getScrollX()) > unbox<int>(session.terminal().cellPixelSize().width))
     {
         auto const modifier = makeModifiers(event->modifiers());
         auto const button = session.getScrollX() > 0 ? VTMouseButton::WheelRight : VTMouseButton::WheelLeft;
@@ -441,7 +441,7 @@ void sendWheelEvent(QWheelEvent* event, TerminalSession& session)
         session.addScrollY(yDelta);
         inputLog()("Accumulate y scroll with current value {} ", session.getScrollY());
     }
-    if (std::abs(session.getScrollY()) > unbox(session.terminal().cellPixelSize().height))
+    if (std::abs(session.getScrollY()) > unbox<int>(session.terminal().cellPixelSize().height))
     {
         auto const modifier = makeModifiers(event->modifiers());
         auto const button = session.getScrollY() > 0 ? VTMouseButton::WheelUp : VTMouseButton::WheelDown;

diff --git a/src/vtbackend/Screen.cpp b/src/vtbackend/Screen.cpp
@@ -566,8 +566,7 @@ void Screen<Cell>::writeTextInternal(char32_t sourceCodepoint)
     else
     {
         auto const extendedWidth = usePreviousCell().appendCharacter(codepoint);
-        if (extendedWidth > 0)
-            clearAndAdvance(extendedWidth);
+        clearAndAdvance(0, extendedWidth);
         _terminal->markCellDirty(_lastCursorPosition);
     }
 
@@ -592,17 +591,19 @@ void Screen<Cell>::writeCharToCurrentAndAdvance(char32_t codepoint) noexcept
     {
         // Erase the left half of the wide char.
         Cell& prevCell = line.useCellAt(_cursor.position.column - 1);
-        prevCell.reset();
+        prevCell.reset(_cursor.graphicsRendition);
     }
 
+    auto const oldWidth = cell.width();
+
     cell.write(_cursor.graphicsRendition,
                codepoint,
                static_cast<uint8_t>(unicode::width(codepoint)),
                _cursor.hyperlink);
 
     _lastCursorPosition = _cursor.position;
 
-    clearAndAdvance(cell.width());
+    clearAndAdvance(oldWidth, cell.width());
 
     // TODO: maybe move selector API up? So we can make this call conditional,
     //       and only call it when something is selected?
@@ -614,32 +615,23 @@ void Screen<Cell>::writeCharToCurrentAndAdvance(char32_t codepoint) noexcept
 
 template <typename Cell>
 CRISPY_REQUIRES(CellConcept<Cell>)
-void Screen<Cell>::clearAndAdvance(int offset) noexcept
+void Screen<Cell>::clearAndAdvance(int oldWidth, int newWidth) noexcept
 {
-    if (offset == 0)
-        return;
-
     bool const cursorInsideMargin =
         _terminal->isModeEnabled(DECMode::LeftRightMargin) && isCursorInsideMargins();
     auto const cellsAvailable = cursorInsideMargin ? *(margin().horizontal.to - _cursor.position.column) - 1
                                                    : *pageSize().columns - *_cursor.position.column - 1;
-    auto const n = min(offset, cellsAvailable);
 
-    if (n == offset)
-    {
-        _cursor.position.column++;
-        auto& line = currentLine();
-        for (int i = 1; i < n; ++i) // XXX It's not even clear if other TEs are doing that, too.
-        {
-            line.useCellAt(_cursor.position.column)
-                .reset(_cursor.graphicsRendition.with(CellFlag::WideCharContinuation), _cursor.hyperlink);
-            _cursor.position.column++;
-        }
-    }
+    auto const sgr = newWidth > 1 ? _cursor.graphicsRendition.with(CellFlag::WideCharContinuation)
+                                  : _cursor.graphicsRendition;
+    auto& line = currentLine();
+    for (int i = 1; i < min(max(oldWidth, newWidth), cellsAvailable); ++i)
+        line.useCellAt(_cursor.position.column + i).reset(sgr, _cursor.hyperlink);
+
+    if (newWidth == min(newWidth, cellsAvailable))
+        _cursor.position.column += ColumnOffset::cast_from(newWidth);
     else if (_cursor.autoWrap)
-    {
         _cursor.wrapPending = true;
-    }
 }
 
 template <typename Cell>

diff --git a/src/vtbackend/Screen.h b/src/vtbackend/Screen.h
@@ -616,7 +616,7 @@ class Screen final: public ScreenBase, public capabilities::StaticDatabase
     void linefeed(ColumnOffset column);
 
     void writeCharToCurrentAndAdvance(char32_t codepoint) noexcept;
-    void clearAndAdvance(int offset) noexcept;
+    void clearAndAdvance(int oldWidth, int newWidth) noexcept;
 
     void scrollUp(LineCount n, GraphicsAttributes sgr, Margin margin);
     void scrollUp(LineCount n, Margin margin);