kicad-developers team mailing list archive

[Tomasz Wlostowski <tomasz.wlostowski@xxxxxxx>]

On 23.11.2016 15:02, jp charras wrote:
> 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.
> 

Hi JP & Alejandro,

I'm hesitant to agree with this idea:
- We risk proliferating more polygon classes this way. Pcbnew operates
almost exclusively on polygon sets. I can't think of a single case where
a special case for a single polygon with holes would be beneficial. For
storing polygon outlines, a SHAPE_LINE_CHAIN should be sufficient.
- It's not possible to predict the result of boolean polygon operations
compile-time, they return one or more polygons, each of them
with/without holes. Therefore, having specialized classes for each of
these cases would make handling them unnecessarily complex (RTTI).
- I would rather improve the iterators in the current SHAPE_POLY_SET
class so that it's easy to access all vertices of each outline/hole.

Cheers,
Tom



> Regards,
> 
>>
>> 2016-11-21 18:32 GMT+01:00 jp charras <jp.charras@xxxxxxxxxx <mailto:jp.charras@xxxxxxxxxx>>:
>>
>>     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 <https://launchpad.net/%7Ekicad-developers>
>>     Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>>     Unsubscribe : https://launchpad.net/~kicad-developers <https://launchpad.net/%7Ekicad-developers>
>>     More help   : https://help.launchpad.net/ListHelp <https://help.launchpad.net/ListHelp>
>>
>>
> 
>