From 8aae7ad9379289e770a72983d0c5151f1da763cc Mon Sep 17 00:00:00 2001 From: valid-ptr Date: Mon, 17 Jan 2022 22:37:07 +0300 Subject: [PATCH 1/3] Fatal exception fixed in DVC on macOS while wxDataViewModel::Cleared call + editing item. --- src/osx/cocoa/dataview.mm | 5 +++++ src/osx/dataview_osx.cpp | 9 ++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/osx/cocoa/dataview.mm b/src/osx/cocoa/dataview.mm index 1859b289d2..65fb498f6e 100644 --- a/src/osx/cocoa/dataview.mm +++ b/src/osx/cocoa/dataview.mm @@ -551,6 +551,11 @@ outlineView:(NSOutlineView*)outlineView { wxUnusedVar(outlineView); + // See the comment in outlineView:objectValueForTableColumn:byItem: below: + // this function can also be called in the same circumstances. + if ( implementation->GetDataViewCtrl()->IsDeleting() ) + return nil; + if ((item == currentParentItem) && (index < ((NSInteger) [self getChildCount]))) return [self getChild:index]; diff --git a/src/osx/dataview_osx.cpp b/src/osx/dataview_osx.cpp index 2a881bad4c..4b4214e439 100644 --- a/src/osx/dataview_osx.cpp +++ b/src/osx/dataview_osx.cpp @@ -251,7 +251,14 @@ bool wxOSXDataViewModelNotifier::ValueChanged(wxDataViewItem const& item, unsign bool wxOSXDataViewModelNotifier::Cleared() { - return m_DataViewCtrlPtr->GetDataViewPeer()->Reload(); + bool noFailureFlag; + + // NOTE: See comments in ItemDeleted method above + m_DataViewCtrlPtr->SetDeleting(true); + noFailureFlag = m_DataViewCtrlPtr->GetDataViewPeer()->Reload(); + m_DataViewCtrlPtr->SetDeleting(false); + + return noFailureFlag; } void wxOSXDataViewModelNotifier::Resort() From 0199a8ce49db28c3ebb609ff7da4b3f80f3774f0 Mon Sep 17 00:00:00 2001 From: valid-ptr Date: Tue, 18 Jan 2022 17:00:15 +0300 Subject: [PATCH 2/3] dataview sample: Added test for wxDataViewModel::Cleared functionality: Ctrl-W will clear MyMusicTreeModel. Ctrl-W can be pressed while editing some item (artist is editable) --- samples/dataview/dataview.cpp | 9 +++++++++ samples/dataview/mymodels.cpp | 15 +++++++++++++++ samples/dataview/mymodels.h | 1 + 3 files changed, 25 insertions(+) diff --git a/samples/dataview/dataview.cpp b/samples/dataview/dataview.cpp index 3e66aa0155..2666d01664 100644 --- a/samples/dataview/dataview.cpp +++ b/samples/dataview/dataview.cpp @@ -84,6 +84,7 @@ private: #endif // wxHAS_GENERIC_DATAVIEWCTRL void OnGetPageInfo(wxCommandEvent& event); void OnDisable(wxCommandEvent& event); + void OnClearMyMusicTreeModel(wxCommandEvent& event); void OnSetForegroundColour(wxCommandEvent& event); void OnIncIndent(wxCommandEvent& event); void OnDecIndent(wxCommandEvent& event); @@ -400,6 +401,7 @@ enum ID_CLEARLOG = wxID_HIGHEST+1, ID_GET_PAGE_INFO, ID_DISABLE, + ID_CLEAR_MODEL, ID_BACKGROUND_COLOUR, ID_FOREGROUND_COLOUR, ID_CUSTOM_HEADER_ATTR, @@ -481,6 +483,7 @@ wxBEGIN_EVENT_TABLE(MyFrame, wxFrame) EVT_MENU( ID_GET_PAGE_INFO, MyFrame::OnGetPageInfo ) EVT_MENU( ID_DISABLE, MyFrame::OnDisable ) + EVT_MENU( ID_CLEAR_MODEL, MyFrame::OnClearMyMusicTreeModel ) EVT_MENU( ID_FOREGROUND_COLOUR, MyFrame::OnSetForegroundColour ) EVT_MENU( ID_BACKGROUND_COLOUR, MyFrame::OnSetBackgroundColour ) EVT_MENU( ID_CUSTOM_HEADER_ATTR, MyFrame::OnCustomHeaderAttr ) @@ -599,6 +602,7 @@ MyFrame::MyFrame(wxFrame *frame, const wxString &title, int x, int y, int w, int file_menu->Append(ID_CLEARLOG, "&Clear log\tCtrl-L"); file_menu->Append(ID_GET_PAGE_INFO, "Show current &page info"); file_menu->AppendCheckItem(ID_DISABLE, "&Disable\tCtrl-D"); + file_menu->Append(ID_CLEAR_MODEL, "&Clear MyMusicTreeModel\tCtrl-W"); file_menu->Append(ID_FOREGROUND_COLOUR, "Set &foreground colour...\tCtrl-S"); file_menu->Append(ID_BACKGROUND_COLOUR, "Set &background colour...\tCtrl-B"); file_menu->AppendCheckItem(ID_CUSTOM_HEADER_ATTR, "C&ustom header attributes"); @@ -1146,6 +1150,11 @@ void MyFrame::OnDisable(wxCommandEvent& event) m_ctrl[m_notebook->GetSelection()]->Enable(!event.IsChecked()); } +void MyFrame::OnClearMyMusicTreeModel(wxCommandEvent& WXUNUSED(event)) +{ + m_music_model->Clear(); +} + void MyFrame::OnSetForegroundColour(wxCommandEvent& WXUNUSED(event)) { wxDataViewCtrl * const dvc = m_ctrl[m_notebook->GetSelection()]; diff --git a/samples/dataview/mymodels.cpp b/samples/dataview/mymodels.cpp index d6a4149459..cbfc403515 100644 --- a/samples/dataview/mymodels.cpp +++ b/samples/dataview/mymodels.cpp @@ -152,6 +152,21 @@ void MyMusicTreeModel::Delete( const wxDataViewItem &item ) // notify control ItemDeleted( parent, item ); } +void MyMusicTreeModel::Clear() +{ + m_pop = NULL; + m_classical = NULL; + m_ninth = NULL; + + while (!m_root->GetChildren().IsEmpty()) + { + MyMusicTreeModelNode* node = m_root->GetNthChild(0); + m_root->GetChildren().Remove(node); + delete node; + } + + Cleared(); +} int MyMusicTreeModel::Compare( const wxDataViewItem &item1, const wxDataViewItem &item2, unsigned int column, bool ascending ) const diff --git a/samples/dataview/mymodels.h b/samples/dataview/mymodels.h index 59733ac61c..d760f96424 100644 --- a/samples/dataview/mymodels.h +++ b/samples/dataview/mymodels.h @@ -136,6 +136,7 @@ public: void AddToClassical( const wxString &title, const wxString &artist, unsigned int year ); void Delete( const wxDataViewItem &item ); + void Clear(); wxDataViewItem GetNinthItem() const { From c701a47176fe24e8bf4d363fb84eae711f04aa63 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 4 Feb 2022 02:27:27 +0100 Subject: [PATCH 3/3] Simplify code using wxDataViewCtrl::m_Deleting in wxOSX Use a RAII-based helper class to reset this flag automatically on scope exit instead of doing it manually. This also ensures that we restore the original value rather than just resetting it which would be more correct in the (admittedly, very unlikely) case when any of the functions changing m_Deleting is called recursively. --- include/wx/osx/dataview.h | 8 ++--- src/osx/dataview_osx.cpp | 65 +++++++++++++++++++++++---------------- 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/include/wx/osx/dataview.h b/include/wx/osx/dataview.h index 1a9033354c..ede2c1b6cd 100644 --- a/include/wx/osx/dataview.h +++ b/include/wx/osx/dataview.h @@ -259,11 +259,6 @@ public: { m_CustomRendererPtr = NewCustomRendererPtr; } - // sets the flag indicating a deletion process: - void SetDeleting(bool deleting) - { - m_Deleting = deleting; - } void AdjustAutosizedColumns() const; @@ -305,6 +300,9 @@ private: // after the actual deletion of the item; then, native callback functions/delegates may try to update data of variables that are already deleted; // if this flag is set all native variable update requests will be ignored + // This class can set (and reset) m_Deleting. + friend class wxOSXDVCScopedDeleter; + void* m_cgContext; // pointer to core graphics context wxDataViewCustomRenderer* m_CustomRendererPtr; // pointer to a valid custom renderer while editing; this class does NOT own the pointer diff --git a/src/osx/dataview_osx.cpp b/src/osx/dataview_osx.cpp index 4b4214e439..1dbde62e06 100644 --- a/src/osx/dataview_osx.cpp +++ b/src/osx/dataview_osx.cpp @@ -49,6 +49,33 @@ wxString ConcatenateDataViewItemValues(wxDataViewCtrl const* dataViewCtrlPtr, wx return dataString; } +// ============================================================================ +// wxOSXDVCScopedDeleter +// ============================================================================ + +// Helper class which exists only to reset m_Deleting on scope exit. +class wxOSXDVCScopedDeleter +{ +public: + explicit wxOSXDVCScopedDeleter(wxDataViewCtrl* dvc) : + m_dvc(dvc), + m_valueOrig(m_dvc->m_Deleting) + { + m_dvc->m_Deleting = true; + } + + ~wxOSXDVCScopedDeleter() + { + m_dvc->m_Deleting = m_valueOrig; + } + +private: + wxDataViewCtrl* const m_dvc; + const bool m_valueOrig; + + wxDECLARE_NO_COPY_CLASS(wxOSXDVCScopedDeleter); +}; + // ============================================================================ // wxOSXDataViewModelNotifier // ============================================================================ @@ -192,42 +219,33 @@ bool wxOSXDataViewModelNotifier::ItemsChanged(wxDataViewItemArray const& items) bool wxOSXDataViewModelNotifier::ItemDeleted(wxDataViewItem const& parent, wxDataViewItem const& item) { - bool noFailureFlag; - - wxCHECK_MSG(item.IsOk(),false,"To be deleted item is invalid."); // when this method is called and currently an item is being edited this item may have already been deleted in the model (the passed item and the being edited item have // not to be identical because the being edited item might be below the passed item in the hierarchy); // to prevent the control trying to ask the model to update an already deleted item the control is informed that currently a deleting process // has been started and that variables can currently not be updated even when requested by the system: - m_DataViewCtrlPtr->SetDeleting(true); - noFailureFlag = m_DataViewCtrlPtr->GetDataViewPeer()->Remove(parent); - // enable automatic updating again: - m_DataViewCtrlPtr->SetDeleting(false); + wxOSXDVCScopedDeleter setDeleting(m_DataViewCtrlPtr); + + bool ok = m_DataViewCtrlPtr->GetDataViewPeer()->Remove(parent); AdjustAutosizedColumns(); - // done: - return noFailureFlag; + + return ok; } bool wxOSXDataViewModelNotifier::ItemsDeleted(wxDataViewItem const& parent, wxDataViewItemArray const& WXUNUSED(items)) { - bool noFailureFlag; - - // when this method is called and currently an item is being edited this item may have already been deleted in the model (the passed item and the being edited item have // not to be identical because the being edited item might be below the passed item in the hierarchy); // to prevent the control trying to ask the model to update an already deleted item the control is informed that currently a deleting process // has been started and that variables can currently not be updated even when requested by the system: - m_DataViewCtrlPtr->SetDeleting(true); + wxOSXDVCScopedDeleter setDeleting(m_DataViewCtrlPtr); // delete all specified items: - noFailureFlag = m_DataViewCtrlPtr->GetDataViewPeer()->Remove(parent); - // enable automatic updating again: - m_DataViewCtrlPtr->SetDeleting(false); + bool ok = m_DataViewCtrlPtr->GetDataViewPeer()->Remove(parent); AdjustAutosizedColumns(); - // done: - return noFailureFlag; + + return ok; } bool wxOSXDataViewModelNotifier::ValueChanged(wxDataViewItem const& item, unsigned int col) @@ -251,14 +269,9 @@ bool wxOSXDataViewModelNotifier::ValueChanged(wxDataViewItem const& item, unsign bool wxOSXDataViewModelNotifier::Cleared() { - bool noFailureFlag; - - // NOTE: See comments in ItemDeleted method above - m_DataViewCtrlPtr->SetDeleting(true); - noFailureFlag = m_DataViewCtrlPtr->GetDataViewPeer()->Reload(); - m_DataViewCtrlPtr->SetDeleting(false); - - return noFailureFlag; + // NOTE: See comments in ItemDeleted method above + wxOSXDVCScopedDeleter setDeleting(m_DataViewCtrlPtr); + return m_DataViewCtrlPtr->GetDataViewPeer()->Reload(); } void wxOSXDataViewModelNotifier::Resort()