From 8e4751a9632bc675aa9779e233f67614103d394b Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 12 Apr 2020 12:03:33 +0200 Subject: [PATCH] LibGUI: Add a way for models to update without invalidating indexes This is really just a workaround to keep SystemMonitor's process table working right wrt selection retention during resorts (while also doing full index invalidation on things like ProfileViewer inversion.) It's starting to feel like the model abstraction is not super great and we'll need a better approach if we want to actually build some more dynamic functionality into our views. --- Applications/SystemMonitor/ProcessModel.cpp | 2 +- DevTools/ProfileViewer/ProfileModel.cpp | 2 +- Libraries/LibGUI/AbstractTableView.cpp | 4 ++-- Libraries/LibGUI/AbstractTableView.h | 2 +- Libraries/LibGUI/AbstractView.cpp | 13 ++++++++----- Libraries/LibGUI/AbstractView.h | 2 +- Libraries/LibGUI/ColumnsView.cpp | 4 ++-- Libraries/LibGUI/ColumnsView.h | 2 +- Libraries/LibGUI/ItemView.cpp | 4 ++-- Libraries/LibGUI/ItemView.h | 2 +- Libraries/LibGUI/ListView.cpp | 4 ++-- Libraries/LibGUI/ListView.h | 2 +- Libraries/LibGUI/Model.cpp | 6 +++--- Libraries/LibGUI/Model.h | 7 ++++++- Libraries/LibGUI/SortingProxyModel.cpp | 4 ++-- Libraries/LibGUI/TreeView.cpp | 4 ++-- Libraries/LibGUI/TreeView.h | 2 +- 17 files changed, 37 insertions(+), 29 deletions(-) diff --git a/Applications/SystemMonitor/ProcessModel.cpp b/Applications/SystemMonitor/ProcessModel.cpp index 9076e82861b..a2796890db3 100644 --- a/Applications/SystemMonitor/ProcessModel.cpp +++ b/Applications/SystemMonitor/ProcessModel.cpp @@ -412,5 +412,5 @@ void ProcessModel::update() if (on_new_cpu_data_point) on_new_cpu_data_point(total_cpu_percent); - did_update(); + did_update(GUI::Model::UpdateFlag::DontInvalidateIndexes); } diff --git a/DevTools/ProfileViewer/ProfileModel.cpp b/DevTools/ProfileViewer/ProfileModel.cpp index 5c7dd56112a..fa77ac2a73b 100644 --- a/DevTools/ProfileViewer/ProfileModel.cpp +++ b/DevTools/ProfileViewer/ProfileModel.cpp @@ -146,5 +146,5 @@ GUI::Variant ProfileModel::data(const GUI::ModelIndex& index, Role role) const void ProfileModel::update() { - did_update(); + did_update(Model::InvalidateAllIndexes); } diff --git a/Libraries/LibGUI/AbstractTableView.cpp b/Libraries/LibGUI/AbstractTableView.cpp index 75773775d71..af35484c859 100644 --- a/Libraries/LibGUI/AbstractTableView.cpp +++ b/Libraries/LibGUI/AbstractTableView.cpp @@ -566,9 +566,9 @@ Gfx::Point AbstractTableView::adjusted_position(const Gfx::Point& position) cons return position.translated(horizontal_scrollbar().value() - frame_thickness(), vertical_scrollbar().value() - frame_thickness()); } -void AbstractTableView::did_update_model() +void AbstractTableView::did_update_model(unsigned flags) { - AbstractView::did_update_model(); + AbstractView::did_update_model(flags); update_column_sizes(); update_content_size(); update(); diff --git a/Libraries/LibGUI/AbstractTableView.h b/Libraries/LibGUI/AbstractTableView.h index 1638e6d03a2..75555406a35 100644 --- a/Libraries/LibGUI/AbstractTableView.h +++ b/Libraries/LibGUI/AbstractTableView.h @@ -75,7 +75,7 @@ protected: virtual ~AbstractTableView() override; AbstractTableView(); - virtual void did_update_model() override; + virtual void did_update_model(unsigned flags) override; virtual void mouseup_event(MouseEvent&) override; virtual void mousedown_event(MouseEvent&) override; virtual void mousemove_event(MouseEvent&) override; diff --git a/Libraries/LibGUI/AbstractView.cpp b/Libraries/LibGUI/AbstractView.cpp index 114aa4253e2..551b9751481 100644 --- a/Libraries/LibGUI/AbstractView.cpp +++ b/Libraries/LibGUI/AbstractView.cpp @@ -55,19 +55,22 @@ void AbstractView::set_model(RefPtr model) m_model = move(model); if (m_model) m_model->register_view({}, *this); - did_update_model(); + did_update_model(GUI::Model::InvalidateAllIndexes); } -void AbstractView::did_update_model() +void AbstractView::did_update_model(unsigned flags) { // FIXME: It's unfortunate that we lose so much view state when the model updates in any way. stop_editing(); m_edit_index = {}; m_hovered_index = {}; - if (model()) { - selection().remove_matching([this](auto& index) { return !model()->is_valid(index); }); - } else { + + dbg() << "did_update_model, flags=" << flags; + dump_backtrace(); + if (!model() || (flags & GUI::Model::InvalidateAllIndexes)) { selection().clear(); + } else { + selection().remove_matching([this](auto& index) { return !model()->is_valid(index); }); } } diff --git a/Libraries/LibGUI/AbstractView.h b/Libraries/LibGUI/AbstractView.h index b48d94f1fee..021da4a3163 100644 --- a/Libraries/LibGUI/AbstractView.h +++ b/Libraries/LibGUI/AbstractView.h @@ -49,7 +49,7 @@ public: void set_editable(bool editable) { m_editable = editable; } virtual bool accepts_focus() const override { return true; } - virtual void did_update_model(); + virtual void did_update_model(unsigned flags); virtual void did_update_selection(); virtual Gfx::Rect content_rect(const ModelIndex&) const { return {}; } diff --git a/Libraries/LibGUI/ColumnsView.cpp b/Libraries/LibGUI/ColumnsView.cpp index b4feee1c381..ae2d14a1fa0 100644 --- a/Libraries/LibGUI/ColumnsView.cpp +++ b/Libraries/LibGUI/ColumnsView.cpp @@ -259,9 +259,9 @@ void ColumnsView::mousedown_event(MouseEvent& event) } } -void ColumnsView::did_update_model() +void ColumnsView::did_update_model(unsigned flags) { - AbstractView::did_update_model(); + AbstractView::did_update_model(flags); // FIXME: Don't drop the columns on minor updates. dbg() << "Model was updated; dropping columns :("; diff --git a/Libraries/LibGUI/ColumnsView.h b/Libraries/LibGUI/ColumnsView.h index 82ff9529c80..a93ffd2b4b2 100644 --- a/Libraries/LibGUI/ColumnsView.h +++ b/Libraries/LibGUI/ColumnsView.h @@ -50,7 +50,7 @@ private: int icon_spacing() const { return 2; } int text_padding() const { return 2; } - virtual void did_update_model() override; + virtual void did_update_model(unsigned flags) override; virtual void paint_event(PaintEvent&) override; virtual void mousedown_event(MouseEvent& event) override; virtual void keydown_event(KeyEvent& event) override; diff --git a/Libraries/LibGUI/ItemView.cpp b/Libraries/LibGUI/ItemView.cpp index d11397d78f3..cc385f8a813 100644 --- a/Libraries/LibGUI/ItemView.cpp +++ b/Libraries/LibGUI/ItemView.cpp @@ -68,9 +68,9 @@ void ItemView::resize_event(ResizeEvent& event) update_content_size(); } -void ItemView::did_update_model() +void ItemView::did_update_model(unsigned flags) { - AbstractView::did_update_model(); + AbstractView::did_update_model(flags); update_content_size(); update(); } diff --git a/Libraries/LibGUI/ItemView.h b/Libraries/LibGUI/ItemView.h index 2433b6513da..32cdb6540fb 100644 --- a/Libraries/LibGUI/ItemView.h +++ b/Libraries/LibGUI/ItemView.h @@ -52,7 +52,7 @@ public: private: ItemView(); - virtual void did_update_model() override; + virtual void did_update_model(unsigned flags) override; virtual void paint_event(PaintEvent&) override; virtual void second_paint_event(PaintEvent&) override; virtual void resize_event(ResizeEvent&) override; diff --git a/Libraries/LibGUI/ListView.cpp b/Libraries/LibGUI/ListView.cpp index 57c957f3d28..bbbbb552e09 100644 --- a/Libraries/LibGUI/ListView.cpp +++ b/Libraries/LibGUI/ListView.cpp @@ -75,9 +75,9 @@ void ListView::resize_event(ResizeEvent& event) AbstractView::resize_event(event); } -void ListView::did_update_model() +void ListView::did_update_model(unsigned flags) { - AbstractView::did_update_model(); + AbstractView::did_update_model(flags); update_content_size(); update(); } diff --git a/Libraries/LibGUI/ListView.h b/Libraries/LibGUI/ListView.h index ba184023257..44abe208e22 100644 --- a/Libraries/LibGUI/ListView.h +++ b/Libraries/LibGUI/ListView.h @@ -56,7 +56,7 @@ public: private: ListView(); - virtual void did_update_model() override; + virtual void did_update_model(unsigned flags) override; virtual void paint_event(PaintEvent&) override; virtual void doubleclick_event(MouseEvent&) override; virtual void keydown_event(KeyEvent&) override; diff --git a/Libraries/LibGUI/Model.cpp b/Libraries/LibGUI/Model.cpp index ff1ae8f7306..2ebc6233094 100644 --- a/Libraries/LibGUI/Model.cpp +++ b/Libraries/LibGUI/Model.cpp @@ -53,12 +53,12 @@ void Model::for_each_view(Function callback) callback(*view); } -void Model::did_update() +void Model::did_update(unsigned flags) { if (on_update) on_update(); - for_each_view([](auto& view) { - view.did_update_model(); + for_each_view([&](auto& view) { + view.did_update_model(flags); }); } diff --git a/Libraries/LibGUI/Model.h b/Libraries/LibGUI/Model.h index bc074440c85..4c5121142e7 100644 --- a/Libraries/LibGUI/Model.h +++ b/Libraries/LibGUI/Model.h @@ -57,6 +57,11 @@ public: Sortable sortable { Sortable::True }; }; + enum UpdateFlag { + DontInvalidateIndexes = 0, + InvalidateAllIndexes = 1 << 0, + }; + enum class Role { Display, Sort, @@ -106,7 +111,7 @@ protected: Model(); void for_each_view(Function); - void did_update(); + void did_update(unsigned flags = UpdateFlag::InvalidateAllIndexes); ModelIndex create_index(int row, int column, const void* data = nullptr) const; diff --git a/Libraries/LibGUI/SortingProxyModel.cpp b/Libraries/LibGUI/SortingProxyModel.cpp index 82062dcca80..7a1d5fac81e 100644 --- a/Libraries/LibGUI/SortingProxyModel.cpp +++ b/Libraries/LibGUI/SortingProxyModel.cpp @@ -118,7 +118,7 @@ void SortingProxyModel::resort() for (int i = 0; i < row_count; ++i) m_row_mappings[i] = i; if (m_key_column == -1) { - did_update(); + did_update(Model::UpdateFlag::DontInvalidateIndexes); return; } quick_sort(m_row_mappings, [&](auto row1, auto row2) -> bool { @@ -133,7 +133,7 @@ void SortingProxyModel::resort() is_less_than = data1 < data2; return m_sort_order == SortOrder::Ascending ? is_less_than : !is_less_than; }); - did_update(); + did_update(Model::UpdateFlag::DontInvalidateIndexes); for_each_view([&](AbstractView& view) { auto& selection = view.selection(); Vector selected_indexes_in_target; diff --git a/Libraries/LibGUI/TreeView.cpp b/Libraries/LibGUI/TreeView.cpp index 93604b0ba61..369e2cf7806 100644 --- a/Libraries/LibGUI/TreeView.cpp +++ b/Libraries/LibGUI/TreeView.cpp @@ -326,10 +326,10 @@ void TreeView::scroll_into_view(const ModelIndex& a_index, Orientation orientati ScrollableWidget::scroll_into_view(found_rect, orientation); } -void TreeView::did_update_model() +void TreeView::did_update_model(unsigned flags) { m_view_metadata.clear(); - AbstractTableView::did_update_model(); + AbstractTableView::did_update_model(flags); } void TreeView::did_update_selection() diff --git a/Libraries/LibGUI/TreeView.h b/Libraries/LibGUI/TreeView.h index bc1d371dfc9..618bca040b8 100644 --- a/Libraries/LibGUI/TreeView.h +++ b/Libraries/LibGUI/TreeView.h @@ -48,7 +48,7 @@ protected: virtual void doubleclick_event(MouseEvent&) override; virtual void keydown_event(KeyEvent&) override; virtual void did_update_selection() override; - virtual void did_update_model() override; + virtual void did_update_model(unsigned flags) override; private: virtual ModelIndex index_at_event_position(const Gfx::Point&, bool& is_toggle) const override;