Qt: Don't reset the model when only changing some data

When changing whether names are resolved, absolute time is used, or the timestamp resolution in the Conversation or Endpoints dialogs, don't reset the entire model, only emit a dataChanged signal. When new data arrives, call layoutChanged instead, which is a little lighter than modelReset and will update all the indices. The underlying GArray from from epan/conversation_table.c is always the same object in a tap update. Among other things, this keeps the selected and current rows in the table the same instead of clearing them. For layoutChanged, scroll so that the selected index (if there is one) stays visible. Ping #16925 (this is an easier case than when applying a filter).
THIS-IS-NOT-A-BACKUP · Feb 7, 2025 · d903f75 · d903f75
1 parent 4898852
commit d903f75
Show file tree

Hide file tree

Showing 5 changed files with 130 additions and 43 deletions.
diff --git a/ui/qt/models/atap_data_model.cpp b/ui/qt/models/atap_data_model.cpp
@@ -228,9 +228,9 @@ void ATapDataModel::updateData(GArray * newData)
     if (_disableTap)
         return;
 
-    beginResetModel();
+    emit layoutAboutToBeChanged();
     storage_ = newData;
-    endResetModel();
+    emit layoutChanged();
 
     if (_type == ATapDataModel::DATAMODEL_CONVERSATION)
         ((ConversationDataModel *)(this))->doDataUpdate();
@@ -241,16 +241,6 @@ bool ATapDataModel::resolveNames() const
     return _resolveNames;
 }
 
-void ATapDataModel::setResolveNames(bool resolve)
-{
-    if (_resolveNames == resolve)
-        return;
-
-    beginResetModel();
-    _resolveNames = resolve;
-    endResetModel();
-}
-
 bool ATapDataModel::allowsNameResolution() const
 {
     if (_protoId < 0)
@@ -277,26 +267,6 @@ bool ATapDataModel::allowsNameResolution() const
     return false;
 }
 
-void ATapDataModel::useAbsoluteTime(bool absolute)
-{
-    if (absolute == _absoluteTime)
-        return;
-
-    beginResetModel();
-    _absoluteTime = absolute;
-    endResetModel();
-}
-
-void ATapDataModel::useNanosecondTimestamps(bool nanoseconds)
-{
-    if (_nanoseconds == nanoseconds)
-        return;
-
-    beginResetModel();
-    _nanoseconds = nanoseconds;
-    endResetModel();
-}
-
 void ATapDataModel::setFilter(QString filter)
 {
     if (_disableTap)
@@ -570,6 +540,35 @@ QVariant EndpointDataModel::data(const QModelIndex &idx, int role) const
     return QVariant();
 }
 
+void EndpointDataModel::setResolveNames(bool resolve)
+{
+    if (_resolveNames == resolve)
+        return;
+
+    _resolveNames = resolve;
+    if (rowCount() > 0) {
+        dataChanged(index(0, ENDP_COLUMN_ADDR), index(rowCount() - 1, ENDP_COLUMN_PORT));
+    }
+}
+
+void EndpointDataModel::useAbsoluteTime(bool absolute)
+{
+    if (absolute == _absoluteTime)
+        return;
+
+    _absoluteTime = absolute;
+    // No columns that depend on absoluteTime
+}
+
+void EndpointDataModel::useNanosecondTimestamps(bool nanoseconds)
+{
+    if (_nanoseconds == nanoseconds)
+        return;
+
+    _nanoseconds = nanoseconds;
+    // No columns that use time precision
+}
+
 ConversationDataModel::ConversationDataModel(int protoId, QString filter, QObject *parent) :
     ATapDataModel(ATapDataModel::DATAMODEL_CONVERSATION, protoId, filter, parent)
 {}
@@ -891,3 +890,38 @@ bool ConversationDataModel::showConversationId(int row) const
         return true;
     return false;
 }
+
+void ConversationDataModel::setResolveNames(bool resolve)
+{
+    if (_resolveNames == resolve)
+        return;
+
+    _resolveNames = resolve;
+    if (rowCount() > 0) {
+        dataChanged(index(0, CONV_COLUMN_SRC_ADDR), index(rowCount() - 1, CONV_COLUMN_DST_PORT));
+    }
+}
+
+void ConversationDataModel::useAbsoluteTime(bool absolute)
+{
+    if (absolute == _absoluteTime)
+        return;
+
+    _absoluteTime = absolute;
+    headerDataChanged(Qt::Horizontal, CONV_COLUMN_START, CONV_COLUMN_START);
+    if (rowCount() > 0) {
+        dataChanged(index(0, CONV_COLUMN_START), index(rowCount() - 1, CONV_COLUMN_START));
+    }
+}
+
+void ConversationDataModel::useNanosecondTimestamps(bool nanoseconds)
+{
+    if (_nanoseconds == nanoseconds)
+        return;
+
+    _nanoseconds = nanoseconds;
+    if (rowCount() > 0) {
+        dataChanged(index(0, CONV_COLUMN_START), index(rowCount() - 1, CONV_COLUMN_DURATION));
+    }
+}
+
diff --git a/ui/qt/models/atap_data_model.h b/ui/qt/models/atap_data_model.h
@@ -133,7 +133,7 @@ class ATapDataModel : public QAbstractListModel
      *
      * @param resolve true if names should be resolved
      */
-    void setResolveNames(bool resolve);
+    virtual void setResolveNames(bool resolve) = 0;
 
     /**
      * @brief Does the model allow names to be resolved
@@ -151,14 +151,16 @@ class ATapDataModel : public QAbstractListModel
      *
      * @param absolute true to use absolute time values
      */
-    void useAbsoluteTime(bool absolute);
+    virtual void useAbsoluteTime(bool absolute) = 0;
 
     /**
      * @brief Use nanosecond timestamps if requested
      *
+     * Otherwise, microsecond time resolution will be displayed
+     *
      * @param nanoseconds use nanosecond timestamps if required and requested
      */
-    void useNanosecondTimestamps(bool nanoseconds);
+    virtual void useNanosecondTimestamps(bool nanoseconds) = 0;
 
     /**
      * @brief Are ports hidden for this model
@@ -234,6 +236,9 @@ class ATapDataModel : public QAbstractListModel
     QString _filter;
 
     bool _absoluteTime;
+    // XXX - There are other possible time precisions besides
+    // microseconds and nanoseconds; e.g., Netmon 2.3 uses
+    // 100 ns, and pcapng can have one of many values.
     bool _nanoseconds;
     bool _resolveNames;
     bool _disableTap;
@@ -279,10 +284,13 @@ class EndpointDataModel : public ATapDataModel
 
     explicit EndpointDataModel(int protoId, QString filter, QObject *parent = nullptr);
 
-    int columnCount(const QModelIndex &parent = QModelIndex()) const;
-    QVariant headerData(int section, Qt::Orientation orientation = Qt::Horizontal, int role = Qt::DisplayRole) const;
-    QVariant data(const QModelIndex &idx, int role = Qt::DisplayRole) const;
+    int columnCount(const QModelIndex &parent = QModelIndex()) const override;
+    QVariant headerData(int section, Qt::Orientation orientation = Qt::Horizontal, int role = Qt::DisplayRole) const override;
+    QVariant data(const QModelIndex &idx, int role = Qt::DisplayRole) const override;
 
+    void setResolveNames(bool resolve) override;
+    void useAbsoluteTime(bool absolute) override;
+    void useNanosecondTimestamps(bool nanoseconds) override;
 };
 
 class ConversationDataModel : public ATapDataModel
@@ -320,9 +328,9 @@ class ConversationDataModel : public ATapDataModel
 
     explicit ConversationDataModel(int protoId, QString filter, QObject *parent = nullptr);
 
-    int columnCount(const QModelIndex &parent = QModelIndex()) const;
-    QVariant headerData(int section, Qt::Orientation orientation = Qt::Horizontal, int role = Qt::DisplayRole) const;
-    QVariant data(const QModelIndex &idx, int role = Qt::DisplayRole) const;
+    int columnCount(const QModelIndex &parent = QModelIndex()) const override;
+    QVariant headerData(int section, Qt::Orientation orientation = Qt::Horizontal, int role = Qt::DisplayRole) const override;
+    QVariant data(const QModelIndex &idx, int role = Qt::DisplayRole) const override;
 
     void doDataUpdate();
 
@@ -336,6 +344,9 @@ class ConversationDataModel : public ATapDataModel
      */
     bool showConversationId(int row = 0) const;
 
+    void setResolveNames(bool resolve) override;
+    void useAbsoluteTime(bool absolute) override;
+    void useNanosecondTimestamps(bool nanoseconds) override;
 };
 
 #endif // ATAP_DATA_MODEL_H
diff --git a/ui/qt/widgets/traffic_tab.cpp b/ui/qt/widgets/traffic_tab.cpp
@@ -150,6 +150,7 @@ QTreeView * TrafficTab::createTree(int protoId)
             }
         });
         connect(proxyModel, &TrafficDataFilterProxy::modelReset, this, &TrafficTab::modelReset);
+        connect(proxyModel, &TrafficDataFilterProxy::layoutChanged, this, &TrafficTab::modelReset);
 
         /* If the columns for the tree have changed, contact the tab. By also having the tab
          * columns changed signal connecting back to the tree, it will propagate to all trees
@@ -356,6 +357,7 @@ QVariant TrafficTab::currentItemData(int role)
     return QVariant();
 }
 
+// update current tab label to include row count
 void TrafficTab::modelReset()
 {
     if (! qobject_cast<TrafficDataFilterProxy *>(sender()))

diff --git a/ui/qt/widgets/traffic_tree.cpp b/ui/qt/widgets/traffic_tree.cpp
@@ -627,14 +627,24 @@ TrafficTree::TrafficTree(QString baseName, GList ** recentColumnList, QWidget *p
 
 void TrafficTree::setModel(QAbstractItemModel * model)
 {
+    QTreeView::setModel(model);
+
+    if (this->model()) {
+        TrafficDataFilterProxy * proxy = qobject_cast<TrafficDataFilterProxy *>(model);
+        if (proxy) {
+            disconnect(_header, &TrafficTreeHeaderView::filterOnColumn, proxy, &TrafficDataFilterProxy::filterForColumn);
+            disconnect(proxy, &TrafficDataFilterProxy::dataChanged, this, &TrafficTree::handleDataChanged);
+            disconnect(proxy, &TrafficDataFilterProxy::layoutChanged, this, &TrafficTree::handleLayoutChanged);
+        }
+    }
     if (model) {
         TrafficDataFilterProxy * proxy = qobject_cast<TrafficDataFilterProxy *>(model);
         if (proxy) {
             connect(_header, &TrafficTreeHeaderView::filterOnColumn, proxy, &TrafficDataFilterProxy::filterForColumn);
+            connect(proxy, &TrafficDataFilterProxy::dataChanged, this, &TrafficTree::handleDataChanged);
+            connect(proxy, &TrafficDataFilterProxy::layoutChanged, this, &TrafficTree::handleLayoutChanged);
         }
     }
-
-    QTreeView::setModel(model);
 }
 
 void TrafficTree::tapListenerEnabled(bool enable)
@@ -838,6 +848,28 @@ void TrafficTree::widenColumnToContents(int col)
     }
 }
 
+
+void TrafficTree::handleDataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight,
+#if QT_VERSION < QT_VERSION_CHECK(6, 0, 0)
+    const QVector<int>
+#else
+    const QList<int>
+#endif
+    )
+{
+    for (int col = topLeft.column(); col <= bottomRight.column(); ++col) {
+        widenColumnToContents(col);
+    }
+}
+
+void TrafficTree::handleLayoutChanged(const QList<QPersistentModelIndex>, QAbstractItemModel::LayoutChangeHint)
+{
+    for (int col = 0; col < model()->columnCount(); ++col) {
+        widenColumnToContents(col);
+    }
+    scrollTo(currentIndex());
+}
+
 void TrafficTree::toggleSaveRawAction()
 {
     if (_exportRole == ATapDataModel::UNFORMATTED_DISPLAYDATA)

diff --git a/ui/qt/widgets/traffic_tree.h b/ui/qt/widgets/traffic_tree.h
@@ -180,6 +180,14 @@ private slots:
     void clipboardAction();
     void resizeAction();
     void toggleSaveRawAction();
+    void handleDataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight,
+#if QT_VERSION < QT_VERSION_CHECK(6, 0, 0)
+        const QVector<int>
+#else
+        const QList<int>
+#endif
+        );
+    void handleLayoutChanged(const QList<QPersistentModelIndex>, QAbstractItemModel::LayoutChangeHint);
 };
 
 #endif // TRAFFIC_TREE_H