kicad-developers team mailing list archive

[Chris Pavlina <pavlina.chris@xxxxxxxxx>]

You probably saw my thread about the bounding boxes for LIB_TEXT and 
LIB_FIELD, which were being merged with the main component bounding box 
incorrectly. I tracked down what the bug was:

in ::GetBoundingBox(), the coordinates are inverted because "Y 
coordinates for LIB_ITEMs are bottom to top". Problem is, the bounding 
boxes aren't used that way, they're expected in most places to be 
noninverted. The only place the LIB_TEXT bounding box should be inverted 
is in ::drawGraphics.

This patch moves the inversion to ::drawGraphics, fixing the bounding 
box bug. The overall bounding box is now calculated correctly:

http://misc.c4757p.com/correct_bbox.png

Compare to the original version, where the text object that I moved 
upwards caused the box to extend downwards:

http://misc.c4757p.com/bbox_wtf.png

This patch also corrects one other minor bug: the bounding box was 
always calculated to include (0,0), even if the component itself doesn't 
include this point.

--
Chris

diff --git a/eeschema/class_libentry.cpp b/eeschema/class_libentry.cpp
index 2ba8989..3c33c51 100644
--- a/eeschema/class_libentry.cpp
+++ b/eeschema/class_libentry.cpp
@@ -1142,6 +1142,7 @@ bool LIB_PART::LoadFootprints( LINE_READER& aLineReader, wxString& aErrorMsg )
 const EDA_RECT LIB_PART::GetBoundingBox( int aUnit, int aConvert ) const
 {
     EDA_RECT bBox( wxPoint( 0, 0 ), wxSize( 0, 0 ) );
+    bool first = true;
 
     for( unsigned ii = 0; ii < drawings.size(); ii++  )
     {
@@ -1157,7 +1158,15 @@ const EDA_RECT LIB_PART::GetBoundingBox( int aUnit, int aConvert ) const
         if ( ( item.Type() == LIB_FIELD_T ) && !( ( LIB_FIELD& ) item ).IsVisible() )
             continue;
 
-        bBox.Merge( item.GetBoundingBox() );
+        EDA_RECT item_bb = item.GetBoundingBox();
+
+        if( first )
+        {
+            bBox = item_bb;
+            first = false;
+        }
+        else
+            bBox.Merge( item_bb );
     }
 
     return bBox;
@@ -1167,6 +1176,7 @@ const EDA_RECT LIB_PART::GetBoundingBox( int aUnit, int aConvert ) const
 const EDA_RECT LIB_PART::GetBodyBoundingBox( int aUnit, int aConvert ) const
 {
     EDA_RECT bBox( wxPoint( 0, 0 ), wxSize( 0, 0 ) );
+    bool first = true;
 
     for( unsigned ii = 0; ii < drawings.size(); ii++  )
     {
@@ -1182,7 +1192,15 @@ const EDA_RECT LIB_PART::GetBodyBoundingBox( int aUnit, int aConvert ) const
         if ( item.Type() == LIB_FIELD_T )
             continue;
 
-        bBox.Merge( item.GetBoundingBox() );
+        EDA_RECT item_bb = item.GetBoundingBox();
+
+        if( first )
+        {
+            bBox = item_bb;
+            first = false;
+        }
+        else
+            bBox.Merge( item_bb );
     }
 
     return bBox;
diff --git a/eeschema/lib_field.cpp b/eeschema/lib_field.cpp
index 3cbc198..f374fa2 100644
--- a/eeschema/lib_field.cpp
+++ b/eeschema/lib_field.cpp
@@ -303,6 +303,12 @@ void LIB_FIELD::drawGraphic( EDA_DRAW_PANEL* aPanel, wxDC* aDC, const wxPoint& a
      * bounding box calculation. */
 #if 0
     EDA_RECT bBox = GetBoundingBox();
+    /* Y coordinates for LIB_ITEMs are bottom to top, so we must invert the bounding
+     * box vertically when usint it to draw the graphics.
+     */
+    bBox.SetY( -bBox.GetY() );
+    bBox.SetSize( bBox.GetSize().x, -bBox.GetSize().y );
+
     EDA_RECT grBox;
     grBox.SetOrigin( aTransform.TransformCoordinate( bBox.GetOrigin() ) );
     grBox.SetEnd( aTransform.TransformCoordinate( bBox.GetEnd() ) );
@@ -505,15 +511,10 @@ wxString LIB_FIELD::GetFullText( int unit )
 
 const EDA_RECT LIB_FIELD::GetBoundingBox() const
 {
-    /* Y coordinates for LIB_ITEMS are bottom to top, so we must invert the Y position when
-     * calling GetTextBox() that works using top to bottom Y axis orientation.
-     */
     EDA_RECT rect = GetTextBox( -1, -1, true );
 
     wxPoint orig = rect.GetOrigin();
     wxPoint end = rect.GetEnd();
-    NEGATE( orig.y);
-    NEGATE( end.y);
 
     RotatePoint( &orig, m_Pos, -m_Orient );
     RotatePoint( &end, m_Pos, -m_Orient );
diff --git a/eeschema/lib_text.cpp b/eeschema/lib_text.cpp
index d4b2a3f..15d0afb 100644
--- a/eeschema/lib_text.cpp
+++ b/eeschema/lib_text.cpp
@@ -388,6 +388,12 @@ void LIB_TEXT::drawGraphic( EDA_DRAW_PANEL* aPanel, wxDC* aDC, const wxPoint& aO
      * and use GetBoundaryBox to know the text coordinate considered as centered
     */
     EDA_RECT bBox = GetBoundingBox();
+
+    /* Y coordinates for LIB_ITEMs are bottom to top, so we must invert the bounding
+     * box vertically when using it to draw the graphics.
+     */
+    bBox.SetY( -bBox.GetY() );
+    bBox.SetSize( bBox.GetSize().x, -bBox.GetSize().y );
     wxPoint txtpos = bBox.Centre();
 
     // Calculate pos accordint to mirror/rotation.
@@ -426,15 +432,10 @@ void LIB_TEXT::GetMsgPanelInfo( MSG_PANEL_ITEMS& aList )
 
 const EDA_RECT LIB_TEXT::GetBoundingBox() const
 {
-    /* Y coordinates for LIB_ITEMS are bottom to top, so we must invert the Y position when
-     * calling GetTextBox() that works using top to bottom Y axis orientation.
-     */
     EDA_RECT rect = GetTextBox( -1, -1, true );
 
     wxPoint orig = rect.GetOrigin();
     wxPoint end = rect.GetEnd();
-    NEGATE( orig.y);
-    NEGATE( end.y);
 
     RotatePoint( &orig, m_Pos, -m_Orient );
     RotatePoint( &end, m_Pos, -m_Orient );