kicad-developers team mailing list archive

[jp charras <jp.charras@xxxxxxxxxx>]

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
> 


-- 
Jean-Pierre CHARRAS