From 8eca23aabe8736d07a0beeb0e2bb893320010290 Mon Sep 17 00:00:00 2001 From: Mikolaj Wielgus Date: Sat, 30 Jan 2021 23:47:39 +0100 Subject: [PATCH] Mark null project initial screen as zoom-initialized The variable `m_Initialized` in `BASE_SCREEN` is used by `SCH_EDIT_FRAME` to mark whether a screen had its zoom level initialized by the "zoom to fit screen" action. When this variable is `false`, the function `SCH_EDIT_FRAME::DisplayCurrentSheet()` performs "zoom to fit screen", modifying the zoom level. This function is indirectly called in the undo routines, so if `m_Initialized` is not set to `true`, a zoom change will occur when the user undoes an operation, a behavior that is undesired. `m_Initialized` was not initialized to `true` for the null schematic (the schematic that is loaded if no project is loaded), causing the aforementioned undesired behavior. To prevent this, I've changed the `SCH_EDIT_FRAME` constructor to set `m_Initialized` to `true`, since it zooms to fit screen already. I've moved `m_Initialized` from `BASE_SCREEN` to `SCH_SCREEN`, as it is used only in Eeschema, and renamed it to `m_zoomInitialized`, a name I believe that better describes what this variable does. I've also introduced the function `SCH_EDIT_FRAME::initScreenZoom()` to group the "zoom to fit screen" action with setting `m_Initialized` to `true`, as they often should occur together. I'd also like to say that I'm not confident whether `SCH_EDIT_FRAME::DisplayCurrentSheet()` should perform the zoom level initialization at this point, but I have decided to not change this behavior for now, as the commit history suggests it's several years old. Fixes https://gitlab.com/kicad/code/kicad/issues/7343 --- common/base_screen.cpp | 1 - eeschema/files-io.cpp | 9 +++------ eeschema/hierarch.cpp | 5 ++--- eeschema/sch_edit_frame.cpp | 9 ++++++++- eeschema/sch_edit_frame.h | 5 +++++ eeschema/sch_screen.cpp | 1 + eeschema/sch_screen.h | 5 ++++- include/base_screen.h | 2 -- 8 files changed, 23 insertions(+), 14 deletions(-) diff --git a/common/base_screen.cpp b/common/base_screen.cpp index d42f471663..bd64e7955f 100644 --- a/common/base_screen.cpp +++ b/common/base_screen.cpp @@ -35,7 +35,6 @@ wxString BASE_SCREEN::m_PageLayoutDescrFileName; // the name of the page layou BASE_SCREEN::BASE_SCREEN( EDA_ITEM* aParent, KICAD_T aType ) : EDA_ITEM( aParent, aType ) { - m_Initialized = false; m_virtualPageNumber = 1; m_pageCount = 1; // Hierarchy: Root: ScreenNumber = 1 m_Center = true; diff --git a/eeschema/files-io.cpp b/eeschema/files-io.cpp index a3e6594807..6705c4940b 100644 --- a/eeschema/files-io.cpp +++ b/eeschema/files-io.cpp @@ -535,13 +535,12 @@ bool SCH_EDIT_FRAME::OpenProjectFiles( const std::vector& aFileSet, in RecalculateConnections( GLOBAL_CLEANUP ); ClearUndoRedoList(); - GetScreen()->m_Initialized = true; } // Load any exclusions from the project file ResolveERCExclusions(); - m_toolManager->RunAction( ACTIONS::zoomFitScreen, true ); + initScreenZoom(); SetSheetNumberAndCount(); RecomputeIntersheetRefs(); @@ -600,7 +599,7 @@ bool SCH_EDIT_FRAME::AppendSchematic() if( !LoadSheetFromFile( GetCurrentSheet().Last(), &GetCurrentSheet(), fullFileName ) ) return false; - m_toolManager->RunAction( ACTIONS::zoomFitScreen, true ); + initScreenZoom(); SetSheetNumberAndCount(); SyncView(); @@ -961,8 +960,6 @@ bool SCH_EDIT_FRAME::importFile( const wxString& aFileName, int aFileType ) SCH_SCREENS schematic( Schematic().Root() ); schematic.UpdateSymbolLinks(); // Update all symbol library links for all sheets. - GetScreen()->m_Initialized = true; - for( SCH_SCREEN* screen = schematic.GetFirst(); screen; screen = schematic.GetNext() ) { for( auto item : screen->Items().OfType( SCH_COMPONENT_T ) ) @@ -988,7 +985,7 @@ bool SCH_EDIT_FRAME::importFile( const wxString& aFileName, int aFileType ) ClearUndoRedoList(); - m_toolManager->RunAction( ACTIONS::zoomFitScreen, true ); + initScreenZoom(); SetSheetNumberAndCount(); SyncView(); UpdateTitle(); diff --git a/eeschema/hierarch.cpp b/eeschema/hierarch.cpp index b1c4091260..57427b235e 100644 --- a/eeschema/hierarch.cpp +++ b/eeschema/hierarch.cpp @@ -300,10 +300,9 @@ void SCH_EDIT_FRAME::DisplayCurrentSheet() GetCurrentSheet().UpdateAllScreenReferences(); SetSheetNumberAndCount(); - if( !screen->m_Initialized ) + if( !screen->m_zoomInitialized ) { - m_toolManager->RunAction( ACTIONS::zoomFitScreen, true ); - screen->m_Initialized = true; + initScreenZoom(); } else { diff --git a/eeschema/sch_edit_frame.cpp b/eeschema/sch_edit_frame.cpp index 7a95c15316..3a74c8d63c 100644 --- a/eeschema/sch_edit_frame.cpp +++ b/eeschema/sch_edit_frame.cpp @@ -274,7 +274,7 @@ SCH_EDIT_FRAME::SCH_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ) : resolveCanvasType(); SwitchCanvas( m_canvasType ); - GetToolManager()->RunAction( ACTIONS::zoomFitScreen, true ); + initScreenZoom(); // This is used temporarily to fix a client size issue on GTK that causes zoom to fit // to calculate the wrong zoom size. See SCH_EDIT_FRAME::onSize(). @@ -1263,6 +1263,13 @@ void SCH_EDIT_FRAME::UpdateTitle() } +void SCH_EDIT_FRAME::initScreenZoom() +{ + m_toolManager->RunAction( ACTIONS::zoomFitScreen, true ); + GetScreen()->m_zoomInitialized = true; +} + + void SCH_EDIT_FRAME::RecalculateConnections( SCH_CLEANUP_FLAGS aCleanupFlags ) { SCHEMATIC_SETTINGS& settings = Schematic().Settings(); diff --git a/eeschema/sch_edit_frame.h b/eeschema/sch_edit_frame.h index 649801c403..e9e4318277 100644 --- a/eeschema/sch_edit_frame.h +++ b/eeschema/sch_edit_frame.h @@ -609,6 +609,11 @@ private: */ void UpdateTitle(); + /** + * Initialize the zoom value of the current screen and mark the screen as zoom-initialized. + */ + void initScreenZoom(); + /** * Verify that the symbol library links \a aSheet and all of it's child sheets have * been remapped to the symbol library table. diff --git a/eeschema/sch_screen.cpp b/eeschema/sch_screen.cpp index e1e0b4087b..c0924fca8e 100644 --- a/eeschema/sch_screen.cpp +++ b/eeschema/sch_screen.cpp @@ -68,6 +68,7 @@ SCH_SCREEN::SCH_SCREEN( EDA_ITEM* aParent ) : { m_modification_sync = 0; m_refCount = 0; + m_zoomInitialized = false; m_LastZoomLevel = 1.0; // Suitable for schematic only. For symbol_editor and viewlib, must be set to true diff --git a/eeschema/sch_screen.h b/eeschema/sch_screen.h index 1b299c629b..ba64563b5d 100644 --- a/eeschema/sch_screen.h +++ b/eeschema/sch_screen.h @@ -116,7 +116,10 @@ private: int m_modification_sync; // inequality with PART_LIBS::GetModificationHash() will // trigger ResolveAll(). - /// List of bus aliases stored in this screen + /// Set to true once the zoom value is initialized with `InitZoom()`. + bool m_zoomInitialized; + + /// List of bus aliases stored in this screen. std::unordered_set< std::shared_ptr< BUS_ALIAS > > m_aliases; /// Library symbols required for this schematic. diff --git a/include/base_screen.h b/include/base_screen.h index 3668fca1b4..17342ce85c 100644 --- a/include/base_screen.h +++ b/include/base_screen.h @@ -103,8 +103,6 @@ public: VECTOR2D m_ScrollCenter; ///< Current scroll center point in logical units. - bool m_Initialized; - protected: /** * The number of #BASE_SCREEN objects in this design.