Rewrite window positioning logic

Now only reposition a window if it is completely on a
disconnected display or if only one corner is on screen
and it is within a region close to the screen border.

CHANGED: Window position on startup should be preserved more

common/eda_base_frame.cpp

@@ -328,69 +328,117 @@ void EDA_BASE_FRAME::CommonSettingsChanged( bool aEnvVarsChanged )
 
 void EDA_BASE_FRAME::LoadWindowSettings( WINDOW_SETTINGS* aCfg )
 {
-    m_FramePos.x = aCfg->pos_x;
+    m_FramePos.x  = aCfg->pos_x;
-    m_FramePos.y = aCfg->pos_y;
+    m_FramePos.y  = aCfg->pos_y;
     m_FrameSize.x = aCfg->size_x;
     m_FrameSize.y = aCfg->size_y;
 
+    wxLogTrace( traceDisplayLocation, "Config position (%d, %d) with size (%d, %d)",
+            m_FramePos.x, m_FramePos.y, m_FrameSize.x, m_FrameSize.y );
+
     // Ensure minimum size is set if the stored config was zero-initialized
     if( m_FrameSize.x < s_minsize_x || m_FrameSize.y < s_minsize_y )
     {
         m_FrameSize.x = s_minsize_x;
         m_FrameSize.y = s_minsize_y;
+
+        wxLogTrace( traceDisplayLocation, "Using minimum size (%d, %d)", m_FrameSize.x, m_FrameSize.y );
     }
 
-    // Ensure window isn't bigger than can be displayed
+    wxPoint upperRight( m_FramePos.x + m_FrameSize.x, m_FramePos.y );
-    int displayIndex = wxDisplay::GetFromPoint( m_FramePos );
+    wxPoint upperLeft( m_FramePos.x, m_FramePos.y );
 
-    if( displayIndex == wxNOT_FOUND )
+    // Check to see if the requested display is still attached to the computer
-        displayIndex = 0;
+    int leftInd  = wxDisplay::GetFromPoint( upperLeft );
+    int rightInd = wxDisplay::GetFromPoint( upperRight );
 
-    wxDisplay display( displayIndex );
+    wxLogTrace( traceDisplayLocation, "Number of displays: %d", wxDisplay::GetCount() );
-    wxRect clientSize = display.GetClientArea();
+    wxLogTrace( traceDisplayLocation, "Previous display indices: %d and %d", leftInd, rightInd );
 
-    // The window may have been saved on a display that is no longer present.
+    if( rightInd == wxNOT_FOUND && leftInd == wxNOT_FOUND )
-    // First, check the window origin and move it if it's off the chosen display
+    {
+        wxLogTrace( traceDisplayLocation, "Previous display not found" );
+
+        // If it isn't attached, use the first display
+        wxDisplay display( 0 );
+        wxRect    clientSize = display.GetClientArea();
+
+        wxLogDebug( "Client size (%d, %d)", clientSize.width, clientSize.height );
 
-    if( m_FramePos.x >= clientSize.x + clientSize.width ||
-        m_FramePos.y >= clientSize.y + clientSize.height )
         m_FramePos = wxDefaultPosition;
 
-    // Now, fix up the size if needed
+        // Ensure the window fits on the display, since the other one could have been larger
+        if( m_FrameSize.x > clientSize.width )
+            m_FrameSize.x = clientSize.width;
 
-    if( m_FrameSize.x + m_FramePos.x > clientSize.x + clientSize.width )
+        if( m_FrameSize.y > clientSize.height )
-    {
+            m_FrameSize.y = clientSize.height;
-        m_FrameSize.x = clientSize.width;
-        m_FramePos.x = 0;
     }
-
+    else
-    if( m_FrameSize.y + m_FramePos.y > clientSize.y + clientSize.height )
     {
-        m_FrameSize.y = clientSize.height;
+        wxRect clientSize;
-        m_FramePos.y = 0;
-    }
 
-    if( m_hasAutoSave )
+        if( leftInd == wxNOT_FOUND )
-        m_autoSaveInterval = Pgm().GetCommonSettings()->m_System.autosave_interval;
+        {
+            // If the top-left point is off-screen, use the display for the top-right point
+            wxDisplay display( rightInd );
+            clientSize = display.GetClientArea();
+        }
+        else
+        {
+            wxDisplay display( leftInd );
+            clientSize = display.GetClientArea();
+        }
+
+// The percentage size (represented in decimal) of the region around the screen's border where
+// an upper corner is not allowed
+#define SCREEN_BORDER_REGION 0.10
+
+        int yLim      = clientSize.y + ( clientSize.height * ( 1.0 - SCREEN_BORDER_REGION ) );
+        int xLimLeft  = clientSize.x + ( clientSize.width  * SCREEN_BORDER_REGION );
+        int xLimRight = clientSize.x + ( clientSize.width  * ( 1.0 - SCREEN_BORDER_REGION ) );
+
+        if( upperLeft.x  > xLimRight ||  // Upper left corner too close to right edge of screen
+            upperRight.x < xLimLeft  ||  // Upper right corner too close to left edge of screen
+            upperRight.y > yLim )        // Upper corner too close to the bottom of the screen
+        {
+            m_FramePos = wxDefaultPosition;
+            wxLogTrace( traceDisplayLocation, "Resetting to default position" );
+        }
+    }
 
     // Ensure Window title bar is visible
-#if defined( __WXMAC__ )
+#if defined( __WXOSX__ )
     // for macOSX, the window must be below system (macOSX) toolbar
-    // Ypos_min = GetMBarHeight(); seems no more exist in new API (subject to change)
     int Ypos_min = 20;
 #else
     int Ypos_min = 0;
 #endif
     if( m_FramePos.y < Ypos_min )
-    {
-        if( m_FrameSize.y + ( Ypos_min - m_FramePos.y ) > clientSize.height)
-            m_FrameSize.y = clientSize.height - Ypos_min;
-
         m_FramePos.y = Ypos_min;
+
+    wxLogTrace( traceDisplayLocation, "Final window position (%d, %d) with size (%d, %d)",
+        m_FramePos.x, m_FramePos.y, m_FrameSize.x, m_FrameSize.y );
+
+    SetSize( m_FramePos.x, m_FramePos.y, m_FrameSize.x, m_FrameSize.y );
+
+    // Center the window if we reset to default
+    if( m_FramePos.x == -1 )
+    {
+        wxLogTrace( traceDisplayLocation, "Centering window" );
+        Center();
+        m_FramePos = GetPosition();
     }
 
+    // Maximize if we were maximized before
     if( aCfg->maximized )
+    {
+        wxLogTrace( traceDisplayLocation, "Maximizing window" );
         Maximize();
+    }
+
+    if( m_hasAutoSave )
+        m_autoSaveInterval = Pgm().GetCommonSettings()->m_System.autosave_interval;
 
     m_perspective = aCfg->perspective;
     m_mruPath = aCfg->mru_path;
@@ -417,6 +465,10 @@ void EDA_BASE_FRAME::SaveWindowSettings( WINDOW_SETTINGS* aCfg )
     aCfg->size_y = m_FrameSize.y;
     aCfg->maximized = IsMaximized();
 
+    wxLogTrace( traceDisplayLocation, "Saving window maximized: %s", IsMaximized() ? "true" : "false" );
+    wxLogTrace( traceDisplayLocation, "Saving config position (%d, %d) with size (%d, %d)",
+            m_FramePos.x, m_FramePos.y, m_FrameSize.x, m_FrameSize.y );
+
     // TODO(JE) should auto-save in common settings be overwritten by every app?
     if( m_hasAutoSave )
         Pgm().GetCommonSettings()->m_System.autosave_interval = m_autoSaveInterval;

common/trace_helpers.cpp

@@ -46,6 +46,7 @@ const wxChar* const traceLocale = wxT( "KICAD_LOCALE" );
 const wxChar* const traceScreen = wxT( "KICAD_SCREEN" );
 const wxChar* const traceZoomScroll = wxT( "KICAD_ZOOM_SCROLL" );
 const wxChar* const traceSymbolResolver = wxT( "KICAD_SYM_RESOLVE" );
+const wxChar* const traceDisplayLocation = wxT( "KICAD_DISPLAY_LOCATION");
 
 
 wxString dump( const wxArrayString& aArray )

eeschema/libedit/lib_edit_frame.cpp

@@ -130,7 +130,6 @@ LIB_EDIT_FRAME::LIB_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ) :
     GetScreen()->SetMaxUndoItems( m_UndoRedoCountMax );
 
     GetCanvas()->GetViewControls()->SetCrossHairCursorPosition( VECTOR2D( 0, 0 ), false );
-    SetSize( m_FramePos.x, m_FramePos.y, m_FrameSize.x, m_FrameSize.y );
 
     GetRenderSettings()->LoadColors( GetColorSettings() );
 
@@ -188,6 +187,9 @@ LIB_EDIT_FRAME::LIB_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ) :
     m_toolManager->RunAction( ACTIONS::zoomFitScreen, true );
 
     SetShutdownBlockReason( _( "Library changes are unsaved" ) );
+
+    // Ensure the window is on top
+    Raise();
 }
 
 

eeschema/sch_edit_frame.cpp

@@ -240,8 +240,6 @@ SCH_EDIT_FRAME::SCH_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ):
 
     CreateScreens();
 
-    SetSize( m_FramePos.x, m_FramePos.y, m_FrameSize.x, m_FrameSize.y );
-
     setupTools();
     ReCreateMenuBar();
     ReCreateHToolbar();
@@ -281,6 +279,9 @@ SCH_EDIT_FRAME::SCH_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ):
 
     // Default shutdown reason until a file is loaded
     SetShutdownBlockReason( _( "New schematic file is unsaved" ) );
+
+    // Ensure the window is on top
+    Raise();
 }
 
 

eeschema/sim/sim_plot_frame.cpp

@@ -136,9 +136,6 @@ SIM_PLOT_FRAME::SIM_PLOT_FRAME( KIWAY* aKiway, wxWindow* aParent )
     icon.CopyFromBitmap( KiBitmap( simulator_xpm ) );
     SetIcon( icon );
 
-    // Gives a minimal size
-    SetSizeHints( 500, 400, -1, -1, -1, -1 );
-
     // Get the previous size and position of windows:
     LoadSettings( config() );
 
@@ -224,6 +221,9 @@ SIM_PLOT_FRAME::SIM_PLOT_FRAME( KIWAY* aKiway, wxWindow* aParent )
     // events
     wxSafeYield();
     setSubWindowsSashSize();
+
+    // Ensure the window is on top
+    Raise();
 }
 
 
@@ -265,8 +265,6 @@ void SIM_PLOT_FRAME::LoadSettings( APP_SETTINGS_BASE* aCfg )
     {
         EDA_BASE_FRAME::LoadSettings( cfg );
 
-        SetSize( m_FramePos.x, m_FramePos.y, m_FrameSize.x, m_FrameSize.y );
-
         // Read subwindows sizes (should be > 0 )
         m_splitterLeftRightSashPosition      = cfg->m_Simulator.plot_panel_width;
         m_splitterPlotAndConsoleSashPosition = cfg->m_Simulator.plot_panel_height;

gerbview/gerbview_frame.cpp

@@ -123,8 +123,6 @@ GERBVIEW_FRAME::GERBVIEW_FRAME( KIWAY* aKiway, wxWindow* aParent )
     // initialize parameters in m_LayersManager
     LoadSettings( config() );
 
-    SetSize( m_FramePos.x, m_FramePos.y, m_FrameSize.x, m_FrameSize.y );
-
     if( m_LastGridSizeId < 0 )
         m_LastGridSizeId = 0;
 
@@ -215,6 +213,9 @@ GERBVIEW_FRAME::GERBVIEW_FRAME( KIWAY* aKiway, wxWindow* aParent )
 
     // Update the checked state of tools
     SyncToolbars();
+
+    // Ensure the window is on top
+    Raise();
 }
 
 

include/trace_helpers.h

@@ -145,6 +145,13 @@ extern const wxChar* const traceLocale;
  */
 extern const wxChar* const traceScreen;
 
+/**
+ * Flag to enable debug output of display positioning logic.
+ *
+ * Use "KICAD_DISPLAY_LOCATION" to enable.
+ */
+extern const wxChar* const traceDisplayLocation;
+
 /**
  * Flag to enable debug output of zoom-scrolling calculations in
  * #KIGFX::ZOOM_CONTROLLER and derivatives.

kicad/kicad_manager_frame.cpp

@@ -114,9 +114,8 @@ KICAD_MANAGER_FRAME::KICAD_MANAGER_FRAME( wxWindow* parent, const wxString& titl
     icon.CopyFromBitmap( KiBitmap( icon_kicad_xpm ) );
     SetIcon( icon );
 
-    // Give the last size and pos to main window
+    // Load the settings
     LoadSettings( config() );
-    SetSize( m_FramePos.x, m_FramePos.y, m_FrameSize.x, m_FrameSize.y );
 
     // Left window: is the box which display tree project
     m_leftWin = new TREE_PROJECT_FRAME( this );
@@ -163,6 +162,9 @@ KICAD_MANAGER_FRAME::KICAD_MANAGER_FRAME( wxWindow* parent, const wxString& titl
     m_auimgr.GetPane( m_leftWin ).MinSize( 200, -1 );
 
     SetTitle( wxString( "KiCad " ) + GetBuildVersion() );
+
+    // Ensure the window is on top
+    Raise();
 }
 
 

pagelayout_editor/pl_editor_frame.cpp

@@ -115,7 +115,6 @@ PL_EDITOR_FRAME::PL_EDITOR_FRAME( KIWAY* aKiway, wxWindow* aParent ) :
     SetCanvas( drawPanel );
 
     LoadSettings( config() );
-    SetSize( m_FramePos.x, m_FramePos.y, m_FrameSize.x, m_FrameSize.y );
 
     wxSize pageSizeIU = GetPageLayout().GetPageSettings().GetSizeIU();
     SetScreen( new PL_EDITOR_SCREEN( pageSizeIU ) );
@@ -197,6 +196,9 @@ PL_EDITOR_FRAME::PL_EDITOR_FRAME( KIWAY* aKiway, wxWindow* aParent ) :
     pglayout.SetPageLayout();
 #endif
     OnNewPageLayout();
+
+    // Ensure the window is on top
+    Raise();
 }
 
 

pcbnew/footprint_edit_frame.cpp

@@ -183,8 +183,6 @@ FOOTPRINT_EDIT_FRAME::FOOTPRINT_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent,
     // this should be OK for all footprint to plot/print
     SetPageSettings( PAGE_INFO( PAGE_INFO::A4 ) );
 
-    SetSize( m_FramePos.x, m_FramePos.y, m_FrameSize.x, m_FrameSize.y );
-
     // Create the manager and dispatcher & route draw panel events to the dispatcher
     setupTools();
 
@@ -234,7 +232,8 @@ FOOTPRINT_EDIT_FRAME::FOOTPRINT_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent,
     // Default shutdown reason until a file is loaded
     SetShutdownBlockReason( _( "Footprint changes are unsaved" ) );
 
-    Raise();            // On some window managers, this is needed
+    // Ensure the window is on top
+    Raise();
     Show( true );
 }
 

pcbnew/pcb_edit_frame.cpp

@@ -212,8 +212,6 @@ PCB_EDIT_FRAME::PCB_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ) :
     // PCB drawings start in the upper left corner.
     GetScreen()->m_Center = false;
 
-    SetSize( m_FramePos.x, m_FramePos.y, m_FrameSize.x, m_FrameSize.y );
-
     GetScreen()->AddGrid( m_UserGridSize, EDA_UNITS::UNSCALED, ID_POPUP_GRID_USER );
     GetScreen()->SetGrid( ID_POPUP_GRID_LEVEL_1000 + m_LastGridSizeId  );
 
@@ -324,6 +322,9 @@ PCB_EDIT_FRAME::PCB_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ) :
     appK2S.SetExt( "exe" );
     #endif
 
+    // Ensure the window is on top
+    Raise();
+
 //    if( !appK2S.FileExists() )
  //       GetMenuBar()->FindItem( ID_GEN_EXPORT_FILE_STEP )->Enable( false );
 }