cabana: fix message view performance issues after #27901 (#27908)

* fix message list issues

* override drawBranches, do nothing

* bypass QTreeView::datachanged

* remove data copy in binaryview::updateState

* rename to x_v2

tools/cabana/binaryview.cc

@@ -281,11 +281,19 @@ void BinaryViewModel::refresh() {
   updateState();
 }
 
+void BinaryViewModel::updateItem(int row, int col, const QString &val, const QColor &color) {
+  auto &item = items[row * column_count + col];
+  if (item.val != val || item.bg_color != color) {
+    item.val = val;
+    item.bg_color = color;
+    auto idx = index(row, col);
+    emit dataChanged(idx, idx, {Qt::DisplayRole});
+  }
+}
+
 void BinaryViewModel::updateState() {
-  auto prev_items = items;
   const auto &last_msg = can->lastMessage(msg_id);
   const auto &binary = last_msg.dat;
-
   // data size may changed.
   if (binary.size() > row_count) {
     beginInsertRows({}, row_count, binary.size() - 1);
@@ -294,29 +302,23 @@ void BinaryViewModel::updateState() {
     endInsertRows();
   }
 
-  double max_f = 255.0;
-  double factor = 0.25;
-  double scaler = max_f / log2(1.0 + factor);
+  const double max_f = 255.0;
+  const double factor = 0.25;
+  const double scaler = max_f / log2(1.0 + factor);
   for (int i = 0; i < binary.size(); ++i) {
     for (int j = 0; j < 8; ++j) {
       auto &item = items[i * column_count + j];
-      item.val = ((binary[i] >> (7 - j)) & 1) != 0 ? '1' : '0';
+      QString val = ((binary[i] >> (7 - j)) & 1) != 0 ? "1" : "0";
       // Bit update frequency based highlighting
       double offset = !item.sigs.empty() ? 50 : 0;
       auto n = last_msg.bit_change_counts[i][7 - j];
       double min_f = n == 0 ? offset : offset + 25;
       double alpha = std::clamp(offset + log2(1.0 + factor * (double)n / (double)last_msg.count) * scaler, min_f, max_f);
-      item.bg_color.setAlpha(alpha);
-    }
-    items[i * column_count + 8].val = toHex(binary[i]);
-    items[i * column_count + 8].bg_color = last_msg.colors[i];
-  }
-
-  for (int i = 0; i < items.size(); ++i) {
-    if (i >= prev_items.size() || prev_items[i].val != items[i].val || prev_items[i].bg_color != items[i].bg_color) {
-      auto idx = index(i / column_count, i % column_count);
-      emit dataChanged(idx, idx);
+      auto color = item.bg_color;
+      color.setAlpha(alpha);
+      updateItem(i, j, val, color);
     }
+    updateItem(i, 8, toHex(binary[i]), last_msg.colors[i]);
   }
 }
 

tools/cabana/binaryview.h

@@ -26,6 +26,7 @@ public:
   BinaryViewModel(QObject *parent) : QAbstractTableModel(parent) {}
   void refresh();
   void updateState();
+  void updateItem(int row, int col, const QString &val, const QColor &color);
   QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const override;
   QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const { return {}; }
   int rowCount(const QModelIndex &parent = QModelIndex()) const override { return row_count; }

tools/cabana/messageswidget.cc

@@ -25,14 +25,16 @@ MessagesWidget::MessagesWidget(QWidget *parent) : QWidget(parent) {
   view = new MessageView(this);
   model = new MessageListModel(this);
   auto delegate = new MessageBytesDelegate(view, settings.multiple_lines_bytes);
-  view->setItemDelegateForColumn(5, delegate);
+  view->setItemDelegate(delegate);
   view->setModel(model);
   view->setSortingEnabled(true);
   view->sortByColumn(0, Qt::AscendingOrder);
+  view->setAllColumnsShowFocus(true);
+  view->setEditTriggers(QAbstractItemView::NoEditTriggers);
   view->setItemsExpandable(false);
   view->setIndentation(0);
   view->setRootIsDecorated(false);
-  view->header()->setStretchLastSection(true);
+  view->header()->setSectionsMovable(false);
   main_layout->addWidget(view);
 
   // suppress
@@ -48,6 +50,7 @@ MessagesWidget::MessagesWidget(QWidget *parent) : QWidget(parent) {
   QObject::connect(multiple_lines_bytes, &QCheckBox::stateChanged, [=](int state) {
     settings.multiple_lines_bytes = (state == Qt::Checked);
     delegate->setMultipleLines(settings.multiple_lines_bytes);
+    view->setUniformRowHeights(!settings.multiple_lines_bytes);
     model->sortMessages();
   });
   QObject::connect(can, &AbstractStream::msgsReceived, model, &MessageListModel::msgsReceived);
@@ -148,7 +151,7 @@ QVariant MessageListModel::data(const QModelIndex &index, int role) const {
       }
     }
     return QVariant::fromValue(colors);
-  } else if (role == BytesRole) {
+  } else if (role == BytesRole && index.column() == 5) {
     return can_data.dat;
   }
   return {};
@@ -268,9 +271,9 @@ void MessageListModel::reset() {
 
 void MessageView::drawRow(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const {
   QTreeView::drawRow(painter, option, index);
-  painter->save();
   const int gridHint = style()->styleHint(QStyle::SH_Table_GridLineColor, &option, this);
   const QColor gridColor = QColor::fromRgba(static_cast<QRgb>(gridHint));
+  QPen old_pen = painter->pen();
   painter->setPen(gridColor);
   painter->drawLine(option.rect.left(), option.rect.bottom(), option.rect.right(), option.rect.bottom());
 
@@ -280,5 +283,12 @@ void MessageView::drawRow(QPainter *painter, const QStyleOptionViewItem &option,
     painter->translate(header()->sectionSize(i), 0);
     painter->drawLine(0, y, 0, y + option.rect.height());
   }
-  painter->restore();
+  painter->setPen(old_pen);
+  painter->resetTransform();
+}
+
+void MessageView::dataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight, const QVector<int> &roles) {
+   // Bypass the slow call to QTreeView::dataChanged.
+   // QTreeView::dataChanged will invalidate the height cache and that's what we don't need in MessageView.
+   QAbstractItemView::dataChanged(topLeft, bottomRight, roles);
 }

tools/cabana/messageswidget.h

@@ -40,6 +40,8 @@ class MessageView : public QTreeView {
 public:
   MessageView(QWidget *parent) : QTreeView(parent) {}
   void drawRow(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const override;
+  void drawBranches(QPainter *painter, const QRect &rect, const QModelIndex &index) const override {}
+  void dataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight, const QVector<int> &roles = QVector<int>()) override;
 };
 
 class MessagesWidget : public QWidget {

tools/cabana/settings.cc

@@ -30,7 +30,7 @@ void Settings::save() {
   s.setValue("geometry", geometry);
   s.setValue("video_splitter_state", video_splitter_state);
   s.setValue("recent_files", recent_files);
-  s.setValue("message_header_state", message_header_state);
+  s.setValue("message_header_state_v2", message_header_state);
   s.setValue("chart_series_type", chart_series_type);
   s.setValue("theme", theme);
   s.setValue("sparkline_range", sparkline_range);
@@ -52,7 +52,7 @@ void Settings::load() {
   geometry = s.value("geometry").toByteArray();
   video_splitter_state = s.value("video_splitter_state").toByteArray();
   recent_files = s.value("recent_files").toStringList();
-  message_header_state = s.value("message_header_state").toByteArray();
+  message_header_state = s.value("message_header_state_v2").toByteArray();
   chart_series_type = s.value("chart_series_type", 0).toInt();
   theme = s.value("theme", 0).toInt();
   sparkline_range = s.value("sparkline_range", 15).toInt();

tools/cabana/util.cc

@@ -53,13 +53,17 @@ void MessageBytesDelegate::setMultipleLines(bool v) {
 }
 
 QSize MessageBytesDelegate::sizeHint(const QStyleOptionViewItem &option, const QModelIndex &index) const {
-  int n = index.data(BytesRole).toByteArray().size();
-  if (n <= 0 || n > 64) return {};
+  int v_margin = QApplication::style()->pixelMetric(QStyle::PM_FocusFrameVMargin) + 1;
+  auto data = index.data(BytesRole);
+  if (!data.isValid()) {
+    return {1, byte_size.height() + 2 * v_margin};
+  }
+  int n = data.toByteArray().size();
+  assert(n > 0 && n <= 64);
 
   QSize size = size_cache[n - 1];
   if (size.isEmpty()) {
     int h_margin = QApplication::style()->pixelMetric(QStyle::PM_FocusFrameHMargin) + 1;
-    int v_margin = QApplication::style()->pixelMetric(QStyle::PM_FocusFrameVMargin) + 1;
     if (!multiple_lines) {
       size.setWidth(h_margin * 2 + n * byte_size.width());
       size.setHeight(byte_size.height() + 2 * v_margin);
@@ -73,18 +77,25 @@ QSize MessageBytesDelegate::sizeHint(const QStyleOptionViewItem &option, const Q
 }
 
 void MessageBytesDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const {
+  auto data = index.data(BytesRole);
+  if (!data.isValid()) {
+    return QStyledItemDelegate::paint(painter, option, index);
+  }
+
+  auto byte_list = data.toByteArray();
   auto colors = index.data(ColorsRole).value<QVector<QColor>>();
-  auto byte_list = index.data(BytesRole).toByteArray();
 
   int v_margin = option.widget->style()->pixelMetric(QStyle::PM_FocusFrameVMargin);
   int h_margin = option.widget->style()->pixelMetric(QStyle::PM_FocusFrameHMargin);
-  painter->save();
   if (option.state & QStyle::State_Selected) {
     painter->fillRect(option.rect, option.palette.highlight());
     painter->setPen(option.palette.color(QPalette::HighlightedText));
+  } else {
+    painter->setPen(option.palette.color(QPalette::Text));
   }
 
   const QPoint pt{option.rect.left() + h_margin, option.rect.top() + v_margin};
+  QFont old_font = painter->font();
   painter->setFont(fixed_font);
   for (int i = 0; i < byte_list.size(); ++i) {
     int row = !multiple_lines ? 0 : i / 8;
@@ -95,7 +106,7 @@ void MessageBytesDelegate::paint(QPainter *painter, const QStyleOptionViewItem &
     }
     painter->drawText(r, Qt::AlignCenter, toHex(byte_list[i]));
   }
-  painter->restore();
+  painter->setFont(old_font);
 }
 
 QColor getColor(const cabana::Signal *sig) {