Fix off-by-1 errors in zoom menus & use zoom steps in GAL canvas.

Also bumps the maximums a bit (at user request).

Fixes: lp:1773215
* https://bugs.launchpad.net/kicad/+bug/1773215

common/draw_frame.cpp

@@ -512,8 +512,7 @@ void EDA_DRAW_FRAME::OnSelectZoom( wxCommandEvent& event )
     }
     else
     {
-        id--;
+        double selectedZoom = GetScreen()->m_ZoomList[id-1];
-        double selectedZoom = GetScreen()->m_ZoomList[id];
 
         if( GetScreen()->GetZoom() == selectedZoom )
             return;

common/tool/common_tools.cpp

@@ -59,37 +59,54 @@ void COMMON_TOOLS::Reset( RESET_REASON aReason )
 
 int COMMON_TOOLS::ZoomInOut( const TOOL_EVENT& aEvent )
 {
-    KIGFX::VIEW* view = m_frame->GetGalCanvas()->GetView();
+    bool direction = aEvent.IsAction( &ACTIONS::zoomIn );
-    KIGFX::VIEW_CONTROLS* ctls = getViewControls();
+    return doZoomInOut( direction, true );
-    double zoomScale = 1.0;
-
-    if( aEvent.IsAction( &ACTIONS::zoomIn ) )
-        zoomScale = 1.3;
-    else if( aEvent.IsAction( &ACTIONS::zoomOut ) )
-        zoomScale = 1/1.3;
-
-    view->SetScale( view->GetScale() * zoomScale, getViewControls()->GetCursorPosition() );
-
-    if( ctls->IsCursorWarpingEnabled() )
-        ctls->CenterOnCursor();
-
-    return 0;
 }
 
 
 int COMMON_TOOLS::ZoomInOutCenter( const TOOL_EVENT& aEvent )
 {
-    KIGFX::VIEW* view = getView();
+    bool direction = aEvent.IsAction( &ACTIONS::zoomInCenter );
-    double zoomScale = 1.0;
+    return doZoomInOut( direction, false );
+}
 
-    if( aEvent.IsAction( &ACTIONS::zoomInCenter ) )
-        zoomScale = 1.3;
-    else if( aEvent.IsAction( &ACTIONS::zoomOutCenter ) )
-        zoomScale = 1/1.3;
 
-    view->SetScale( view->GetScale() * zoomScale );
+int COMMON_TOOLS::doZoomInOut( bool aDirection, bool aCenterOnCursor )
+{
+    double zoom = m_frame->GetGalCanvas()->GetLegacyZoom();
 
-    return 0;
+    // Step must be AT LEAST 1.3
+    if( aDirection )
+        zoom /= 1.3;
+    else
+        zoom *= 1.3;
+
+    // Now look for the next closest menu step
+    std::vector<double>& zoomList = m_frame->GetScreen()->m_ZoomList;
+    int idx;
+
+    if( aDirection )
+    {
+        for( idx = zoomList.size() - 1; idx >= 0; --idx )
+        {
+            if( zoomList[idx] <= zoom )
+                break;
+        }
+        if( idx < 0 )
+            idx = 0;        // if we ran off the end then peg to the end
+    }
+    else
+    {
+        for( idx = 0; idx < zoomList.size(); ++idx )
+        {
+            if( zoomList[idx] >= zoom )
+                break;
+        }
+        if( idx >= zoomList.size() )
+            idx = zoomList.size() - 1;        // if we ran off the end then peg to the end
+    }
+
+    return doZoomToPreset( idx + 1, aCenterOnCursor );
 }
 
 
@@ -148,6 +165,13 @@ int COMMON_TOOLS::ZoomFitScreen( const TOOL_EVENT& aEvent )
 int COMMON_TOOLS::ZoomPreset( const TOOL_EVENT& aEvent )
 {
     unsigned int idx = aEvent.Parameter<intptr_t>();
+    return doZoomToPreset( idx, false );
+}
+
+
+// Note: idx == 0 is Auto; idx == 1 is first entry in zoomList
+int COMMON_TOOLS::doZoomToPreset( int idx, bool aCenterOnCursor )
+{
     std::vector<double>& zoomList = m_frame->GetScreen()->m_ZoomList;
     KIGFX::VIEW* view = m_frame->GetGalCanvas()->GetView();
     KIGFX::GAL* gal = m_frame->GetGalCanvas()->GetGAL();
@@ -156,17 +180,25 @@ int COMMON_TOOLS::ZoomPreset( const TOOL_EVENT& aEvent )
 
     if( idx == 0 )      // Zoom Auto
     {
-        return ZoomFitScreen( aEvent );
+        TOOL_EVENT dummy;
-    }
+        return ZoomFitScreen( dummy );
-    else if( idx >= zoomList.size() )
-    {
-        assert( false );
-        return 0;
     }
+    else
+        idx--;
 
     double selectedZoom = zoomList[idx];
     double zoomFactor = gal->GetWorldScale() / gal->GetZoomFactor();
-    view->SetScale( 1.0 / ( zoomFactor * selectedZoom ) );
+    double scale = 1.0 / ( zoomFactor * selectedZoom );
+
+    if( aCenterOnCursor )
+    {
+        view->SetScale( scale, getViewControls()->GetCursorPosition() );
+
+        if( getViewControls()->IsCursorWarpingEnabled() )
+            getViewControls()->CenterOnCursor();
+    }
+    else
+        view->SetScale( scale );
 
     return 0;
 }

common/tool/zoom_menu.cpp

@@ -45,7 +45,7 @@ ZOOM_MENU::ZOOM_MENU( EDA_DRAW_FRAME* aParent ) : m_parent( aParent )
 
     for( int i = 0; i < maxZoomIds; ++i )
     {
-        Append( ID_POPUP_ZOOM_LEVEL_START + i,
+        Append( ID_POPUP_ZOOM_LEVEL_START+1 + i,   // ID_POPUP_ZOOM_LEVEL_START == Auto
             wxString::Format( _( "Zoom: %.2f" ), aParent->GetZoomLevelCoeff() / screen->m_ZoomList[i] ),
             wxEmptyString, wxITEM_CHECK );
     }
@@ -64,10 +64,14 @@ OPT_TOOL_EVENT ZOOM_MENU::eventHandler( const wxMenuEvent& aEvent )
 
 void ZOOM_MENU::update()
 {
-    double zoom = m_parent->GetScreen()->GetZoom();
+    BASE_SCREEN* screen = m_parent->GetScreen();
+    double zoom = screen->GetZoom();
     const std::vector<double>& zoomList = m_parent->GetScreen()->m_ZoomList;
 
     // Check the current zoom
-    for( unsigned int i = 0; i < GetMenuItemCount(); ++i )
+    int maxZoomIds = std::min( ID_POPUP_ZOOM_LEVEL_END - ID_POPUP_ZOOM_LEVEL_START,
-        Check( ID_POPUP_ZOOM_LEVEL_START + i, std::fabs( zoomList[i] - zoom ) < 1e-6 );
+                               (int) screen->m_ZoomList.size() );
+
+    for( int i = 0; i < maxZoomIds; ++i )
+        Check( ID_POPUP_ZOOM_LEVEL_START+1 + i, std::fabs( zoomList[i] - zoom ) < 1e-6 );
 }

common/view/view.cpp

@@ -283,7 +283,7 @@ void VIEW::OnDestroy( VIEW_ITEM* aItem )
 VIEW::VIEW( bool aIsDynamic ) :
     m_enableOrderModifier( true ),
     m_scale( 4.0 ),
-    m_minScale( 4.0 ), m_maxScale( 15000 ),
+    m_minScale( 1.0 ), m_maxScale( 75000.0 ),
     m_mirrorX( false ), m_mirrorY( false ),
     m_painter( NULL ),
     m_gal( NULL ),

gerbview/gbr_screen.cpp

@@ -45,14 +45,14 @@
 */
 static const double gbrZoomList[] =
 {
+    ZOOM_FACTOR( 0.022 ),
+    ZOOM_FACTOR( 0.035 ),
     ZOOM_FACTOR( 0.05 ),
-    ZOOM_FACTOR( 0.075 ),
+    ZOOM_FACTOR( 0.08 ),
-    ZOOM_FACTOR( 0.1 ),
+    ZOOM_FACTOR( 0.13 ),
-    ZOOM_FACTOR( 0.15 ),
+    ZOOM_FACTOR( 0.22 ),
-    ZOOM_FACTOR( 0.2 ),
+    ZOOM_FACTOR( 0.35 ),
-    ZOOM_FACTOR( 0.3 ),
+    ZOOM_FACTOR( 0.6 ),
-    ZOOM_FACTOR( 0.44 ),
-    ZOOM_FACTOR( 0.66 ),
     ZOOM_FACTOR( 1.0 ),
     ZOOM_FACTOR( 1.5 ),
     ZOOM_FACTOR( 2.2 ),
@@ -65,9 +65,7 @@ static const double gbrZoomList[] =
     ZOOM_FACTOR( 50.0 ),
     ZOOM_FACTOR( 80.0 ),
     ZOOM_FACTOR( 130.0 ),
-    ZOOM_FACTOR( 200.0 ),
+    ZOOM_FACTOR( 220.0 )
-    ZOOM_FACTOR( 350.0 ),
-    ZOOM_FACTOR( 500.0 )
 };
 
 

include/tool/common_tools.h

@@ -66,6 +66,11 @@ private:
     ///> Pointer to the currently used edit frame.
     EDA_DRAW_FRAME* m_frame;
 
+    int doZoomInOut( bool aDirection, bool aCenterOnCursor );
+
+    ///> Note: idx == 0 is Auto; idx == 1 is first entry in zoomList
+    int doZoomToPreset( int idx, bool aCenterOnCursor );
+
     ///> Applies the legacy canvas grid settings for GAL.
     void updateGrid();
 };

pcbnew/pcb_base_frame.cpp

@@ -676,12 +676,14 @@ void PCB_BASE_FRAME::OnUpdateSelectZoom( wxUpdateUIEvent& aEvent )
     if( m_zoomSelectBox == NULL || m_auxiliaryToolBar == NULL )
         return;
 
-    int current = 0;
+    int current = 0;    // display Auto if no match found
+
+    // check for a match within 1%
     double zoom = IsGalCanvasActive() ? GetGalCanvas()->GetLegacyZoom() : GetScreen()->GetZoom();
 
     for( unsigned i = 0; i < GetScreen()->m_ZoomList.size(); i++ )
     {
-        if( std::fabs( zoom - GetScreen()->m_ZoomList[i] ) < 1e-6 )
+        if( std::fabs( zoom - GetScreen()->m_ZoomList[i] ) < ( zoom / 100.0 ) )
         {
             current = i + 1;
             break;

pcbnew/pcb_screen.cpp

@@ -60,27 +60,27 @@
 
 static const double pcbZoomList[] =
 {
-    ZOOM_FACTOR( 0.1 ),
+    ZOOM_FACTOR( 0.035 ),
-    ZOOM_FACTOR( 0.2 ),
+    ZOOM_FACTOR( 0.05 ),
-    ZOOM_FACTOR( 0.3 ),
+    ZOOM_FACTOR( 0.08 ),
-    ZOOM_FACTOR( 0.5 ),
+    ZOOM_FACTOR( 0.13 ),
+    ZOOM_FACTOR( 0.22 ),
+    ZOOM_FACTOR( 0.35 ),
+    ZOOM_FACTOR( 0.6 ),
     ZOOM_FACTOR( 1.0 ),
     ZOOM_FACTOR( 1.5 ),
-    ZOOM_FACTOR( 2.0 ),
+    ZOOM_FACTOR( 2.2 ),
-    ZOOM_FACTOR( 3.0 ),
+    ZOOM_FACTOR( 3.5 ),
-    ZOOM_FACTOR( 4.5 ),
+    ZOOM_FACTOR( 5.0 ),
-    ZOOM_FACTOR( 6.0 ),
     ZOOM_FACTOR( 8.0 ),
-    ZOOM_FACTOR( 11.0 ),
+    ZOOM_FACTOR( 13.0 ),
-    ZOOM_FACTOR( 15.0 ),
+    ZOOM_FACTOR( 20.0 ),
-    ZOOM_FACTOR( 22.0 ),
     ZOOM_FACTOR( 35.0 ),
     ZOOM_FACTOR( 50.0 ),
     ZOOM_FACTOR( 80.0 ),
-    ZOOM_FACTOR( 110.0 ),
+    ZOOM_FACTOR( 130.0 ),
-    ZOOM_FACTOR( 150.0 ),
+    ZOOM_FACTOR( 220.0 ),
-    ZOOM_FACTOR( 200.0 ),
+    ZOOM_FACTOR( 350.0 )
-    ZOOM_FACTOR( 300.0 ),
 /*
     The largest distance that wx can support is INT_MAX, since it represents
     distance often in a wxCoord or wxSize. As a scalar, a distance is always