Fix visual glitches with injected isolates

Author: IgKh

core/katvan_editor.cpp

@@ -206,7 +206,7 @@ QMenu* Editor::createInsertMenu()
     menu->addSeparator();
 
     QAction* insertInlineMathAction = menu->addAction(tr("Inline &Math"), this, [this]() {
-        insertSurroundingMarks(utils::LRI_MARK + QStringLiteral("$"), QStringLiteral("$") + utils::PDI_MARK);
+        insertSurroundingMarks(QStringLiteral("$"), QStringLiteral("$"));
     });
     insertInlineMathAction->setIcon(utils::fontIcon(QLatin1Char('$')));
     insertInlineMathAction->setShortcut(Qt::CTRL | Qt::Key_M);

core/katvan_editorlayout.cpp

@@ -70,18 +70,16 @@ namespace katvan {
  *   test logic).
  */
 
-constexpr int CURSOR_WIDTH = 1;
-
 class LayoutBlockData : public QTextBlockUserData
 {
 public:
     static constexpr BlockDataKind DATA_KIND = BlockDataKind::LAYOUT;
 
-    LayoutBlockData() : revision(-1) {}
+    LayoutBlockData() : textHash(qHash(QStringView())) {}
 
     std::unique_ptr<QTextLayout> displayLayout;
     QList<ushort> displayOffsets;
-    int revision;
+    size_t textHash;
 };
 
 static int adjustPosToDisplay(const QList<ushort>& displayOffsets, int pos)
@@ -112,6 +110,7 @@ static int adjustPosFromDisplay(const QList<ushort>& displayOffsets, int pos)
 EditorLayout::EditorLayout(QTextDocument* document, CodeModel* codeModel)
     : QAbstractTextDocumentLayout(document)
     , d_codeModel(codeModel)
+    , d_cursorWidth(1)
     , d_documentSize(0, 0)
 {
 }
@@ -204,7 +203,7 @@ void EditorLayout::draw(QPainter* painter, const QAbstractTextDocumentLayout::Pa
 
     // Make sure that the right margin is NOT part of the clip rectangle, so
     // that it won't be painted over by a FullWidthSelection
-    qreal maxX = document()->textWidth() - document()->documentMargin() + CURSOR_WIDTH;
+    qreal maxX = document()->textWidth() - document()->documentMargin() + d_cursorWidth;
     clip.setRight(qMin(clip.right(), maxX));
 
     QTextBlock block = findContainingBlock(clip.top());
@@ -266,7 +265,7 @@ void EditorLayout::draw(QPainter* painter, const QAbstractTextDocumentLayout::Pa
             layout->drawCursor(painter,
                 QPointF(),
                 adjustPosToDisplay(layoutData->displayOffsets, cursorPosInBlock),
-                CURSOR_WIDTH);
+                d_cursorWidth);
         }
 
         block = block.next();
@@ -336,10 +335,17 @@ void EditorLayout::documentChanged(int position, int charsRemoved, int charsAdde
 
 static void buildDisplayLayout(const QTextBlock& block, LayoutBlockData* blockData)
 {
-    blockData->revision = block.revision();
+    QString text = block.text();
+    size_t newHash = qHash(text);
+
+    // Calculating a display layout is expensive, only do it if content actually changed
+    if (newHash == blockData->textHash) {
+        return;
+    }
+    blockData->textHash = newHash;
 
     IsolatesBlockData* isolateData = BlockData::get<IsolatesBlockData>(block);
-    if (isolateData == nullptr /*|| isolateData->isolates().isEmpty()*/) {
+    if (isolateData == nullptr || isolateData->isolates().isEmpty()) {
         blockData->displayOffsets.clear();
         blockData->displayLayout.reset();
         return;
@@ -354,12 +360,14 @@ static void buildDisplayLayout(const QTextBlock& block, LayoutBlockData* blockDa
     // not edit purposes. This moves around positions of visible characters
     // relative to the editable block content stored in the QTextDocument, so
     // we need to save an offset mapping.
-    QString text = block.text();
 
     auto& offsets = blockData->displayOffsets;
     offsets.resize(text.size());
     offsets.fill(0);
 
+    QMap<int, int> isolatesStartingAt;
+    QMap<int, int> isolatesEndingAt;
+
     for (const auto& isolate : isolates) {
         QChar startChar;
         switch (isolate.dir) {
@@ -380,6 +388,9 @@ static void buildDisplayLayout(const QTextBlock& block, LayoutBlockData* blockDa
         for (int i = end + 1; i < offsets.length(); i++) {
             offsets[i] += 2;
         }
+
+        isolatesStartingAt[start]++;
+        isolatesEndingAt[end]++;
     }
 
     // Patch format ranges to be be correct for adjusted text
@@ -389,6 +400,32 @@ static void buildDisplayLayout(const QTextBlock& block, LayoutBlockData* blockDa
         r.start += startOffset;
     }
 
+    // The Qt method QWidgetTextControlPrivate::rectForPosition directly
+    // uses the QTextLine objects from the default layout to determine cursor
+    // and selection rectangles position and height. It is essential that
+    // our display layout's lines have the same bounding rectangles - otherwise
+    // there are various visual glitches related to mouse selection.
+    //
+    // In theory, we only add invisible control characters which shouldn't affect
+    // text size, BUT - due to them being control chars most fonts will (rightfully)
+    // claim to not cover them. We must prevent Qt from falling back to another font
+    // that might claim coverage, and has a bigger ascent or descent height than
+    // any font used for visible characters. To achieve that, add a format
+    // for each control char we injected that prevents font merging.
+    QTextLayout::FormatRange r;
+    r.format.setFontStyleStrategy(QFont::NoFontMerging);
+
+    for (const auto& [pos, count] : isolatesStartingAt.asKeyValueRange()) {
+        r.start = pos + offsets[pos] - count;
+        r.length = count;
+        formats.append(r);
+    }
+    for (const auto& [pos, count] : isolatesEndingAt.asKeyValueRange()) {
+        r.start = pos + offsets[pos] + 1;
+        r.length = count;
+        formats.append(r);
+    }
+
     blockData->displayLayout = std::make_unique<QTextLayout>(text);
     blockData->displayLayout->setFormats(formats);
 }
@@ -411,17 +448,19 @@ void EditorLayout::layoutBlock(QTextBlock& block, qreal topY)
     doBlockLayout(defaultLayout, option, topY);
     block.setLineCount(defaultLayout->lineCount());
 
-    // Calculating a display layout is expensive, only do it if content actually changed
-    if (block.revision() != blockData->revision || block.revision() == 1) {
-        buildDisplayLayout(block, blockData);
-    }
+    buildDisplayLayout(block, blockData);
 
     QTextLayout* displayLayout = blockData->displayLayout.get();
     if (displayLayout != nullptr) {
         displayLayout->setFont(document()->defaultFont());
         displayLayout->setCursorMoveStyle(document()->defaultCursorMoveStyle());
 
         doBlockLayout(displayLayout, option, topY);
+
+        if (displayLayout->boundingRect() != defaultLayout->boundingRect()) {
+            qWarning() << "Block" << block.blockNumber() << "display bounding rect differs from default one!"
+                << displayLayout->boundingRect() << "vs" << defaultLayout->boundingRect();
+        }
     }
 }
 

core/katvan_editorlayout.h

@@ -28,6 +28,10 @@ class EditorLayout : public QAbstractTextDocumentLayout
 {
     Q_OBJECT
 
+    // Undocumented property that QTextEdit (actually QWidgetTextControlPrivate)
+    // wants to exist on document layouts
+    Q_PROPERTY(int cursorWidth MEMBER d_cursorWidth)
+
 public:
     EditorLayout(QTextDocument* document, CodeModel* codeModel);
 
@@ -51,6 +55,7 @@ class EditorLayout : public QAbstractTextDocumentLayout
 
     CodeModel* d_codeModel;
 
+    int d_cursorWidth;
     QSizeF d_documentSize;
 };
 

demo.typ

@@ -4,7 +4,7 @@
 #set heading(numbering: "1.")
 
 = סדרת פיבונאצ'י
-סדרת פיבונאצ'י מוגדרת באמצעות נוסחת הנסיגה ⁦$F_n = F_(n-1) + F_(n+2)$⁩. ניתן גם לייצג אותה באמצעות _הנוסחא הסגורה_:
+סדרת פיבונאצ'י מוגדרת באמצעות נוסחת הנסיגה $F_n = F_(n-1) + F_(n+2)$. ניתן ג   ם לייצג אותה באמצעות _הנוסחא הסגורה_:
 
 $ F_n = round(1 / sqrt(5) phi.alt^n), quad phi.alt = (1 + sqrt(5)) / 2 $
 
@@ -15,7 +15,7 @@ $ F_n = round(1 / sqrt(5) phi.alt^n), quad phi.alt = (1 + sqrt(5)) / 2 $
     else { fib(n - 1) + fib(n - 2) }
 )
 
-⁦#count⁩ המספרים הראשונים בסדרה הם:
+‏#count המספרים הראשונים בסדרה הם:
 
 #text(dir: ltr,
     align(center,