kicad-developers team mailing list archive

[Jonas Diemer <diemer@...>]

Am Samstag 15 März 2008 05:00:37 schrieb Dick Hollenbeck:
> This is good work. Some remarks:

Thanks, also thanks for the remarks. I have attached a patch (against latest 
svn) that simplifies my code using the new EDA_Rect::Merge() code (for which 
I added a const). Please merge that to svn.

> ** I'm wondering if you think we should put these GetBoundingBox()
> functions nearer to the corresponding Draw() functions so that a person
> can see by looking at the Draw() function how big the rectangle is.
> Currently many of those functions are in eeredraw.cpp. Give this some
> thought and let us know.

Well, as far as I am concerned, the partitioning of functions to cpp files 
does not make much sense in many other cases too (e. g. "cmpclass.cpp" 
and "component_class.cpp" scream for being merged). 

I usually have separate .h/.cpp files for each class (e.g. 
have "SchComponentStruct.h" and "SchComponentStruct.cpp" files for 
EDA_SchComponentStruct). In that case, the corresponding Draw() and 
GetBoundingBox() functions can be very near, which I would prefer. 
What do you guys think about such partitioning?

> ** We are using uncrustify as our C++ beautifier and basically, as a
> simple means of saying what code should look like. There is a project
> config file for it uncrustify.cfg. It would be wonderful if you were
> to run your more significant edits through the beautifier before
> generating your patches.

Ok, I'll give it a try.

> Thanks for your help, your contributions will help make the program
> better for everyone. Similar edits can still be made to pcbnew if you
> get ambitious.

Well, currently, I am more concerned about artifacts in eeschema. We have 
approached those for Deletes, but for Moves, there are still ugly artifacts.

I have tried the same trick in WinEDA_SchematicFrame::StartMovePart() (see 
eeschema_redraw-move.patch), which gets rid of the artifacts but adds new 
ones around circles... I have no Idea how to fix this though, other than 
posting dirty rects again in ShowWhileMoving()...

However, I think this second patch is worth integrating, since it further 
reduces the visual artifacts in eeschema. The cost should be much less than 
my original fix.

Next step is to fix artifacts for block commands and possibly other places.

Best regards
 --Boundary-00=_g382Hy5GcTJq601 Content-Type: text/x-diff;
charset="utf-8";
name="eeschema_redraw-2.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="eeschema_redraw-2.patch"

diff -r 660a67ed1740 -r d4625ea3ee6d kicad/common/base_struct.cpp
--- a/kicad/common/base_struct.cpp	Sat Mar 15 10:24:03 2008 +0000
+++ b/kicad/common/base_struct.cpp	Sat Mar 15 13:54:20 2008 +0100
@@ -767,7 +767,7 @@ EDA_Rect& EDA_Rect::Inflate( wxCoord dx,
* mainly used to calculate bounding boxes
* @param aRect = given rect to merge with this
*/
-void EDA_Rect::Merge( EDA_Rect& aRect )
+void EDA_Rect::Merge( const EDA_Rect& aRect )
{
Normalize(); // ensure width and height >= 0
EDA_Rect rect = aRect;
@@ -775,8 +775,8 @@ void EDA_Rect::Merge( EDA_Rect& aRect )
wxPoint end = GetEnd();
wxPoint rect_end = rect.GetEnd();

-	// Change origin and size in order to contain the given rect
-	m_Pos.x = MIN( m_Pos.x, rect.m_Pos.x );
+ // Change origin and size in order to contain the given rect
+ m_Pos.x = MIN( m_Pos.x, rect.m_Pos.x );
m_Pos.y = MIN( m_Pos.y, rect.m_Pos.y );
end.x = MAX( end.x, rect_end.x );
end.y = MAX( end.y, rect_end.y );
diff -r 660a67ed1740 -r d4625ea3ee6d kicad/eeschema/cmpclass.cpp
--- a/kicad/eeschema/cmpclass.cpp	Sat Mar 15 10:24:03 2008 +0000
+++ b/kicad/eeschema/cmpclass.cpp	Sat Mar 15 13:54:20 2008 +0100
@@ -272,32 +272,23 @@ EDA_Rect EDA_SchComponentStruct::GetBoun
EDA_Rect EDA_SchComponentStruct::GetBoundingBox()
{
const int PADDING = 40;
- int xmin, xmax, ymin, ymax;

// This gives a reasonable approximation (but some things are missing so...
EDA_Rect ret = GetBoundaryBox();
- xmin = ret.m_Pos.x;
- ymin = ret.m_Pos.y;
- xmax = ret.m_Pos.x + ret.m_Size.x;
- ymax = ret.m_Pos.y + ret.m_Size.y;

// Include BoundingBoxes of fields
for( int i = REFERENCE; i < NUMBER_OF_FIELDS; i++ )
{
- EDA_Rect box = m_Field[i].GetBoundaryBox();
- xmin = MIN( xmin, box.m_Pos.x);
- ymin = MIN( ymin, box.m_Pos.y);
- xmax = MAX( xmax, box.m_Pos.x + box.m_Size.x);
- ymax = MAX( ymax, box.m_Pos.y + box.m_Size.y);
+ ret.Merge( m_Field[i].GetBoundaryBox() );
}

// ... add padding TODO: improve this
- ret.m_Pos.x = xmin - PADDING;
- ret.m_Pos.y = ymin - PADDING;
- ret.m_Size.x = xmax - xmin + 2*PADDING;
- ret.m_Size.y = ymax - ymin + 2*PADDING;
+ ret.m_Pos.x -= PADDING;
+ ret.m_Pos.y -= PADDING;
+ ret.m_Size.x += 2*PADDING;
+ ret.m_Size.y += 2*PADDING;

- D( printf("final box: %d,%d, %d,%d\n", ret.m_Pos.x, ret.m_Pos.y, ret.m_Size.x, ret.m_Size.y); )
+// D( printf("final box: %d,%d, %d,%d\n", ret.m_Pos.x, ret.m_Pos.y, ret.m_Size.x, ret.m_Size.y); )
return ret;
}

diff -r 660a67ed1740 -r d4625ea3ee6d kicad/include/base_struct.h
--- a/kicad/include/base_struct.h	Sat Mar 15 10:24:03 2008 +0000
+++ b/kicad/include/base_struct.h	Sat Mar 15 13:54:20 2008 +0100
@@ -200,7 +200,7 @@ public:
* mainly used to calculate bounding boxes
* @param aRect = given rect to merge with this
*/
-	void Merge( EDA_Rect & aRect );
+	void Merge( const EDA_Rect & aRect );

};

--Boundary-00=_g382Hy5GcTJq601 Content-Type: text/x-diff;
charset="utf-8";
name="eeschema_redraw-move.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="eeschema_redraw-move.patch"

diff -r d4625ea3ee6d kicad/eeschema/getpart.cpp
--- a/kicad/eeschema/getpart.cpp	Sat Mar 15 13:54:20 2008 +0100
+++ b/kicad/eeschema/getpart.cpp	Sat Mar 15 14:13:44 2008 +0100
@@ -260,11 +260,15 @@ static void ShowWhileMoving( WinEDA_Draw
panel->m_Parent->GetScreen()->GetCurItem();

/* Effacement du composant */
- if( erase )
+ if( erase ){
DrawStructsInGhost( panel, DC, DrawLibItem, 0, 0 );
+#if 1 // fix artifacts around circles...
+ panel->PostDirtyRect( DrawLibItem->GetBoundingBox() );
+#endif
+ }

-	move_vector.x = panel->m_Parent->GetScreen()->m_Curseur.x - DrawLibItem->m_Pos.x;
-	move_vector.y = panel->m_Parent->GetScreen()->m_Curseur.y - DrawLibItem->m_Pos.y;
+ move_vector.x = panel->m_Parent->GetScreen()->m_Curseur.x - DrawLibItem->m_Pos.x;
+ move_vector.y = panel->m_Parent->GetScreen()->m_Curseur.y - DrawLibItem->m_Pos.y;
MoveOneStruct( DrawLibItem, move_vector );

DrawStructsInGhost( panel, DC, DrawLibItem, 0, 0 );
@@ -493,6 +497,9 @@ void WinEDA_SchematicFrame::StartMovePar
memcpy( OldTransMat, Component->m_Transform, sizeof(OldTransMat) );

RedrawOneStruct( DrawPanel, DC, Component, g_XorMode );
+ // Redraw screen to get rid of artifacs. TODO: This doesn't seem to work perfecly for circles.
+ DrawPanel->PostDirtyRect( Component->GetBoundingBox() );
+
Component->m_Flags |= IS_MOVED;
DrawPanel->ManageCurseur( DrawPanel, DC, FALSE );
DrawPanel->m_AutoPAN_Request = TRUE;
 --Boundary-00=_g382Hy5GcTJq601--