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

@@ -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();
         }
     }
 }

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();