https://lists.launchpad.net/kicad-developers/msg14286.html

common/dialog_shim.cpp

@@ -27,29 +27,30 @@
 #include <kiway_player.h>
 #include <wx/evtloop.h>
 
-/*
-    Quasi-Modal Mode Explained:
 
-    The gtk calls in wxDialog::ShowModal() cause event routing problems if that
-    modal dialog then tries to use KIWAY_PLAYER::ShowModal().  The latter shows up
-    and mostly works but does not respond to the window decoration close button.
-    There is no way to get around this without reversing the gtk calls temporarily.
+/// Toggle a window's "enable" status to disabled, then enabled on destruction.
+class WDO_ENABLE_DISABLE
+{
+    wxWindow* m_win;
 
-    Quasi-Modal mode is our own almost modal mode which disables only the parent
-    of the DIALOG_SHIM, leaving other frames operable and while staying captured in the
-    nested event loop.  This avoids the gtk calls and leaves event routing pure
-    and sufficient to operate the KIWAY_PLAYER::ShowModal() properly.  When using
-    ShowQuasiModal() you have to use EndQuasiModal() in your dialogs and not
-    EndModal().  There is also IsQuasiModal() but its value can only be true
-    when the nested event loop is active.  Do not mix the modal and quasi-modal
-    functions.  Use one set or the other.
+public:
 
-    You might find this behavior preferable over a pure modal mode, and it was said
-    that only the Mac has this natively, but now other platforms have something
-    similar.  You CAN use it anywhere for any dialog.  But you MUST use it when
-    you want to use KIWAY_PLAYER::ShowModal() from a dialog event.
-*/
+    WDO_ENABLE_DISABLE( wxWindow* aWindow ) :
+        m_win( aWindow )
+    {
+        if( m_win )
+            m_win->Disable();
+    }
 
+    ~WDO_ENABLE_DISABLE()
+    {
+        if( m_win )
+        {
+            m_win->Enable();
+            m_win->SetFocus(); // let's focus back on the parent window
+        }
+    }
+};
 
 
 DIALOG_SHIM::DIALOG_SHIM( wxWindow* aParent, wxWindowID id, const wxString& title,
@@ -57,7 +58,8 @@ DIALOG_SHIM::DIALOG_SHIM( wxWindow* aParent, wxWindowID id, const wxString& titl
     wxDialog( aParent, id, title, pos, size, style, name ),
     KIWAY_HOLDER( 0 ),
     m_qmodal_loop( 0 ),
-    m_qmodal_showing( false )
+    m_qmodal_showing( false ),
+    m_qmodal_parent_disabler( 0 )
 {
     // pray that aParent is either a KIWAY_PLAYER or DIALOG_SHIM derivation.
     KIWAY_HOLDER* h = dynamic_cast<KIWAY_HOLDER*>( aParent );
@@ -78,6 +80,8 @@ DIALOG_SHIM::~DIALOG_SHIM()
     // if the dialog is quasi-modal, this will end its event loop
     if( IsQuasiModal() )
         EndQuasiModal( wxID_CANCEL );
+
+    delete m_qmodal_parent_disabler;    // usually NULL by now
 }
 
 
@@ -140,6 +144,82 @@ bool DIALOG_SHIM::Enable( bool enable )
 }
 
 
+#if DLGSHIM_USE_SETFOCUS
+
+static bool findWindowRecursively( const wxWindowList& children, const wxWindow* wanted )
+{
+    for( wxWindowList::const_iterator it = children.begin();  it != children.end();  ++it )
+    {
+        const wxWindow* child = *it;
+
+        if( wanted == child )
+            return true;
+        else
+        {
+            if( findWindowRecursively( child->GetChildren(), wanted ) )
+                return true;
+        }
+    }
+
+    return false;
+}
+
+
+static bool findWindowRecursively( const wxWindow* topmost, const wxWindow* wanted )
+{
+    // wanted may be NULL and that is ok.
+
+    if( wanted == topmost )
+        return true;
+
+    return findWindowRecursively( topmost->GetChildren(), wanted );
+}
+
+
+/// Set the focus if it is not already set in a derived constructor to a specific control.
+void DIALOG_SHIM::onInit( wxInitDialogEvent& aEvent )
+{
+    wxWindow* focusWnd = wxWindow::FindFocus();
+
+    // If focusWnd is not already this window or a child of it, then SetFocus().
+    // Otherwise the derived class's constructor SetFocus() already to a specific
+    // child control.
+
+    if( !findWindowRecursively( this, focusWnd ) )
+    {
+        // Linux wxGTK needs this to allow the ESCAPE key to close a wxDialog window.
+        SetFocus();
+    }
+
+    aEvent.Skip();     // derived class's handler should be called too
+}
+#endif
+
+
+/*
+    Quasi-Modal Mode Explained:
+
+    The gtk calls in wxDialog::ShowModal() cause event routing problems if that
+    modal dialog then tries to use KIWAY_PLAYER::ShowModal().  The latter shows up
+    and mostly works but does not respond to the window decoration close button.
+    There is no way to get around this without reversing the gtk calls temporarily.
+
+    Quasi-Modal mode is our own almost modal mode which disables only the parent
+    of the DIALOG_SHIM, leaving other frames operable and while staying captured in the
+    nested event loop.  This avoids the gtk calls and leaves event routing pure
+    and sufficient to operate the KIWAY_PLAYER::ShowModal() properly.  When using
+    ShowQuasiModal() you have to use EndQuasiModal() in your dialogs and not
+    EndModal().  There is also IsQuasiModal() but its value can only be true
+    when the nested event loop is active.  Do not mix the modal and quasi-modal
+    functions.  Use one set or the other.
+
+    You might find this behavior preferable over a pure modal mode, and it was said
+    that only the Mac has this natively, but now other platforms have something
+    similar.  You CAN use it anywhere for any dialog.  But you MUST use it when
+    you want to use KIWAY_PLAYER::ShowModal() from a dialog event.
+*/
+
+
 #if !wxCHECK_VERSION( 2, 9, 4 )
 wxWindow* DIALOG_SHIM::CheckIfCanBeUsedAsParent( wxWindow* parent ) const
 {
@@ -207,24 +287,231 @@ wxWindow* DIALOG_SHIM::GetParentForModalDialog(wxWindow *parent, long style) con
 #endif
 
 
-int DIALOG_SHIM::ShowQuasiModal()
+/*
+/// wxEventLoopActivator but with a friend so it
+/// has access to m_evtLoopOld, and it does not SetActive() as that is
+/// done inside base class Run().
+class ELOOP_ACTIVATOR
 {
-    // toggle a window's "enable" status to disabled, then enabled on exit.
-    // exception safe.
-    struct ENABLE_DISABLE
+    friend class EVENT_LOOP;
+
+public:
+    ELOOP_ACTIVATOR( WX_EVENT_LOOP* evtLoop )
     {
-        wxWindow* m_win;
-        ENABLE_DISABLE( wxWindow* aWindow ) : m_win( aWindow ) { if( m_win ) m_win->Disable(); }
-        ~ENABLE_DISABLE()
+        m_evtLoopOld = wxEventLoopBase::GetActive();
+        // wxEventLoopBase::SetActive( evtLoop );
+    }
+
+    ~ELOOP_ACTIVATOR()
+    {
+        // restore the previously active event loop
+        wxEventLoopBase::SetActive( m_evtLoopOld );
+    }
+
+private:
+    WX_EVENT_LOOP* m_evtLoopOld;
+};
+*/
+
+
+class EVENT_LOOP : public WX_EVENT_LOOP
+{
+public:
+
+    EVENT_LOOP()
+    {
+        ;
+    }
+
+    ~EVENT_LOOP()
+    {
+    }
+
+#if 0   // does not work any better than inherited wxGuiEventLoop functions:
+
+    // sets the "should exit" flag and wakes up the loop so that it terminates
+    // soon
+    void ScheduleExit( int rc = 0 )
+    {
+        wxCHECK_RET( IsInsideRun(), wxT("can't call ScheduleExit() if not running") );
+
+        m_exitcode = rc;
+        m_shouldExit = true;
+
+        OnExit();
+
+        // all we have to do to exit from the loop is to (maybe) wake it up so that
+        // it can notice that Exit() had been called
+        //
+        // in particular, do *not* use here calls such as PostQuitMessage() (under
+        // MSW) which terminate the current event loop here because we're not sure
+        // that it is going to be processed by the correct event loop: it would be
+        // possible that another one is started and terminated by mistake if we do
+        // this
+        WakeUp();
+    }
+
+    int Run()
+    {
+        // event loops are not recursive, you need to create another loop!
+        //wxCHECK_MSG( !IsInsideRun(), -1, wxT("can't reenter a message loop") );
+
+        // ProcessIdle() and ProcessEvents() below may throw so the code here should
+        // be exception-safe, hence we must use local objects for all actions we
+        // should undo
+        wxEventLoopActivator activate(this);
+
+        // We might be called again, after a previous call to ScheduleExit(), so
+        // reset this flag.
+        m_shouldExit = false;
+
+        // Set this variable to true for the duration of this method.
+        setInsideRun( true );
+
+        struct SET_FALSE
+        {
+            EVENT_LOOP* m_loop;
+            SET_FALSE( EVENT_LOOP* aLoop ) : m_loop( aLoop ) {}
+            ~SET_FALSE() { m_loop->setInsideRun( false ); }
+        } t( this );
+
+        // Finally really run the loop.
+        return DoRun();
+    }
+
+    bool ProcessEvents()
+    {
+        // process pending wx events first as they correspond to low-level events
+        // which happened before, i.e. typically pending events were queued by a
+        // previous call to Dispatch() and if we didn't process them now the next
+        // call to it might enqueue them again (as happens with e.g. socket events
+        // which would be generated as long as there is input available on socket
+        // and this input is only removed from it when pending event handlers are
+        // executed)
+        if( wxTheApp )
+        {
+            wxTheApp->ProcessPendingEvents();
+
+            // One of the pending event handlers could have decided to exit the
+            // loop so check for the flag before trying to dispatch more events
+            // (which could block indefinitely if no more are coming).
+            if( m_shouldExit )
+                return false;
+        }
+
+        return Dispatch();
+    }
+
+
+    int DoRun()
+    {
+        // we must ensure that OnExit() is called even if an exception is thrown
+        // from inside ProcessEvents() but we must call it from Exit() in normal
+        // situations because it is supposed to be called synchronously,
+        // wxModalEventLoop depends on this (so we can't just use ON_BLOCK_EXIT or
+        // something similar here)
+    #if wxUSE_EXCEPTIONS
+        for ( ;; )
         {
-            if( m_win )
+            try
+            {
+    #endif // wxUSE_EXCEPTIONS
+
+                // this is the event loop itself
+                for ( ;; )
+                {
+                    // generate and process idle events for as long as we don't
+                    // have anything else to do
+                    while ( !m_shouldExit && !Pending() && ProcessIdle() )
+                        ;
+
+                    if ( m_shouldExit )
+                        break;
+
+                    // a message came or no more idle processing to do, dispatch
+                    // all the pending events and call Dispatch() to wait for the
+                    // next message
+                    if ( !ProcessEvents() )
+                    {
+                        // we got WM_QUIT
+                        break;
+                    }
+                }
+
+                // Process the remaining queued messages, both at the level of the
+                // underlying toolkit level (Pending/Dispatch()) and wx level
+                // (Has/ProcessPendingEvents()).
+                //
+                // We do run the risk of never exiting this loop if pending event
+                // handlers endlessly generate new events but they shouldn't do
+                // this in a well-behaved program and we shouldn't just discard the
+                // events we already have, they might be important.
+                for ( ;; )
+                {
+                    bool hasMoreEvents = false;
+                    if ( wxTheApp && wxTheApp->HasPendingEvents() )
+                    {
+                        wxTheApp->ProcessPendingEvents();
+                        hasMoreEvents = true;
+                    }
+
+                    if ( Pending() )
+                    {
+                        Dispatch();
+                        hasMoreEvents = true;
+                    }
+
+                    if ( !hasMoreEvents )
+                        break;
+                }
+
+    #if wxUSE_EXCEPTIONS
+                // exit the outer loop as well
+                break;
+            }
+            catch ( ... )
             {
-                m_win->Enable();
-                m_win->SetFocus(); // let's focus back on the parent window
+                try
+                {
+                    if ( !wxTheApp || !wxTheApp->OnExceptionInMainLoop() )
+                    {
+                        OnExit();
+                        break;
+                    }
+                    //else: continue running the event loop
+                }
+                catch ( ... )
+                {
+                    // OnException() throwed, possibly rethrowing the same
+                    // exception again: very good, but we still need OnExit() to
+                    // be called
+                    OnExit();
+                    throw;
+                }
             }
         }
-    };
+    #endif // wxUSE_EXCEPTIONS
+
+        return m_exitcode;
+    }
+
+protected:
+    int     m_exitcode;
 
+    /* this only works if you add
+    friend class EVENT_LOOP
+    to EventLoopBase
+    */
+    void setInsideRun( bool aValue )
+    {
+        m_isInsideRun = aValue;
+    }
+#endif
+};
+
+
+int DIALOG_SHIM::ShowQuasiModal()
+{
     // This is an exception safe way to zero a pointer before returning.
     // Yes, even though DismissModal() clears this first normally, this is
     // here in case there's an exception before the dialog is dismissed.
@@ -235,7 +522,6 @@ int DIALOG_SHIM::ShowQuasiModal()
         ~NULLER() { m_what = 0; }   // indeed, set it to NULL on destruction
     } clear_this( (void*&) m_qmodal_loop );
 
-
     // release the mouse if it's currently captured as the window having it
     // will be disabled when this dialog is shown -- but will still keep the
     // capture making it impossible to do anything in the modal dialog itself
@@ -249,20 +535,17 @@ int DIALOG_SHIM::ShowQuasiModal()
     // Show the optimal parent
     DBG( if( parent ) printf( "%s: optimal parent: %s\n", __func__, typeid(*parent).name() );)
 
-    ENABLE_DISABLE  toggle( parent );       // quasi-modal: disable only my "optimal" parent
+    wxASSERT_MSG( !m_qmodal_parent_disabler,
+            wxT( "Caller using ShowQuasiModal() twice on same window?" ) );
+
+    // quasi-modal: disable only my "optimal" parent
+    m_qmodal_parent_disabler = new WDO_ENABLE_DISABLE( parent );
 
     Show( true );
 
     m_qmodal_showing = true;
 
-    WX_EVENT_LOOP           event_loop;
-
-#if wxCHECK_VERSION( 2, 9, 4 )  // 2.9.4 is only approximate.
-    // new code needs this, old code does it in wxEventLoop::Run() and cannot
-    // tolerate it here. Where that boundary is as a version number, I don't know.
-    // A closer look at the subversion repo for wx would tell.
-    wxEventLoopActivator    event_loop_stacker( &event_loop );
-#endif
+    EVENT_LOOP event_loop;
 
     m_qmodal_loop = &event_loop;
 
@@ -286,61 +569,19 @@ void DIALOG_SHIM::EndQuasiModal( int retCode )
 
     if( m_qmodal_loop )
     {
-        m_qmodal_loop->Exit();
-        m_qmodal_loop = NULL;
-    }
-
-    Show( false );
-}
-
-
-#if DLGSHIM_USE_SETFOCUS
-
-static bool findWindowRecursively( const wxWindowList& children, const wxWindow* wanted )
-{
-    for( wxWindowList::const_iterator it = children.begin();  it != children.end();  ++it )
-    {
-        const wxWindow* child = *it;
-
-        if( wanted == child )
-            return true;
+#if wxCHECK_VERSION( 2, 9, 4 )  // 2.9.4 is only approximate, might be 3, 0, 0.
+        if( m_qmodal_loop->IsRunning() )
+            m_qmodal_loop->Exit( 0 );
         else
-        {
-            if( findWindowRecursively( child->GetChildren(), wanted ) )
-                return true;
-        }
+            m_qmodal_loop->ScheduleExit( 0 );
+#else
+        m_qmodal_loop->Exit( 0 );
+#endif
+        m_qmodal_loop = NULL;
     }
 
-    return false;
-}
-
+    delete m_qmodal_parent_disabler;
+    m_qmodal_parent_disabler = 0;
 
-static bool findWindowRecursively( const wxWindow* topmost, const wxWindow* wanted )
-{
-    // wanted may be NULL and that is ok.
-
-    if( wanted == topmost )
-        return true;
-
-    return findWindowRecursively( topmost->GetChildren(), wanted );
-}
-
-
-/// Set the focus if it is not already set in a derived constructor to a specific control.
-void DIALOG_SHIM::onInit( wxInitDialogEvent& aEvent )
-{
-    wxWindow* focusWnd = wxWindow::FindFocus();
-
-    // If focusWnd is not already this window or a child of it, then SetFocus().
-    // Otherwise the derived class's constructor SetFocus() already to a specific
-    // child control.
-
-    if( !findWindowRecursively( this, focusWnd ) )
-    {
-        // Linux wxGTK needs this to allow the ESCAPE key to close a wxDialog window.
-        SetFocus();
-    }
-
-    aEvent.Skip();     // derived class's handler should be called too
+    Show( false );
 }
-#endif

common/kiway_player.cpp

@@ -104,14 +104,7 @@ bool KIWAY_PLAYER::ShowModal( wxString* aResult, wxWindow* aResultantFocusWindow
         // re-enables only those that were disabled on exit
         wxWindowDisabler    toggle( this );
 
-        WX_EVENT_LOOP           event_loop;
-
-#if wxCHECK_VERSION( 2, 9, 4 )  // 2.9.4 is only approximate.
-        // new code needs this, old code does it in wxEventLoop::Run() and cannot
-        // tolerate it here. Where that boundary is as a version number, I don't know.
-        // A closer look at the subversion repo for wx would tell.
-        wxEventLoopActivator    event_loop_stacker( &event_loop );
-#endif
+        WX_EVENT_LOOP          event_loop;
 
         m_modal_loop = &event_loop;
 

eeschema/dialogs/dialog_edit_component_in_schematic.cpp

@@ -148,6 +148,10 @@ void SCH_EDIT_FRAME::EditComponent( SCH_COMPONENT* aComponent )
     // make sure the chipnameTextCtrl is wide enough to hold any unusually long chip names:
     EnsureTextCtrlWidth( dlg->chipnameTextCtrl );
 
+    // This dialog itself subsequently can invoke a KIWAY_PLAYER as a quasimodal
+    // frame. Therefore this dialog as a modal frame parent, MUST be run under
+    // quasimodal mode for the quasimodal frame support to work.  So don't use
+    // the QUASIMODAL macros here.
     dlg->ShowQuasiModal();
 
     m_canvas->SetIgnoreMouseEvents( false );

eeschema/dialogs/dialog_edit_libentry_fields_in_lib.cpp

@@ -142,6 +142,10 @@ void LIB_EDIT_FRAME::InstallFieldsEditorDialog( wxCommandEvent& event )
 
     DIALOG_EDIT_LIBENTRY_FIELDS_IN_LIB dlg( this, GetCurPart() );
 
+    // This dialog itself subsequently can invoke a KIWAY_PLAYER as a quasimodal
+    // frame. Therefore this dialog as a modal frame parent, MUST be run under
+    // quasimodal mode for the quasimodal frame support to work.  So don't use
+    // the QUASIMODAL macros here.
     int abort = dlg.ShowQuasiModal();
 
     if( abort )

include/dialog_shim.h

@@ -35,14 +35,20 @@
  #define DLGSHIM_USE_SETFOCUS      0
 #endif
 
-#if wxCHECK_VERSION( 2, 9, 4 )
- #define WX_EVENT_LOOP      wxGUIEventLoop
+class WDO_ENABLE_DISABLE;
+class EVENT_LOOP;
+
+// These macros are for DIALOG_SHIM only, NOT for KIWAY_PLAYER.  KIWAY_PLAYER
+// has its own support for quasi modal and its platform specific issues are different
+// than for a wxDialog.
+#if wxCHECK_VERSION( 3, 0, 0 )
+ #define SHOWQUASIMODAL     ShowQuasiModal
+ #define ENDQUASIMODAL      EndQuasiModal
 #else
- #define WX_EVENT_LOOP      wxEventLoop
+ #define SHOWQUASIMODAL     ShowModal
+ #define ENDQUASIMODAL      EndModal
 #endif
 
-class WX_EVENT_LOOP;
-
 
 /**
  * Class DIALOG_SHIM
@@ -86,9 +92,9 @@ protected:
     std::string m_hash_key;     // alternate for class_map when classname re-used.
 
     // variables for quasi-modal behavior support, only used by a few derivatives.
-    WX_EVENT_LOOP*  m_qmodal_loop;      // points to nested event_loop, NULL means not qmodal and dismissed
-    bool            m_qmodal_showing;
-
+    EVENT_LOOP*         m_qmodal_loop;      // points to nested event_loop, NULL means not qmodal and dismissed
+    bool                m_qmodal_showing;
+    WDO_ENABLE_DISABLE* m_qmodal_parent_disabler;
 
 #if DLGSHIM_USE_SETFOCUS
 private:

pcbnew/dialogs/dialog_edit_module_for_BoardEditor.cpp

@@ -182,7 +182,7 @@ void DIALOG_MODULE_BOARD_EDITOR::InitBoardProperties()
 
 void DIALOG_MODULE_BOARD_EDITOR::OnCancelClick( wxCommandEvent& event )
 {
-    EndModal( -1 );
+    ENDQUASIMODAL( -1 );
 }
 
 
@@ -194,7 +194,7 @@ void DIALOG_MODULE_BOARD_EDITOR::GotoModuleEditor( wxCommandEvent& event )
         m_Parent->OnModify();
     }
 
-    EndModal( 2 );
+    ENDQUASIMODAL( 2 );
 }
 
 
@@ -204,7 +204,7 @@ void DIALOG_MODULE_BOARD_EDITOR::ExchangeModule( wxCommandEvent& event )
 
     // Warning: m_CurrentModule was deleted by exchange module
     m_Parent->SetCurItem( NULL );
-    EndModal( 0 );
+    ENDQUASIMODAL( 0 );
 }
 
 
@@ -241,8 +241,6 @@ void DIALOG_MODULE_BOARD_EDITOR::ModuleOrientEvent( wxCommandEvent& event )
 
 void DIALOG_MODULE_BOARD_EDITOR::InitModeditProperties()
 {
-    SetFocus();
-
     wxString default_path;
     wxGetEnv( wxT( KISYS3DMOD ), &default_path );
 #ifdef __WINDOWS__
@@ -675,7 +673,7 @@ void DIALOG_MODULE_BOARD_EDITOR::OnOkClick( wxCommandEvent& event )
 
     m_Parent->OnModify();
 
-    EndModal( 1 );
+    ENDQUASIMODAL( 1 );
 
     if( m_DC )
     {

pcbnew/editmod.cpp

@@ -59,12 +59,12 @@ void PCB_EDIT_FRAME::InstallModuleOptionsFrame( MODULE* Module, wxDC* DC )
     DIALOG_MODULE_BOARD_EDITOR* dialog = new DIALOG_MODULE_BOARD_EDITOR( this, Module, NULL );
 #endif
 
-    int retvalue = dialog->ShowModal(); /* retvalue =
-                                         *  -1 if abort,
-                                         *  0 if exchange module,
-                                         *  1 for normal edition
-                                         *  and 2 for a goto editor command
-                                         */
+    int retvalue = dialog->SHOWQUASIMODAL();    /* retvalue =
+                                                 *  -1 if abort,
+                                                 *  0 if exchange module,
+                                                 *  1 for normal edition
+                                                 *  and 2 for a goto editor command
+                                                 */
     dialog->Destroy();
 
 #ifdef __WXMAC__