← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Refactor LAYER_ID to be the one and only layer definition

 

Hi Orson,

It's an interesting idea, I will have to look at it more.  But, doesn't
this still allow the programmer to accidentally overlap two enum values?  I
can add checks to prevent this from happening elsewhere in the code, but it
seems less clean to me.

Best,
-Jon

On Mon, Mar 13, 2017 at 1:52 PM, Maciej Suminski <maciej.suminski@xxxxxxx>
wrote:

> Hi,
>
> How about emulating enum inheritance [1]? I suppose it would be the
> cleanest solution allowing us to clearly specify what kind of layer is
> expected for certain methods. This is even better kind of type checking.
>
> Cheers,
> Orson
>
> 1. https://www.codeproject.com/kb/cpp/inheritenum.aspx
>
> On 03/13/2017 02:50 PM, Jon Evans wrote:
> > Hi Wayne,
> >
> > I understand this might seem like too big a change.
> > Here is what I was thinking when I thought that combining everything
> would
> > be a good solution.
> >
> > - If there is more than one enum, then functions that consume data from
> > more than one app (i.e. things in common/GAL) have to cast to int, so you
> > lose type checking that the enum gives you for free (or your type
> checking
> > gets more complicated, because the range of valid values is different for
> > each application)
> >
> > - If there is more than one enum, it's easier to duplicate layers for no
> > good reason (i.e. GerbView and Pcbnew have different layer ids for "grid"
> > right now)
> >
> > - I want to combine the color settings for all applications under the
> hood
> > (users will still be able to configure different colors for each
> > application).  This change will let color settings take LAYER_ID instead
> of
> > int, and there will only be one key/value mapping of colors -- no more
> > difference between "GetLayerColor" and "GetItemColor".  There will be no
> > clashes between the meaning of a layer id (int type) between different
> > applications.
> >
> > - Bringing Eeschema into this now is just early groundwork for Eeschema
> GAL
> > port (as well as unified color settings)
> >
> > If you will not accept this change, I have to think about a different
> > proposal that will make the different layer types in different
> applications
> > a bit more manageable than they are today.  I understand how having one
> > giant enum for LAYER_ID seems more complicated, I'm just worried that
> > having several different enums will make the code that consumes LAYER_ID
> > more complicated, especially if the applications become more integrated
> > with each other and start sharing more code.
> >
> > Best,
> > Jon
> >
> > On Mon, Mar 13, 2017 at 8:21 AM, Wayne Stambaugh <stambaughw@xxxxxxxxx>
> > wrote:
> >
> >> Jon,
> >>
> >> I misunderstood your original intent.  I don't think cluttering the
> >> board layer enums with all of the virtual layer and schematic layer
> >> enums is a good idea.  It just seems like overkill to me.  I thought you
> >> were going to create a separate enum for virtual board layers.  You
> >> could always start the virtual board layer enums from the last board
> >> layer enum if you need unique enums.  I would also prefer the schematic
> >> layer enums be separate from the board layer enums for clarity.  Anyone
> >> else have any thoughts on this?
> >>
> >> Cheers,
> >>
> >> Wayne
> >>
> >> On 3/12/2017 11:24 PM, Jon Evans wrote:
> >>> Hi all,
> >>>
> >>> Per the other thread, this patch unifies the layer definitions between
> >>> Pcbnew, GerbView, and Eeschema.  It removes the need for ITEM_GAL_LAYER
> >>> and some other macros, and it will simplify the implementation of
> >>> cross-application color themes and using GAL in multiple applications.
> >>>
> >>> Note that this patch introduces some temporary weirdness in a few
> >>> places, such as in COLORS_DESIGN_SETTINGS (there is now a single array
> >>> for color storage, but it's still referred to by two sets of
> >>> getters/setters).  This is because I wanted to keep this refactor as
> >>> simple as possible, as I plan to follow it up with an overhaul of color
> >>> settings when I share my color themes work.  I didn't want to do work
> >>> that I would soon end up getting rid of anyway.
> >>>
> >>> Best,
> >>> Jon
> >>>
> >>>
> >>> _______________________________________________
> >>> 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
> >>>
> >>
> >> _______________________________________________
> >> 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
> >>
> >
> >
> >
> > _______________________________________________
> > 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
> >
>
> _______________________________________________
> 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
>

Follow ups

References