From dd4a40bcc104a2096c79bf128b61cf67c16db11d Mon Sep 17 00:00:00 2001
From: Josh Maxwell <cctechwiz@gmail.com>
Date: Tue, 13 Oct 2020 16:01:04 -0600
Subject: [PATCH] Fixes buggy line highlighting (#2444)

* Calling updateCursorPosition before moving cursor.

Previously the call to readCurrentDisassemblyOffset in
updateCursorPosition was causing essentially an off-by-one bug since the
cursor was moved prior to checking the current offset.

* Separated highlightCurrentLine and highlightPCLine logic so they can be
called independently when needed.

Previously logic for highlighting the PC was included in highlighting
the current line. This caused the PC to not be highlighed when the
current line was not on-screen and being highlighted.
---
 src/widgets/DisassemblyWidget.cpp | 35 +++++++++++++++++++++++--------
 src/widgets/DisassemblyWidget.h   | 11 ++++++++++
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/src/widgets/DisassemblyWidget.cpp b/src/widgets/DisassemblyWidget.cpp
index f76917a1..76463141 100644
--- a/src/widgets/DisassemblyWidget.cpp
+++ b/src/widgets/DisassemblyWidget.cpp
@@ -315,6 +315,10 @@ void DisassemblyWidget::refreshDisasm(RVA offset)
         bottomOffset = topOffset;
     }
 
+    connectCursorPositionChanged(false);
+
+    updateCursorPosition();
+
     // remove additional lines
     QTextCursor tc = mDisasTextEdit->textCursor();
     tc.movePosition(QTextCursor::Start);
@@ -323,10 +327,6 @@ void DisassemblyWidget::refreshDisasm(RVA offset)
     tc.movePosition(QTextCursor::End, QTextCursor::KeepAnchor);
     tc.removeSelectedText();
 
-    connectCursorPositionChanged(false);
-
-    updateCursorPosition();
-
     mDisasTextEdit->setLockScroll(false);
     mDisasTextEdit->horizontalScrollBar()->setValue(horizontalScrollValue);
 
@@ -376,7 +376,6 @@ void DisassemblyWidget::highlightCurrentLine()
     QList<QTextEdit::ExtraSelection> extraSelections;
 
     QColor highlightColor = ConfigColor("lineHighlight");
-    QColor highlightPCColor = ConfigColor("highlightPC");
 
     // Highlight the current word
     QTextCursor cursor = mDisasTextEdit->textCursor();
@@ -409,9 +408,18 @@ void DisassemblyWidget::highlightCurrentLine()
     // Highlight all the words in the document same as the current one
     extraSelections.append(createSameWordsSelections(mDisasTextEdit, curHighlightedWord));
 
-    // highlight PC line
+    mDisasTextEdit->setExtraSelections(extraSelections);
+}
+
+void DisassemblyWidget::highlightPCLine()
+{
     RVA PCAddr = Core()->getProgramCounterValue();
-    highlightSelection.cursor = cursor;
+
+    QColor highlightPCColor = ConfigColor("highlightPC");
+
+    QList<QTextEdit::ExtraSelection> pcSelections;
+    QTextEdit::ExtraSelection highlightSelection;
+    highlightSelection.cursor = mDisasTextEdit->textCursor();
     highlightSelection.cursor.movePosition(QTextCursor::Start);
     if (PCAddr != RVA_INVALID) {
         while (true) {
@@ -420,7 +428,7 @@ void DisassemblyWidget::highlightCurrentLine()
                 highlightSelection.format.setBackground(highlightPCColor);
                 highlightSelection.format.setProperty(QTextFormat::FullWidthSelection, true);
                 highlightSelection.cursor.clearSelection();
-                extraSelections.append(highlightSelection);
+                pcSelections.append(highlightSelection);
             } else if (lineOffset != RVA_INVALID && lineOffset > PCAddr) {
                 break;
             }
@@ -433,7 +441,11 @@ void DisassemblyWidget::highlightCurrentLine()
         }
     }
 
-    mDisasTextEdit->setExtraSelections(extraSelections);
+    // Don't override any extraSelections already set
+    QList<QTextEdit::ExtraSelection> currentSelections = mDisasTextEdit->extraSelections();
+    currentSelections.append(pcSelections);
+
+    mDisasTextEdit->setExtraSelections(currentSelections);
 }
 
 void DisassemblyWidget::showDisasContextMenu(const QPoint &pt)
@@ -513,6 +525,9 @@ void DisassemblyWidget::updateCursorPosition()
             mDisasTextEdit->setTextCursor(originalCursor);
         }
     }
+
+    highlightPCLine();
+
     connectCursorPositionChanged(false);
 }
 
@@ -545,6 +560,7 @@ void DisassemblyWidget::cursorPositionChanged()
     seekable->seek(offset);
     seekFromCursor = false;
     highlightCurrentLine();
+    highlightPCLine();
     mCtxMenu->setCanCopy(mDisasTextEdit->textCursor().hasSelection());
     if (mDisasTextEdit->textCursor().hasSelection()) {
         // A word is selected so use it
@@ -613,6 +629,7 @@ void DisassemblyWidget::moveCursorRelative(bool up, bool page)
         if (offset != seekable->getOffset()) {
             seekable->seek(offset);
             highlightCurrentLine();
+            highlightPCLine();
         }
     }
 }
diff --git a/src/widgets/DisassemblyWidget.h b/src/widgets/DisassemblyWidget.h
index 36aaf3ec..e4b6eead 100644
--- a/src/widgets/DisassemblyWidget.h
+++ b/src/widgets/DisassemblyWidget.h
@@ -28,7 +28,18 @@ public:
     static QString getWidgetType();
 
 public slots:
+    /**
+     * @brief Highlights the currently selected line and updates the 
+     * highlighting of the same words under the cursor in the visible screen.
+     * This overrides all previous highlighting.
+     */
     void highlightCurrentLine();
+    /**
+     * @brief Adds the PC line highlighting to the other current highlighting.
+     * This should be called after highlightCurrentLine since that function
+     * overrides all previous highlighting.
+     */
+    void highlightPCLine();
     void showDisasContextMenu(const QPoint &pt);
     void fontsUpdatedSlot();
     void colorsUpdatedSlot();