pcbnew CPolyLine::AppendCorner should call CloseLastContour

[Miles McCoo]

running latest code. Version info below.

I have a proposed fix below for which I'm happy to submit a patch. I don't yet know the code
enough to predict what side effects this could have. Hence, I'm filing this bug report.

Description
-----------
For those that use the python scripting interface to create zones, it is easy to not know that
one must call CloseLastContour() on an outline after any calls to AppendCorner

If CloseLastContour is not called, saving the layout will yield a corrupt kicad_pcb file. One of the needed closing ')' is omitted.

This problem has also been mentioned here:
https://forum.kicad.info/t/yet-another-python-teardrop-script-adds-and-deletes-teardrops-to-a-pcb-v0-3-0/3388

To reproduce,
-------------
run this script:
https://github.com/mmccoo/kicad_mmccoo/blob/master/zonebug/zonebug.py

on any pcb. The pcb needs at least one net.

Then saveas and try to load the resulting file.

Proposed fix
------------
add
 m_CornersList->CloseLastContour();

to the end of
 void CPolyLine::AppendCorner( int x, int y )

This is the error message given when trying to load:
Error loading board.
IO_ERROR: Expecting 'net, layer, tstamp, hatch, priority, connect_pads, min_thickness, fill, polygon, filled_polygon, or fill_segments' in input/source
'/bubba/electronicsDS/kicad/leddriver2/bug.kicad_pcb'
line 2947, offset 4

from dsnlexer.cpp : Expecting() line:369

on ubuntu 16.04 using
cmake -DCMAKE_BUILD_TYPE=Debug -DwxWidgets_USE_DEBUG=ON -DKICAD_SCRIPTING_WXPYTHON=ON -DKICAD_SCRIPTING=ON -DKICAD_SCRIPTING_MODULES=ON -DCMAKE_INSTALL_PREFIX=`realpath ../install` ../kicad

Application: pcbnew
Version: (2017-03-15 revision 3cc90ce)-master, debug build
Libraries: wxWidgets 3.0.2
           libcurl/7.47.0 GnuTLS/3.4.10 zlib/1.2.8 libidn/1.32 librtmp/2.3
Platform: Linux 4.4.0-66-generic x86_64, 64 bit, Little endian, wxGTK
- Build Info -
wxWidgets: 3.0.2 (wchar_t,wx containers,compatible with 2.8)
Boost: 1.58.0
Curl: 7.47.0
KiCad - Compiler: GCC 5.4.0 with C++ ABI 1009
        Settings: USE_WX_GRAPHICS_CONTEXT=OFF
                  USE_WX_OVERLAY=OFF
                  KICAD_SCRIPTING=ON
                  KICAD_SCRIPTING_MODULES=ON
                  KICAD_SCRIPTING_WXPYTHON=ON
                  KICAD_SCRIPTING_ACTION_MENU=OFF
                  BUILD_GITHUB_PLUGIN=ON
                  KICAD_USE_OCE=OFF

[jean-pierre charras (jp-charras)]

void CPolyLine::AppendCorner( int x, int y ) appends a corner to a polyline.
It is not intended to close the current polyline.
You can (and must) close the *current* contour only after the last point is entered.

m_CornersList->CloseLastContour() closes the current polyline, and a new polyline can be started.

(after CloseLastContour(), call AppendCorner() add the first point of a *new* polyline
(for a zone: a new hole in the outline already defined).

The Kicad code is good.
The Python script is incorrect.

[Miles McCoo (mmccoo)]

is there a downside to closing a polyline early or more than once?

is there a rule "you may NOT close the polyline early. bad things will happen if you do"?

will you have an incorrect polyline if you make redundant calls to closecontour?

where possible, it should be easy to do things correctly. It should be hard to do it wrong.

[jean-pierre charras (jp-charras)]

What is exactly your problem?

I am thinking I already answered your questions about CloseLastContour() meaning.

A contour must be closed once it is *finished*.
Next calls to AppendCorner() add a corner to the *new* contour, if the previous is closed.
InsertCorner() add a corner to the given contour.

[Miles McCoo (mmccoo)]

I was trying to prevent confusion by others who don't know that a contour has to be finished.

I hadn't noticed before that always closing a contour causes a new one to be created each time.

You did say that in your original response, I just didn't absorb it.