Fix incorrect (too small) bounding box EDA_TEXT, noticeable when chars like j or { are used in zone fill functions.

I tried to merge some constants used both in legacy mode and gal mode (which were, before this patch, separate constants).
There is still a serious work to avoid different calculation code for the same text in draw and plot functions.
Work in progress to merge these calculation functions.

common/drawtxt.cpp

@@ -39,24 +39,29 @@
 #include <class_drawpanel.h>
 #include <class_base_screen.h>
 
+#include <gal/stroke_font.h>
+
 
 #include <newstroke_font.h>
 #include <plot_common.h>
 
-/* factor used to calculate actual size of shapes from hershey fonts
+using namespace KIGFX;
- * (could be adjusted depending on the font name)
+
- * Its value is choosen in order to have letters like M, P .. vertical size
+
- * equal to the vertical char size parameter
+// A ugly hack to avoid UTF8::uni_iter method not found at link stage
- * Of course some shapes can be bigger or smaller than the vertical char size
+// in gerbview and pl_editor.
- * parameter
+// Hoping I (JPC) can remove that soon.
- */
+void dummy()
-#define HERSHEY_SCALE_FACTOR 1 / 21.0
+{
-double s_HersheyScaleFactor = HERSHEY_SCALE_FACTOR;
+    UTF8 text = "x";
+    for( UTF8::uni_iter it = text.ubegin(), end = text.uend(); it < end; ++it )
+        ;
+}
 
 
 int OverbarPositionY( int size_v )
 {
-    return KiROUND( size_v * 1.22 );
+    return KiROUND( size_v * STROKE_FONT::OVERBAR_HEIGHT );
 }
 
 
@@ -184,13 +189,13 @@ int GraphicTextWidth( const wxString& aText, int aXSize, bool aItalic, bool aWid
         // Get metrics
         int         xsta    = *shape_ptr++ - 'R';
         int         xsto    = *shape_ptr++ - 'R';
-        tally += KiROUND( aXSize * (xsto - xsta) * s_HersheyScaleFactor );
+        tally += KiROUND( aXSize * (xsto - xsta) * STROKE_FONT::STROKE_FONT_SCALE );
     }
 
     // For italic correction, add 1/8 size
     if( aItalic )
     {
-        tally += KiROUND( aXSize * 0.125 );
+        tally += KiROUND( aXSize * STROKE_FONT::ITALIC_TILT );
     }
 
     return tally;
@@ -415,7 +420,7 @@ void DrawGraphicText( EDA_RECT* aClipBox,
 
     if( aItalic )
     {
-        overbar_italic_comp = OverbarPositionY( size_v ) / 8;
+        overbar_italic_comp = KiROUND( OverbarPositionY( size_v ) * STROKE_FONT::ITALIC_TILT );
 
         if( italic_reverse )
         {
@@ -519,13 +524,14 @@ void DrawGraphicText( EDA_RECT* aClipBox,
             {
                 wxPoint currpoint;
                 hc1 -= xsta; hc2 -= 10;    // Align the midpoint
-                hc1  = KiROUND( hc1 * size_h * s_HersheyScaleFactor );
+                hc1  = KiROUND( hc1 * size_h * STROKE_FONT::STROKE_FONT_SCALE );
-                hc2  = KiROUND( hc2 * size_v * s_HersheyScaleFactor );
+                hc2  = KiROUND( hc2 * size_v * STROKE_FONT::STROKE_FONT_SCALE );
 
                 // To simulate an italic font,
                 // add a x offset depending on the y offset
                 if( aItalic )
-                    hc1 -= KiROUND( italic_reverse ? -hc2 / 8.0 : hc2 / 8.0 );
+                    hc1 -= KiROUND( italic_reverse ? -hc2 * STROKE_FONT::ITALIC_TILT
+                                                          : hc2 * STROKE_FONT::ITALIC_TILT );
 
                 currpoint.x = hc1 + current_char_pos.x;
                 currpoint.y = hc2 + current_char_pos.y;
@@ -541,7 +547,7 @@ void DrawGraphicText( EDA_RECT* aClipBox,
         ptr++;
 
         // Apply the advance width
-        current_char_pos.x += KiROUND( size_h * (xsto - xsta) * s_HersheyScaleFactor );
+        current_char_pos.x += KiROUND( size_h * (xsto - xsta) * STROKE_FONT::STROKE_FONT_SCALE );
     }
 
     if( overbars % 2 )

common/eda_text.cpp

@@ -1,8 +1,8 @@
 /*
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
- * Copyright (C) 2004 Jean-Pierre Charras, jean-pierre.charras@gipsa-lab.inpg.com
+ * Copyright (C) 2016 Jean-Pierre Charras, jp.charras at wanadoo.fr
- * Copyright (C) 2004-2011 KiCad Developers, see change_log.txt for contributors.
+ * Copyright (C) 2004-2016 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
@@ -46,6 +46,8 @@
 #error "Cannot resolve units formatting due to no definition of EESCHEMA or PCBNEW."
 #endif
 
+#include <gal/stroke_font.h>
+
 #include <convert_to_biu.h>
 
 EDA_TEXT::EDA_TEXT( const wxString& text )
@@ -106,15 +108,16 @@ wxString EDA_TEXT::ShortenedShownText() const
 }
 
 
-/**
+/*
- * Function GetInterline
+ * calculate the distance (pitch) between 2 text lines
- * return the distance between 2 text lines
+ * the distance includes the interline + room for chars like j { and [
- * has meaning only for multiline texts
+ * Is used for multiline texts, but also for single line texts, to calculate
+ * the text bounding box
  */
 int EDA_TEXT::GetInterline( int aTextThickness ) const
 {
     int thickness = aTextThickness <= 0 ? m_Thickness : aTextThickness;
-    return (( m_Size.y * 14 ) / 10) + thickness;
+    return KiROUND( m_Size.y * KIGFX::STROKE_FONT::INTERLINE_PITCH_RATIO ) + thickness;
 }
 
 EDA_RECT EDA_TEXT::GetTextBox( int aLine, int aThickness, bool aInvertY ) const
@@ -130,7 +133,7 @@ EDA_RECT EDA_TEXT::GetTextBox( int aLine, int aThickness, bool aInvertY ) const
     {
         wxStringSplit( text, strings, '\n' );
 
-        if ( strings.GetCount() )     // GetCount() == 0 for void strings
+        if( strings.GetCount() )     // GetCount() == 0 for void strings
         {
             if( aLine >= 0 && (aLine < (int)strings.GetCount()) )
                 text = strings.Item( aLine );
@@ -153,11 +156,15 @@ EDA_RECT EDA_TEXT::GetTextBox( int aLine, int aThickness, bool aInvertY ) const
     else
         rect.SetOrigin( m_Pos );
 
-    // extra dy interval for letters like j and y and ]
+    // The bbox vertical size returned by GetInterline( aThickness )
-    int extra_dy = dy - m_Size.y;
+    // includes letters like j and y and ] + interval between lines.
-    rect.Move( wxPoint( 0, -extra_dy / 2 ) ); // move origin by the half extra interval
+    // The interval below the last line is not usefull, and we can use its half value
+    // as vertical margin above the text
+    // the full interval is roughly m_Size.y * 0.4 - aThickness/2
+    rect.Move( wxPoint( 0, aThickness/4 - KiROUND( m_Size.y * 0.2 ) ) );
 
     // for multiline texts and aLine < 0, merge all rectangles
+    // ( if aLine < 0, we want the full text bounding box )
     if( m_MultilineAllowed && aLine < 0 )
     {
         for( unsigned ii = 1; ii < strings.GetCount(); ii++ )
@@ -232,7 +239,6 @@ EDA_RECT EDA_TEXT::GetTextBox( int aLine, int aThickness, bool aInvertY ) const
         }
     }
 
-    rect.Inflate( thickness / 2 );
     rect.Normalize();       // Make h and v sizes always >= 0
 
     return rect;

common/gal/stroke_font.cpp

@@ -32,9 +32,11 @@
 
 using namespace KIGFX;
 
+const double STROKE_FONT::INTERLINE_PITCH_RATIO = 1.5;
 const double STROKE_FONT::OVERBAR_HEIGHT = 1.22;
 const double STROKE_FONT::BOLD_FACTOR = 1.3;
-const double STROKE_FONT::HERSHEY_SCALE = 1.0 / 21.0;
+const double STROKE_FONT::STROKE_FONT_SCALE = 1.0 / 21.0;
+const double STROKE_FONT::ITALIC_TILT = 1.0 / 8;
 
 STROKE_FONT::STROKE_FONT( GAL* aGal ) :
     m_gal( aGal ),
@@ -81,8 +83,8 @@ bool STROKE_FONT::LoadNewStrokeFont( const char* const aNewStrokeFont[], int aNe
             if( i < 2 )
             {
                 // The first two values contain the width of the char
-                glyphStartX     = ( coordinate[0] - 'R' ) * HERSHEY_SCALE;
+                glyphStartX     = ( coordinate[0] - 'R' ) * STROKE_FONT_SCALE;
-                glyphEndX       = ( coordinate[1] - 'R' ) * HERSHEY_SCALE;
+                glyphEndX       = ( coordinate[1] - 'R' ) * STROKE_FONT_SCALE;
                 glyphBoundingX  = VECTOR2D( 0, glyphEndX - glyphStartX );
             }
             else if( ( coordinate[0] == ' ' ) && ( coordinate[1] == 'R' ) )
@@ -97,9 +99,9 @@ bool STROKE_FONT::LoadNewStrokeFont( const char* const aNewStrokeFont[], int aNe
             {
                 // Every coordinate description of the Hershey format has an offset,
                 // it has to be subtracted
-                point.x = (double) ( coordinate[0] - 'R' ) * HERSHEY_SCALE - glyphStartX;
+                point.x = (double) ( coordinate[0] - 'R' ) * STROKE_FONT_SCALE - glyphStartX;
 				// -10 is here to keep GAL rendering consistent with the legacy gfx stuff
-                point.y = (double) ( coordinate[1] - 'R' - 10) * HERSHEY_SCALE;
+                point.y = (double) ( coordinate[1] - 'R' - 10) * STROKE_FONT_SCALE;
                 pointList.push_back( point );
             }
 
@@ -121,7 +123,7 @@ bool STROKE_FONT::LoadNewStrokeFont( const char* const aNewStrokeFont[], int aNe
 
 int STROKE_FONT::getInterline() const
 {
-    return ( m_glyphSize.y * 14 ) / 10 + m_gal->GetLineWidth();
+    return KiROUND( m_glyphSize.y * INTERLINE_PITCH_RATIO ) + m_gal->GetLineWidth();
 }
 
 
@@ -307,11 +309,11 @@ void STROKE_FONT::drawSingleLineText( const UTF8& aText )
         {
             if( m_mirrored )
             {
-                overbar_italic_comp = (-m_glyphSize.y * OVERBAR_HEIGHT) / 8;
+                overbar_italic_comp = (-m_glyphSize.y * OVERBAR_HEIGHT) / ITALIC_TILT;
             }
             else
             {
-                overbar_italic_comp = (m_glyphSize.y * OVERBAR_HEIGHT) / 8;
+                overbar_italic_comp = (m_glyphSize.y * OVERBAR_HEIGHT) / ITALIC_TILT;
             }
         }
 

eeschema/CMakeLists.txt

@@ -252,6 +252,7 @@ target_link_libraries( eeschema_kiface
     common
     bitmaps
     polygon
+    gal
     ${wxWidgets_LIBRARIES}
     ${GDI_PLUS_LIBRARIES}
     )

gerbview/CMakeLists.txt

@@ -133,6 +133,7 @@ target_link_libraries( gerbview_kiface
     common
     polygon
     bitmaps
+    gal
     ${wxWidgets_LIBRARIES}
     ${GDI_PLUS_LIBRARIES}
     )

include/gal/stroke_font.h

@@ -198,6 +198,16 @@ private:
             return std::count( aText.begin(), aText.end() - 1, '\n' ) + 1;
     }
 
+public:
+    // These members are declared public only to be (temporary, I am expecting)
+    // used in legacy canvas, to avoid multiple declarations of the same constants,
+    // having multiple declarations of the same constants is really a thing to avoid.
+    //
+    // They will be private later, when the legacy canvas is removed.
+
+    ///> Factor that determines the pitch between 2 lines.
+    static const double INTERLINE_PITCH_RATIO;
+
     ///> Factor that determines relative height of overbar.
     static const double OVERBAR_HEIGHT;
 
@@ -205,7 +215,11 @@ private:
     static const double BOLD_FACTOR;
 
     ///> Scale factor for a glyph
-    static const double HERSHEY_SCALE;
+    static const double STROKE_FONT_SCALE;
+
+    ///> Tilt factor for italic style (the is is the scaling factor
+    ///> on dY relative coordinates to give a tilst shape
+    static const double ITALIC_TILT;
 };
 } // namespace KIGFX
 

include/newstroke_font.h

@@ -31,4 +31,5 @@
  */
 extern const char* const newstroke_font[];        //The font
 extern const int         newstroke_font_bufsize;  //font buffer size
+
 #endif /* __NEWSTROKE_FONT_H__ */

pagelayout_editor/CMakeLists.txt

@@ -102,6 +102,7 @@ target_link_libraries( pl_editor_kiface
     common
     polygon
     bitmaps
+    gal
     ${wxWidgets_LIBRARIES}
     ${GDI_PLUS_LIBRARIES}
     )

pcbnew/dialogs/dialog_create_array.cpp

@@ -293,7 +293,7 @@ void DIALOG_CREATE_ARRAY::OnOkClick( wxCommandEvent& event )
     }
 
     else
-        wxMessageBox( _(" Bad parameters" ) );
+        wxMessageBox( _("Bad parameters" ) );
 }