From 4f456c8a193022cc88abcd365419fff080419398 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 16 Jan 2018 13:04:54 +0100 Subject: [PATCH 1/2] Refactor "changed" notifiers in generic wxDataViewCtrl code No real changes, just add the new DoItemChanged() used from both ItemChanged() and ValueChanged() notifications to avoid repeating the same code in both functions. --- src/generic/datavgen.cpp | 66 ++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 37 deletions(-) diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 8a584ce69a..9f27cdcfc5 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -704,7 +704,10 @@ public: // notifications from wxDataViewModel bool ItemAdded( const wxDataViewItem &parent, const wxDataViewItem &item ); bool ItemDeleted( const wxDataViewItem &parent, const wxDataViewItem &item ); - bool ItemChanged( const wxDataViewItem &item ); + bool ItemChanged( const wxDataViewItem &item ) + { + return DoItemChanged(item, wxNOT_FOUND); + } bool ValueChanged( const wxDataViewItem &item, unsigned int model_column ); bool Cleared(); void Resort() @@ -891,6 +894,10 @@ private: void DrawCellBackground( wxDataViewRenderer* cell, wxDC& dc, const wxRect& rect ); + // Common part of {Item,Value}Changed(): if view_column is wxNOT_FOUND, + // assumes that all columns were modified, otherwise just this one. + bool DoItemChanged(const wxDataViewItem& item, int view_column); + private: wxDataViewCtrl *m_owner; int m_lineHeight; @@ -2926,17 +2933,34 @@ bool wxDataViewMainWindow::ItemDeleted(const wxDataViewItem& parent, return true; } -bool wxDataViewMainWindow::ItemChanged(const wxDataViewItem & item) +bool wxDataViewMainWindow::DoItemChanged(const wxDataViewItem & item, int view_column) { // Move this node to its new correct place after it was updated. + // + // In principle, we could skip the call to PutInSortOrder() if the modified + // column is not the sort column, but in real-world applications it's fully + // possible and likely that custom compare uses not only the selected model + // column but also falls back to other values for comparison. To ensure + // consistency it is better to treat a value change as if it was an item + // change. wxDataViewTreeNode* const node = FindNode(item); wxCHECK_MSG( node, false, "invalid item" ); node->PutInSortOrder(this); - GetOwner()->InvalidateColBestWidths(); + wxDataViewColumn* column; + if ( view_column == wxNOT_FOUND ) + { + column = NULL; + GetOwner()->InvalidateColBestWidths(); + } + else + { + column = m_owner->GetColumn(view_column); + GetOwner()->InvalidateColBestWidth(view_column); + } // Send event - wxDataViewEvent le(wxEVT_DATAVIEW_ITEM_VALUE_CHANGED, m_owner, item); + wxDataViewEvent le(wxEVT_DATAVIEW_ITEM_VALUE_CHANGED, m_owner, column, item); m_owner->ProcessWindowEvent(le); return true; @@ -2948,39 +2972,7 @@ bool wxDataViewMainWindow::ValueChanged( const wxDataViewItem & item, unsigned i if ( view_column == wxNOT_FOUND ) return false; - // NOTE: to be valid, we cannot use e.g. INT_MAX - 1 -/*#define MAX_VIRTUAL_WIDTH 100000 - - wxRect rect( 0, row*m_lineHeight, MAX_VIRTUAL_WIDTH, m_lineHeight ); - m_owner->CalcScrolledPosition( rect.x, rect.y, &rect.x, &rect.y ); - Refresh( true, &rect ); - - return true; -*/ - - // In principle, we could skip the call to PutInSortOrder() if the modified - // column is not the sort column, but in real-world applications it's fully - // possible and likely that custom compare uses not only the selected model - // column but also falls back to other values for comparison. To ensure - // consistency it is better to treat a value change as if it was an item - // change. - // - // Alternatively, one could require the user to use ItemChanged() rather - // than ValueChanged() when the changed value may affect sorting, and add a - // check for model_column vs sort column here. - - wxDataViewTreeNode* const node = FindNode(item); - wxCHECK_MSG( node, false, "invalid item" ); - node->PutInSortOrder(this); - - GetOwner()->InvalidateColBestWidth(view_column); - - // Send event - wxDataViewColumn* const column = m_owner->GetColumn(view_column); - wxDataViewEvent le(wxEVT_DATAVIEW_ITEM_VALUE_CHANGED, m_owner, column, item); - m_owner->ProcessWindowEvent(le); - - return true; + return DoItemChanged(item, view_column); } bool wxDataViewMainWindow::Cleared() From 77c7c80696a4566e10719e6b154b5d07717a3323 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 16 Jan 2018 13:08:52 +0100 Subject: [PATCH 2/2] Update modified items in generic wxDataViewCtrl immediately Recent optimizations avoiding resort on item change (see https://github.com/wxWidgets/wxWidgets/pull/642) have also optimized away refreshing the modified item, which was done implicitly, as a side effect of resorting, before, so the changes were not reflected on display immediately any longer. Fix this by simply refreshing the item explicitly and also add a way to test for the correct behaviour in the sample. --- samples/dataview/dataview.cpp | 18 ++++++++++++++++++ src/generic/datavgen.cpp | 3 +++ 2 files changed, 21 insertions(+) diff --git a/samples/dataview/dataview.cpp b/samples/dataview/dataview.cpp index b6fa7328b4..a53b1c7be3 100644 --- a/samples/dataview/dataview.cpp +++ b/samples/dataview/dataview.cpp @@ -102,6 +102,7 @@ private: void OnExpand(wxCommandEvent& event); void OnShowCurrent(wxCommandEvent& event); void OnSetNinthCurrent(wxCommandEvent& event); + void OnChangeNinthTitle(wxCommandEvent& event); void OnPrependList(wxCommandEvent& event); void OnDeleteList(wxCommandEvent& event); @@ -342,6 +343,7 @@ enum ID_EXPAND = 105, ID_SHOW_CURRENT, ID_SET_NINTH_CURRENT, + ID_CHANGE_NINTH_TITLE, ID_PREPEND_LIST = 200, ID_DELETE_LIST = 201, @@ -384,6 +386,7 @@ wxBEGIN_EVENT_TABLE(MyFrame, wxFrame) EVT_BUTTON( ID_EXPAND, MyFrame::OnExpand ) EVT_BUTTON( ID_SHOW_CURRENT, MyFrame::OnShowCurrent ) EVT_BUTTON( ID_SET_NINTH_CURRENT, MyFrame::OnSetNinthCurrent ) + EVT_BUTTON( ID_CHANGE_NINTH_TITLE, MyFrame::OnChangeNinthTitle ) EVT_BUTTON( ID_PREPEND_LIST, MyFrame::OnPrependList ) EVT_BUTTON( ID_DELETE_LIST, MyFrame::OnDeleteList ) @@ -512,6 +515,8 @@ MyFrame::MyFrame(wxFrame *frame, const wxString &title, int x, int y, int w, int "&Show current"), border); sizerCurrent->Add(new wxButton(firstPanel, ID_SET_NINTH_CURRENT, "Make &ninth symphony current"), border); + sizerCurrent->Add(new wxButton(firstPanel, ID_CHANGE_NINTH_TITLE, + "Change ninth &title"), border); wxSizer *firstPanelSz = new wxBoxSizer( wxVERTICAL ); m_ctrl[0]->SetMinSize(wxSize(-1, 200)); @@ -1137,6 +1142,19 @@ void MyFrame::OnSetNinthCurrent(wxCommandEvent& WXUNUSED(event)) m_ctrl[0]->SetCurrentItem(item); } +void MyFrame::OnChangeNinthTitle(wxCommandEvent& WXUNUSED(event)) +{ + wxDataViewItem item(m_music_model->GetNinthItem()); + if ( !item.IsOk() ) + { + wxLogError( "Cannot change the ninth symphony title: it was removed!" ); + return; + } + + m_music_model->SetValue("Symphony No. 9", item, 0); + m_music_model->ItemChanged(item); +} + void MyFrame::OnValueChanged( wxDataViewEvent &event ) { wxString title = m_music_model->GetTitle( event.GetItem() ); diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 9f27cdcfc5..75d37d1357 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -2959,6 +2959,9 @@ bool wxDataViewMainWindow::DoItemChanged(const wxDataViewItem & item, int view_c GetOwner()->InvalidateColBestWidth(view_column); } + // Update the displayed value(s). + RefreshRow(GetRowByItem(item)); + // Send event wxDataViewEvent le(wxEVT_DATAVIEW_ITEM_VALUE_CHANGED, m_owner, column, item); m_owner->ProcessWindowEvent(le);