From 3b67e3d0a43e0d581c6b0e2e8c0d519df8baff5d Mon Sep 17 00:00:00 2001 From: Ian McInerney Date: Sat, 18 Apr 2020 00:32:29 +0100 Subject: [PATCH] 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 | 112 +++++++++++++++++++------- common/trace_helpers.cpp | 1 + eeschema/libedit/lib_edit_frame.cpp | 4 +- eeschema/sch_edit_frame.cpp | 5 +- eeschema/sim/sim_plot_frame.cpp | 8 +- gerbview/gerbview_frame.cpp | 5 +- include/trace_helpers.h | 7 ++ kicad/kicad_manager_frame.cpp | 6 +- pagelayout_editor/pl_editor_frame.cpp | 4 +- pcbnew/footprint_edit_frame.cpp | 5 +- pcbnew/pcb_edit_frame.cpp | 5 +- 11 files changed, 114 insertions(+), 48 deletions(-) diff --git a/common/eda_base_frame.cpp b/common/eda_base_frame.cpp index b85ee2fd31..dcb50f5900 100644 --- a/common/eda_base_frame.cpp +++ b/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.y = aCfg->pos_y; + m_FramePos.x = aCfg->pos_x; + 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 - int displayIndex = wxDisplay::GetFromPoint( m_FramePos ); + wxPoint upperRight( m_FramePos.x + m_FrameSize.x, m_FramePos.y ); + wxPoint upperLeft( m_FramePos.x, m_FramePos.y ); - if( displayIndex == wxNOT_FOUND ) - displayIndex = 0; + // Check to see if the requested display is still attached to the computer + int leftInd = wxDisplay::GetFromPoint( upperLeft ); + int rightInd = wxDisplay::GetFromPoint( upperRight ); - wxDisplay display( displayIndex ); - wxRect clientSize = display.GetClientArea(); + wxLogTrace( traceDisplayLocation, "Number of displays: %d", wxDisplay::GetCount() ); + wxLogTrace( traceDisplayLocation, "Previous display indices: %d and %d", leftInd, rightInd ); - // The window may have been saved on a display that is no longer present. - // First, check the window origin and move it if it's off the chosen display + if( rightInd == wxNOT_FOUND && leftInd == wxNOT_FOUND ) + { + 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 ) - { - m_FrameSize.x = clientSize.width; - m_FramePos.x = 0; + if( m_FrameSize.y > clientSize.height ) + m_FrameSize.y = clientSize.height; } - - if( m_FrameSize.y + m_FramePos.y > clientSize.y + clientSize.height ) + else { - m_FrameSize.y = clientSize.height; - m_FramePos.y = 0; - } + wxRect clientSize; - if( m_hasAutoSave ) - m_autoSaveInterval = Pgm().GetCommonSettings()->m_System.autosave_interval; + if( leftInd == wxNOT_FOUND ) + { + // 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; diff --git a/common/trace_helpers.cpp b/common/trace_helpers.cpp index b4cda831b9..1508267f48 100644 --- a/common/trace_helpers.cpp +++ b/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 ) diff --git a/eeschema/libedit/lib_edit_frame.cpp b/eeschema/libedit/lib_edit_frame.cpp index e097c79a02..ed8bd5a608 100644 --- a/eeschema/libedit/lib_edit_frame.cpp +++ b/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(); } diff --git a/eeschema/sch_edit_frame.cpp b/eeschema/sch_edit_frame.cpp index 3fc4405f02..7f1cd27cd2 100644 --- a/eeschema/sch_edit_frame.cpp +++ b/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(); } diff --git a/eeschema/sim/sim_plot_frame.cpp b/eeschema/sim/sim_plot_frame.cpp index dfb5bce30a..b8b4a1e8d0 100644 --- a/eeschema/sim/sim_plot_frame.cpp +++ b/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; diff --git a/gerbview/gerbview_frame.cpp b/gerbview/gerbview_frame.cpp index c10014ccb8..4ca1644086 100644 --- a/gerbview/gerbview_frame.cpp +++ b/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(); } diff --git a/include/trace_helpers.h b/include/trace_helpers.h index 55caa48a11..1f56040090 100644 --- a/include/trace_helpers.h +++ b/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. diff --git a/kicad/kicad_manager_frame.cpp b/kicad/kicad_manager_frame.cpp index 73c6ff0d74..281ba36fab 100644 --- a/kicad/kicad_manager_frame.cpp +++ b/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(); } diff --git a/pagelayout_editor/pl_editor_frame.cpp b/pagelayout_editor/pl_editor_frame.cpp index 51b2eb8506..efc674eeb2 100644 --- a/pagelayout_editor/pl_editor_frame.cpp +++ b/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(); } diff --git a/pcbnew/footprint_edit_frame.cpp b/pcbnew/footprint_edit_frame.cpp index f425d85a85..367381cb69 100644 --- a/pcbnew/footprint_edit_frame.cpp +++ b/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 ); } diff --git a/pcbnew/pcb_edit_frame.cpp b/pcbnew/pcb_edit_frame.cpp index 599fd0b054..917f27a79f 100644 --- a/pcbnew/pcb_edit_frame.cpp +++ b/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 ); }