kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxxxx>]

Lorenzo,

I like a lot of the work you did here and I appreciate your efforts and
enthusiasm.  I have a few comments about this patch.

1) It is fine for folks to test but there are too many changes at least
for me to review and commit at one time.  For committing purposes I
would prefer this be broken into four separate patches.  One each for
the EDA_COLOR_T, EDA_DRAW_MODE_T, PCB_LAYER_MASK, and the Eeshema
m_Layer removal.  It's not necessarily the size of the patch.  It's the
number of things it changes that makes me take pause.

2) Please convert tabs to 4 spaces.

3) Please remove trailing white space.

4) It appears you are using Windows as your development platform because
the new files you added have ^M as the line ending.  Your editor should
be able to strip these out and configured not to use them.

I'm extremely busy right now so I don't have any free time to test this.
 Hopefully there are some other folks out there who can help out with
the testing.  Thanks again for your efforts.

Wayne

On 5/9/2012 5:35 AM, Lorenzo Marcantonio wrote:
> The witch hunt gave its fruits. The patch is a biggie, now I'm testing
> it but obviously some bug could have crept it (aka the 'shit happens'
> principle).
> 
> This would contribute to the C++-ification of kicad and also remove some
> slight nearly-impossible-to-hit bugs (bonus point: what's the difference
> between FIRST_NO_COPPER_LAYER and FIRST_NON_COPPER_LAYER? now the
> compiler tags the error).
> 
> What happened:
> 
> - On the exterior *nothing*, I hope. It's only an innard rework.
> 
> - PCB/gerber layers representation changed from int to enum (and
>   corresponding operators added). Newer g++ flags as error the assigment
>   of an int to and enum! So it's more like a 'lightweight' class...
>   We now have the PCB_LAYER_NUMBER and the PCB_LAYER_MASK. The
>   GetLayerMask() was put inline in the header so we'll never seen the
>   ugly (1 << layer) construct. PCB_LAYER_MASK has the appropriate
>   boolean operators so they're handy to manipulate. Bonus: it's a big
>   step for adding new layers since these are some big abstractions
>   anyway (at least they flag every place where a layer/mask is used).
>   Calls to LayerFromInt and static_cast mark 'difficult' places.
> 
> - Many #defines converted to static const; that's the C++ style
> 
> - The same enum-conversion was applied to colors. Now colors are not
>   anymore anonymous ints but the EDA_COLOR_T is fully employed. As
>   someone already know EDA_COLOR_T also carries some flags and the alpha
>   should value in the higher bits. Some functions help encapsulate this.
>   Also used the MakeColour when possible instead of accessing the raw
>   rgb fields.
> 
> - GRDrawMode encapsulated as an enum, too (it's a bit field struct,
>   anyway, but enums help the compiler keeping them separate).
> 
> - Completely removed m_Layer from the SCH_ITEM classes; now these are
>   identified by the class only (i.e. no more subtype for lines which
>   before could represent wires, buses or even graphic lines). The
>   refactored classes were the line and bus entry. A factory load
>   function is used to create the right class depending on the input
>   record.
> 
> - Removed SCH_POLYLINE since it was never implemented anyway
> 
> - Some global renamed with the proper prefix (eg. Drc_On -> g_Drc_on)
> 
> (still have to find the source for the 'same color of the background'
> message, anyway...)
> 
> 
> 
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp