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/app.h b/include/wx/qt/app.h index 8ce685eb62..2c7d51ff97 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; + wxScopedPtr m_qtApplication; int m_qtArgc; - char **m_qtArgv; + wxScopedArray m_qtArgv; wxDECLARE_DYNAMIC_CLASS_NO_COPY( wxApp ); }; diff --git a/include/wx/qt/statusbar.h b/include/wx/qt/statusbar.h index 7e89426d25..df9ae9891d 100644 --- a/include/wx/qt/statusbar.h +++ b/include/wx/qt/statusbar.h @@ -45,7 +45,7 @@ private: void UpdateFields(); QStatusBar *m_qtStatusBar; - QList< QLabel* > *m_qtPanes; + wxVector m_qtPanes; wxDECLARE_DYNAMIC_CLASS(wxStatusBar); }; diff --git a/include/wx/qt/window.h b/include/wx/qt/window.h index e9d14424bf..681a28732e 100644 --- a/include/wx/qt/window.h +++ b/include/wx/qt/window.h @@ -216,24 +216,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 + wxScopedPtr m_qtPainter; // always allocated bool m_mouseInside; #if wxUSE_ACCEL - QList< QShortcut* > *m_qtShortcuts; - wxQtShortcutHandler *m_qtShortcutHandler; + wxVector m_qtShortcuts; // owned by whatever GetHandle() returns + wxScopedPtr m_qtShortcutHandler; // always allocated bool m_processingShortcut; #endif // wxUSE_ACCEL 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 diff --git a/src/qt/accel.cpp b/src/qt/accel.cpp index f12ab0b202..8037e95887 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/app.cpp b/src/qt/app.cpp index 9fcd4e7681..a1a4b67d72 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.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/choice.cpp b/src/qt/choice.cpp index da605c67ba..9399820bf6 100644 --- a/src/qt/choice.cpp +++ b/src/qt/choice.cpp @@ -20,6 +20,8 @@ namespace class LexicalSortProxyModel : public QSortFilterProxyModel { public: + explicit 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); diff --git a/src/qt/dc.cpp b/src/qt/dc.cpp index 0143c5b6a0..5f1324abe9 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; diff --git a/src/qt/dcclient.cpp b/src/qt/dcclient.cpp index 1a8efb2ab2..6ce6a4ff2e 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 ) @@ -99,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() ); 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; } diff --git a/src/qt/statusbar.cpp b/src/qt/statusbar.cpp index cbf8664d89..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 = new QList < QLabel* >; 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 @@ -106,18 +105,19 @@ void wxStatusBar::Refresh( bool eraseBackground, const wxRect *rect ) void wxStatusBar::Init() { m_qtStatusBar = NULL; - m_qtPanes = NULL; } 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++) { @@ -125,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 ) { diff --git a/src/qt/window.cpp b/src/qt/window.cpp index 0eadc15715..c6afec9af8 100644 --- a/src/qt/window.cpp +++ b/src/qt/window.cpp @@ -211,19 +211,16 @@ 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; - - m_dropTarget = NULL; } wxWindowQt::wxWindowQt() @@ -251,13 +248,6 @@ wxWindowQt::~wxWindowQt() DestroyChildren(); // This also destroys scrollbars - delete m_qtPainter; - -#if wxUSE_ACCEL - m_qtShortcutHandler->deleteLater(); - delete m_qtShortcuts; -#endif - #if wxUSE_DRAG_AND_DROP SetDropTarget(NULL); #endif @@ -266,23 +256,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 +597,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 ) @@ -706,6 +683,7 @@ void wxWindowQt::SetDropTarget( wxDropTarget *dropTarget ) if ( m_dropTarget != NULL ) { m_dropTarget->Disconnect(); + delete m_dropTarget; } m_dropTarget = dropTarget; @@ -1006,26 +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(); - - // Create new shortcuts (use GetHandle() so all events inside - // the window are handled, not only in the container subwindow) - delete m_qtShortcuts; + delete *it; } - m_qtShortcuts = 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, &wxQtShortcutHandler::activated ); - QObject::connect( s, &QShortcut::activatedAmbiguously, m_qtShortcutHandler, &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 @@ -1191,7 +1168,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; @@ -1574,5 +1551,5 @@ void wxWindowQt::QtSetPicture( QPicture* pict ) QPainter *wxWindowQt::QtGetPainter() { - return m_qtPainter; + return m_qtPainter.get(); }