Re: [Kicad-developers] CPolyLine refactor

2016-11-23 Thread jp charras 
Le 23/11/2016 à 13:32, Alejandro Garcia Montoro a écrit :
> Hi!
> 
> Thanks for the tips :)
> 
> I have some more doubts, one of them quite important: we have a class for 
> storing a set of polygons
> with holes, SHAPE_POLY_SET, but there is no explicit class to store a single 
> polygon with holes.
> There's a POLYGON typedef that could do the trick, but I think it would be a 
> better design if we
> created a SHAPE_POLYGON class that contained the methods from SHAPE_POLY_SET 
> (basically the
> iterator, the management of holes and vertices and the overrided from SHAPE) 
> and SHAPE_POLY_SET
> would be just a wrapper over SHAPE_POLYGON, basically a vector of them with a 
> couple of the methods
> that are now implemented (probably the ones from the Clipper library). 
> Furthermore, the
> SHAPE_POLY_SET::ITERATOR class should be also refactored and completed, as 
> for now it just iterates
> through the vertices of the outlines of the polygons, not through the 
> vertices of its holes.
> 
> I think I could send a patch soon with these changes, before going ahead with 
> the work I'm doing for
> the CPolyLine refactor (which is somewhat deeper than I though on the first 
> time), as I see a better
> correspondence between CPolyLine and SHAPE_POLYGON than between the former 
> and SHAPE_POLY_SET.
> 
> However, I still need a second opinion on this, I don't know if I'm 
> misunderstanding something.
> 
> Best,
> Alejandro

I think extracting POLYGON class from SHAPE_POLY_SET to make it a full separate 
class is a good idea.
Perhaps you could call it SHAPE_POLYGON_WITH_HOLE, to be more explicit and 
avoid mistakes with a
simple usual polygon.

Regards,

> 
> 2016-11-21 18:32 GMT+01:00 jp charras  >:
> 
> Le 21/11/2016 à 18:07, Tomasz Wlostowski a écrit :
> > On 21.11.2016 13:56, Alejandro Garcia Montoro wrote:
> >> Hi!
> >>
> >> I'm working on the CPolyLine refactor into SHAPE_POLY_SET. Before 
> coding
> >> anything more, I want to discuss with you the changes I have to do, 
> just
> >> to be sure I'm not doing any useless work.
> >>
> >> The classes that use CPolyLine, and that have to be modified, are:
> >>
> >>   * ZONE_CONTAINER: This class has two CPolyLine members: m_Poly and
> >> m_smoothedPoly. The latter is not used, but the former is the one
> >> that has all the information about the zone. It's necessary to
> >> carefully refactor this class, as the interface of CPolyLine and
> >> SHAPE_POLY_SET is not exactly the same, e.g., the Hatch-related
> >> functions are not yet implemented in SHAPE_POLY_SET or the code in
> >> DrawWhileCreateOutline access the methods of the old CPOLYGONS_LIST
> >> class.
> >
> > Hi Alejandro,
> 
> m_smoothedPoly temporary stores the outline (from m_Poly) before building 
> the filled areas in zone,
> because the zone outline can be smoothed or chamfered before filling this 
> zone.
> 
> >
> > If it's only about hatching, it's (as far as I know) only for the
> > purpose of displaying a hatched zone outline (JP, please confirm). Then
> > it belongs to the renderer (be it legacy or GAL), not to the polygon 
> class.
> 
> Yes, the hatches are only for rendering purposes.
> (more easy to see the different zone outlines, especially when zones 
> overlap, and more easy to see
> complex outlines with holes).
> 
> 
> >>   * BOARD, PCB_PAINTER and POINT_EDITOR: These classes use instances of
> >> the ZONE_CONTAINER class, that still has methods that return
> >> CPolyLine objects. The work on these classes should be more or less
> >> straightforward when the refactor on the first one is finished.
> > OK.
> >>
> >> I would like to set up some tests on these classes, just to be sure 
> that
> >> the refactor does not change anything. Does anyone have some tests 
> using
> >> these classes that I could reuse?
> > I don't ;-)
> >>
> >> Furthermore, some more instances of CPolyLine HATCH_STYLE enum are
> >> found. I am not sure that the hatch style should be inside
> >> SHAPE_POLY_SET, that contains just geometric information: what do you 
> think?
> > As said above, I don't think SHAPE_POLY_SET should contain hatching
> > functions.
> >
> > Best,
> > Tom
> >>
> >> Any comments on this will be appreciated.
> >>
> >> Regards,
> >> Alejandro
> 
> 
> --
> Jean-Pierre CHARRAS
> 
> ___
> Mailing list: https://launchpad.net/~kicad-developers 
> 
> Post to : kicad-developers@lists.launchpad.net 
> 
> Unsubscribe : https://launchpad.net/~kicad-developers 
> 
> More help   : https://help.launchpad.net/ListHelp 
> <

Re: [Kicad-developers] CPolyLine refactor

2016-11-23 Thread Alejandro Garcia Montoro 
Hi!

Thanks for the tips :)

I have some more doubts, one of them quite important: we have a class for
storing a set of polygons with holes, SHAPE_POLY_SET, but there is no
explicit class to store a single polygon with holes. There's a POLYGON
typedef that could do the trick, but I think it would be a better design if
we created a SHAPE_POLYGON class that contained the methods from
SHAPE_POLY_SET (basically the iterator, the management of holes and
vertices and the overrided from SHAPE) and SHAPE_POLY_SET would be just a
wrapper over SHAPE_POLYGON, basically a vector of them with a couple of the
methods that are now implemented (probably the ones from the Clipper
library). Furthermore, the SHAPE_POLY_SET::ITERATOR class should be also
refactored and completed, as for now it just iterates through the vertices
of the outlines of the polygons, not through the vertices of its holes.

I think I could send a patch soon with these changes, before going ahead
with the work I'm doing for the CPolyLine refactor (which is somewhat
deeper than I though on the first time), as I see a better correspondence
between CPolyLine and SHAPE_POLYGON than between the former and
SHAPE_POLY_SET.

However, I still need a second opinion on this, I don't know if I'm
misunderstanding something.

Best,
Alejandro

2016-11-21 18:32 GMT+01:00 jp charras :

> Le 21/11/2016 à 18:07, Tomasz Wlostowski a écrit :
> > On 21.11.2016 13:56, Alejandro Garcia Montoro wrote:
> >> Hi!
> >>
> >> I'm working on the CPolyLine refactor into SHAPE_POLY_SET. Before coding
> >> anything more, I want to discuss with you the changes I have to do, just
> >> to be sure I'm not doing any useless work.
> >>
> >> The classes that use CPolyLine, and that have to be modified, are:
> >>
> >>   * ZONE_CONTAINER: This class has two CPolyLine members: m_Poly and
> >> m_smoothedPoly. The latter is not used, but the former is the one
> >> that has all the information about the zone. It's necessary to
> >> carefully refactor this class, as the interface of CPolyLine and
> >> SHAPE_POLY_SET is not exactly the same, e.g., the Hatch-related
> >> functions are not yet implemented in SHAPE_POLY_SET or the code in
> >> DrawWhileCreateOutline access the methods of the old CPOLYGONS_LIST
> >> class.
> >
> > Hi Alejandro,
>
> m_smoothedPoly temporary stores the outline (from m_Poly) before building
> the filled areas in zone,
> because the zone outline can be smoothed or chamfered before filling this
> zone.
>
> >
> > If it's only about hatching, it's (as far as I know) only for the
> > purpose of displaying a hatched zone outline (JP, please confirm). Then
> > it belongs to the renderer (be it legacy or GAL), not to the polygon
> class.
>
> Yes, the hatches are only for rendering purposes.
> (more easy to see the different zone outlines, especially when zones
> overlap, and more easy to see
> complex outlines with holes).
>
>
> >>   * BOARD, PCB_PAINTER and POINT_EDITOR: These classes use instances of
> >> the ZONE_CONTAINER class, that still has methods that return
> >> CPolyLine objects. The work on these classes should be more or less
> >> straightforward when the refactor on the first one is finished.
> > OK.
> >>
> >> I would like to set up some tests on these classes, just to be sure that
> >> the refactor does not change anything. Does anyone have some tests using
> >> these classes that I could reuse?
> > I don't ;-)
> >>
> >> Furthermore, some more instances of CPolyLine HATCH_STYLE enum are
> >> found. I am not sure that the hatch style should be inside
> >> SHAPE_POLY_SET, that contains just geometric information: what do you
> think?
> > As said above, I don't think SHAPE_POLY_SET should contain hatching
> > functions.
> >
> > Best,
> > Tom
> >>
> >> Any comments on this will be appreciated.
> >>
> >> Regards,
> >> Alejandro
>
>
> --
> Jean-Pierre CHARRAS
>
> ___
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>
___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp