kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

On 6/5/19 7:20 AM, jp charras wrote:
> Le 04/06/2019 à 22:51, Wayne Stambaugh a écrit :
>> On 6/4/19 4:25 PM, Seth Hillbrand wrote:
>>> On 2019-05-31 07:25, Wayne Stambaugh wrote:
>>>> On 5/30/19 4:53 PM, Seth Hillbrand wrote:
>>>>> On 2019-05-30 15:00, jp charras wrote:
>>>>>> Le 29/05/2019 à 21:31, Seth Hillbrand a écrit :
>>>>>>> On 2019-05-29 10:33, jp charras wrote:
>>>>>>>> Attached a patch that modify the way filled areas (solid polygons)
>>>>>>>> are
>>>>>>>> built in copper areas.
>>>>>>>>
>>>>>>>> Currently, solid polygons are slightly smaller than the exact
>>>>>>>> area, and
>>>>>>>> the polygon outlines have a thickness to fill the exact area.
>>>>>>>> With this patch, polygon outlines have no thickness and the polygons
>>>>>>>> have the exact area.
>>>>>>>>
>>>>>>>> To test it on a given zone, the zone setting must be edited with the
>>>>>>>> "Fill polys without thick outline" checked.
>>>>>>>
>>>>>>>
>>>>>>> Hi JP-
>>>>>>>
>>>>>>> Why did you decide to make this a user option?  Is there some feature
>>>>>>> that it prevents that a user would want for some areas but not for
>>>>>>> others?
>>>>>>>
>>>>>>> I tested it with a large board and it reduces the polygon point
>>>>>>> count by
>>>>>>> almost 50% (!) for complex fills.  If I zoom in on an edge, it appears
>>>>>>> that the approximation count is substantially coarsened by the patch. 
>>>>>>> See attached image.  The edge on the right is with the new option
>>>>>>> enabled.  The edge of the left is without the new option.
>>>>>>>
>>>>>>> I didn't find any other issues.  Large boards were much faster and
>>>>>>> DRC /
>>>>>>> plotting appear consistent between options (with the exception noted
>>>>>>> above)
>>>>>>>
>>>>>>> -Seth
>>>>>>
>>>>>>
>>>>>> Thanks Seth for your test.
>>>>>>
>>>>>> Currently, having a user option is useful to test and compare the 2
>>>>>> options (the current way, and the new way).
>>>>>
>>>>> OK.  Makes sense.  Instead of changing the board file, can we put the
>>>>> option in the advanced config file to enable our testing?  It would be
>>>>> nice to avoid changing the file format here.
>>>>
>>>> I agree that the board file format should not change for rendering
>>>> configuration.  Please make this a user option.
>>>
>>>
>>> Hi JP-
>>>
>>> Did you mean to push eb1faebf1?
>>>
>>> -Seth
>>
>> I thought this was not going to require any board file version changes.
>>  Did I miss something here?
>>
>> Wayne
>>
> 
> Some clarification:
> 
> I pushed some changes because maintaining my local branch against all
> changes in master branch was not always easy.

I know keeping things rebased given the current development churn but
you should have held off committing the file format change before
committing the rest of the code that actually does anything with the
file format changes.  How much longer before you merge the rest of the
changes?  If it's going to be too long, we should probably revert your
initial commit.

> 
> I do not yet committed the new zone fill algo living in zone_filler.
> It should happen very soon, after a few other tests.
> (note also, to be enabled, the advanced config file must be set)

If this is the case, why did we just get a message on the dev mailing
list complaining about file compatibility with 5.1?  If commit eb1faebf
uses the advanced config (which I'm not seeing) and the default is
disabled, why would this happen?  Did you forget to merge the advanced
config part of your changes?

> 
> Now, about changes in file format, here are my thoughts:
> 
> New zone filler algo is not "just" a rendering change.
> It modify many other things.
> 
> These changes are not not due to a code change, but due to the fact the
> size/shape of solid polygons in zones are actually modified:
> Gerber files are modified.
> Connectivity is modified.
> 
> Moreover, in order to remove thick outlines, the solid polygon shapes,
> once they are build, are inflated by zone min thickness/2>
> I am really confident with the Clipper Library reliability, and the
> Inflate function.
> But previous experiences with other polygon libraries lead me to be
> *very*careful.
> I know Inflate transform is one of most hard to manage transform.
> 
> I remember when we used boost::polygon, the first issue encountered was
> reported one year after we started using it, and it was the reason we
> was forced to use clipper.
> 
> Therefore I want to be able to use the old (reliable...) way to build
> the solid polygons, and to use the new way (without outline thickness).
> 
> It imply the zones filled with the new way must have a flag to keep
> trace of that.

Fair enough.  I misunderstood the original intent of this change.

> 
> There is also an other (and important) problem:
> We always try to keep Pcbnew compatible with old board files.
> Without this flag, this is not possible:
> * A old board generates broken Gerber files, unless zones are refilled,
> but it imply a board change (and the fact the user knows that...).
> By broken Gerber files I mean zones without thick lines can have very
> thin areas, not compatible with the zone settings.
> * A new board generates also broken Gerber files, unless zones are
> refilled, because filled areas are too big.
> It can happens if for some reasons we need to go back to the "old"
> (current) filling algo.
> I hope it does not happens, but who know.
> 
> Note also files saved by the current pcbnew version are not compatible
> with 5.1.2 stable versions, due to the fact the "max_error" parameter is
> now stored in file.
> 
> So to avoid predictable issues, I prefer have a flag stored in file (the
> cost is low), and a user settings to select the way the solid areas are
> built, at least for now, just in case...
>