From cb748195626bdb8bbc30dee91ab40f60d9f83e38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20R=C4=83ceanu?= Date: Wed, 30 Jan 2019 20:48:46 +0200 Subject: [PATCH 01/15] Don't leak QApplication and command line arguments --- include/wx/qt/app.h | 6 ++++-- src/qt/app.cpp | 13 +++++-------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/include/wx/qt/app.h b/include/wx/qt/app.h index 8ce685eb62..68d8e701d1 100644 --- a/include/wx/qt/app.h +++ b/include/wx/qt/app.h @@ -9,6 +9,8 @@ #ifndef _WX_QT_APP_H_ #define _WX_QT_APP_H_ +#include + class QApplication; class WXDLLIMPEXP_CORE wxApp : public wxAppBase { @@ -19,9 +21,9 @@ public: virtual bool Initialize(int& argc, wxChar **argv); private: - QApplication *m_qtApplication; + QScopedPointer m_qtApplication; int m_qtArgc; - char **m_qtArgv; + QScopedArrayPointer m_qtArgv; wxDECLARE_DYNAMIC_CLASS_NO_COPY( wxApp ); }; diff --git a/src/qt/app.cpp b/src/qt/app.cpp index 9fcd4e7681..47aa92d2f2 100644 --- a/src/qt/app.cpp +++ b/src/qt/app.cpp @@ -19,19 +19,16 @@ wxIMPLEMENT_DYNAMIC_CLASS(wxApp, wxEvtHandler); wxApp::wxApp() { - m_qtApplication = NULL; m_qtArgc = 0; - m_qtArgv = NULL; } wxApp::~wxApp() { - // Only delete if the app was actually initialized - if ( m_qtApplication != NULL ) + // Delete command line arguments + for ( int i = 0; i < m_qtArgc; ++i ) { - m_qtApplication->deleteLater(); - delete [] m_qtArgv; + delete m_qtArgv[i]; } } @@ -49,7 +46,7 @@ bool wxApp::Initialize( int &argc, wxChar **argv ) // TODO: Check whether new/strdup etc. can be replaced with std::vector<>. // Clone and store arguments - m_qtArgv = new char *[argc + 1]; + m_qtArgv.reset(new char* [argc + 1]); for ( int i = 0; i < argc; i++ ) { m_qtArgv[i] = wxStrdupA(wxConvUTF8.cWX2MB(argv[i])); @@ -57,7 +54,7 @@ bool wxApp::Initialize( int &argc, wxChar **argv ) m_qtArgv[argc] = NULL; m_qtArgc = argc; - m_qtApplication = new QApplication( m_qtArgc, m_qtArgv ); + m_qtApplication.reset(new QApplication(m_qtArgc, m_qtArgv.data())); // Use the args returned by Qt as it may have deleted (processed) some of them // Using QApplication::arguments() forces argument processing From c6354696d4a65fa50fad872495059119584fa442 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20R=C4=83ceanu?= Date: Mon, 25 Feb 2019 00:00:39 +0200 Subject: [PATCH 02/15] Do not leak the sort proxy model --- src/qt/choice.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qt/choice.cpp b/src/qt/choice.cpp index da605c67ba..ef7f4bfa48 100644 --- a/src/qt/choice.cpp +++ b/src/qt/choice.cpp @@ -20,6 +20,8 @@ namespace class LexicalSortProxyModel : public QSortFilterProxyModel { public: + LexicalSortProxyModel(QObject* owner) : QSortFilterProxyModel(owner) {} + bool lessThan( const QModelIndex &left, const QModelIndex &right ) const wxOVERRIDE { const QVariant leftData = sourceModel()->data( left ); @@ -77,7 +79,7 @@ wxChoice::wxChoice() : void wxChoice::QtInitSort( QComboBox *combo ) { - QSortFilterProxyModel *proxyModel = new LexicalSortProxyModel(); + QSortFilterProxyModel *proxyModel = new LexicalSortProxyModel(combo); proxyModel->setSourceModel(combo->model()); combo->model()->setParent(proxyModel); combo->setModel(proxyModel); From 595a9945b55688421896a3bc24ff6b76cdb8f872 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20R=C4=83ceanu?= Date: Tue, 26 Feb 2019 23:13:12 +0200 Subject: [PATCH 03/15] Do not leak the list of panes --- include/wx/qt/statusbar.h | 4 +++- src/qt/statusbar.cpp | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/include/wx/qt/statusbar.h b/include/wx/qt/statusbar.h index 7e89426d25..7d289327fc 100644 --- a/include/wx/qt/statusbar.h +++ b/include/wx/qt/statusbar.h @@ -10,6 +10,8 @@ #include "wx/statusbr.h" +#include + class QLabel; class QStatusBar; @@ -45,7 +47,7 @@ private: void UpdateFields(); QStatusBar *m_qtStatusBar; - QList< QLabel* > *m_qtPanes; + QScopedPointer< QList > m_qtPanes; // should this really be a pointer? wxDECLARE_DYNAMIC_CLASS(wxStatusBar); }; diff --git a/src/qt/statusbar.cpp b/src/qt/statusbar.cpp index cbf8664d89..fa30dbb23a 100644 --- a/src/qt/statusbar.cpp +++ b/src/qt/statusbar.cpp @@ -48,7 +48,7 @@ bool wxStatusBar::Create(wxWindow *parent, wxWindowID WXUNUSED(winid), long style, const wxString& WXUNUSED(name)) { m_qtStatusBar = new wxQtStatusBar( parent, this ); - m_qtPanes = new QList < QLabel* >; + m_qtPanes.reset(new QList()); if ( style & wxSTB_SIZEGRIP ) m_qtStatusBar->setSizeGripEnabled(true); @@ -106,7 +106,6 @@ void wxStatusBar::Refresh( bool eraseBackground, const wxRect *rect ) void wxStatusBar::Init() { m_qtStatusBar = NULL; - m_qtPanes = NULL; } void wxStatusBar::UpdateFields() From b91d2a93ee48b4c3396c1b5d8b0abc57e76836e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20R=C4=83ceanu?= Date: Tue, 26 Feb 2019 23:21:13 +0200 Subject: [PATCH 04/15] Set sizer to the main frame and avoid leaking it --- samples/minimal/minimal.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/samples/minimal/minimal.cpp b/samples/minimal/minimal.cpp index f765a70dac..0d91f7fc75 100644 --- a/samples/minimal/minimal.cpp +++ b/samples/minimal/minimal.cpp @@ -170,6 +170,7 @@ MyFrame::MyFrame(const wxString& title) wxButton* aboutBtn = new wxButton(this, wxID_ANY, "About..."); aboutBtn->Bind(wxEVT_BUTTON, &MyFrame::OnAbout, this); sizer->Add(aboutBtn, wxSizerFlags().Center()); + SetSizer(sizer); #endif // wxUSE_MENUS/!wxUSE_MENUS #if wxUSE_STATUSBAR From d34017f656813adcf36f5ffc9392df1c83fdd329 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20R=C4=83ceanu?= Date: Tue, 26 Feb 2019 23:31:54 +0200 Subject: [PATCH 05/15] Initialize member variable --- src/qt/dc.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qt/dc.cpp b/src/qt/dc.cpp index dfde8af7d1..f3989ab32e 100644 --- a/src/qt/dc.cpp +++ b/src/qt/dc.cpp @@ -49,6 +49,7 @@ wxQtDCImpl::wxQtDCImpl( wxDC *owner ) : wxDCImpl( owner ) { m_qtPixmap = NULL; + m_qtPainter = NULL; m_rasterColourOp = wxQtNONE; m_qtPenColor = new QColor; m_qtBrushColor = new QColor; From e6e6b1ea38bff089c292b014d3a175b7d7b8c58c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20R=C4=83ceanu?= Date: Tue, 26 Feb 2019 23:32:38 +0200 Subject: [PATCH 06/15] Do not initialize member variables of base classes --- src/qt/dcclient.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/qt/dcclient.cpp b/src/qt/dcclient.cpp index 1a8efb2ab2..6d49661d2b 100644 --- a/src/qt/dcclient.cpp +++ b/src/qt/dcclient.cpp @@ -90,8 +90,6 @@ wxIMPLEMENT_CLASS(wxClientDCImpl,wxWindowDCImpl); wxClientDCImpl::wxClientDCImpl( wxDC *owner ) : wxWindowDCImpl( owner ) { - m_window = NULL; - m_qtPainter = NULL; } wxClientDCImpl::wxClientDCImpl( wxDC *owner, wxWindow *win ) From dc4854f916461d68ea403cdbda507993acb69c64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20R=C4=83ceanu?= Date: Tue, 26 Feb 2019 23:36:14 +0200 Subject: [PATCH 07/15] Do not reallocate base class member, which also led to a leak --- src/qt/dcclient.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/qt/dcclient.cpp b/src/qt/dcclient.cpp index 6d49661d2b..6ce6a4ff2e 100644 --- a/src/qt/dcclient.cpp +++ b/src/qt/dcclient.cpp @@ -97,8 +97,6 @@ wxClientDCImpl::wxClientDCImpl( wxDC *owner, wxWindow *win ) { m_window = win; - m_qtPainter = new QPainter(); - m_pict.reset(new QPicture()); m_ok = m_qtPainter->begin( m_pict.get() ); From 315a9460c00c60628e9803502c1641a9de35dbac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20R=C4=83ceanu?= Date: Wed, 27 Feb 2019 01:12:03 +0200 Subject: [PATCH 08/15] Remove memory leaks by using smart pointers or explicit delete --- include/wx/qt/window.h | 16 +++++++------ src/qt/window.cpp | 51 +++++++++++++++--------------------------- 2 files changed, 27 insertions(+), 40 deletions(-) diff --git a/include/wx/qt/window.h b/include/wx/qt/window.h index e9d14424bf..7951ed5436 100644 --- a/include/wx/qt/window.h +++ b/include/wx/qt/window.h @@ -9,6 +9,8 @@ #ifndef _WX_QT_WINDOW_H_ #define _WX_QT_WINDOW_H_ +#include + class QShortcut; template < class T > class QList; @@ -216,24 +218,24 @@ protected: private: void Init(); - QScrollArea *m_qtContainer; + QScrollArea *m_qtContainer; // either NULL or the same as m_qtWindow pointer - QScrollBar *m_horzScrollBar; - QScrollBar *m_vertScrollBar; + QScrollBar *m_horzScrollBar; // owned by m_qtWindow when allocated + QScrollBar *m_vertScrollBar; // owned by m_qtWindow when allocated QScrollBar *QtGetScrollBar( int orientation ) const; QScrollBar *QtSetScrollBar( int orientation, QScrollBar *scrollBar=NULL ); bool QtSetBackgroundStyle(); - QPicture *m_qtPicture; - QPainter *m_qtPainter; + QPicture *m_qtPicture; // not owned + QScopedPointer m_qtPainter; // always allocated bool m_mouseInside; #if wxUSE_ACCEL - QList< QShortcut* > *m_qtShortcuts; - wxQtShortcutHandler *m_qtShortcutHandler; + QScopedPointer< QList > m_qtShortcuts; // always allocated + QScopedPointer m_qtShortcutHandler; // always allocated bool m_processingShortcut; #endif // wxUSE_ACCEL diff --git a/src/qt/window.cpp b/src/qt/window.cpp index 2e8094f1c2..07bff82592 100644 --- a/src/qt/window.cpp +++ b/src/qt/window.cpp @@ -211,14 +211,13 @@ void wxWindowQt::Init() m_vertScrollBar = NULL; m_qtPicture = NULL; - m_qtPainter = new QPainter(); + m_qtPainter.reset(new QPainter()); m_mouseInside = false; #if wxUSE_ACCEL - m_qtShortcutHandler = new wxQtShortcutHandler( this ); + m_qtShortcutHandler.reset(new wxQtShortcutHandler(this)); m_processingShortcut = false; - m_qtShortcuts = NULL; #endif m_qtWindow = NULL; m_qtContainer = NULL; @@ -251,11 +250,14 @@ wxWindowQt::~wxWindowQt() DestroyChildren(); // This also destroys scrollbars - delete m_qtPainter; - #if wxUSE_ACCEL - m_qtShortcutHandler->deleteLater(); - delete m_qtShortcuts; + if ( m_qtShortcuts ) + { + for ( int i = 0; i < m_qtShortcuts->size(); ++i ) + { + delete m_qtShortcuts->at(i); + } + } #endif #if wxUSE_DRAG_AND_DROP @@ -266,23 +268,8 @@ wxWindowQt::~wxWindowQt() if (m_qtWindow) { wxLogTrace(TRACE_QT_WINDOW, wxT("wxWindow::~wxWindow %s m_qtWindow=%p"), GetName(), m_qtWindow); - // Avoid sending further signals (i.e. if deleting the current page) - m_qtWindow->blockSignals(true); - // Reset the pointer to avoid handling pending event and signals - QtStoreWindowPointer( GetHandle(), NULL ); - if ( m_horzScrollBar ) - { - QtStoreWindowPointer( m_horzScrollBar, NULL ); - m_horzScrollBar->deleteLater(); - } - if ( m_vertScrollBar ) - { - QtStoreWindowPointer( m_vertScrollBar, NULL ); - m_vertScrollBar->deleteLater(); - } - // Delete QWidget when control return to event loop (safer) - m_qtWindow->deleteLater(); - m_qtWindow = NULL; + + delete m_qtWindow; } else { @@ -622,6 +609,8 @@ QScrollBar *wxWindowQt::QtSetScrollBar( int orientation, QScrollBar *scrollBar ) void wxWindowQt::SetScrollbar( int orientation, int pos, int thumbvisible, int range, bool WXUNUSED(refresh) ) { + wxCHECK_RET(GetHandle(), "Window has not been created"); + //If not exist, create the scrollbar QScrollBar *scrollBar = QtGetScrollBar( orientation ); if ( scrollBar == NULL ) @@ -1013,19 +1002,15 @@ void wxWindowQt::SetAcceleratorTable( const wxAcceleratorTable& accel ) // Disable previously set accelerators while ( !m_qtShortcuts->isEmpty() ) delete m_qtShortcuts->takeFirst(); - - // Create new shortcuts (use GetHandle() so all events inside - // the window are handled, not only in the container subwindow) - delete m_qtShortcuts; } - m_qtShortcuts = accel.ConvertShortcutTable( GetHandle() ); + m_qtShortcuts.reset(accel.ConvertShortcutTable(GetHandle())); // Connect shortcuts to window Q_FOREACH( QShortcut *s, *m_qtShortcuts ) { - QObject::connect( s, &QShortcut::activated, m_qtShortcutHandler, &wxQtShortcutHandler::activated ); - QObject::connect( s, &QShortcut::activatedAmbiguously, m_qtShortcutHandler, &wxQtShortcutHandler::activated ); + QObject::connect( s, &QShortcut::activated, m_qtShortcutHandler.get(), &wxQtShortcutHandler::activated ); + QObject::connect( s, &QShortcut::activatedAmbiguously, m_qtShortcutHandler.get(), &wxQtShortcutHandler::activated ); } } #endif // wxUSE_ACCEL @@ -1191,7 +1176,7 @@ bool wxWindowQt::QtHandlePaintEvent ( QWidget *handler, QPaintEvent *event ) else { // Data from wxClientDC, paint it - m_qtPicture->play( m_qtPainter ); + m_qtPicture->play( m_qtPainter.get() ); // Reset picture m_qtPicture->setData( NULL, 0 ); handled = true; @@ -1572,5 +1557,5 @@ void wxWindowQt::QtSetPicture( QPicture* pict ) QPainter *wxWindowQt::QtGetPainter() { - return m_qtPainter; + return m_qtPainter.get(); } From 82523b3d3baa635bf48f2b6da3fba5dd9e4d3a59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20R=C4=83ceanu?= Date: Wed, 27 Feb 2019 01:39:14 +0200 Subject: [PATCH 09/15] Do not initialize base class variable and do not leak old allocation --- src/qt/window.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qt/window.cpp b/src/qt/window.cpp index 07bff82592..81ee557427 100644 --- a/src/qt/window.cpp +++ b/src/qt/window.cpp @@ -221,8 +221,6 @@ void wxWindowQt::Init() #endif m_qtWindow = NULL; m_qtContainer = NULL; - - m_dropTarget = NULL; } wxWindowQt::wxWindowQt() @@ -695,6 +693,7 @@ void wxWindowQt::SetDropTarget( wxDropTarget *dropTarget ) if ( m_dropTarget != NULL ) { m_dropTarget->Disconnect(); + delete m_dropTarget; } m_dropTarget = dropTarget; From c00187eaeb7ae96102da5bc930caa398faa95eb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20R=C4=83ceanu?= Date: Wed, 27 Feb 2019 02:07:06 +0200 Subject: [PATCH 10/15] Do not leak menus and menu items --- src/qt/menu.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/qt/menu.cpp b/src/qt/menu.cpp index 0619cf4c48..61d9d5935f 100644 --- a/src/qt/menu.cpp +++ b/src/qt/menu.cpp @@ -88,6 +88,12 @@ static void InsertMenuItemAction( const wxMenu *menu, const wxMenuItem *previous break; case wxITEM_NORMAL: { + // If the inserted action is a submenu, set the owner for the submenu. + if ( item->IsSubMenu() ) + { + item->GetSubMenu()->GetHandle()->setParent(qtMenu, Qt::Popup); + } + wxWindowID id = item->GetId(); if ( wxIsStockID( id ) ) { @@ -113,6 +119,9 @@ static void InsertMenuItemAction( const wxMenu *menu, const wxMenuItem *previous // Insert the action into the actual menu: QAction *successiveItemAction = ( successiveItem != NULL ) ? successiveItem->GetHandle() : NULL; qtMenu->insertAction( successiveItemAction, itemAction ); + // Menu items in Qt can be part of multiple menus, so a menu will not take ownership + // when one is added to it. Take it explicitly, otherwise it will create a memory leak. + itemAction->setParent(qtMenu); } wxMenuItem *wxMenu::DoAppend(wxMenuItem *item) @@ -211,6 +220,9 @@ bool wxMenuBar::Append( wxMenu *menu, const wxString& title ) QMenu *qtMenu = SetTitle( menu, title ); m_qtMenuBar->addMenu( qtMenu ); + // Menus in Qt can be reused as popups, so a menu bar will not take ownership when + // one is added to it. Take it explicitly, otherwise there will be a memory leak. + qtMenu->setParent(m_qtMenuBar, Qt::Popup); // must specify window type for correct display! return true; } @@ -233,6 +245,7 @@ bool wxMenuBar::Insert(size_t pos, wxMenu *menu, const wxString& title) QMenu *qtMenu = SetTitle( menu, title ); QAction *qtAction = GetActionAt( m_qtMenuBar, pos ); m_qtMenuBar->insertMenu( qtAction, qtMenu ); + qtMenu->setParent(m_qtMenuBar, Qt::Popup); // must specify window type for correct display! return true; } From 113822d024795140572c64b8cd6afae695c53d86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20R=C4=83ceanu?= Date: Wed, 27 Feb 2019 16:58:35 +0200 Subject: [PATCH 11/15] Change member variable type --- include/wx/qt/statusbar.h | 4 +--- src/qt/statusbar.cpp | 17 +++++++++-------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/include/wx/qt/statusbar.h b/include/wx/qt/statusbar.h index 7d289327fc..df9ae9891d 100644 --- a/include/wx/qt/statusbar.h +++ b/include/wx/qt/statusbar.h @@ -10,8 +10,6 @@ #include "wx/statusbr.h" -#include - class QLabel; class QStatusBar; @@ -47,7 +45,7 @@ private: void UpdateFields(); QStatusBar *m_qtStatusBar; - QScopedPointer< QList > m_qtPanes; // should this really be a pointer? + wxVector m_qtPanes; wxDECLARE_DYNAMIC_CLASS(wxStatusBar); }; diff --git a/src/qt/statusbar.cpp b/src/qt/statusbar.cpp index fa30dbb23a..729fa66535 100644 --- a/src/qt/statusbar.cpp +++ b/src/qt/statusbar.cpp @@ -48,7 +48,6 @@ bool wxStatusBar::Create(wxWindow *parent, wxWindowID WXUNUSED(winid), long style, const wxString& WXUNUSED(name)) { m_qtStatusBar = new wxQtStatusBar( parent, this ); - m_qtPanes.reset(new QList()); if ( style & wxSTB_SIZEGRIP ) m_qtStatusBar->setSizeGripEnabled(true); @@ -65,10 +64,10 @@ bool wxStatusBar::GetFieldRect(int i, wxRect& rect) const wxCHECK_MSG( (i >= 0) && ((size_t)i < m_panes.GetCount()), false, "invalid statusbar field index" ); - if ( static_cast(m_qtPanes->count()) != m_panes.GetCount() ) + if ( m_qtPanes.size() != m_panes.GetCount() ) const_cast(this)->UpdateFields(); - rect = wxQtConvertRect((*m_qtPanes)[i]->geometry()); + rect = wxQtConvertRect(m_qtPanes[i]->geometry()); return true; } @@ -89,10 +88,10 @@ int wxStatusBar::GetBorderY() const void wxStatusBar::DoUpdateStatusText(int number) { - if ( static_cast(m_qtPanes->count()) != m_panes.GetCount() ) + if ( m_qtPanes.size() != m_panes.GetCount() ) UpdateFields(); - (*m_qtPanes)[number]->setText( wxQtConvertString( m_panes[number].GetText() ) ); + m_qtPanes[number]->setText( wxQtConvertString( m_panes[number].GetText() ) ); } // Called each time number/size of panes changes @@ -112,11 +111,13 @@ void wxStatusBar::UpdateFields() { // is it a good idea to recreate all the panes every update? - while ( !m_qtPanes->isEmpty() ) + for ( wxVector::const_iterator it = m_qtPanes.begin(); + it != m_qtPanes.end(); ++it ) { //Remove all panes - delete m_qtPanes->takeLast(); + delete *it; } + m_qtPanes.clear(); for (size_t i = 0; i < m_panes.GetCount(); i++) { @@ -124,7 +125,7 @@ void wxStatusBar::UpdateFields() int width = m_panes[i].GetWidth(); QLabel *pane = new QLabel( m_qtStatusBar ); - m_qtPanes->append( pane ); + m_qtPanes.push_back(pane); if ( width >= 0 ) { From 1578240b6e49064927e71ded817515737c74f343 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20R=C4=83ceanu?= Date: Wed, 27 Feb 2019 17:42:01 +0200 Subject: [PATCH 12/15] Change a couple of data types and simplify code a bit --- include/wx/qt/accel.h | 2 +- include/wx/qt/window.h | 2 +- src/qt/accel.cpp | 11 ++++++----- src/qt/window.cpp | 29 +++++++++++------------------ 4 files changed, 19 insertions(+), 25 deletions(-) diff --git a/include/wx/qt/accel.h b/include/wx/qt/accel.h index d402f4941a..ee9734f6c7 100644 --- a/include/wx/qt/accel.h +++ b/include/wx/qt/accel.h @@ -43,7 +43,7 @@ public: wxAcceleratorTable(int n, const wxAcceleratorEntry entries[]); // Implementation - QList < QShortcut* > *ConvertShortcutTable( QWidget *parent ) const; + wxVector ConvertShortcutTable( QWidget *parent ) const; bool Ok() const { return IsOk(); } bool IsOk() const; diff --git a/include/wx/qt/window.h b/include/wx/qt/window.h index 7951ed5436..25290319c6 100644 --- a/include/wx/qt/window.h +++ b/include/wx/qt/window.h @@ -234,7 +234,7 @@ private: bool m_mouseInside; #if wxUSE_ACCEL - QScopedPointer< QList > m_qtShortcuts; // always allocated + wxVector m_qtShortcuts; // owned by whatever GetHandle() returns QScopedPointer m_qtShortcutHandler; // always allocated bool m_processingShortcut; #endif // wxUSE_ACCEL diff --git a/src/qt/accel.cpp b/src/qt/accel.cpp index 6e258f4db8..ea35fb28fc 100644 --- a/src/qt/accel.cpp +++ b/src/qt/accel.cpp @@ -82,16 +82,17 @@ wxAcceleratorTable::wxAcceleratorTable(int n, const wxAcceleratorEntry entries[] } } -QList< QShortcut* > *wxAcceleratorTable::ConvertShortcutTable( QWidget *parent ) const +wxVector wxAcceleratorTable::ConvertShortcutTable( QWidget *parent ) const { - QList< QShortcut* > *qtList = new QList< QShortcut* >; + wxVector shortcuts; - for ( wxAccelList::compatibility_iterator node = M_ACCELDATA->m_accels.GetFirst(); node; node = node->GetNext() ) + for ( wxAccelList::compatibility_iterator node = M_ACCELDATA->m_accels.GetFirst(); + node; node = node->GetNext() ) { - qtList->push_back(ConvertAccelerator( node->GetData(), parent )); + shortcuts.push_back(ConvertAccelerator(node->GetData(), parent)); } - return qtList; + return shortcuts; } wxObjectRefData *wxAcceleratorTable::CreateRefData() const diff --git a/src/qt/window.cpp b/src/qt/window.cpp index 81ee557427..bfc7672e0e 100644 --- a/src/qt/window.cpp +++ b/src/qt/window.cpp @@ -248,16 +248,6 @@ wxWindowQt::~wxWindowQt() DestroyChildren(); // This also destroys scrollbars -#if wxUSE_ACCEL - if ( m_qtShortcuts ) - { - for ( int i = 0; i < m_qtShortcuts->size(); ++i ) - { - delete m_qtShortcuts->at(i); - } - } -#endif - #if wxUSE_DRAG_AND_DROP SetDropTarget(NULL); #endif @@ -994,22 +984,25 @@ bool wxWindowQt::DoPopupMenu(wxMenu *menu, int x, int y) #if wxUSE_ACCEL void wxWindowQt::SetAcceleratorTable( const wxAcceleratorTable& accel ) { + wxCHECK_RET(GetHandle(), "Window has not been created"); + wxWindowBase::SetAcceleratorTable( accel ); - if ( m_qtShortcuts ) + // Disable previously set accelerators + for ( wxVector::const_iterator it = m_qtShortcuts.begin(); + it != m_qtShortcuts.end(); ++it ) { - // Disable previously set accelerators - while ( !m_qtShortcuts->isEmpty() ) - delete m_qtShortcuts->takeFirst(); + delete *it; } - m_qtShortcuts.reset(accel.ConvertShortcutTable(GetHandle())); + m_qtShortcuts = accel.ConvertShortcutTable(GetHandle()); // Connect shortcuts to window - Q_FOREACH( QShortcut *s, *m_qtShortcuts ) + for ( wxVector::const_iterator it = m_qtShortcuts.begin(); + it != m_qtShortcuts.end(); ++it ) { - QObject::connect( s, &QShortcut::activated, m_qtShortcutHandler.get(), &wxQtShortcutHandler::activated ); - QObject::connect( s, &QShortcut::activatedAmbiguously, m_qtShortcutHandler.get(), &wxQtShortcutHandler::activated ); + QObject::connect( *it, &QShortcut::activated, m_qtShortcutHandler.get(), &wxQtShortcutHandler::activated ); + QObject::connect( *it, &QShortcut::activatedAmbiguously, m_qtShortcutHandler.get(), &wxQtShortcutHandler::activated ); } } #endif // wxUSE_ACCEL From 66f6559a588c746e107317325266da8a4dc082cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20R=C4=83ceanu?= Date: Wed, 27 Feb 2019 17:46:02 +0200 Subject: [PATCH 13/15] Replace accessor with an older version --- src/qt/window.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qt/window.cpp b/src/qt/window.cpp index bfc7672e0e..70a0d29f56 100644 --- a/src/qt/window.cpp +++ b/src/qt/window.cpp @@ -1001,8 +1001,8 @@ void wxWindowQt::SetAcceleratorTable( const wxAcceleratorTable& accel ) for ( wxVector::const_iterator it = m_qtShortcuts.begin(); it != m_qtShortcuts.end(); ++it ) { - QObject::connect( *it, &QShortcut::activated, m_qtShortcutHandler.get(), &wxQtShortcutHandler::activated ); - QObject::connect( *it, &QShortcut::activatedAmbiguously, m_qtShortcutHandler.get(), &wxQtShortcutHandler::activated ); + QObject::connect( *it, &QShortcut::activated, m_qtShortcutHandler.data(), &wxQtShortcutHandler::activated ); + QObject::connect( *it, &QShortcut::activatedAmbiguously, m_qtShortcutHandler.data(), &wxQtShortcutHandler::activated ); } } #endif // wxUSE_ACCEL @@ -1168,7 +1168,7 @@ bool wxWindowQt::QtHandlePaintEvent ( QWidget *handler, QPaintEvent *event ) else { // Data from wxClientDC, paint it - m_qtPicture->play( m_qtPainter.get() ); + m_qtPicture->play( m_qtPainter.data() ); // Reset picture m_qtPicture->setData( NULL, 0 ); handled = true; @@ -1549,5 +1549,5 @@ void wxWindowQt::QtSetPicture( QPicture* pict ) QPainter *wxWindowQt::QtGetPainter() { - return m_qtPainter.get(); + return m_qtPainter.data(); } From 934698d8ac86b505bb91c66a8668195075767f21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20R=C4=83ceanu?= Date: Wed, 27 Feb 2019 22:06:47 +0200 Subject: [PATCH 14/15] Change Qt smart pointers with wx ones to fix compilation --- include/wx/qt/app.h | 6 +++--- include/wx/qt/window.h | 6 ++---- src/qt/app.cpp | 2 +- src/qt/window.cpp | 8 ++++---- 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/include/wx/qt/app.h b/include/wx/qt/app.h index 68d8e701d1..2c7d51ff97 100644 --- a/include/wx/qt/app.h +++ b/include/wx/qt/app.h @@ -9,7 +9,7 @@ #ifndef _WX_QT_APP_H_ #define _WX_QT_APP_H_ -#include +#include class QApplication; class WXDLLIMPEXP_CORE wxApp : public wxAppBase @@ -21,9 +21,9 @@ public: virtual bool Initialize(int& argc, wxChar **argv); private: - QScopedPointer m_qtApplication; + wxScopedPtr m_qtApplication; int m_qtArgc; - QScopedArrayPointer m_qtArgv; + wxScopedArray m_qtArgv; wxDECLARE_DYNAMIC_CLASS_NO_COPY( wxApp ); }; diff --git a/include/wx/qt/window.h b/include/wx/qt/window.h index 25290319c6..681a28732e 100644 --- a/include/wx/qt/window.h +++ b/include/wx/qt/window.h @@ -9,8 +9,6 @@ #ifndef _WX_QT_WINDOW_H_ #define _WX_QT_WINDOW_H_ -#include - class QShortcut; template < class T > class QList; @@ -229,13 +227,13 @@ private: bool QtSetBackgroundStyle(); QPicture *m_qtPicture; // not owned - QScopedPointer m_qtPainter; // always allocated + wxScopedPtr m_qtPainter; // always allocated bool m_mouseInside; #if wxUSE_ACCEL wxVector m_qtShortcuts; // owned by whatever GetHandle() returns - QScopedPointer m_qtShortcutHandler; // always allocated + wxScopedPtr m_qtShortcutHandler; // always allocated bool m_processingShortcut; #endif // wxUSE_ACCEL diff --git a/src/qt/app.cpp b/src/qt/app.cpp index 47aa92d2f2..a1a4b67d72 100644 --- a/src/qt/app.cpp +++ b/src/qt/app.cpp @@ -54,7 +54,7 @@ bool wxApp::Initialize( int &argc, wxChar **argv ) m_qtArgv[argc] = NULL; m_qtArgc = argc; - m_qtApplication.reset(new QApplication(m_qtArgc, m_qtArgv.data())); + m_qtApplication.reset(new QApplication(m_qtArgc, m_qtArgv.get())); // Use the args returned by Qt as it may have deleted (processed) some of them // Using QApplication::arguments() forces argument processing diff --git a/src/qt/window.cpp b/src/qt/window.cpp index 70a0d29f56..bfc7672e0e 100644 --- a/src/qt/window.cpp +++ b/src/qt/window.cpp @@ -1001,8 +1001,8 @@ void wxWindowQt::SetAcceleratorTable( const wxAcceleratorTable& accel ) for ( wxVector::const_iterator it = m_qtShortcuts.begin(); it != m_qtShortcuts.end(); ++it ) { - QObject::connect( *it, &QShortcut::activated, m_qtShortcutHandler.data(), &wxQtShortcutHandler::activated ); - QObject::connect( *it, &QShortcut::activatedAmbiguously, m_qtShortcutHandler.data(), &wxQtShortcutHandler::activated ); + QObject::connect( *it, &QShortcut::activated, m_qtShortcutHandler.get(), &wxQtShortcutHandler::activated ); + QObject::connect( *it, &QShortcut::activatedAmbiguously, m_qtShortcutHandler.get(), &wxQtShortcutHandler::activated ); } } #endif // wxUSE_ACCEL @@ -1168,7 +1168,7 @@ bool wxWindowQt::QtHandlePaintEvent ( QWidget *handler, QPaintEvent *event ) else { // Data from wxClientDC, paint it - m_qtPicture->play( m_qtPainter.data() ); + m_qtPicture->play( m_qtPainter.get() ); // Reset picture m_qtPicture->setData( NULL, 0 ); handled = true; @@ -1549,5 +1549,5 @@ void wxWindowQt::QtSetPicture( QPicture* pict ) QPainter *wxWindowQt::QtGetPainter() { - return m_qtPainter.data(); + return m_qtPainter.get(); } From 6374aef924a72301c9631055ec1513a82e9456ed Mon Sep 17 00:00:00 2001 From: VZ Date: Wed, 27 Feb 2019 22:11:50 +0200 Subject: [PATCH 15/15] Use explicit constructor for better type restriction Co-Authored-By: catalinr --- src/qt/choice.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qt/choice.cpp b/src/qt/choice.cpp index ef7f4bfa48..9399820bf6 100644 --- a/src/qt/choice.cpp +++ b/src/qt/choice.cpp @@ -20,7 +20,7 @@ namespace class LexicalSortProxyModel : public QSortFilterProxyModel { public: - LexicalSortProxyModel(QObject* owner) : QSortFilterProxyModel(owner) {} + explicit LexicalSortProxyModel(QObject* owner) : QSortFilterProxyModel(owner) {} bool lessThan( const QModelIndex &left, const QModelIndex &right ) const wxOVERRIDE {