Skip to content

Commit

Permalink
Fix incorrect (too small) bounding box EDA_TEXT, noticeable when char…
Browse files Browse the repository at this point in the history
…s 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.
  • Loading branch information
jp-charras committed Mar 11, 2016
1 parent 3fd179c commit 99e81ae
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 39 deletions.
40 changes: 23 additions & 17 deletions common/drawtxt.cpp
Expand Up @@ -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
* (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
* Of course some shapes can be bigger or smaller than the vertical char size
* parameter
*/
#define HERSHEY_SCALE_FACTOR 1 / 21.0
double s_HersheyScaleFactor = HERSHEY_SCALE_FACTOR;
using namespace KIGFX;


// A ugly hack to avoid UTF8::uni_iter method not found at link stage
// in gerbview and pl_editor.
// Hoping I (JPC) can remove that soon.
void dummy()
{
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 );
}


Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 )
{
Expand Down Expand Up @@ -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 );
hc2 = KiROUND( hc2 * size_v * s_HersheyScaleFactor );
hc1 = KiROUND( hc1 * size_h * STROKE_FONT::STROKE_FONT_SCALE );
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;
Expand All @@ -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 )
Expand Down
30 changes: 18 additions & 12 deletions 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) 2004-2011 KiCad Developers, see change_log.txt for contributors.
* Copyright (C) 2016 Jean-Pierre Charras, jp.charras at wanadoo.fr
* 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
Expand Down Expand Up @@ -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 )
Expand Down Expand Up @@ -106,15 +108,16 @@ wxString EDA_TEXT::ShortenedShownText() const
}


/**
* Function GetInterline
* return the distance between 2 text lines
* has meaning only for multiline texts
/*
* calculate the distance (pitch) between 2 text lines
* the distance includes the interline + room for chars like j { and [
* 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
Expand All @@ -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 );
Expand All @@ -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 ]
int extra_dy = dy - m_Size.y;
rect.Move( wxPoint( 0, -extra_dy / 2 ) ); // move origin by the half extra interval
// The bbox vertical size returned by GetInterline( aThickness )
// includes letters like j and y and ] + interval between lines.
// 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++ )
Expand Down Expand Up @@ -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;
Expand Down
18 changes: 10 additions & 8 deletions common/gal/stroke_font.cpp
Expand Up @@ -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 ),
Expand Down Expand Up @@ -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;
glyphEndX = ( coordinate[1] - 'R' ) * HERSHEY_SCALE;
glyphStartX = ( coordinate[0] - 'R' ) * STROKE_FONT_SCALE;
glyphEndX = ( coordinate[1] - 'R' ) * STROKE_FONT_SCALE;
glyphBoundingX = VECTOR2D( 0, glyphEndX - glyphStartX );
}
else if( ( coordinate[0] == ' ' ) && ( coordinate[1] == 'R' ) )
Expand All @@ -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 );
}

Expand All @@ -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();
}


Expand Down Expand Up @@ -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;
}
}

Expand Down
1 change: 1 addition & 0 deletions eeschema/CMakeLists.txt
Expand Up @@ -254,6 +254,7 @@ target_link_libraries( eeschema_kiface
common
bitmaps
polygon
gal
${wxWidgets_LIBRARIES}
${GDI_PLUS_LIBRARIES}
)
Expand Down
1 change: 1 addition & 0 deletions gerbview/CMakeLists.txt
Expand Up @@ -125,6 +125,7 @@ target_link_libraries( gerbview_kiface
common
polygon
bitmaps
gal
${wxWidgets_LIBRARIES}
${GDI_PLUS_LIBRARIES}
)
Expand Down
16 changes: 15 additions & 1 deletion include/gal/stroke_font.h
Expand Up @@ -198,14 +198,28 @@ class STROKE_FONT
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;

///> Factor that determines relative line width for bold text.
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

Expand Down
1 change: 1 addition & 0 deletions include/newstroke_font.h
Expand Up @@ -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__ */
1 change: 1 addition & 0 deletions pagelayout_editor/CMakeLists.txt
Expand Up @@ -94,6 +94,7 @@ target_link_libraries( pl_editor_kiface
common
polygon
bitmaps
gal
${wxWidgets_LIBRARIES}
${GDI_PLUS_LIBRARIES}
)
Expand Down
2 changes: 1 addition & 1 deletion pcbnew/dialogs/dialog_create_array.cpp
Expand Up @@ -293,7 +293,7 @@ void DIALOG_CREATE_ARRAY::OnOkClick( wxCommandEvent& event )
}

else
wxMessageBox( _(" Bad parameters" ) );
wxMessageBox( _("Bad parameters" ) );
}


Expand Down

0 comments on commit 99e81ae

Please sign in to comment.