kicad-developers team mailing list archive

[Dick Hollenbeck <dick@...>]

Hi Jonas,

> Hi,
>
> I have had a look at your changes, Dick, and added some Stuff for other 
> objects (junctions and components).
>
> I have attached a patch. Am I on the right track? If yes, commit my changes to 
> svn please. If no, please give me some hints on what to improve.
>
> I noticed that there are some functions GetBoundaryBox() (in classes 
> PartTextStruct, EDA_LibComponentStruct, EDA_SchComponentStruct) which seem to 
> have similar functionality. However, they are not exactly what I needed: 
> EDA_SchComponentStruct, for instance, does only return the BBox of the 
> component itself, not the associated text fields. So I have used that only 
> for a basis of my EDA_SchComponentStruct::GetBoundingBox() and joined the 
> BBoxes of the text field myself. Here, I cheated a bit to save calculating 
> the boundary of pin ends... :-)
>
> Best regards,
> Jonas.

This is good work. Some remarks:

** 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.

** EDA_BaseStruct is shared by pcbnew, eeschema, gerbvew, etc. and 
because we had one implementation of the GetBoundingBox() which would 
could not be easily made "const", I dropped that from the anchoring 
prototype in EDA_BaseStruct. That is, it is no longer a "const" 
virtual function.

** The virtual is not required in the derived classes, so I removed 
those, as the compiler knows that they are all virtual by inheritance 
from EDA_BaseStruct.

** The Doxygen comments are inherited from the header file when the 
DOXYGEN html docs are generated, so I fixed up the base class's comment 
for EDA_BaseStruct::GetBoundingBox because it is inherited when the 
documentation files are generated unless overridden. I also made the 
comment somewhat generic so that text will make sense in any derived 
classes. The config file for doxygen is setup to give priority to 
header files when extracting function comments, and that is why I 
usually won't duplicate the function comments in both places.

** Any immediately back to back PostDirtyRect() calls be OK, and will 
cause each rectangle to be combined by the wxWidgets environment into an 
intersecting rectangle. For an example of this, see
void WinEDA_PcbFrame::Delete_net( wxDC* DC, TRACK* Track ) in 
pcbnew/deltrack.cpp where it is called from a loop.

** 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.

This below could be made into a script for folks to beautify a single 
file. It would be nice to add that script to the project in the base 
source directory. Maybe genericize the uncrustify path, otherwise it 
might be close now:


#!/bin/bash

# A script to beautify one file in situ. With subversion to fall back on,
# there is little danger that the file will be destroyed because of a bug
# in the beautifier, but for this to be the case, you should be in a 
position
# to "revert" to a prior version. Keep "revert" in mind.

# Build the latest uncrustify from its sourceforge subversion repository
# or install a recent binary package


if [ $# -ne 1 ] ; then
echo "Usage `basename $0` <filename>"
exit 1
fi

# use indentoutput.cpp as a temporary file, then move it back to original
/svn/uncrustify/src/uncrustify -f "$1" -c uncrustify.cfg -o indentoutput.cpp
mv indentoutput.cpp "$1"

###################################

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.


Dick