← Back to team overview

kicad-developers team mailing list archive

About polygons in Kicad (was Segmentation fault when using Clipper and SHAPE_POLY_SET in 3D viewer)

 

Le 06/07/2015 23:11, Tomasz Wlostowski a écrit :
> On 06.07.2015 19:02, Wayne Stambaugh wrote:
>> On 7/6/2015 9:56 AM, Tomasz Wlostowski wrote:
>>> On 06.07.2015 15:51, jp charras wrote:
>>>> Le 06/07/2015 15:19, Maciej Sumiński a écrit :
>>>>> On 07/06/2015 02:30 PM, Tomasz Wlostowski wrote:
>>>>>> Fixes to SHAPE_POLY_SET slitting/fracturing algo: - Tom's too 
>>>>>> dumb to use C++ iterators (so he uses pointers now ;) - Some
>>>>>> speed optimization
>>>>>
>>>>> I have tested the patch with all demos and one case that crashed
>>>>> KiCad before and all worked fine. The changes have been committed
>>>>> in 5889, thanks Tom.
>>>>>
>>>>> Cheers, Orson
>>>>>
>>>>
>>>> It works fine for me too.
>>>> Hat off, Tomasz.
>>>
>>> Thanks.
>>>
>>> Let's remove boost::polygon dependency now.
>>>
>>> Tom
>>
>> Please!  At lease with Clipper, we know that we can resolve any issues
>> ourselves rather than depending on the Boost developers who haven't been
>> very helpful when issues are found in Boost.
> Hi,
> 
> We can proceed in two ways:
> - Modify all calls of bpl to use SHAPE_POLY_SET. Provide functions for
> importing/exporting between SHAPE_POLY_SET and CPOLYGONS_LIST/CPolyLine.
> - Rethink all these classes and design a single class that will
> represent a polygon set.
> 
> I'm in favor of the latter, even though it's quite a lot of work. On the
> other hand, it might save us work in the future (e.g. polygon-related
> DRC checks or handling polygons/keepouts in P&S).
> 
> BTW. I noticed quite a lot of overlap in the functionality of CPolyLine
> and CPOLYGONS_LIST. Is it just legacy or was it a deliberate design
> decision to have two polygon classes?
> 
> Cheers,
> Tom
> 

The overlap in the functionality of CPolyLine and CPOLYGONS_LIST is
mostly legacy.
The initial purpose of CPolyLine was the support of copper zones.
The first draft was coming from FreePCB to create a decent zone support.

Over the time, polygons were used for other things than copper zones (
3D viewer for instance) and the CPOLYGONS_LIST class ( coming from
CPolyLine ) was created.


Now, to remove boost::polygon, I am also in favor of the latter way.

Exactly, just provide the 2 functions for importing/exporting between
SHAPE_POLY_SET and CPOLYGONS_LIST/CPolyLine, because they are already in
use (and the code duplicated) in 3D viewer and zone filling algo, and
replace only calls of bpl which crashed, in stable version.
This is mostly done, because it is the difference between 2 sets of
polygons which crashes, but there are 2 or 3 places where the difference
is still computed (although we did not seen crashes, because they are
made on basic shapes)

We have the time to remove all calls to BPL which currently work fine.
And also we need time to be sure Clipper has no issue or find and fix them.
Remember the first bug report about BPL bug was received one year after
the version of Kicad using BPL was released.

And, like Tom suggest, rethink all these classes and design a single
class that will represent a polygon set, with basic transforms currently
in use in Kicad and graphic shape edition very easy to use.

We need this base class in zones and polygon-related DRC checks or
handling polygons/keepouts in P&S, but also in advanced pads,
footprints, micro-wave applications, advanced selection of items (block
selection, Altium like rooms) ...

-- 
Jean-Pierre CHARRAS


References