Geometry Boost List: [geometry] reading WKT (was: #8376: difference of non-closed polygons returns closed polygon)

Subject: [geometry] reading WKT (was: #8376: difference of non-closed polygons returns closed polygon)
From: Adam Wulkiewicz (adam.wulkiewicz_at_[hidden])
Date: 2014-10-11 15:12:14

Adam Wulkiewicz wrote:
> Volker Schöch wrote:
>>> I tried this example as well (see attached program and output).
>>> What I see is that the output has 18 points (I can see that from 
>>> both the dsv printout of the geometry, as well as from 
>>> bg::num_points). So the resulting multi-polygon is not closed, but 
>>> when you output it through bg::wkt, it is shown as closed.
>> Thank you very much for double-checking! In the mean time I, too, 
>> found that things may not be what they appear to be, but I could not 
>> yet nail it. Apparently, not only wkt but also read_wkt mix up the 
>> closedness. I got caught by this because I have a number of unit 
>> tests in place based on read_wkt, which are now triggering like crazy.
>>
>> I'm glad that it's just wkt/read_wkt, and not the actual algorithm, 
>> that is at fault.
>>
>> Should I file a ticket for the wkt/read_wkt issue? It interferes with 
>> debugging, unit-testing, and with reporting issues, too!
>
> FYI, some time ago WKT was fixed to always output closed Polygons. 
> AFAIU this change was made because an output must be consistent and 
> representation-independent. Not to mention that in WKT language 
> Polygons are just defined as closed, so it's also a matter of 
> conformance. Consider e.g. creating WKTs for open polygons in BG and 
> then loading them using other software.
>
> For convenience:
> http://boost-geometry.203548.n3.nabble.com/problem-with-wkt-io-td4025641.html 
>
> https://svn.boost.org/trac/boost/ticket/9217
> https://github.com/boostorg/geometry/commit/5f6f6212d624397f943761cd68fe52693438f37f 
>
>
> Is this related to your problem?
>
> Regarding read_wkt AFAIR it checks if the last Point is the same as 
> the first one and do whatever needs to be done with it. I don't 
> remember if it also appends the missing closing Point, probably it is.
> Assuming that I'm right, maybe it should throw an exception if the 
> input was invalid (not closed) if that's a problem? We probably 
> shouldn't assume that we know why the input is incorrect, maybe the 
> last Point is missing on purpose and should be added, or maybe the 
> last Point is different by mistake so should be fixed/replaced. But 
> I'm not sure if the behavior should be changed at this point, plus 
> nobody complains about.
>
> So you should file a ticket if you think that something's wrong or 
> should be done differently.
> Or we could first consider all aspects in detail on the list.

It seems that read_wkt() function always naiively fills the Polygon with 
data, not taking into account the closure. This means that open Polygons 
contains duplicated Points if "proper" closed WKT is passed. I consider 
this as a bug, what do you think?
If we decided to fix it, do you think that read_wkt() for open Polygons 
should:
1. check the last Point and if it was the same as the first one, not 
append it
2. check the correctness of the input and e.g. throw if the WKT wasn't 
closed
3. always assume that a valid WKT is passed and always ommit the last Point
4. ?

1 is probably the best because it's backward compatible.

The opposite situation could also be possible. Filling closed Polygon 
with open WKT would probably result in invalid Polygon.
Though this could be handled by bg::correct().
Btw, maybe bg::correct() should not only append missing Points for 
closed Polygons but also remove duplicated for open ones? For consistency.

Regards,
Adam