Qt: Use QTextLayout in ByteViewText.

Use QTextLayout to draw each line in ByteViewText instead of drawing
fragments ourselves. Build our pixel-to-byte-offset map when we draw our
first line, which should hopefully make it more accurate. This should
fix layout and hover issues on some systems.

Start moving common code to DataPrinter.

Mark prefs.gui_hex_dump_highlight_style GTK+ only.

Bug: 11844
Change-Id: Ifda16ae7dc1a5ea22570c0bfd0eb20cee621bfc9
Reviewed-on: https://code.wireshark.org/review/24717
Reviewed-by: Gerald Combs <gerald@wireshark.org>
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Roland Knall <rknall@gmail.com>

1 files changed, 200 insertions, 204 deletions

diff --git a/ui/qt/widgets/byte_view_text.cpp b/ui/qt/widgets/byte_view_text.cpp
index 93612e514b..c66af61761 100644
--- a/ui/qt/widgets/byte_view_text.cpp
+++ b/ui/qt/widgets/byte_view_text.cpp
@@ -4,24 +4,9 @@
  * By Gerald Combs <gerald@wireshark.org>
  * Copyright 1998 Gerald Combs
  *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version 2
- * of the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ * SPDX-License-Identifier: GPL-2.0+
  */
 
-// Some code based on QHexView by Even Teran
-// https://code.google.com/p/qhexview/
-
 #include "byte_view_text.h"
 
 #include <epan/charsets.h>
@@ -32,6 +17,7 @@
 #include "wireshark_application.h"
 #include "ui/recent.h"
 
+#include <ui/qt/utils/data_printer.h>
 #include <ui/qt/utils/variant_pointer.h>
 
 #include <QActionGroup>
@@ -40,39 +26,43 @@
 #include <QScrollBar>
 #include <QStyle>
 #include <QStyleOption>
+#include <QTextLayout>
 
 // To do:
 // - Add recent settings and context menu items to show/hide the offset,
 //   and ASCII/EBCDIC.
 // - Add a UTF-8 and possibly UTF-xx option to the ASCII display.
 // - Add "copy bytes as" context menu items.
-
-// We don't obey the gui.hex_dump_highlight_style preference. If you
-// would like to add support for this you'll probably have to call
-// QPainter::drawText for each individual character.
+// - Add back hover.
+// - Move more common metrics to DataPrinter.
 
 Q_DECLARE_METATYPE(bytes_view_type)
 Q_DECLARE_METATYPE(packet_char_enc)
 
 ByteViewText::ByteViewText(QByteArray data, packet_char_enc encoding, QWidget *parent) :
     QAbstractScrollArea(parent),
-    bold_highlight_(false),
+    layout_(new QTextLayout()),
     encoding_(encoding),
     hovered_byte_offset_(-1),
     hovered_byte_lock_(false),
-    p_bound_(0, 0),
-    f_bound_(0, 0),
-    fa_bound_(0, 0),
+    proto_start_(0),
+    proto_len_(0),
+    field_start_(0),
+    field_len_(0),
+    field_a_start_(0),
+    field_a_len_(0),
     show_offset_(true),
     show_hex_(true),
     show_ascii_(true),
     row_width_(recent.gui_bytes_view == BYTES_HEX ? 16 : 8),
-    one_em_(0),
     font_width_(0),
-    line_spacing_(0),
-    margin_(0)
+    line_height_(0)
 {
     data_ = data;
+    layout_->setCacheEnabled(true);
+
+    offset_normal_fg_ = ColorUtils::alphaBlend(palette().windowText(), palette().window(), 0.35);
+    offset_field_fg_ = ColorUtils::alphaBlend(palette().windowText(), palette().window(), 0.65);
 
     createContextMenu();
 
@@ -86,6 +76,7 @@ ByteViewText::ByteViewText(QByteArray data, packet_char_enc encoding, QWidget *p
 ByteViewText::~ByteViewText()
 {
     ctx_menu_.clear();
+    delete(layout_);
 }
 
 void ByteViewText::createContextMenu()
@@ -139,11 +130,6 @@ QByteArray ByteViewText::viewData()
     return data_;
 }
 
-void ByteViewText::setHighlightStyle(bool bold)
-{
-    bold_highlight_ = bold;
-}
-
 bool ByteViewText::isEmpty() const
 {
     return data_.isEmpty();
@@ -151,22 +137,22 @@ bool ByteViewText::isEmpty() const
 
 QSize ByteViewText::minimumSizeHint() const
 {
-    // Allow panel to be shrinked to any size
+    // Allow panel to shrink to any size
     return QSize();
 }
 
-void ByteViewText::markProtocol(int start, int end)
+void ByteViewText::markProtocol(int start, int length)
 {
-    p_bound_ = QPair<guint, guint>(qMax(0, start), qMax(0, end));
-    p_bound_save_ = p_bound_;
+    proto_start_ = start;
+    proto_len_ = length;
     viewport()->update();
 }
 
-void ByteViewText::markField(int start, int end)
+void ByteViewText::markField(int start, int length)
 {
-    f_bound_ = QPair<guint, guint>(qMax(0, start), qMax(0, end));
+    field_start_ = start;
+    field_len_ = length;
     scrollToByte(start);
-    f_bound_save_ = f_bound_;
     viewport()->update();
 }
 
@@ -177,10 +163,10 @@ void ByteViewText::moveToOffset(int pos)
 }
 
 
-void ByteViewText::markAppendix(int start, int end)
+void ByteViewText::markAppendix(int start, int length)
 {
-    fa_bound_ = QPair<guint, guint>(qMax(0, start), qMax(0, end));
-    fa_bound_save_ = f_bound_;
+    field_a_start_ = start;
+    field_a_len_ = length;
     viewport()->update();
 }
 
@@ -190,11 +176,12 @@ void ByteViewText::setMonospaceFont(const QFont &mono_font)
 
     const QFontMetricsF fm(mono_font);
     font_width_  = fm.width('M');
-    line_spacing_ = fm.lineSpacing() + 0.5;
-    one_em_ = fm.height();
-    margin_ = fm.height() / 2;
 
     setFont(mono_font);
+    layout_->setFont(mono_font);
+
+    // We should probably use ProtoTree::rowHeight.
+    line_height_ = fontMetrics().height();
 
     updateScrollbars();
     viewport()->update();
@@ -210,14 +197,12 @@ void ByteViewText::paintEvent(QPaintEvent *)
     int row_y = 0;
 
     // Starting byte offset
-    guint offset = (guint) verticalScrollBar()->value() * row_width_;
+    int offset = verticalScrollBar()->value() * row_width_;
 
     // Clear the area
     painter.fillRect(viewport()->rect(), palette().base());
 
-    // Offset background
-    offset_normal_fg_.setColor(ColorUtils::alphaBlend(palette().windowText(), palette().window(), 0.35));
-    offset_field_fg_.setColor(ColorUtils::alphaBlend(palette().windowText(), palette().window(), 0.65));
+    // Offset background. We want the entire height to be filled.
     if (show_offset_) {
         QRect offset_rect = QRect(viewport()->rect());
         offset_rect.setWidth(offsetPixels());
@@ -228,34 +213,17 @@ void ByteViewText::paintEvent(QPaintEvent *)
         return;
     }
 
-    // Map window coordinates to byte offsets
-    x_pos_to_column_.clear();
-    for (guint i = 0; i < row_width_; i++) {
-        int sep_width = (i / separator_interval_) * font_width_;
-        if (show_hex_) {
-            // Hittable pixels extend 1/2 space on either side of the hex digits
-            int pixels_per_byte = (recent.gui_bytes_view == BYTES_HEX ? 3 : 9) * font_width_;
-            int hex_x = offsetPixels() + margin_ + sep_width + (i * pixels_per_byte) - (font_width_ / 2);
-            for (int j = 0; j <= pixels_per_byte; j++) {
-                x_pos_to_column_[hex_x + j] = i;
-            }
-        }
-        if (show_ascii_) {
-            int ascii_x = offsetPixels() + hexPixels() + margin_ + sep_width + (i * font_width_);
-            for (int j = 0; j <= font_width_; j++) {
-                x_pos_to_column_[ascii_x + j] = i;
-            }
-        }
-    }
-
     // Data rows
     int widget_height = height();
     painter.save();
-    while( (int) (row_y + line_spacing_) < widget_height && (int) offset < (int) data_.count()) {
-        drawOffsetLine(painter, offset, row_y);
+
+    x_pos_to_column_.clear();
+    while( (int) (row_y + line_height_) < widget_height && offset < (int) data_.count()) {
+        drawLine(&painter, offset, row_y);
         offset += row_width_;
-        row_y += line_spacing_;
+        row_y += line_height_;
     }
+
     painter.restore();
 
     QStyleOptionFocusRect option;
@@ -304,187 +272,206 @@ void ByteViewText::contextMenuEvent(QContextMenuEvent *event)
 
 // Private
 
-const int ByteViewText::separator_interval_ = 8; // Insert a space after this many bytes
+const int ByteViewText::separator_interval_ = DataPrinter::separatorInterval();
 
 // Draw a line of byte view text for a given offset.
-// Text with different styles are split into fragments and passed to
-// flushOffsetFragment. Font character widths aren't necessarily whole
-// numbers so we track our X coordinate position using using floats.
-void ByteViewText::drawOffsetLine(QPainter &painter, const guint offset, const int row_y)
+// Text highlighting is handled using QTextLayout::FormatRange.
+void ByteViewText::drawLine(QPainter *painter, const int offset, const int row_y)
 {
     if (isEmpty()) {
         return;
     }
-    guint tvb_len = data_.count();
-    guint max_pos = qMin(offset + row_width_, tvb_len);
+
+    // Build our pixel to byte offset vector the first time through.
+    bool build_x_pos = x_pos_to_column_.empty() ? true : false;
+    int tvb_len = data_.count();
+    int max_tvb_pos = qMin(offset + row_width_, tvb_len) - 1;
+    QList<QTextLayout::FormatRange> fmt_list;
 
     static const guchar hexchars[16] = {
         '0', '1', '2', '3', '4', '5', '6', '7',
         '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' };
 
-    QString text;
-    HighlightMode hl_mode = ModeNormal, offset_mode = ModeOffsetNormal;
-    qreal hex_x = offsetPixels() + margin_;
-    qreal ascii_x = offsetPixels() + hexPixels() + margin_;
+    QString line;
+    HighlightMode offset_mode = ModeOffsetNormal;
+
+    // Offset.
+    if (show_offset_) {
+        line = QString(" %1 ").arg(offset, offsetChars(false), 16, QChar('0'));
+        if (build_x_pos) {
+            x_pos_to_column_.fill(-1, fontMetrics().width(line));
+        }
+    }
 
     // Hex
     if (show_hex_) {
-        for (guint tvb_pos = offset; tvb_pos < max_pos; tvb_pos++) {
-            HighlightMode hex_state = ModeNormal;
-            bool add_space = tvb_pos != offset;
-            bool draw_hover = tvb_pos == hovered_byte_offset_;
-
-            if ((tvb_pos >= f_bound_.first && tvb_pos < f_bound_.second) || (tvb_pos >= fa_bound_.first && tvb_pos < fa_bound_.second)) {
-                hex_state = ModeField;
-                offset_mode = ModeOffsetField;
-            } else if (tvb_pos >= p_bound_.first && tvb_pos < p_bound_.second) {
-                hex_state = ModeProtocol;
-            }
+        int ascii_start = line.length() + DataPrinter::hexChars() + 3;
+        // Extra hover space before and after each byte.
+        int slop = font_width_ / 2;
 
-            if (hex_state != hl_mode || draw_hover) {
-                if ((hl_mode == ModeNormal || (hl_mode == ModeProtocol && hex_state == ModeField) || draw_hover) && add_space) {
-                    add_space = false;
-                    text += ' ';
-                    /* insert a space every separator_interval_ bytes */
-                    if ((tvb_pos % separator_interval_) == 0)
-                        text += ' ';
-                }
-                hex_x += flushOffsetFragment(painter, hex_x, row_y, hl_mode, text);
-                hl_mode = hex_state;
-            }
+        if (build_x_pos) {
+            x_pos_to_column_.append(QVector<int>().fill(-1, slop));
+        }
 
-            if (add_space) {
-                text += ' ';
-                /* insert a space every separator_interval_ bytes */
-                if ((tvb_pos % separator_interval_) == 0)
-                    text += ' ';
+        for (int tvb_pos = offset; tvb_pos <= max_tvb_pos; tvb_pos++) {
+            line += ' ';
+            /* insert a space every separator_interval_ bytes */
+            if ((tvb_pos != offset) && ((tvb_pos % separator_interval_) == 0)) {
+                line += ' ';
+                x_pos_to_column_.append(QVector<int>().fill(tvb_pos - offset - 1, font_width_));
             }
 
             switch (recent.gui_bytes_view) {
             case BYTES_HEX:
-                text += hexchars[(data_[tvb_pos] & 0xf0) >> 4];
-                text += hexchars[data_[tvb_pos] & 0x0f];
+                line += hexchars[(data_[tvb_pos] & 0xf0) >> 4];
+                line += hexchars[data_[tvb_pos] & 0x0f];
                 break;
             case BYTES_BITS:
                 /* XXX, bitmask */
-                for (int j = 7; j >= 0; j--)
-                    text += (data_[tvb_pos] & (1 << j)) ? '1' : '0';
+                for (int j = 7; j >= 0; j--) {
+                    line += (data_[tvb_pos] & (1 << j)) ? '1' : '0';
+                }
                 break;
             }
-            if (draw_hover) {
-                hex_x += flushOffsetFragment(painter, hex_x, row_y, ModeHover, text);
+            if (build_x_pos) {
+                x_pos_to_column_.append(QVector<int>().fill(tvb_pos - offset, fontMetrics().width(line) - x_pos_to_column_.size() + slop));
             }
         }
+        line += QString(ascii_start - line.length(), ' ');
+        if (build_x_pos) {
+            x_pos_to_column_.append(QVector<int>().fill(-1, fontMetrics().width(line) - x_pos_to_column_.size()));
+        }
+
+        addHexFormatRange(fmt_list, proto_start_, proto_len_, offset, max_tvb_pos, ModeProtocol);
+        if (addHexFormatRange(fmt_list, field_start_, field_len_, offset, max_tvb_pos, ModeField)) {
+            offset_mode = ModeOffsetField;
+        }
+        addHexFormatRange(fmt_list, field_a_start_, field_a_len_, offset, max_tvb_pos, ModeField);
+        if (hovered_byte_offset_ >= offset && hovered_byte_offset_ <= max_tvb_pos) {
+            addHexFormatRange(fmt_list, hovered_byte_offset_, hovered_byte_offset_ + 1, offset, max_tvb_pos, ModeField);
+        }
     }
-    if (text.length() > 0) {
-        flushOffsetFragment(painter, hex_x, row_y, hl_mode, text);
-    }
-    hl_mode = ModeNormal;
 
     // ASCII
     if (show_ascii_) {
-        for (guint tvb_pos = offset; tvb_pos < max_pos; tvb_pos++) {
-            HighlightMode ascii_state = ModeNormal;
-            bool add_space = tvb_pos != offset;
-            bool highlight_text = tvb_pos == hovered_byte_offset_;
-
-            if ((tvb_pos >= f_bound_.first && tvb_pos < f_bound_.second) || (tvb_pos >= fa_bound_.first && tvb_pos < fa_bound_.second)) {
-                ascii_state = ModeField;
-                offset_mode = ModeOffsetField;
-            } else if (tvb_pos >= p_bound_.first && tvb_pos < p_bound_.second) {
-                ascii_state = ModeProtocol;
-            }
-
-            if (ascii_state != hl_mode || highlight_text) {
-                if ((hl_mode == ModeNormal || (hl_mode == ModeProtocol && ascii_state == ModeField) || highlight_text) && add_space) {
-                    add_space = false;
-                    /* insert a space every separator_interval_ bytes */
-                    if ((tvb_pos % separator_interval_) == 0)
-                        text += ' ';
+        for (int tvb_pos = offset; tvb_pos <= max_tvb_pos; tvb_pos++) {
+            /* insert a space every separator_interval_ bytes */
+            if ((tvb_pos != offset) && ((tvb_pos % separator_interval_) == 0)) {
+                line += ' ';
+                if (build_x_pos) {
+                    x_pos_to_column_.append(QVector<int>().fill(tvb_pos - offset - 1, font_width_ / 2));
                 }
-                ascii_x += flushOffsetFragment(painter, ascii_x, row_y, hl_mode, text);
-                hl_mode = ascii_state;
-            }
-
-            if (add_space) {
-                /* insert a space every separator_interval_ bytes */
-                if ((tvb_pos % separator_interval_) == 0)
-                    text += ' ';
             }
 
             guchar c = (encoding_ == PACKET_CHAR_ENC_CHAR_EBCDIC) ?
                         EBCDIC_to_ASCII1(data_[tvb_pos]) :
                         data_[tvb_pos];
 
-            text += g_ascii_isprint(c) ? c : '.';
-            if (highlight_text) {
-                ascii_x += flushOffsetFragment(painter, ascii_x, row_y, ModeHover, text);
+            if (g_ascii_isprint(c)) {
+                line += c;
+            } else {
+                line += UTF8_MIDDLE_DOT;
+            }
+            if (build_x_pos) {
+                x_pos_to_column_.append(QVector<int>().fill(tvb_pos - offset, fontMetrics().width(line) - x_pos_to_column_.size()));
             }
         }
-    }
-    if (text.length() > 0) {
-        flushOffsetFragment(painter, ascii_x, row_y, hl_mode, text);
+        addAsciiFormatRange(fmt_list, proto_start_, proto_len_, offset, max_tvb_pos, ModeProtocol);
+        if (addAsciiFormatRange(fmt_list, field_start_, field_len_, offset, max_tvb_pos, ModeField)) {
+            offset_mode = ModeOffsetField;
+        }
+        addAsciiFormatRange(fmt_list, field_a_start_, field_a_len_, offset, max_tvb_pos, ModeField);
+        if (hovered_byte_offset_ >= offset && hovered_byte_offset_ <= max_tvb_pos) {
+            addAsciiFormatRange(fmt_list, hovered_byte_offset_, hovered_byte_offset_ + 1, offset, max_tvb_pos, ModeField);
+        }
     }
 
-    // Offset. Must be drawn last in order for offset_state to be set.
-    if (show_offset_) {
-        text = QString("%1").arg(offset, offsetChars(), 16, QChar('0'));
-        flushOffsetFragment(painter, margin_, row_y, offset_mode, text);
-    }
+    // XXX Fields won't be highlighted if neither hex nor ascii are enabled.
+    addFormatRange(fmt_list, 0, offsetChars(), offset_mode);
+
+    layout_->clearLayout();
+    layout_->clearAdditionalFormats();
+    layout_->setText(line);
+    layout_->setAdditionalFormats(fmt_list);
+    layout_->beginLayout();
+    QTextLine tl = layout_->createLine();
+    tl.setLineWidth(totalPixels());
+    tl.setPosition(QPointF(0.0, 0.0));
+    layout_->endLayout();
+    layout_->draw(painter, QPointF(0.0, row_y));
 }
 
-// Draws a fragment of byte view text at the specifiec location using colors
-// for the specified state. Clears the text and returns the pixel width of the
-// drawn text.
-qreal ByteViewText::flushOffsetFragment(QPainter &painter, qreal x, int y, HighlightMode mode, QString &text)
+bool ByteViewText::addFormatRange(QList<QTextLayout::FormatRange> &fmt_list, int start, int length, HighlightMode mode)
 {
-    if (text.length() < 1) {
-        return 0;
-    }
-    QFontMetricsF fm(mono_font_);
-    qreal width = fm.width(text);
-    QRectF area(x, y, width, line_spacing_);
-    // Background
-    switch (mode) {
-    case ModeField:
-        painter.fillRect(area, palette().highlight());
-        break;
-    case ModeProtocol:
-        painter.fillRect(area, palette().window());
-        break;
-    case ModeHover:
-        painter.fillRect(area, ColorUtils::byteViewHoverColor(true));
-        break;
-    default:
-        break;
+    if (length < 1 || mode == ModeNormal) {
+        return false;
     }
 
-    // Text
-    QBrush text_brush;
+    QTextLayout::FormatRange format_range;
+    format_range.start = start;
+    format_range.length = length;
+    format_range.format.setProperty(QTextFormat::LineHeight, line_height_);
     switch (mode) {
     case ModeNormal:
-    case ModeProtocol:
-    default:
-        text_brush = palette().windowText();
         break;
     case ModeField:
-        text_brush = palette().highlightedText();
+        format_range.format.setBackground(palette().highlight());
+        break;
+    case ModeProtocol:
+        format_range.format.setBackground(palette().window());
         break;
     case ModeOffsetNormal:
-        text_brush = offset_normal_fg_;
+        format_range.format.setForeground(offset_normal_fg_);
         break;
     case ModeOffsetField:
-        text_brush = offset_field_fg_;
+        format_range.format.setForeground(offset_field_fg_);
         break;
     case ModeHover:
-        text_brush = ColorUtils::byteViewHoverColor(false);
+        format_range.format.setForeground(ColorUtils::byteViewHoverColor(false));
+        format_range.format.setBackground(ColorUtils::byteViewHoverColor(true));
         break;
     }
+    fmt_list << format_range;
+    return true;
+}
 
-    painter.setPen(QPen(text_brush.color()));
-    painter.drawText(area, Qt::AlignTop, text);
-    text.clear();
-    return width;
+bool ByteViewText::addHexFormatRange(QList<QTextLayout::FormatRange> &fmt_list, int mark_start, int mark_length, int tvb_offset, int max_tvb_pos, ByteViewText::HighlightMode mode)
+{
+    int mark_end = mark_start + mark_length - 1;
+    if (mark_start < 0 || mark_length < 1) return false;
+    if (mark_start > max_tvb_pos && mark_end < tvb_offset) return false;
+
+    int chars_per_byte = recent.gui_bytes_view == BYTES_HEX ? 3 : 9;
+    int byte_start = qMax(tvb_offset, mark_start) - tvb_offset;
+    int byte_end = qMin(max_tvb_pos, mark_end) - tvb_offset;
+    int fmt_start = offsetChars() + 1 // offset + spacing
+            + (byte_start / separator_interval_)
+            + (byte_start * chars_per_byte);
+    int fmt_length = offsetChars() + 1 // offset + spacing
+            + (byte_end / separator_interval_)
+            + (byte_end * chars_per_byte)
+            + 2 // Both the high and low nibbles.
+            - fmt_start;
+    return addFormatRange(fmt_list, fmt_start, fmt_length, mode);
+}
+
+bool ByteViewText::addAsciiFormatRange(QList<QTextLayout::FormatRange> &fmt_list, int mark_start, int mark_length, int tvb_offset, int max_tvb_pos, ByteViewText::HighlightMode mode)
+{
+    int mark_end = mark_start + mark_length - 1;
+    if (mark_start < 0 || mark_length < 1) return false;
+    if (mark_start > max_tvb_pos && mark_end < tvb_offset) return false;
+
+    int byte_start = qMax(tvb_offset, mark_start) - tvb_offset;
+    int byte_end = qMin(max_tvb_pos, mark_end) - tvb_offset;
+    int fmt_start = offsetChars() + DataPrinter::hexChars() + 3 // offset + hex + spacing
+            + (byte_start / separator_interval_)
+            + byte_start;
+    int fmt_length = offsetChars() + DataPrinter::hexChars() + 3 // offset + hex + spacing
+            + (byte_end / separator_interval_)
+            + byte_end
+            + 1 // Just one character.
+            - fmt_start;
+    return addFormatRange(fmt_list, fmt_start, fmt_length, mode);
 }
 
 void ByteViewText::scrollToByte(int byte)
@@ -493,19 +480,22 @@ void ByteViewText::scrollToByte(int byte)
 }
 
 // Offset character width
-int ByteViewText::offsetChars()
+int ByteViewText::offsetChars(bool include_pad)
 {
+    int padding = include_pad ? 2 : 0;
     if (! isEmpty() && data_.count() > 0xffff) {
-        return 8;
+        return 8 + padding;
     }
-    return 4;
+    return 4 + padding;
 }
 
 // Offset pixel width
 int ByteViewText::offsetPixels()
 {
     if (show_offset_) {
-        return offsetChars() * font_width_ + one_em_;
+        // One pad space before and after
+        QString zeroes = QString(offsetChars(), '0');
+        return fontMetrics().width(zeroes);
     }
     return 0;
 }
@@ -514,8 +504,9 @@ int ByteViewText::offsetPixels()
 int ByteViewText::hexPixels()
 {
     if (show_hex_) {
-        int digits_per_byte = recent.gui_bytes_view == BYTES_HEX ? 3 : 9;
-        return (((row_width_ * digits_per_byte) + ((row_width_ - 1) / separator_interval_)) * font_width_) + one_em_;
+        // One pad space before and after
+        QString zeroes = QString(DataPrinter::hexChars() + 2, '0');
+        return fontMetrics().width(zeroes);
     }
     return 0;
 }
@@ -523,7 +514,10 @@ int ByteViewText::hexPixels()
 int ByteViewText::asciiPixels()
 {
     if (show_ascii_) {
-        return ((row_width_ + ((row_width_ - 1) / separator_interval_)) * font_width_) + one_em_;
+        // Two pad spaces before, one after
+        int ascii_chars = (row_width_ + ((row_width_ - 1) / separator_interval_));
+        QString zeroes = QString(ascii_chars + 3, '0');
+        return fontMetrics().width(zeroes);
     }
     return 0;
 }
@@ -533,20 +527,22 @@ int ByteViewText::totalPixels()
     return offsetPixels() + hexPixels() + asciiPixels();
 }
 
+// We do chunky (per-character) scrolling because it makes some of the
+// math easier. Should we do smooth scrolling?
 void ByteViewText::updateScrollbars()
 {
     const int length = data_.count();
     if (length > 0) {
-        qint64 maxval = length / row_width_ + ((length % row_width_) ? 1 : 0) - viewport()->height() / line_spacing_;
+        int all_lines_height = length / row_width_ + ((length % row_width_) ? 1 : 0) - viewport()->height() / line_height_;
 
-        verticalScrollBar()->setRange(0, int(qMax((qint64)0, maxval)));
-        horizontalScrollBar()->setRange(0, qMax(0, static_cast<int>((totalPixels() - viewport()->width()) / font_width_)));
+        verticalScrollBar()->setRange(0, qMax(0, all_lines_height));
+        horizontalScrollBar()->setRange(0, qMax(0, int((totalPixels() - viewport()->width()) / font_width_)));
     }
 }
 
 int ByteViewText::byteOffsetAtPixel(QPoint pos)
 {
-    int byte = (verticalScrollBar()->value() + (pos.y() / line_spacing_)) * row_width_;
+    int byte = (verticalScrollBar()->value() + (pos.y() / line_height_)) * row_width_;
     int x = (horizontalScrollBar()->value() * font_width_) + pos.x();
     int col = x_pos_to_column_.value(x, -1);