← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Measurement tool for GAL

 

Hi John,

All I can say is: it looks really cool! I would like to merge, but
before I do so, would you consider the attached fixup patch? If so, I
simply update the existing set of patches and merge everything.

Changes:
- Changed std::vector<> to const std::vector<>& in
PREVIEW::SetConstantGlyphHeight()

- One of the new files had Tom as the copyright holder and incorrect
date, changed to "(c) 2017 KiCad Developers"

- Changed "Measure between two points" to "Measure distance between two
points". I am not a native English speaker, so I need your advice here.

- A few code formatting fixes.

Cheers,
Orson

On 03/10/2017 12:18 PM, John Beard wrote:
> Hi Nick,
> 
> That's because the label formatting just uses a fixed 2 DP display for
> mm (it uses 4 for inches, and 1 for degrees). It's not a perfect
> solution, but it does line up nicely when the labels are on the left
> of the line. Stripping trailing zeroes would make it ragged when you
> zoom in a bit and get "1, 1.25, 1.5, 1.75, 2".
> 
> Cheers,
> 
> John
> 
> On Fri, Mar 10, 2017 at 4:34 PM, Nick Østergaard <oe.nick@xxxxxxxxx> wrote:
>> It certainly looks quite nice. I have not yet tested the patch, but I
>> wonder why it shall show the integer labels here with decimals behind
>> it when they are all zero?
>>
>> 2017-03-09 18:18 GMT+01:00 John Beard <john.j.beard@xxxxxxxxx>:
>>> By the way, here's a screenshot of the ruler tool:
>>>
>>> Cheers,
>>>
>>> John
>>>
>>> On Fri, Mar 10, 2017 at 1:11 AM, John Beard <john.j.beard@xxxxxxxxx> wrote:
>>>> Hi,
>>>>
>>>> Here's a tool I'm had on my list for a while - a ruler tool.
>>>>
>>>> This patch set introduces a new directory, for "preview items" which
>>>> are EDA_ITEMS that are only used for transient previews, such as
>>>> drawing aids and selection boxes. These are located in common, so that
>>>> they can (in theory) be used in any GAL canvas, not just
>>>> Pcbnew/Modedit.
>>>>
>>>> There are also some functions that hopefully will be useful for other
>>>> preview items in future in the utils file.
>>>>
>>>> There's an oustanding bug with the OpenGL GAL in that it cannot draw
>>>> spaces in text, so there are a couple of places where it pre-emptively
>>>> strips spaces that should be removed when that bug is fixed.
>>>> (https://bugs.launchpad.net/kicad/+bug/1668455).
>>>>
>>>> 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
>>>
> 
> _______________________________________________
> 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 644481eaef5a31d06228be3fa6f193b14bdaebb7 Mon Sep 17 00:00:00 2001
From: Maciej Suminski <maciej.suminski@xxxxxxx>
Date: Fri, 10 Mar 2017 14:36:37 +0100
Subject: [PATCH] fixup! Add a ruler tool to pcbnew GAL

---
 common/preview_items/preview_utils.cpp |  6 ++++--
 common/preview_items/ruler_item.cpp    | 19 ++++++++-----------
 include/preview_items/preview_utils.h  |  2 +-
 pcbnew/tool_modedit.cpp                |  2 +-
 pcbnew/tool_pcb.cpp                    |  2 +-
 pcbnew/tools/edit_tool.cpp             |  4 ++--
 6 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/common/preview_items/preview_utils.cpp b/common/preview_items/preview_utils.cpp
index bdd223794..6cef9059b 100644
--- a/common/preview_items/preview_utils.cpp
+++ b/common/preview_items/preview_utils.cpp
@@ -75,6 +75,7 @@ static wxString getDimensionUnit( EDA_UNITS_T aUnits )
     return wxEmptyString;
 }
 
+
 static wxString formatPreviewDimension( double aVal, EDA_UNITS_T aUnits )
 {
     int precision = 4;
@@ -108,6 +109,7 @@ static wxString formatPreviewDimension( double aVal, EDA_UNITS_T aUnits )
     return str;
 }
 
+
 wxString KIGFX::PREVIEW::DimensionLabel( const wxString& prefix,
             double aVal, EDA_UNITS_T aUnits )
 {
@@ -133,7 +135,7 @@ void KIGFX::PREVIEW::SetConstantGlyphHeight( KIGFX::GAL& aGal, double aHeight )
 
 void KIGFX::PREVIEW::DrawTextNextToCursor( KIGFX::GAL& aGal,
         const VECTOR2D& aCursorPos, const VECTOR2D& aTextQuadrant,
-        std::vector<wxString> aStrings )
+        const std::vector<wxString>& aStrings )
 {
     auto glyphSize = aGal.GetGlyphSize();
 
@@ -167,7 +169,7 @@ void KIGFX::PREVIEW::DrawTextNextToCursor( KIGFX::GAL& aGal,
     aGal.SetIsFill( false );
 
     // write strings top-to-bottom
-    for( auto& str : aStrings )
+    for( const auto& str : aStrings )
     {
         textPos.y += linePitch;
         aGal.BitmapText( str, textPos, 0.0 );
diff --git a/common/preview_items/ruler_item.cpp b/common/preview_items/ruler_item.cpp
index 15d77ad4f..331fc45fb 100644
--- a/common/preview_items/ruler_item.cpp
+++ b/common/preview_items/ruler_item.cpp
@@ -1,8 +1,7 @@
 /*
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
- * Copyright (C) 2013 CERN
- * @author Tomasz Wlostowski <tomasz.wlostowski@xxxxxxx>
+ * Copyright (C) 2017 Kicad Developers, see change_log.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
@@ -48,13 +47,12 @@ static void drawCursorStrings( KIGFX::GAL& aGal, const VECTOR2D& aCursor,
     cursorStrings.push_back( DimensionLabel( "r", aRulerVec.EuclideanNorm(), g_UserUnit ) );
 
     double degs = RAD2DECIDEG( -aRulerVec.Angle() );
-    cursorStrings.push_back( DimensionLabel("θ", degs, DEGREES ) );
+    cursorStrings.push_back( DimensionLabel( "θ", degs, DEGREES ) );
 
     for( auto& str: cursorStrings )
     {
         // FIXME: remove spaces that choke OpenGL lp:1668455
-        str.erase( std::remove( str.begin(), str.end(), ' ' ),
-                   str.end() );
+        str.erase( std::remove( str.begin(), str.end(), ' ' ), str.end() );
     }
 
     auto temp = aRulerVec;
@@ -99,10 +97,10 @@ static TICK_FORMAT getTickFormatForScale( double aScale, double& aTickSpace )
     {
         const auto pixelSpace = aTickSpace * aScale;
 
-        if( pixelSpace >= maxTickDensity)
+        if( pixelSpace >= maxTickDensity )
             break;
 
-        tickFormat = (tickFormat + 1) % tickFormats.size();
+        tickFormat = ( tickFormat + 1 ) % tickFormats.size();
         aTickSpace *= tickFormats[tickFormat].divisionBase;
     }
 
@@ -146,7 +144,7 @@ void drawTicksAlongLine( KIGFX::GAL& aGal, const VECTOR2D& aOrigin,
     // text and ticks are dimmed
     aGal.SetStrokeColor( PreviewOverlayDefaultColor().WithAlpha( PreviewOverlayDeemphAlpha( true ) ) );
 
-    const auto labelOffset = tickLine.Resize( aMinorTickLen * (majorTickLengthFactor + 1) );
+    const auto labelOffset = tickLine.Resize( aMinorTickLen * ( majorTickLengthFactor + 1 ) );
 
     for( int i = 0; i < numTicks; ++i )
     {
@@ -160,7 +158,7 @@ void drawTicksAlongLine( KIGFX::GAL& aGal, const VECTOR2D& aOrigin,
             drawLabel = true;
             length *= majorTickLengthFactor;
         }
-        else if ( tickF.midStep && i % tickF.midStep == 0 )
+        else if( tickF.midStep && i % tickF.midStep == 0 )
         {
             drawLabel = true;
             length *= midTickLengthFactor;
@@ -173,8 +171,7 @@ void drawTicksAlongLine( KIGFX::GAL& aGal, const VECTOR2D& aOrigin,
             wxString label = DimensionLabel( "", tickSpace * i, g_UserUnit );
 
             // FIXME: spaces choke OpenGL lp:1668455
-            label.erase( std::remove( label.begin(), label.end(), ' ' ),
-                      label.end() );
+            label.erase( std::remove( label.begin(), label.end(), ' ' ), label.end() );
 
             aGal.BitmapText( label, tickPos + labelOffset, labelAngle );
         }
diff --git a/include/preview_items/preview_utils.h b/include/preview_items/preview_utils.h
index 31a08fd5d..c4c711b60 100644
--- a/include/preview_items/preview_utils.h
+++ b/include/preview_items/preview_utils.h
@@ -82,7 +82,7 @@ void SetConstantGlyphHeight( KIGFX::GAL& aGal, double aHeight );
  */
 void DrawTextNextToCursor( KIGFX::GAL& aGal,
         const VECTOR2D& aCursorPos, const VECTOR2D& aTextQuadrant,
-        std::vector<wxString> aStrings );
+        const std::vector<wxString>& aStrings );
 
 } // PREVIEW
 } // KIGFX
diff --git a/pcbnew/tool_modedit.cpp b/pcbnew/tool_modedit.cpp
index 42dd2411a..e28956030 100644
--- a/pcbnew/tool_modedit.cpp
+++ b/pcbnew/tool_modedit.cpp
@@ -198,7 +198,7 @@ void FOOTPRINT_EDIT_FRAME::ReCreateVToolbar()
 
     m_drawToolBar->AddTool( ID_MODEDIT_MEASUREMENT_TOOL, wxEmptyString,
                             KiBitmap( measurement_xpm ),
-                            _( "Measure between two points" ),
+                            _( "Measure distance between two points" ),
                             wxITEM_CHECK );
 
     m_drawToolBar->Realize();
diff --git a/pcbnew/tool_pcb.cpp b/pcbnew/tool_pcb.cpp
index 28fdedb7d..d51bd21b0 100644
--- a/pcbnew/tool_pcb.cpp
+++ b/pcbnew/tool_pcb.cpp
@@ -486,7 +486,7 @@ void PCB_EDIT_FRAME::ReCreateVToolbar()
 
     m_drawToolBar->AddTool( ID_PCB_MEASUREMENT_TOOL, wxEmptyString,
                             KiBitmap( measurement_xpm ),
-                            _( "Activate the measurement tool" ),
+                            _( "Measure distance between two points" ),
                             wxITEM_CHECK );
 
     m_drawToolBar->Realize();
diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp
index 9f64b4d60..932065ca4 100644
--- a/pcbnew/tools/edit_tool.cpp
+++ b/pcbnew/tools/edit_tool.cpp
@@ -154,7 +154,7 @@ TOOL_ACTION PCB_ACTIONS::editModifiedSelection( "pcbnew.InteractiveEdit.Modified
 
 TOOL_ACTION PCB_ACTIONS::measureTool( "pcbnew.InteractiveEdit.measureTool",
         AS_GLOBAL, MD_CTRL + MD_SHIFT + 'M',
-        _( "Measure tool" ), _( "Interactively measure between points" ),
+        _( "Measure tool" ), _( "Interactively measure distance between points" ),
         nullptr, AF_ACTIVATE );
 
 
@@ -943,7 +943,7 @@ int EDIT_TOOL::MeasureTool( const TOOL_EVENT& aEvent )
     Activate();
     frame()->SetToolID( EditingModules() ? ID_MODEDIT_MEASUREMENT_TOOL
                                          : ID_PCB_MEASUREMENT_TOOL,
-                        wxCURSOR_PENCIL, _( "Measure between two points" ) );
+                        wxCURSOR_PENCIL, _( "Measure distance between two points" ) );
 
     KIGFX::PREVIEW::RULER_ITEM ruler;
     view.Add( &ruler );
-- 
2.12.0

Attachment: signature.asc
Description: OpenPGP digital signature


Follow ups

References