From: Soeren Apel Date: Wed, 13 May 2020 15:17:50 +0000 (+0200) Subject: TabularDecView: Fix model issues and improve ViewModeVisible X-Git-Url: https://sigrok.org/gitweb/?p=pulseview.git;a=commitdiff_plain;h=6f43db70c63c683d546566eda0be7f182f614655 TabularDecView: Fix model issues and improve ViewModeVisible Instead of trying to be smart on the model side, we use a filtering proxy instead to filter non-visible rows. This works much better and the performance penalty seems to be very small. --- diff --git a/pv/views/tabular_decoder/model.cpp b/pv/views/tabular_decoder/model.cpp index 938e6add..3733e138 100644 --- a/pv/views/tabular_decoder/model.cpp +++ b/pv/views/tabular_decoder/model.cpp @@ -44,20 +44,24 @@ AnnotationCollectionModel::AnnotationCollectionModel(QObject* parent) : all_annotations_(nullptr), dataset_(nullptr), signal_(nullptr), + first_hidden_column_(0), prev_segment_(0), prev_last_row_(0), - start_index_(0), - end_index_(0), had_highlight_before_(false), hide_hidden_(false) { - // TBD Maybe use empty columns as indentation levels to indicate stacked decoders - header_data_.emplace_back(tr("Sample")); // Column #0 - header_data_.emplace_back(tr("Time")); // Column #1 - header_data_.emplace_back(tr("Decoder")); // Column #2 - header_data_.emplace_back(tr("Ann Row")); // Column #3 - header_data_.emplace_back(tr("Ann Class")); // Column #4 - header_data_.emplace_back(tr("Value")); // Column #5 + // Note: when adding entries, consider ViewVisibleFilterProxyModel::filterAcceptsRow() + + uint8_t i = 0; + header_data_.emplace_back(tr("Sample")); i++; // Column #0 + header_data_.emplace_back(tr("Time")); i++; // Column #1 + header_data_.emplace_back(tr("Decoder")); i++; // Column #2 + header_data_.emplace_back(tr("Ann Row")); i++; // Column #3 + header_data_.emplace_back(tr("Ann Class")); i++; // Column #4 + header_data_.emplace_back(tr("Value")); i++; // Column #5 + + first_hidden_column_ = i; + header_data_.emplace_back("End Sample"); // Column #6, hidden } int AnnotationCollectionModel::get_hierarchy_level(const Annotation* ann) const @@ -88,6 +92,7 @@ QVariant AnnotationCollectionModel::data_from_ann(const Annotation* ann, int ind case 3: return QVariant(ann->row()->description()); // Column #3, Ann Row case 4: return QVariant(ann->ann_class_description()); // Column #4, Ann Class case 5: return QVariant(ann->longest_annotation()); // Column #5, Value + case 6: return QVariant((qulonglong)ann->end_sample()); // Column #6, End Sample default: return QVariant(); } } @@ -151,6 +156,11 @@ Qt::ItemFlags AnnotationCollectionModel::flags(const QModelIndex& index) const return Qt::ItemIsEnabled | Qt::ItemIsSelectable | Qt::ItemNeverHasChildren; } +uint8_t AnnotationCollectionModel::first_hidden_column() const +{ + return first_hidden_column_; +} + QVariant AnnotationCollectionModel::headerData(int section, Qt::Orientation orientation, int role) const { @@ -171,13 +181,8 @@ QModelIndex AnnotationCollectionModel::index(int row, int column, QModelIndex idx; - if (start_index_ == end_index_) { - if ((size_t)row < dataset_->size()) - idx = createIndex(row, column, (void*)dataset_->at(row)); - } else { - if ((size_t)row < (end_index_ - start_index_)) - idx = createIndex(row, column, (void*)dataset_->at(start_index_ + row)); - } + if ((size_t)row < dataset_->size()) + idx = createIndex(row, column, (void*)dataset_->at(row)); return idx; } @@ -196,10 +201,7 @@ int AnnotationCollectionModel::rowCount(const QModelIndex& parent_idx) const if (!dataset_) return 0; - if (start_index_ == end_index_) - return dataset_->size(); - else - return (end_index_ - start_index_); + return dataset_->size(); } int AnnotationCollectionModel::columnCount(const QModelIndex& parent_idx) const @@ -240,14 +242,11 @@ void AnnotationCollectionModel::set_signal_and_segment(data::DecodeSignal* signa return; } - // Re-apply the requested sample range - set_sample_range(start_sample_, end_sample_); - const size_t new_row_count = dataset_->size() - 1; // Force the view associated with this model to update when the segment changes if (prev_segment_ != current_segment) { - dataChanged(QModelIndex(), QModelIndex()); + dataChanged(index(0, 0), index(new_row_count, 0)); layoutChanged(); } else { // Force the view associated with this model to update when we have more annotations @@ -261,62 +260,6 @@ void AnnotationCollectionModel::set_signal_and_segment(data::DecodeSignal* signa prev_last_row_ = new_row_count; } -void AnnotationCollectionModel::set_sample_range(uint64_t start_sample, uint64_t end_sample) -{ - // Check if there's even anything to reset - if ((start_sample == end_sample) && (start_index_ == end_index_)) - return; - - if (!dataset_ || dataset_->empty() || (end_sample == 0)) { - start_index_ = 0; - end_index_ = 0; - start_sample_ = 0; - end_sample_ = 0; - - dataChanged(QModelIndex(), QModelIndex()); - layoutChanged(); - return; - } - - start_sample_ = start_sample; - end_sample_ = end_sample; - - // Determine first and last indices into the annotation list - int64_t i = -1; - bool ann_outside_range; - do { - i++; - - if (i == (int64_t)dataset_->size()) { - start_index_ = 0; - end_index_ = 0; - - dataChanged(QModelIndex(), QModelIndex()); - layoutChanged(); - return; - } - const Annotation* ann = (*dataset_)[i]; - ann_outside_range = - ((ann->start_sample() < start_sample) && (ann->end_sample() < start_sample)); - } while (ann_outside_range); - start_index_ = i; - - // Ideally, we would be able to set end_index_ to the last annotation that - // is within range. However, as annotations in the list are sorted by - // start sample and hierarchy level, we may encounter this scenario: - // [long annotation that spans across view] - // [short annotations that aren't seen] - // [short annotations that are seen] - // ..in which our output would only show the first long annotations. - // For this reason, we simply show everything after the first visible - // annotation for now. - - end_index_ = dataset_->size(); - - dataChanged(index(0, 0), index((end_index_ - start_index_), 0)); - layoutChanged(); -} - void AnnotationCollectionModel::set_hide_hidden(bool hide_hidden) { hide_hidden_ = hide_hidden; @@ -329,11 +272,8 @@ void AnnotationCollectionModel::set_hide_hidden(bool hide_hidden) all_annotations_without_hidden_.clear(); // To conserve memory } - // Re-apply the requested sample range - set_sample_range(start_sample_, end_sample_); - if (dataset_) - dataChanged(index(0, 0), index(dataset_->size(), 0)); + dataChanged(index(0, 0), index(dataset_->size() - 1, 0)); else dataChanged(QModelIndex(), QModelIndex()); @@ -362,15 +302,16 @@ void AnnotationCollectionModel::update_annotations_without_hidden() all_annotations_without_hidden_.resize(count); } -void AnnotationCollectionModel::update_highlighted_rows(QModelIndex first, +QModelIndex AnnotationCollectionModel::update_highlighted_rows(QModelIndex first, QModelIndex last, int64_t sample_num) { bool has_highlight = false; + QModelIndex result; highlight_sample_num_ = sample_num; if (!dataset_ || dataset_->empty()) - return; + return result; if (sample_num >= 0) { last = last.sibling(last.row() + 1, 0); @@ -385,6 +326,7 @@ void AnnotationCollectionModel::update_highlighted_rows(QModelIndex first, if (((int64_t)ann->start_sample() <= sample_num) && ((int64_t)ann->end_sample() >= sample_num)) { + result = index; has_highlight = true; break; } @@ -393,10 +335,14 @@ void AnnotationCollectionModel::update_highlighted_rows(QModelIndex first, } while (index != last); } - if (has_highlight || had_highlight_before_) + if (has_highlight || had_highlight_before_) { dataChanged(first, last); + layoutChanged(); + } had_highlight_before_ = has_highlight; + + return result; } void AnnotationCollectionModel::on_annotation_visibility_changed() @@ -406,11 +352,8 @@ void AnnotationCollectionModel::on_annotation_visibility_changed() update_annotations_without_hidden(); - // Re-apply the requested sample range - set_sample_range(start_sample_, end_sample_); - if (dataset_) - dataChanged(index(0, 0), index(dataset_->size(), 0)); + dataChanged(index(0, 0), index(dataset_->size() - 1, 0)); else dataChanged(QModelIndex(), QModelIndex()); diff --git a/pv/views/tabular_decoder/view.cpp b/pv/views/tabular_decoder/view.cpp index 2283d68c..1ed6f95b 100644 --- a/pv/views/tabular_decoder/view.cpp +++ b/pv/views/tabular_decoder/view.cpp @@ -63,7 +63,50 @@ const char* ViewModeNames[ViewModeCount] = { "Show visible in main view" }; -QSize QCustomTableView::minimumSizeHint() const + +CustomFilterProxyModel::CustomFilterProxyModel(QObject* parent) : + QSortFilterProxyModel(parent) +{ +} + +bool CustomFilterProxyModel::filterAcceptsRow(int sourceRow, + const QModelIndex &sourceParent) const +{ + (void)sourceParent; + assert(sourceModel() != nullptr); + + const QModelIndex ann_start_sample_idx = sourceModel()->index(sourceRow, 0); + const uint64_t ann_start_sample = + sourceModel()->data(ann_start_sample_idx, Qt::DisplayRole).toULongLong(); + + const QModelIndex ann_end_sample_idx = sourceModel()->index(sourceRow, 6); + const uint64_t ann_end_sample = + sourceModel()->data(ann_end_sample_idx, Qt::DisplayRole).toULongLong(); + + // We consider all annotations as visible that either + // a) begin to the left of the range and end within the range or + // b) begin and end within the range or + // c) begin within the range and end to the right of the range + // ...which is equivalent to the negation of "begins and ends outside the range" + + const bool left_of_range = (ann_end_sample < range_start_sample_); + const bool right_of_range = (ann_start_sample > range_end_sample_); + const bool entirely_outside_of_range = left_of_range || right_of_range; + + return !entirely_outside_of_range; +} + +void CustomFilterProxyModel::set_sample_range(uint64_t start_sample, + uint64_t end_sample) +{ + range_start_sample_ = start_sample; + range_end_sample_ = end_sample; + + invalidateFilter(); +} + + +QSize CustomTableView::minimumSizeHint() const { QSize size(QTableView::sizeHint()); @@ -77,7 +120,7 @@ QSize QCustomTableView::minimumSizeHint() const return size; } -QSize QCustomTableView::sizeHint() const +QSize CustomTableView::sizeHint() const { return minimumSizeHint(); } @@ -93,8 +136,9 @@ View::View(Session &session, bool is_main_view, QMainWindow *parent) : view_mode_selector_(new QComboBox()), save_button_(new QToolButton()), save_action_(new QAction(this)), - table_view_(new QCustomTableView()), - model_(new AnnotationCollectionModel()), + table_view_(new CustomTableView()), + model_(new AnnotationCollectionModel(this)), + filter_proxy_model_(new CustomFilterProxyModel(this)), signal_(nullptr) { QVBoxLayout *root_layout = new QVBoxLayout(this); @@ -153,13 +197,18 @@ View::View(Session &session, bool is_main_view, QMainWindow *parent) : save_button_->setDefaultAction(save_action_); save_button_->setPopupMode(QToolButton::MenuButtonPopup); - // Set up the table view - table_view_->setModel(model_); + // Set up the models and the table view + filter_proxy_model_->setSourceModel(model_); + table_view_->setModel(filter_proxy_model_); + table_view_->setSelectionBehavior(QAbstractItemView::SelectRows); table_view_->setSelectionMode(QAbstractItemView::ContiguousSelection); - table_view_->setSortingEnabled(false); + table_view_->setSortingEnabled(true); table_view_->sortByColumn(0, Qt::AscendingOrder); + for (uint8_t i = model_->first_hidden_column(); i < model_->columnCount(); i++) + table_view_->setColumnHidden(i, true); + const int font_height = QFontMetrics(QApplication::font()).height(); table_view_->verticalHeader()->setDefaultSectionSize((font_height * 5) / 4); table_view_->verticalHeader()->setVisible(false); @@ -417,15 +466,19 @@ void View::on_view_mode_changed(int index) int64_t start_sample = md_obj->value(MetadataValueStartSample).toLongLong(); int64_t end_sample = md_obj->value(MetadataValueEndSample).toLongLong(); - model_->set_sample_range(max((int64_t)0, start_sample), + filter_proxy_model_->set_sample_range(max((int64_t)0, start_sample), max((int64_t)0, end_sample)); + + // Force repaint, otherwise the new selection may not show immediately +// table_view_->viewport()->update(); } else { - // Reset the model's data range - model_->set_sample_range(0, 0); + // Use the data model directly + table_view_->setModel(model_); } if (index == ViewModeLatest) - table_view_->scrollTo(model_->index(model_->rowCount() - 1, 0), + table_view_->scrollTo( + filter_proxy_model_->mapFromSource(model_->index(model_->rowCount() - 1, 0)), QAbstractItemView::PositionAtBottom); } @@ -583,16 +636,26 @@ void View::on_metadata_object_changed(MetadataObject* obj, int64_t start_sample = obj->value(MetadataValueStartSample).toLongLong(); int64_t end_sample = obj->value(MetadataValueEndSample).toLongLong(); - model_->set_sample_range(max((int64_t)0, start_sample), + filter_proxy_model_->set_sample_range(max((int64_t)0, start_sample), max((int64_t)0, end_sample)); } if (obj->type() == MetadataObjMousePos) { - QModelIndex first_visual_idx = table_view_->indexAt(table_view_->rect().topLeft()); - QModelIndex last_visual_idx = table_view_->indexAt(table_view_->rect().bottomLeft()); - - model_->update_highlighted_rows(first_visual_idx, last_visual_idx, - obj->value(MetadataValueStartSample).toLongLong()); + QModelIndex first_visible_idx = + filter_proxy_model_->mapToSource(filter_proxy_model_->index(0, 0)); + QModelIndex last_visible_idx = + filter_proxy_model_->mapToSource(filter_proxy_model_->index(filter_proxy_model_->rowCount() - 1, 0)); + + if (first_visible_idx.isValid()) { + const QModelIndex first_highlighted_idx = + model_->update_highlighted_rows(first_visible_idx, last_visible_idx, + obj->value(MetadataValueStartSample).toLongLong()); + + if (view_mode_selector_->currentIndex() == ViewModeVisible) { + const QModelIndex idx = filter_proxy_model_->mapFromSource(first_highlighted_idx); + table_view_->scrollTo(idx, QAbstractItemView::EnsureVisible); + } + } } } diff --git a/pv/views/tabular_decoder/view.hpp b/pv/views/tabular_decoder/view.hpp index 891c227c..534e3d9d 100644 --- a/pv/views/tabular_decoder/view.hpp +++ b/pv/views/tabular_decoder/view.hpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -68,6 +69,7 @@ public: QVariant data(const QModelIndex& index, int role) const override; Qt::ItemFlags flags(const QModelIndex& index) const override; + uint8_t first_hidden_column() const; QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const override; QModelIndex index(int row, int column, @@ -79,11 +81,10 @@ public: int columnCount(const QModelIndex& parent_idx = QModelIndex()) const override; void set_signal_and_segment(data::DecodeSignal* signal, uint32_t current_segment); - void set_sample_range(uint64_t start_sample, uint64_t end_sample); void set_hide_hidden(bool hide_hidden); void update_annotations_without_hidden(); - void update_highlighted_rows(QModelIndex first, QModelIndex last, + QModelIndex update_highlighted_rows(QModelIndex first, QModelIndex last, int64_t sample_num); private Q_SLOTS: @@ -95,16 +96,33 @@ private: deque all_annotations_without_hidden_; const deque* dataset_; data::DecodeSignal* signal_; + uint8_t first_hidden_column_; uint32_t prev_segment_; uint64_t prev_last_row_; - uint64_t start_sample_, end_sample_, start_index_, end_index_; int64_t highlight_sample_num_; bool had_highlight_before_; bool hide_hidden_; }; -class QCustomTableView : public QTableView +class CustomFilterProxyModel : public QSortFilterProxyModel +{ + Q_OBJECT + +public: + CustomFilterProxyModel(QObject* parent = 0); + + void set_sample_range(uint64_t start_sample, uint64_t end_sample); + +protected: + bool filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const override; + +private: + uint64_t range_start_sample_, range_end_sample_; +}; + + +class CustomTableView : public QTableView { Q_OBJECT @@ -178,9 +196,9 @@ private: QToolButton* save_button_; QAction* save_action_; - QCustomTableView* table_view_; - + CustomTableView* table_view_; AnnotationCollectionModel* model_; + CustomFilterProxyModel* filter_proxy_model_; data::DecodeSignal* signal_; const data::decode::Decoder* decoder_;