← Back to team overview

kicad-developers team mailing list archive

[PATCH] Move ZoomFitScreen and ZoomPreset to common

 

Hi all,

This patch finishes off the refactor I did a few days ago, by enabling
ZoomFitScreen to work without knowledge of the BOARD class.

In order to make this work, I changed the way GetBoundingBox and
ComputeBoundingBox work so that the call sites of GetBoundingBox don't have
to also call ComputeBoundingBox if they don't need to use the
aBoardEdgesOnly flag.

Those with good knowledge of BOARD should review this to make sure I caught
all the instances where we should mark the bounding box as needing to be
re-computed.

Best,
Jon
From 153167789cffbedb8861f2832f640ef86f3d4505 Mon Sep 17 00:00:00 2001
From: Jon Evans <jon@xxxxxxxxxxxxx>
Date: Wed, 22 Feb 2017 23:31:26 -0500
Subject: [PATCH] Move ZoomFitScreen and ZoomPreset from PCBNEW_CONTROL to
 COMMON_TOOLS

In order to prevent call sites of BOARD::GetBoundingBox() needing to
call BOARD::ComputeBoundingBox(), also implement a dirty-flag in BOARD
that will re-compute the bounding box when board objects change.

This allows COMMON_TOOLS to implement ZoomFitScreen without knowledge
of the BOARD class.
---
 common/tool/common_tools.cpp    | 65 +++++++++++++++++++++++++++++++++++++++
 include/tool/common_tools.h     |  2 ++
 pcbnew/class_board.cpp          | 29 +++++++++++++++++-
 pcbnew/class_board.h            |  9 ++++--
 pcbnew/tools/pcbnew_control.cpp | 68 -----------------------------------------
 pcbnew/tools/pcbnew_control.h   |  4 ---
 6 files changed, 101 insertions(+), 76 deletions(-)

diff --git a/common/tool/common_tools.cpp b/common/tool/common_tools.cpp
index b5b7b07..9dbe2fd 100644
--- a/common/tool/common_tools.cpp
+++ b/common/tool/common_tools.cpp
@@ -98,6 +98,69 @@ int COMMON_TOOLS::ZoomCenter( const TOOL_EVENT& aEvent )
 }
 
 
+int COMMON_TOOLS::ZoomFitScreen( const TOOL_EVENT& aEvent )
+{
+    KIGFX::VIEW* view = getView();
+    EDA_DRAW_PANEL_GAL* galCanvas = m_frame->GetGalCanvas();
+    EDA_ITEM* model = getModel<EDA_ITEM>();
+
+    BOX2I boardBBox = model->ViewBBox();
+    VECTOR2D scrollbarSize = VECTOR2D( galCanvas->GetSize() - galCanvas->GetClientSize() );
+    VECTOR2D screenSize = view->ToWorld( galCanvas->GetClientSize(), false );
+
+    if( boardBBox.GetWidth() == 0 || boardBBox.GetHeight() == 0 )
+    {
+        // Empty view
+        view->SetScale( 17.0 );     // works fine for the standard worksheet frame
+
+        view->SetCenter( screenSize / 2.0 );
+    }
+    else
+    {
+        VECTOR2D vsize = boardBBox.GetSize();
+        double scale = view->GetScale() / std::max( fabs( vsize.x / screenSize.x ),
+                                                    fabs( vsize.y / screenSize.y ) );
+
+        view->SetScale( scale );
+        view->SetCenter( boardBBox.Centre() );
+    }
+
+
+    // Take scrollbars into account
+    VECTOR2D worldScrollbarSize = view->ToWorld( scrollbarSize, false );
+    view->SetCenter( view->GetCenter() + worldScrollbarSize / 2.0 );
+
+    return 0;
+}
+
+
+int COMMON_TOOLS::ZoomPreset( const TOOL_EVENT& aEvent )
+{
+    unsigned int idx = aEvent.Parameter<intptr_t>();
+    std::vector<double>& zoomList = m_frame->GetScreen()->m_ZoomList;
+    KIGFX::VIEW* view = m_frame->GetGalCanvas()->GetView();
+    KIGFX::GAL* gal = m_frame->GetGalCanvas()->GetGAL();
+
+    m_frame->SetPresetZoom( idx );
+
+    if( idx == 0 )      // Zoom Auto
+    {
+        return ZoomFitScreen( aEvent );
+    }
+    else if( idx >= zoomList.size() )
+    {
+        assert( false );
+        return 0;
+    }
+
+    double selectedZoom = zoomList[idx];
+    double zoomFactor = gal->GetWorldScale() / gal->GetZoomFactor();
+    view->SetScale( 1.0 / ( zoomFactor * selectedZoom ) );
+
+    return 0;
+}
+
+
 // Grid control
 int COMMON_TOOLS::GridNext( const TOOL_EVENT& aEvent )
 {
@@ -135,6 +198,8 @@ void COMMON_TOOLS::SetTransitions()
     Go( &COMMON_TOOLS::ZoomInOutCenter,    ACTIONS::zoomInCenter.MakeEvent() );
     Go( &COMMON_TOOLS::ZoomInOutCenter,    ACTIONS::zoomOutCenter.MakeEvent() );
     Go( &COMMON_TOOLS::ZoomCenter,         ACTIONS::zoomCenter.MakeEvent() );
+    Go( &COMMON_TOOLS::ZoomFitScreen,      ACTIONS::zoomFitScreen.MakeEvent() );
+    Go( &COMMON_TOOLS::ZoomPreset,         ACTIONS::zoomPreset.MakeEvent() );
 
     Go( &COMMON_TOOLS::GridNext,           ACTIONS::gridNext.MakeEvent() );
     Go( &COMMON_TOOLS::GridPrev,           ACTIONS::gridPrev.MakeEvent() );
diff --git a/include/tool/common_tools.h b/include/tool/common_tools.h
index c37d0cd..9f60402 100644
--- a/include/tool/common_tools.h
+++ b/include/tool/common_tools.h
@@ -48,6 +48,8 @@ public:
     int ZoomInOut( const TOOL_EVENT& aEvent );
     int ZoomInOutCenter( const TOOL_EVENT& aEvent );
     int ZoomCenter( const TOOL_EVENT& aEvent );
+    int ZoomFitScreen( const TOOL_EVENT& aEvent );
+    int ZoomPreset( const TOOL_EVENT& aEvent );
 
     // Grid control
     int GridNext( const TOOL_EVENT& aEvent );
diff --git a/pcbnew/class_board.cpp b/pcbnew/class_board.cpp
index 4d672ab..05d41d2 100644
--- a/pcbnew/class_board.cpp
+++ b/pcbnew/class_board.cpp
@@ -107,6 +107,8 @@ BOARD::BOARD() :
 
     // Initialize ratsnest
     m_ratsnest = new RN_DATA( this );
+
+    m_boundingBoxInvalid = true;
 }
 
 
@@ -173,6 +175,8 @@ void BOARD::Move( const wxPoint& aMoveVector )        // overload
     };
 
     Visit( inspector, NULL, top_level_board_stuff );
+
+    m_boundingBoxInvalid = true;
 }
 
 
@@ -943,6 +947,7 @@ void BOARD::Add( BOARD_ITEM* aBoardItem, ADD_MODE aMode )
 
     aBoardItem->SetParent( this );
     m_ratsnest->Add( aBoardItem );
+    m_boundingBoxInvalid = true;
 }
 
 
@@ -1012,6 +1017,7 @@ void BOARD::Remove( BOARD_ITEM* aBoardItem )
     }
 
     m_ratsnest->Remove( aBoardItem );
+    m_boundingBoxInvalid = true;
 }
 
 
@@ -1054,7 +1060,7 @@ unsigned BOARD::GetNodesCount() const
 }
 
 
-EDA_RECT BOARD::ComputeBoundingBox( bool aBoardEdgesOnly )
+EDA_RECT BOARD::ComputeBoundingBox( bool aBoardEdgesOnly ) const
 {
     bool hasItems = false;
     EDA_RECT area;
@@ -1125,10 +1131,22 @@ EDA_RECT BOARD::ComputeBoundingBox( bool aBoardEdgesOnly )
 
     m_BoundingBox = area;   // save for BOARD::GetBoundingBox()
 
+    m_boundingBoxInvalid = false;
+
     return area;
 }
 
 
+const EDA_RECT BOARD::GetBoundingBox() const
+{
+    if( m_boundingBoxInvalid )
+    {
+        ComputeBoundingBox();
+    }
+    return m_BoundingBox;
+}
+
+
 // virtual, see pcbstruct.h
 void BOARD::GetMsgPanelInfo( std::vector< MSG_PANEL_ITEM >& aList )
 {
@@ -2328,6 +2346,8 @@ ZONE_CONTAINER* BOARD::AddArea( PICKED_ITEMS_LIST* aNewZonesList, int aNetcode,
         aNewZonesList->PushItem( picker );
     }
 
+    m_boundingBoxInvalid = true;
+
     return new_area;
 }
 
@@ -2347,6 +2367,8 @@ void BOARD::RemoveArea( PICKED_ITEMS_LIST* aDeletedList, ZONE_CONTAINER* area_to
     {
         Delete( area_to_remove );
     }
+
+    m_boundingBoxInvalid = true;
 }
 
 
@@ -2364,6 +2386,9 @@ ZONE_CONTAINER* BOARD::InsertArea( int netcode, int iarea, LAYER_ID layer, int x
         m_ZoneDescriptorList.push_back( new_area );
 
     new_area->Outline()->Start( layer, x, y, hatch );
+
+    m_boundingBoxInvalid = true;
+
     return new_area;
 }
 
@@ -2835,6 +2860,8 @@ void BOARD::ReplaceNetlist( NETLIST& aNetlist, bool aDeleteSinglePadNets,
     }
 
     std::swap( newFootprints, *aNewFootprints );
+
+    m_boundingBoxInvalid = true;
 }
 
 
diff --git a/pcbnew/class_board.h b/pcbnew/class_board.h
index 1045e6d..7d31d5a 100644
--- a/pcbnew/class_board.h
+++ b/pcbnew/class_board.h
@@ -185,7 +185,7 @@ private:
 
     int                     m_fileFormatVersionAtLoad;  ///< the version loaded from the file
 
-    EDA_RECT                m_BoundingBox;
+    mutable EDA_RECT        m_BoundingBox;
     NETINFO_LIST            m_NetInfo;              ///< net info list (name, design constraints ..
     RN_DATA*                m_ratsnest;
 
@@ -202,6 +202,9 @@ private:
     /// Number of unconnected nets in the current rats nest.
     int                     m_unconnectedNetCount;
 
+    ///> True if the board's bounding box needs to be recomputed.
+    mutable bool            m_boundingBoxInvalid;
+
     /**
      * Function chainMarkedSegments
      * is used by MarkTrace() to set the BUSY flag of connected segments of the trace
@@ -825,7 +828,7 @@ public:
      * @return EDA_RECT - the board's bounding box
      * @see PCB_BASE_FRAME::GetBoardBoundingBox() which calls this and doctors the result
      */
-    EDA_RECT ComputeBoundingBox( bool aBoardEdgesOnly = false );
+    EDA_RECT ComputeBoundingBox( bool aBoardEdgesOnly = false ) const;
 
     /**
      * Function GetBoundingBox
@@ -833,7 +836,7 @@ public:
      * as long as the BOARD has not changed.  Remember, ComputeBoundingBox()'s
      * aBoardEdgesOnly argument is considered in this return value also.
      */
-    const EDA_RECT GetBoundingBox() const override { return m_BoundingBox; }
+    const EDA_RECT GetBoundingBox() const override;
 
     void SetBoundingBox( const EDA_RECT& aBox ) { m_BoundingBox = aBox; }
 
diff --git a/pcbnew/tools/pcbnew_control.cpp b/pcbnew/tools/pcbnew_control.cpp
index fabc005..42e5122 100644
--- a/pcbnew/tools/pcbnew_control.cpp
+++ b/pcbnew/tools/pcbnew_control.cpp
@@ -253,70 +253,6 @@ void PCBNEW_CONTROL::Reset( RESET_REASON aReason )
 }
 
 
-int PCBNEW_CONTROL::ZoomFitScreen( const TOOL_EVENT& aEvent )
-{
-    KIGFX::VIEW* view = getView();
-    EDA_DRAW_PANEL_GAL* galCanvas = m_frame->GetGalCanvas();
-    BOARD* board = getModel<BOARD>();
-    board->ComputeBoundingBox();
-
-    BOX2I boardBBox = board->ViewBBox();
-    VECTOR2D scrollbarSize = VECTOR2D( galCanvas->GetSize() - galCanvas->GetClientSize() );
-    VECTOR2D screenSize = view->ToWorld( galCanvas->GetClientSize(), false );
-
-    if( boardBBox.GetWidth() == 0 || boardBBox.GetHeight() == 0 )
-    {
-        // Empty view
-        view->SetScale( 17.0 );     // works fine for the standard worksheet frame
-
-        view->SetCenter( screenSize / 2.0 );
-    }
-    else
-    {
-        VECTOR2D vsize = boardBBox.GetSize();
-        double scale = view->GetScale() / std::max( fabs( vsize.x / screenSize.x ),
-                                                    fabs( vsize.y / screenSize.y ) );
-
-        view->SetScale( scale );
-        view->SetCenter( boardBBox.Centre() );
-    }
-
-
-    // Take scrollbars into account
-    VECTOR2D worldScrollbarSize = view->ToWorld( scrollbarSize, false );
-    view->SetCenter( view->GetCenter() + worldScrollbarSize / 2.0 );
-
-    return 0;
-}
-
-
-int PCBNEW_CONTROL::ZoomPreset( const TOOL_EVENT& aEvent )
-{
-    unsigned int idx = aEvent.Parameter<intptr_t>();
-    std::vector<double>& zoomList = m_frame->GetScreen()->m_ZoomList;
-    KIGFX::VIEW* view = m_frame->GetGalCanvas()->GetView();
-    KIGFX::GAL* gal = m_frame->GetGalCanvas()->GetGAL();
-
-    m_frame->SetPresetZoom( idx );
-
-    if( idx == 0 )      // Zoom Auto
-    {
-        return ZoomFitScreen( aEvent );
-    }
-    else if( idx >= zoomList.size() )
-    {
-        assert( false );
-        return 0;
-    }
-
-    double selectedZoom = zoomList[idx];
-    double zoomFactor = gal->GetWorldScale() / gal->GetZoomFactor();
-    view->SetScale( 1.0 / ( zoomFactor * selectedZoom ) );
-
-    return 0;
-}
-
-
 int PCBNEW_CONTROL::TrackDisplayMode( const TOOL_EVENT& aEvent )
 {
     auto painter = static_cast<KIGFX::PCB_PAINTER*>( getView()->GetPainter() );
@@ -1000,10 +936,6 @@ int PCBNEW_CONTROL::ToBeDone( const TOOL_EVENT& aEvent )
 
 void PCBNEW_CONTROL::SetTransitions()
 {
-    // View controls
-    Go( &PCBNEW_CONTROL::ZoomFitScreen,      ACTIONS::zoomFitScreen.MakeEvent() );
-    Go( &PCBNEW_CONTROL::ZoomPreset,         ACTIONS::zoomPreset.MakeEvent() );
-
     // Display modes
     Go( &PCBNEW_CONTROL::TrackDisplayMode,   PCB_ACTIONS::trackDisplayMode.MakeEvent() );
     Go( &PCBNEW_CONTROL::PadDisplayMode,     PCB_ACTIONS::padDisplayMode.MakeEvent() );
diff --git a/pcbnew/tools/pcbnew_control.h b/pcbnew/tools/pcbnew_control.h
index 6ca4ee1..f48db51 100644
--- a/pcbnew/tools/pcbnew_control.h
+++ b/pcbnew/tools/pcbnew_control.h
@@ -47,10 +47,6 @@ public:
     /// @copydoc TOOL_INTERACTIVE::Reset()
     void Reset( RESET_REASON aReason ) override;
 
-    // View controls
-    int ZoomFitScreen( const TOOL_EVENT& aEvent );
-    int ZoomPreset( const TOOL_EVENT& aEvent );
-
     // Display modes
     int TrackDisplayMode( const TOOL_EVENT& aEvent );
     int PadDisplayMode( const TOOL_EVENT& aEvent );
-- 
2.7.4


Follow ups