Qt: Allow multiple lines in the packet list.

Remove the setUniformRowHeights(true) call in the PacketList
constructor. This means that the packet list now calculates its height
by querying the Qt::SizeHintRole for every item instead of simply
multiplying item_height * number_of_rows.

Implement SizeHintRole in PacketListModel::data so that size
calculations aren't unbearably slow. We don't have any row text until an
item is drawn (via DisplayRole), so items with multiple lines end up
being rendered twice. Note where we make assumptions on line heights.

Although we call gtk_tree_view_set_fixed_height_mode() in the GTK+ UI we
don't have this problem there. GTK+ is apparently less strict about
enforcing row heights.

Bug: 10924
Change-Id: I98e9f4f5f321c2e03f18498e0a7e7556f88792a1
Reviewed-on: https://code.wireshark.org/review/7430
Petri-Dish: Evan Huus <eapache@gmail.com>
Reviewed-by: Evan Huus <eapache@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michal Labedzki <michal.labedzki@tieto.com>
Reviewed-by: Anders Broman <a.broman58@gmail.com>

5 files changed, 65 insertions, 22 deletions

diff --git a/ui/qt/packet_list.cpp b/ui/qt/packet_list.cpp
index 555a15881e..d24e95a1c9 100644
--- a/ui/qt/packet_list.cpp
+++ b/ui/qt/packet_list.cpp
@@ -238,7 +238,6 @@ PacketList::PacketList(QWidget *parent) :
     setItemsExpandable(false);
     setRootIsDecorated(false);
     setSortingEnabled(true);
-    setUniformRowHeights(true);
     setAccessibleName("Packet list");
     setItemDelegateForColumn(0, &related_packet_delegate_);
 
@@ -912,7 +911,11 @@ void PacketList::setCaptureFile(capture_file *cf)
 
 void PacketList::setMonospaceFont(const QFont &mono_font)
 {
-    packet_list_model_->setMonospaceFont(mono_font);
+    setFont(mono_font);
+    // qtreeview.cpp does something similar in Qt 5 so this *should* be
+    // safe...
+    int row_height = itemDelegate()->sizeHint(viewOptions(), QModelIndex()).height();
+    packet_list_model_->setMonospaceFont(mono_font, row_height);
     redrawVisiblePackets();
 }
 
diff --git a/ui/qt/packet_list_model.cpp b/ui/qt/packet_list_model.cpp
index b10af13a18..56135e4731 100644
--- a/ui/qt/packet_list_model.cpp
+++ b/ui/qt/packet_list_model.cpp
@@ -38,9 +38,14 @@
 #include <QModelIndex>
 
 PacketListModel::PacketListModel(QObject *parent, capture_file *cf) :
-    QAbstractItemModel(parent)
+    QAbstractItemModel(parent),
+    row_height_(-1),
+    line_spacing_(0)
 {
     setCaptureFile(cf);
+    connect(this, SIGNAL(itemHeightChanged(QModelIndex)),
+            this, SLOT(emitItemHeightChanged(QModelIndex)),
+            Qt::QueuedConnection);
 }
 
 void PacketListModel::setCaptureFile(capture_file *cf)
@@ -50,11 +55,8 @@ void PacketListModel::setCaptureFile(capture_file *cf)
 }
 
 // Packet list records have no children (for now, at least).
-QModelIndex PacketListModel::index(int row, int column, const QModelIndex &parent)
-            const
+QModelIndex PacketListModel::index(int row, int column, const QModelIndex &) const
 {
-    Q_UNUSED(parent);
-
     if (row >= visible_rows_.count() || row < 0 || !cap_file_ || column >= prefs.num_cols)
         return QModelIndex();
 
@@ -64,9 +66,8 @@ QModelIndex PacketListModel::index(int row, int column, const QModelIndex &paren
 }
 
 // Everything is under the root.
-QModelIndex PacketListModel::parent(const QModelIndex &index) const
+QModelIndex PacketListModel::parent(const QModelIndex &) const
 {
-    Q_UNUSED(index);
     return QModelIndex();
 }
 
@@ -133,9 +134,12 @@ int PacketListModel::columnTextSize(const char *str)
     return fm.width(str);
 }
 
-void PacketListModel::setMonospaceFont(const QFont &mono_font)
+void PacketListModel::setMonospaceFont(const QFont &mono_font, int row_height)
 {
+    QFontMetrics fm(mono_font_);
     mono_font_ = mono_font;
+    row_height_ = row_height;
+    line_spacing_ = fm.lineSpacing();
 }
 
 // The Qt MVC documentation suggests using QSortFilterProxyModel for sorting
@@ -235,6 +239,11 @@ bool PacketListModel::recordLessThan(PacketListRecord *r1, PacketListRecord *r2)
     }
 }
 
+void PacketListModel::emitItemHeightChanged(const QModelIndex &index)
+{
+    emit dataChanged(index, index);
+}
+
 int PacketListModel::rowCount(const QModelIndex &parent) const
 {
     if (parent.column() >= prefs.num_cols)
@@ -243,10 +252,8 @@ int PacketListModel::rowCount(const QModelIndex &parent) const
     return visible_rows_.count();
 }
 
-int PacketListModel::columnCount(const QModelIndex &parent) const
+int PacketListModel::columnCount(const QModelIndex &) const
 {
-    Q_UNUSED(parent);
-
     return prefs.num_cols;
 }
 
@@ -313,7 +320,20 @@ QVariant PacketListModel::data(const QModelIndex &index, int role) const
     case Qt::DisplayRole:
     {
         int column = index.column();
-        return record->columnString(cap_file_, column);
+        QVariant column_string = record->columnString(cap_file_, column);
+        // We don't know an item's sizeHint until we fetch its text here.
+        // Assume each line count is 1. If the line count changes, emit
+        // itemHeightChanged which triggers another redraw (including a
+        // fetch of SizeHintRole and DisplayRole) in the next event loop.
+        if (column == 0 && record->lineCountChanged())
+            emit itemHeightChanged(index);
+        return column_string;
+    }
+    case Qt::SizeHintRole:
+    {
+        // We assume that inter-line spacing is 0.
+        QSize size = QSize(-1, row_height_ + ((record->lineCount() - 1) * line_spacing_));
+        return size;
     }
     default:
         return QVariant();
diff --git a/ui/qt/packet_list_model.h b/ui/qt/packet_list_model.h
index fedde05149..db229ce38b 100644
--- a/ui/qt/packet_list_model.h
+++ b/ui/qt/packet_list_model.h
@@ -45,14 +45,14 @@ public:
     explicit PacketListModel(QObject *parent = 0, capture_file *cf = NULL);
     void setCaptureFile(capture_file *cf);
     QModelIndex index(int row, int column,
-                      const QModelIndex &parent = QModelIndex()) const;
-    QModelIndex parent(const QModelIndex &index) const;
+                      const QModelIndex & = QModelIndex()) const;
+    QModelIndex parent(const QModelIndex &) const;
     int packetNumberToRow(int packet_num) const;
     guint recreateVisibleRows();
     void clear();
 
     int rowCount(const QModelIndex &parent = QModelIndex()) const;
-    int columnCount(const QModelIndex &parent = QModelIndex()) const;
+    int columnCount(const QModelIndex & = QModelIndex()) const;
     QVariant data(const QModelIndex &index, int role) const;
     QVariant headerData(int section, Qt::Orientation orientation,
                              int role = Qt::DisplayRole) const;
@@ -66,9 +66,10 @@ public:
 
 signals:
     void goToPacket(int);
+    void itemHeightChanged(const QModelIndex &index) const;
 
 public slots:
-    void setMonospaceFont(const QFont &mono_font);
+    void setMonospaceFont(const QFont &mono_font, int row_height);
     void sort(int column, Qt::SortOrder order = Qt::AscendingOrder);
 
 private:
@@ -79,13 +80,17 @@ private:
     QVector<PacketListRecord *> physical_rows_;
     QMap<int, int> number_to_row_;
 
-    int header_height_;
+    int row_height_;
+    int line_spacing_;
 
     static int sort_column_;
     static int text_sort_column_;
     static Qt::SortOrder sort_order_;
     static capture_file *sort_cap_file_;
     static bool recordLessThan(PacketListRecord *r1, PacketListRecord *r2);
+
+private slots:
+    void emitItemHeightChanged(const QModelIndex &index);
 };
 
 #endif // PACKET_LIST_MODEL_H
diff --git a/ui/qt/packet_list_record.cpp b/ui/qt/packet_list_record.cpp
index 1bacfd66d9..d1875f964e 100644
--- a/ui/qt/packet_list_record.cpp
+++ b/ui/qt/packet_list_record.cpp
@@ -39,6 +39,8 @@ unsigned PacketListRecord::col_data_ver_ = 1;
 
 PacketListRecord::PacketListRecord(frame_data *frameData) :
     fdata_(frameData),
+    lines_(1),
+    line_count_changed_(false),
     data_ver_(0),
     colorized_(false),
     conv_(NULL)
@@ -178,8 +180,11 @@ void PacketListRecord::cacheColumnStrings(column_info *cinfo)
     }
 
     col_text_.clear();
+    lines_ = 1;
+    line_count_changed_ = false;
 
     for (int column = 0; column < cinfo->num_cols; ++column) {
+        int col_lines = 1;
 
 #ifdef MINIMIZE_STRING_COPYING
         int text_col = cinfo_column_.value(column, -1);
@@ -239,16 +244,23 @@ void PacketListRecord::cacheColumnStrings(column_info *cinfo)
         }
 #else // MINIMIZE_STRING_COPYING
         // XXX Use QContiguousCache?
+        QByteArray col_text;
         if (!get_column_resolved(column) && cinfo->col_expr.col_expr_val[column]) {
             /* Use the unresolved value in col_expr_val */
-            col_text_.append(cinfo->col_expr.col_expr_val[column]);
+            col_text = cinfo->col_expr.col_expr_val[column];
         } else {
             int text_col = cinfo_column_.value(column, -1);
 
             if (text_col < 0) {
                 col_fill_in_frame_data(fdata_, cinfo, column, FALSE);
             }
-            col_text_.append(cinfo->col_data[column]);
+            col_text = cinfo->col_data[column];
+        }
+        col_text_.append(col_text);
+        col_lines += col_text.count('\n');
+        if (col_lines > lines_) {
+            lines_ = col_lines;
+            line_count_changed_ = true;
         }
 #endif // MINIMIZE_STRING_COPYING
     }
diff --git a/ui/qt/packet_list_record.h b/ui/qt/packet_list_record.h
index 8fea32a0de..085c96c7d4 100644
--- a/ui/qt/packet_list_record.h
+++ b/ui/qt/packet_list_record.h
@@ -51,15 +51,18 @@ public:
     int columnTextSize(const char *str);
     static void resetColumns(column_info *cinfo);
     void resetColorized();
+    inline int lineCount() { return lines_; }
+    inline int lineCountChanged() { return line_count_changed_; }
 
 private:
     /** The column text for some columns */
     QList<QByteArray> col_text_;
 
     frame_data *fdata_;
+    int lines_;
+    bool line_count_changed_;
     static QMap<int, int> cinfo_column_;
 
-
     /** Data versions. Used to invalidate col_text_ */
     static unsigned col_data_ver_;
     unsigned data_ver_;