From d6d4a1a9838624d490ea5aa64b27e47951ba53e9 Mon Sep 17 00:00:00 2001 From: Jonathan Li Date: Mon, 22 Jun 2015 18:56:31 +0100 Subject: [PATCH 1/4] linux: wx3.0: fix broken messagebox handling Do not do additional processing on an event after EndModal is called. This fixes a message box bug on Linux wx3.0 builds. Without this fix, Release and Devel builds emit cancel signals even if 'Ok' or other buttons are pressed, and Debug builds trip an assertion with the message "EndModal called twice or ShowModal not called." --- pcsx2/gui/Dialogs/ConfirmationDialogs.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pcsx2/gui/Dialogs/ConfirmationDialogs.cpp b/pcsx2/gui/Dialogs/ConfirmationDialogs.cpp index 7756e97d88..70d1f3e48a 100644 --- a/pcsx2/gui/Dialogs/ConfirmationDialogs.cpp +++ b/pcsx2/gui/Dialogs/ConfirmationDialogs.cpp @@ -298,10 +298,11 @@ ModalButtonPanel::ModalButtonPanel( wxWindow* parent, const MsgButtons& buttons void ModalButtonPanel::OnActionButtonClicked( wxCommandEvent& evt ) { - evt.Skip(); wxWindow* toplevel = wxGetTopLevelParent( this ); if( wxDialog* dialog = wxDynamicCast( toplevel, wxDialog ) ) dialog->EndModal( evt.GetId() ); + // If the dialog doesn't close, and you're using it for a modeless dialog - hint: + // read the name of the class. ;) } void ModalButtonPanel::AddCustomButton( wxWindowID id, const wxString& label ) From d9447de4929e679e3af06f782450e9a43f9cfca8 Mon Sep 17 00:00:00 2001 From: Jonathan Li Date: Wed, 24 Jun 2015 23:26:22 +0100 Subject: [PATCH 2/4] linux: Fix close button on minimisable modal dialogs Keep Windows platform specific fixes ifdef'd to Windows only. --- common/src/Utilities/wxHelpers.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/common/src/Utilities/wxHelpers.cpp b/common/src/Utilities/wxHelpers.cpp index a0d189e3b4..578e035af2 100644 --- a/common/src/Utilities/wxHelpers.cpp +++ b/common/src/Utilities/wxHelpers.cpp @@ -149,10 +149,16 @@ wxDialogWithHelpers::~wxDialogWithHelpers() throw() void wxDialogWithHelpers::Init( const pxDialogCreationFlags& cflags ) { + // Note to self: if any comments indicate platform specific behaviour then + // ifdef them out to see if they fix the issue. I wasted too much time + // figuring out why the close box wouldn't work on wxGTK modal dialogs that + // had a minimise button. +#if _WIN32 // This fixes it so that the dialogs show up in the task bar in Vista: // (otherwise they go stupid iconized mode if the user minimizes them) if( cflags.hasMinimizeBox ) SetExtraStyle(GetExtraStyle() & ~wxTOPLEVEL_EX_DIALOG); +#endif m_extraButtonSizer = NULL; @@ -161,11 +167,6 @@ void wxDialogWithHelpers::Init( const pxDialogCreationFlags& cflags ) delete wxHelpProvider::Set( new wxSimpleHelpProvider() ); #endif - // GTK/Linux Note: currently the Close (X) button doesn't appear to work in dialogs. Docs - // indicate that it should, so I presume the problem is in wxWidgets and that (hopefully!) - // an updated version will fix it later. I tried to fix it using a manual Connect but it - // didn't do any good. (problem could also be my Co-Linux / x-window manager) - Connect( pxEvt_OnDialogCreated, wxCommandEventHandler (wxDialogWithHelpers::OnDialogCreated) ); Connect( wxID_OK, wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler (wxDialogWithHelpers::OnOkCancel) ); From 2a4bd85f53687af3f76b611ea02bc2f9b12dc449 Mon Sep 17 00:00:00 2001 From: Jonathan Li Date: Thu, 25 Jun 2015 16:46:20 +0100 Subject: [PATCH 3/4] gui: Fix looping dialog event handling The dialog event handling is a bit messed up. An ok/cancel event sends a close event, which sends a cancel event and repeats. This would actually be an infinite loop if wxWidgets didn't detect a loop. Rework the event handling to avoid the loop and to remember the positions of modal dialogs as well. --- common/include/Utilities/wxGuiTools.h | 3 +-- common/src/Utilities/wxHelpers.cpp | 21 ++++++++----------- pcsx2/gui/Dialogs/BaseConfigurationDialog.cpp | 11 ---------- pcsx2/gui/Dialogs/ConfigurationDialog.h | 1 - 4 files changed, 10 insertions(+), 26 deletions(-) diff --git a/common/include/Utilities/wxGuiTools.h b/common/include/Utilities/wxGuiTools.h index 8068558ab9..1f15ceda55 100644 --- a/common/include/Utilities/wxGuiTools.h +++ b/common/include/Utilities/wxGuiTools.h @@ -512,11 +512,11 @@ public: void Init( const pxDialogCreationFlags& cflags ); void AddOkCancel( wxSizer& sizer, bool hasApply=false ); void AddOkCancel( wxSizer* sizer=NULL, bool hasApply=false ); + void RememberPosition(); virtual void SmartCenterFit(); virtual int ShowModal(); virtual bool Show( bool show=true ); - virtual bool Destroy(); // Must return the same thing as GetNameStatic; a name ideal for use in uniquely // identifying dialogs. (this version is the 'instance' version, which is called @@ -536,7 +536,6 @@ public: protected: void OnDialogCreated( wxCommandEvent& evt ); void OnOkCancel(wxCommandEvent& evt); - void OnCloseWindow(wxCloseEvent& event); bool ShouldPreventAppExit() const { return false; } diff --git a/common/src/Utilities/wxHelpers.cpp b/common/src/Utilities/wxHelpers.cpp index 578e035af2..dbb2bd1f0b 100644 --- a/common/src/Utilities/wxHelpers.cpp +++ b/common/src/Utilities/wxHelpers.cpp @@ -171,7 +171,6 @@ void wxDialogWithHelpers::Init( const pxDialogCreationFlags& cflags ) Connect( wxID_OK, wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler (wxDialogWithHelpers::OnOkCancel) ); Connect( wxID_CANCEL, wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler (wxDialogWithHelpers::OnOkCancel) ); - Connect( wxEVT_CLOSE_WINDOW, wxCloseEventHandler (wxDialogWithHelpers::OnCloseWindow) ); wxCommandEvent createEvent( pxEvt_OnDialogCreated ); createEvent.SetId( GetId() ); @@ -287,7 +286,7 @@ pxStaticText& wxDialogWithHelpers::Heading( const wxString& label ) return *new pxStaticHeading( this, label ); } -bool wxDialogWithHelpers::Destroy() +void wxDialogWithHelpers::RememberPosition() { // Save the dialog position if the dialog is named... // FIXME : This doesn't get called if the app is exited by alt-f4'ing the main app window. @@ -312,20 +311,18 @@ bool wxDialogWithHelpers::Destroy() saver.Entry( dlgName + L"_Pos", pos, screenRect.GetPosition() ); } } - - return _parent::Destroy(); -} - -void wxDialogWithHelpers::OnCloseWindow( wxCloseEvent& evt ) -{ - if( !IsModal() ) Destroy(); - evt.Skip(); } void wxDialogWithHelpers::OnOkCancel( wxCommandEvent& evt ) { - Close(); - evt.Skip(); + RememberPosition(); + + // Modal dialogs should be destroyed after ShowModal returns, otherwise there + // might be double delete problems if the dialog was declared on the stack. + if (!IsModal()) + Destroy(); + else + evt.Skip(); } void wxDialogWithHelpers::AddOkCancel( wxSizer &sizer, bool hasApply ) diff --git a/pcsx2/gui/Dialogs/BaseConfigurationDialog.cpp b/pcsx2/gui/Dialogs/BaseConfigurationDialog.cpp index 977ea86bcf..d6846c5510 100644 --- a/pcsx2/gui/Dialogs/BaseConfigurationDialog.cpp +++ b/pcsx2/gui/Dialogs/BaseConfigurationDialog.cpp @@ -144,8 +144,6 @@ Dialogs::BaseConfigurationDialog::BaseConfigurationDialog( wxWindow* parent, con Connect( wxID_APPLY, wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( BaseConfigurationDialog::OnApply_Click ) ); Connect( wxID_SAVE, wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( BaseConfigurationDialog::OnScreenshot_Click ) ); - Connect( wxEVT_CLOSE_WINDOW, wxCloseEventHandler(BaseConfigurationDialog::OnCloseWindow) ); - Connect( pxEvt_SetSettingsPage, wxCommandEventHandler( BaseConfigurationDialog::OnSetSettingsPage ) ); // ---------------------------------------------------------------------------- @@ -230,14 +228,6 @@ void Dialogs::BaseConfigurationDialog::OnSomethingChanged( wxCommandEvent& evt ) SomethingChanged(); } - -void Dialogs::BaseConfigurationDialog::OnCloseWindow( wxCloseEvent& evt ) -{ - if( !IsModal() ) Destroy(); - evt.Skip(); -} - - void Dialogs::BaseConfigurationDialog::AllowApplyActivation( bool allow ) { m_allowApplyActivation = allow; @@ -280,7 +270,6 @@ void Dialogs::BaseConfigurationDialog::OnApply_Click( wxCommandEvent& evt ) SomethingChanged_StateModified_IsChanged(); } -//avih: FIXME: ? for some reason, this OnCancel_Click is called twice when clicking cancel or closing the dialog (Jake's code?). void Dialogs::BaseConfigurationDialog::OnCancel_Click( wxCommandEvent& evt ) { //same as for Ok/Apply: let SysConfigDialog clean-up the presets and derivatives (menu system) if needed. diff --git a/pcsx2/gui/Dialogs/ConfigurationDialog.h b/pcsx2/gui/Dialogs/ConfigurationDialog.h index f8a34ae669..ea044e4135 100644 --- a/pcsx2/gui/Dialogs/ConfigurationDialog.h +++ b/pcsx2/gui/Dialogs/ConfigurationDialog.h @@ -76,7 +76,6 @@ namespace Dialogs void OnCancel_Click( wxCommandEvent& evt ); void OnApply_Click( wxCommandEvent& evt ); void OnScreenshot_Click( wxCommandEvent& evt ); - void OnCloseWindow( wxCloseEvent& evt ); void OnSetSettingsPage( wxCommandEvent& evt ); void OnSomethingChanged( wxCommandEvent& evt ); From 9a171a5928fea6cba7119df1adc70a648912fc9d Mon Sep 17 00:00:00 2001 From: Jonathan Li Date: Thu, 25 Jun 2015 16:53:10 +0100 Subject: [PATCH 4/4] gui: Make Msgbox more consistent Just use ShowModal. That will invoke pxMessageDialog on the main thread anyway. --- pcsx2/gui/MessageBoxes.cpp | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/pcsx2/gui/MessageBoxes.cpp b/pcsx2/gui/MessageBoxes.cpp index 5b34674f26..b7e77e4caf 100644 --- a/pcsx2/gui/MessageBoxes.cpp +++ b/pcsx2/gui/MessageBoxes.cpp @@ -181,30 +181,12 @@ namespace Msgbox // true if OK, false if cancel. bool OkCancel( const wxString& text, const wxString& caption, int icon ) { - MsgButtons buttons( MsgButtons().OKCancel() ); - - if( wxThread::IsMain() ) - { - return wxID_OK == pxMessageDialog( caption, text, buttons ); - } - else - { - return wxID_OK == ShowModal( caption, text, buttons ); - } + return ShowModal(caption, text, MsgButtons().OKCancel()) == wxID_OK; } bool YesNo( const wxString& text, const wxString& caption, int icon ) { - MsgButtons buttons( MsgButtons().YesNo() ); - - if( wxThread::IsMain() ) - { - return wxID_YES == pxMessageDialog( caption, text, buttons ); - } - else - { - return wxID_YES == ShowModal( caption, text, buttons ); - } + return ShowModal(caption, text, MsgButtons().YesNo()) == wxID_YES; } int Assertion( const wxString& text, const wxString& stacktrace )