← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] CPolyLine -> SHAPE_POLY_SET refactor

 

Where do we stand on a fix for this?

On 4/10/2017 3:18 AM, jp charras wrote:
> Le 10/04/2017 à 08:39, Nick Østergaard a écrit :
>> Hi JP,
>>
>> Yes I can confirm that it seems to work now, but since you mentioned
>> that it was not fixed properly yet, I thought it was worth
>> highlighting in this thread. Are unittests missing here for the
>> geometry library or is this on a higher level?
> 
> About SEG class, unittests works.
> 
> It is more the fact when using a SEG class, you can now modify an other entity without clearly
> notice in your code the other entity will be modified, by side effect, because now the SEG class
> stores a reference to the other entity, not a copy of values.
> 
> It is this side effect I do not like.
> 
> About bugs reports for zones, zones had 3 different bugs:
> * The outlines was broken when adding a corner (GAL) due to the SEG class change.
> * Cutout outlines were incorrectly saved and read in .kicad_pcb files.
> * Cutout outlines were incorrectly managed in GAL, when created: they were added as main outlines,
> not holes in an existing main outline.
> 
> Still remains serious issues in GAL mode about self intersecting and overlapping polygons after
> creation or edition.
> These issues exist since the beginning.
> (They are not too hard to fix, because methods to fix them are written for the legacy mode, but need
> a better knowledge I have to correctly manage undo/redo on GAL, due to the fact the number of
> existing or new zones can change after cleaning or editing zones)
> 
> Note also usability issues in zones pop up menu (missing option to delete a cutout, and most
> commands disabled when the zone tool is activated).
> 
>>
>> Nick
>>
>> 2017-04-10 8:21 GMT+02:00 jp charras <jp.charras@xxxxxxxxxx>:
>>> Le 09/04/2017 à 22:18, Nick Østergaard a écrit :
>>>> Hi Alejandro,
>>>>
>>>> It seems like there is a use case you may not have considered. The zone cutout.
>>>
>>> I am thinking I recently fixed this issue.
>>>
>>> However I am not thrilled by commit f68ce306bdca0a2f5a1a234497ede6550ca79a0b for geometry/seg.h
>>> Instead of storing coordinates, SEG stores after this commit references to VECTOR2I of an other entity.
>>> It means when you (for some reason) modify a coordinate value in a SEG instance, you can modify also
>>> the other entity, that is not expected.
>>> (I saw that when I fixed the zone cutout issues).
>>> It was one (but not the only one) reason cutouts did not work.
>>>
>>> However I am really not satisfied by the way polygon zones are created or edited in GAL:
>>> they can easily be self intersecting (when created or when a corner is moved or deleted) or overlapping.
>>>
>>> In Kicad, polygons *cannot be* self intersecting (not accepted in Gerber files).
>>>
>>> In Legacy mode, they are tested and modified (break into 2 or more polygons, and/or merged with
>>> similar zones) after each change.
>>> This is mandatory to avoid broken zones.
>>>
>>>>
>>>> See https://bugs.launchpad.net/kicad/+bug/1679795
>>>>
>>>>
>>>> 2017-03-24 11:01 GMT+01:00 Maciej Sumiński <maciej.suminski@xxxxxxx>:
>>>>> TL;DR: Everything seems fine, I am going to merge the branch today.
>>>>>
>>>>> For the record: I got the board that was showing the differences. I
>>>>> refilled zones using the master branch, then once again with the polygon
>>>>> refactor patch applied. Diff of zone polygons shows no difference.
>>>>>
>>>>> There were differences between the zones in the original file and the
>>>>> just refilled, so there could be a change in the filling algorithm in
>>>>> the meantime.
>>>>>
>>>>> I am going to merge the branch soon. Thank you Alejandro, I know it was
>>>>> a rough road, but we are really grateful for your work. Well done!
>>>>>
>>>>> Regards,
>>>>> Orson
>>>>>
>>>>> On 03/23/2017 09:19 AM, Maciej Sumiński wrote:
>>>>>> Finally I had some time to test the branch and I could not find any
>>>>>> problems, hence I would like to merge it. Let me know if there are any
>>>>>> objections.
>>>>>>
>>>>>> @Nick:
>>>>>> Could you give more details? I placed keepout zones and refilled zones
>>>>>> for all demo boards, and every time I get exactly the same zones (diffed
>>>>>> two .kicad_pcb files).
>>>>>>
>>>>>> Regards,
>>>>>> Orson
>>>>>>
>>>>>> On 02/18/2017 08:13 PM, Nick Østergaard wrote:
>>>>>>> I have noticed that Alejandro's branch does not have a clearance
>>>>>>> distance to a keepout zone, which the old filling algorithm has.
>>>>>>>
>>>>>>> I am not sure what is really desired, but this could potentially break
>>>>>>> old designs, although I like the new way where the zone goes to the
>>>>>>> keepout edge.
>>>>>>>
>>>>>>> 2017-02-17 20:24 GMT+01:00 jp charras <jp.charras@xxxxxxxxxx>:
>>>>>>>> Le 17/02/2017 à 19:41, Alejandro Garcia Montoro a écrit :
>>>>>>>>> Hi!
>>>>>>>>>
>>>>>>>>> The errors were caused by some asserts that contained functions that needed to be called... My bad.
>>>>>>>>> Now the asserts are gone and the errors are handled via out_of_range exceptions (they were related
>>>>>>>>> with possible illegal memory access),
>>>>>>>>>
>>>>>>>>> JP, I finally saw the zone filling error in the release build!
>>>>>>>>>
>>>>>>>>> Nick and JP, if you can pull and test the branch again and see if the errors you saw are fixed, that
>>>>>>>>> would be great. Thank you.
>>>>>>>>>
>>>>>>>>> Best,
>>>>>>>>> Alejandro
>>>>>>>>
>>>>>>>>
>>>>>>>> At first glance, the issues I previously saw are gone.
>>>>>>>> Thanks.
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Jean-Pierre CHARRAS
>>>
>>>
>>> --
>>> Jean-Pierre CHARRAS
>>>
>>> _______________________________________________
>>> 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
>>
> 
> 


References