kicad-developers team mailing list archive

[Maciej Sumiński <maciej.suminski@xxxxxxx>]

Hi John,

Thank you very much, I see lots of great improvements here. Your branch
is a good candidate for merge, but there is one problem. If I type a
value in INCREMENT_TEXT_CTRL and hit Enter, pcbnew segfaults. I suspect
that kill focus event handler is executed after the window is gone, but
I am not fully sure.

There are also a few minor patches, see the attachments.

Cheers,
Orson

On 02/16/2017 07:01 AM, John Beard wrote:
> Hi,
> 
> Here is a branch with a set of patches to add the ability to set the
> grid line/dot size and the minimum grid spacing (to avoid very dense
> grids when using thicker lines).
> 
> https://code.launchpad.net/~john-j-beard/kicad/+git/kicad/+ref/grid_dot_size
> 
> This also includes moving the grid display settings our of the Set
> Grid dialog in to the Display Preferences dialog. As an item in the
> Dimensions menu, Set Grid isn't really a suitable place, especially as
> there are now more options.
> 
> This was a fairly annoying change to make in the code, as the current
> GAL properties structure was only consumed by the OpenGL GAL canvas.
> However, now, all GAL canvases have access to it, which opens the way
> for more display options to be added easily (though if the Display
> Options dialog gets more options, we might need to put some tabs in,
> like Eeschema).
> 
> There are also some supporting infrastructure changes:
> 
> * A class to bind a text entry and spin buttons together without a lot
> of verbose repeating of event bindings.
> * Utility functions to use a mapping table to decouple internal enum
> values from the values written into config files and radio button
> selections
> * Over-visible header causing slow recompilation, hidden a bit better
> 
> Cheers,
> 
> John
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
> 

From 34ff0b241e462f1f86faf6635a03275491abc958 Mon Sep 17 00:00:00 2001
From: Maciej Suminski <maciej.suminski@xxxxxxx>
Date: Thu, 16 Feb 2017 16:31:55 +0100
Subject: [PATCH 1/3] Code formatting

---
 common/gal/graphics_abstraction_layer.cpp |  3 ++-
 common/incremental_text_ctrl.cpp          |  6 ++----
 include/config_map.h                      |  4 ++--
 include/gal/cairo/cairo_gal.h             |  2 +-
 include/gal/gal_display_options.h         |  6 +++---
 include/gal/graphics_abstraction_layer.h  |  2 +-
 pcbnew/dialogs/dialog_display_options.cpp | 10 +++++-----
 pcbnew/dialogs/dialog_display_options.h   |  4 ++--
 8 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/common/gal/graphics_abstraction_layer.cpp b/common/gal/graphics_abstraction_layer.cpp
index 08017995c..b579f8aff 100644
--- a/common/gal/graphics_abstraction_layer.cpp
+++ b/common/gal/graphics_abstraction_layer.cpp
@@ -2,7 +2,7 @@
  * This program source code file is part of KICAD, a free EDA CAD application.
  *
  * Copyright (C) 2012 Torsten Hueter, torstenhtr <at> gmx.de
- * Copyright (C) 2012 Kicad Developers, see change_log.txt for contributors.
+ * Copyright (C) 2012-2017 Kicad Developers, see change_log.txt for contributors.
  *
  * Graphics Abstraction Layer (GAL) - base class
  *
@@ -79,6 +79,7 @@ GAL::~GAL()
 {
 }
 
+
 void GAL::OnGalDisplayOptionsChanged( const GAL_DISPLAY_OPTIONS& aOptions )
 {
     // defer to the child class first
diff --git a/common/incremental_text_ctrl.cpp b/common/incremental_text_ctrl.cpp
index 2ab5ecbdf..23261ce61 100644
--- a/common/incremental_text_ctrl.cpp
+++ b/common/incremental_text_ctrl.cpp
@@ -1,8 +1,7 @@
 /*
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
- * Copyright (C) 2015 Jean-Pierre Charras, jean-pierre.charras at wanadoo.fr
- * Copyright (C) 1992-2016 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2017 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -37,8 +36,7 @@ static bool validateFloatField( const wxString& aStr )
     // a single . or , doesn't count as number, although valid in a float
     if( aStr.size() == 1 )
     {
-        if( (aStr.compare( "." ) == 0) ||
-            (aStr.compare( "," ) == 0) )
+        if( ( aStr.compare( "." ) == 0 ) || ( aStr.compare( "," ) == 0 ) )
             return false;
     }
 
diff --git a/include/config_map.h b/include/config_map.h
index 2c7ae91bd..53e05749d 100644
--- a/include/config_map.h
+++ b/include/config_map.h
@@ -24,7 +24,7 @@
 #ifndef CONFIG_MAP__H_
 #define CONFIG_MAP__H_
 
-#include <map>
+#include <vector>
 
 namespace UTIL
 {
@@ -32,7 +32,7 @@ namespace UTIL
 /**
  * A config value table is a list of native values (usually enums)
  * to a different set of values, for example, the values used to
- * represent the enum in a config file, or the index used to represnet
+ * represent the enum in a config file, or the index used to represent
  * it in a selection list.
  *
  * It can be important to decouple from the internal representation,
diff --git a/include/gal/cairo/cairo_gal.h b/include/gal/cairo/cairo_gal.h
index d661a247c..fa46bba89 100644
--- a/include/gal/cairo/cairo_gal.h
+++ b/include/gal/cairo/cairo_gal.h
@@ -353,7 +353,7 @@ private:
 
     int wxBufferWidth;
 
-    ///< Cairo-specific update handlers
+    ///> Cairo-specific update handlers
     bool updatedGalDisplayOptions( const GAL_DISPLAY_OPTIONS& aOptions ) override;
 
     void flushPath();
diff --git a/include/gal/gal_display_options.h b/include/gal/gal_display_options.h
index a5f103c54..83868c6d0 100644
--- a/include/gal/gal_display_options.h
+++ b/include/gal/gal_display_options.h
@@ -72,13 +72,13 @@ namespace KIGFX
 
         OPENGL_ANTIALIASING_MODE gl_antialiasing_mode;
 
-        ///< The grid style to draw the grid in
+        ///> The grid style to draw the grid in
         KIGFX::GRID_STYLE m_gridStyle;
 
-        ///< Thickness to render grid lines/dots
+        ///> Thickness to render grid lines/dots
         double m_gridLineWidth;
 
-        ///< Minimum pixel distance between displayed grid lines
+        ///> Minimum pixel distance between displayed grid lines
         double m_gridMinSpacing;
     };
 
diff --git a/include/gal/graphics_abstraction_layer.h b/include/gal/graphics_abstraction_layer.h
index 5d946e1d1..8c00d511c 100644
--- a/include/gal/graphics_abstraction_layer.h
+++ b/include/gal/graphics_abstraction_layer.h
@@ -2,7 +2,7 @@
  * This program source code file is part of KICAD, a free EDA CAD application.
  *
  * Copyright (C) 2012 Torsten Hueter, torstenhtr <at> gmx.de
- * Copyright (C) 2016 Kicad Developers, see change_log.txt for contributors.
+ * Copyright (C) 2016-2017 Kicad Developers, see change_log.txt for contributors.
  *
  * Graphics Abstraction Layer (GAL) - base class
  *
diff --git a/pcbnew/dialogs/dialog_display_options.cpp b/pcbnew/dialogs/dialog_display_options.cpp
index fda112891..d9b85a37b 100644
--- a/pcbnew/dialogs/dialog_display_options.cpp
+++ b/pcbnew/dialogs/dialog_display_options.cpp
@@ -2,7 +2,7 @@
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
  * Copyright (C) 2015 Jean-Pierre Charras, jean-pierre.charras at wanadoo.fr
- * Copyright (C) 1992-2016 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 1992-2017 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -98,14 +98,14 @@ DIALOG_DISPLAY_OPTIONS::DIALOG_DISPLAY_OPTIONS( PCB_EDIT_FRAME* parent ) :
 
     // bind the spin button and text box
     m_gridSizeIncrementer = std::make_unique<SPIN_INCREMENTAL_TEXT_CTRL>(
-                *m_gridLineWidthSpinBtn, *m_gridLineWidth);
+                *m_gridLineWidthSpinBtn, *m_gridLineWidth );
 
     m_gridSizeIncrementer->SetStep( gridThicknessMin, gridThicknessMax,
                                     gridThicknessStep );
     m_gridSizeIncrementer->SetPrecision( 1 );
 
     m_gridMinSpacingIncrementer = std::make_unique<SPIN_INCREMENTAL_TEXT_CTRL>(
-                *m_gridMinSpacingSpinBtn, *m_gridMinSpacing);
+                *m_gridMinSpacingSpinBtn, *m_gridMinSpacing );
 
     m_gridMinSpacingIncrementer->SetStep( gridMinSpacingMin, gridMinSpacingMax,
                                           gridMinSpacingStep );
@@ -164,7 +164,7 @@ void DIALOG_DISPLAY_OPTIONS::OnCancelClick( wxCommandEvent& event )
 
 /* Update variables with new options
 */
-void DIALOG_DISPLAY_OPTIONS::OnOkClick(wxCommandEvent& event)
+void DIALOG_DISPLAY_OPTIONS::OnOkClick( wxCommandEvent& event )
 {
     DISPLAY_OPTIONS* displ_opts = (DISPLAY_OPTIONS*)m_Parent->GetDisplayOptions();
     KIGFX::GAL_DISPLAY_OPTIONS& gal_opts = m_Parent->GetGalDisplayOptions();
@@ -186,7 +186,7 @@ void DIALOG_DISPLAY_OPTIONS::OnOkClick(wxCommandEvent& event)
 
     displ_opts->m_DisplayPadNum = m_OptDisplayPadNumber->GetValue();
 
-    m_Parent->SetElementVisibility( PCB_VISIBLE(NO_CONNECTS_VISIBLE),
+    m_Parent->SetElementVisibility( PCB_VISIBLE( NO_CONNECTS_VISIBLE ),
                                     m_OptDisplayPadNoConn->GetValue() );
 
     displ_opts->m_DisplayDrawItemsFill = not m_OptDisplayDrawings->GetValue();
diff --git a/pcbnew/dialogs/dialog_display_options.h b/pcbnew/dialogs/dialog_display_options.h
index 9a3fa720c..61006771f 100644
--- a/pcbnew/dialogs/dialog_display_options.h
+++ b/pcbnew/dialogs/dialog_display_options.h
@@ -2,7 +2,7 @@
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
  * Copyright (C) 2010-2014 Jean-Pierre Charras, jean-pierre.charras at wanadoo.fr
- * Copyright (C) 1992-2014 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 1992-2017 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -41,7 +41,7 @@ private:
 
 public:
    DIALOG_DISPLAY_OPTIONS( PCB_EDIT_FRAME* parent );
-   ~DIALOG_DISPLAY_OPTIONS( ) { };
+   ~DIALOG_DISPLAY_OPTIONS() {}
    void OnOkClick( wxCommandEvent& event ) override;
    void OnCancelClick( wxCommandEvent& event ) override;
 };
-- 
2.11.0

From 221a14d2a93875e0a6623a30d02d10fd4a8fd6c0 Mon Sep 17 00:00:00 2001
From: Maciej Suminski <maciej.suminski@xxxxxxx>
Date: Thu, 16 Feb 2017 17:11:53 +0100
Subject: [PATCH 2/3] Assert to verify min & max values for
 INCREMENTAL_TEXT_CTRL

---
 common/incremental_text_ctrl.cpp | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/common/incremental_text_ctrl.cpp b/common/incremental_text_ctrl.cpp
index 23261ce61..2c80f358c 100644
--- a/common/incremental_text_ctrl.cpp
+++ b/common/incremental_text_ctrl.cpp
@@ -52,11 +52,12 @@ INCREMENTAL_TEXT_CTRL::INCREMENTAL_TEXT_CTRL() :
 {}
 
 
-void INCREMENTAL_TEXT_CTRL::SetStep( double aMin, double aMax,
-    STEP_FUNCTION aStepFunc )
+void INCREMENTAL_TEXT_CTRL::SetStep( double aMin, double aMax, STEP_FUNCTION aStepFunc )
 {
-    m_minVal = aMin;
-    m_maxVal = aMax;
+    wxASSERT( aMin <= aMax );
+
+    m_minVal = std::min( aMin, aMax );
+    m_maxVal = std::max( aMin, aMax );
     m_stepFunc = aStepFunc;
 
     // finally, clamp the current value and re-display
@@ -80,6 +81,7 @@ void INCREMENTAL_TEXT_CTRL::updateTextValue()
 void INCREMENTAL_TEXT_CTRL::incrementCtrlBy( double aInc )
 {
     const wxString txt = getCtrlText();
+
     if( !validateFloatField( txt ) )
         return;
 
-- 
2.11.0

From f7920281a067a3e29dfdd8160ca683b74f003e5d Mon Sep 17 00:00:00 2001
From: Maciej Suminski <maciej.suminski@xxxxxxx>
Date: Thu, 16 Feb 2017 17:12:51 +0100
Subject: [PATCH 3/3] Correct min & max grid thickness value

---
 pcbnew/dialogs/dialog_display_options.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pcbnew/dialogs/dialog_display_options.cpp b/pcbnew/dialogs/dialog_display_options.cpp
index d9b85a37b..e4fd07f63 100644
--- a/pcbnew/dialogs/dialog_display_options.cpp
+++ b/pcbnew/dialogs/dialog_display_options.cpp
@@ -48,8 +48,8 @@
 /*
  * Spin control parameters
  */
-static const double gridThicknessMax = 0.5;
-static const double gridThicknessMin = 10.0;
+static const double gridThicknessMax = 10.0;
+static const double gridThicknessMin = 0.5;
 static const double gridThicknessStep = 0.5;
 
 static const double gridMinSpacingMin = 5;
-- 
2.11.0

Attachment: signature.asc
Description: OpenPGP digital signature