PR575 seems to break shapefile about tilemaker HOT 20 OPEN

Gah. Do you know any other locations where this happens?

from tilemaker.

surfed around the coastlines for visual testing and it seems to appear in all regions. I also don't see a pattern in zoomlevel or similar. Will have to dig deeper ...

e.g. southeast of Boston #9.31/42.0623/-70.2313

between Athens and Mykonos
#8.7/37.7811/24.4236

Cadiz / Spain
#12.23/36.5034/-6.28439

Shetland Islands
#8.18/60.165/-1.144

from tilemaker.

Reading the shape gives (and always had given in the past) some messages, maybe its related as follow up-effect
This is for us-northeast. But none of these is near the Boston coordinate (the second is nearest ~400km away)

Shapefile entity #1628 type 5 is invalid. Parts:28. Reason:Geometry has invalid self-intersections. A self-intersection point was found at (-110.642, 25.7673); method: i; operations: u/i; segment IDs {source, multi, ring, segment}: {0, 0, -1, 214}/{0, 0, -1, 216}... failed to correct. Reason: Geometry has invalid self-intersections. A self-intersection point was found at (-110.642, 25.7673); method: i; operations: u/i; segment IDs {source, multi, ring, segment}: {0, 0, -1, 214}/{0, 0, -1, 216}
Shapefile entity #3197 type 5 is invalid. Parts:219. Reason:Geometry has invalid self-intersections. A self-intersection point was found at (-71.4807, 45.5092); method: i; operations: i/u; segment IDs {source, multi, ring, segment}: {0, 0, -1, 30261}/{0, 0, -1, 30263}... failed to correct. Reason: Geometry has invalid self-intersections. A self-intersection point was found at (-71.4807, 45.5092); method: i; operations: i/u; segment IDs {source, multi, ring, segment}: {0, 0, -1, 30261}/{0, 0, -1, 30263}
Shapefile entity #8142 type 5 is invalid. Parts:3153. Reason:Geometry has invalid self-intersections. A self-intersection point was found at (9.87713, 83.8271); method: i; operations: i/u; segment IDs {source, multi, ring, segment}: {0, 0, 2917, 3}/{0, 0, 2917, 5}... failed to correct. Reason: Geometry has invalid self-intersections. A self-intersection point was found at (9.87713, 83.8271); method: i; operations: i/u; segment IDs {source, multi, ring, segment}: {0, 0, 2917, 3}/{0, 0, 2917, 5}

from tilemaker.

Great, thank you. I'll see what I can find. I don't think those output errors are connected though you never know.

from tilemaker.

Try with #581. It uses the latest version of the geometry correct library which seems to fix this. I've tested with Cadiz and the Shetlands and in both cases it now works.

Compiling might be a bit tricksy - if it fails to build, let me know what version of Boost you have installed.

from tilemaker.

First the code stuff (as i don't know if this has side effects on the test):
compile on debian 12 with boost 1.74 crashed with the message below, so i changed the line
#if BOOST_VERSION < 107400
in correct.hpp to a higher number, then it compiled, but using the code part intended for boost < 1.74

Then the test:
First tests with europe and US it seems to be much better - nearly all of the effects mentioned in the issue and a couple of others found internally are gone - but Athens and a new find north of the bahamas (#9/26.3340/-77.3300 ) remain
Test with planet/other regions open
If this might be an affect of my change to boost version check i might try with boost newer version / backports if i manage to get it running, but i'm not a c++ guru

Initial compile error
In file included from src/geom.cpp:7:
./include/geometry/correct.hpp: In function ‘void geometry::impl::dissolve_find_intersections(const ring_t&, std::map<pseudo_vertice_key, pseudo_vertice<point_t>, compare_pseudo_vertice_key>&, std::set<pseudo_vertice_key, compare_pseudo_vertice_key>&)’:
./include/geometry/correct.hpp:130:26: error: ‘boost::geometry::strategies’ has not been declared
130 | boost::geometry::strategies::relate::cartesian<> strategy;
| ^~~~~~~~~~
./include/geometry/correct.hpp:130:56: error: expected primary-expression before ‘>’ token
130 | boost::geometry::strategies::relate::cartesian<> strategy;
| ^
./include/geometry/correct.hpp:130:58: error: ‘strategy’ was not declared in this scope; did you mean ‘boost::geometry::strategy’?
130 | boost::geometry::strategies::relate::cartesian<> strategy;
| ^~~~~~~~
| boost::geometry::strategy
In file included from /usr/include/boost/geometry/strategies/strategies.hpp:25,
from /usr/include/boost/geometry/geometry.hpp:58:
/usr/include/boost/geometry/strategies/tags.hpp:26:11: note: ‘boost::geometry::strategy’ declared here
26 | namespace strategy
| ^~~~~~~~
src/geom.cpp: In function ‘void simplify_ring(const GeometryType&, GeometryType&, double, const simplify_rtree&)’:
src/geom.cpp:19:5: warning: this ‘for’ clause does not guard... [-Wmisleading-indentation]
19 | for(std::size_t i = 0; i < input.size(); ++i)
| ^~~
src/geom.cpp:22:9: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘for’
22 | simplify_rtree rtree(
| ^~~~~~~~~~~~~~
src/geom.cpp: In instantiation of ‘void simplify_ring(const GeometryType&, GeometryType&, double, const simplify_rtree&) [with GeometryType = boost::geometry::model::ring<boost::geometry::model::d2::point_xy, true, true, std::vector, std::allocator>; simplify_rtree = boost::geometry::index::rtree<boost::geometry::model::segment<boost::geometry::model::d2::point_xy >, boost::geometry::index::quadratic<16> >]’:
src/geom.cpp:89:16: required from here
src/geom.cpp:57:29: warning: unused variable ‘result’ [-Wunused-variable]
57 | for(auto const &result: rtree | boost::geometry::index::adaptors::queried(boost::geometry::index::intersects(line)))
| ^~~~~~
src/geom.cpp:59:29: warning: unused variable ‘result’ [-Wunused-variable]
59 | for(auto const &result: outer_rtree | boost::geometry::index::adaptors::queried(boost::geometry::index::intersects(line)))
| ^~~~~~
make: *** [Makefile:98: src/geom.o] Error 1

from tilemaker.

Thanks for checking the Boost version - I have 1.76 on one machine and 1.68 on another so was slightly guessing, but a look at the commits makes it look like 1.76 is the breakpoint so I've updated that. Otherwise that shouldn't affect the behaviour. I'll take a look at those two examples.

from tilemaker.

I've had a fairly in-depth look at this and it doesn't look like an easy one to fix - I suspect the simplify code is generating a problematic geometry even before we get to clipping it. My gut is to merge #581 which makes things better and then keep iterating on the few remaining failures.

A peninsula on the eastern side of Andros which looks like this

is being simplified to this (note the self-intersections)

from tilemaker.

dang, all works great with extracts like europe or the us, but the planet doesn't finish as 2 threads seem to be stuck in an endless loop consuming 100% cpu since > 1 day. I managed to get the stacktrace with gdb but this points to geom correct i have no more info (can i get this with gdb ?)
Killing these processes would result in corrupt database and without knowing objectid it will be hard to find. Maybe to approach this could there be a way to e.g. say in writing "try this geometry (or tile) for x minutes and if doesnt succeed, discard, write empty and print failing id", just to get the hint on the problem ?

Generated points: 191992085, lines: 554101566, polygons: 657148788
Zoom level 14, writing tile 280752513 of 280754513

gdb -p 90546
(gdb) bt
#0 0x000055b2bc899a2a in void geometry::impl::correct<geometry::impl::fill_non_zero_winding<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator> >, void (boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator>&, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator>&), void (boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator> const&, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator> const&, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator>&), boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, boost::geometry::model::ring<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::allocator>, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator> >(boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator> const&, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator>&, double, geometry::impl::fill_non_zero_winding<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator> > const&, void ( const&)(boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator>&, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator>&), void ( const&)(boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator> const&, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator> const&, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator>&)) [clone .constprop.0] [clone .isra.0] ()
#1 0x000055b2bc898d68 in void geometry::impl::correct<geometry::impl::fill_non_zero_winding<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator> >, void (boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator>&, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator>&), void (boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator> const&, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator> const&, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator>&), boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, boost::geometry::model::ring<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::allocator>, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator> >(boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator> const&, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator>&, double, geometry::impl::fill_non_zero_winding<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator> > const&, void ( const&)(boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator>&, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator>&), void ( const&)(boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator> const&, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator> const&, boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator>&)) [clone .constprop.0] [clone .isra.0] ()
#2 0x000055b2bc89af4c in make_valid(boost::geometry::model::multi_polygon<boost::geometry::model::polygon<boost::geometry::model::d2::point_xy<double, boost::geometry::cs::cartesian>, true, true, std::vector, std::vector, std::allocator, std::allocator>, std::vector, std::allocator>&) ()
#3 0x000055b2bc6eaaa2 in buildWayGeometry(OSMStore&, OutputObject const&, TileBbox const&) ()
#4 0x000055b2bc8239dc in ProcessObjects(OSMStore&, __gnu_cxx::__normal_iterator<OutputObjectRef const*, std::vector<OutputObjectRef, std::allocator > >, __gnu_cxx::__normal_iterator<OutputObjectRef const*, std::vector<OutputObjectRef, std::allocator > >, SharedData&, double, double, bool, unsigned int, TileBbox const&, vector_tile::Tile_Layer*, std::vector<std::__cxx11::basic_string<char, std::char_traits, std::allocator >, std::allocator<std::cxx11::basic_string<char, std::char_traits, std::allocator > > >&, std::vector<vector_tile::Tile_Value, std::allocator<vector_tile::Tile_Value> >&) ()
#5 0x000055b2bc824902 in ProcessLayer(OSMStore&, TileCoordinates, unsigned int, std::vector<OutputObjectRef, std::allocator > const&, vector_tile::Tile&, TileBbox const&, std::vector<unsigned int, std::allocator > const&, SharedData&) ()
#6 0x000055b2bc825066 in outputProc(boost::asio::thread_pool&, SharedData&, OSMStore&, std::vector<OutputObjectRef, std::allocator > const&, TileCoordinates, unsigned int)
()
#7 0x000055b2bc85f21e in main::{lambda()#5}::operator()() const ()
#8 0x000055b2bc85f4ca in boost::asio::detail::executor_op<main::{lambda()#5}, std::allocator, boost::asio::detail::scheduler_operation>::do_complete(void*, boost::asio::detail::scheduler_operation*, boost::system::error_code const&, unsigned long) ()
#9 0x000055b2bc73fca1 in boost::asio::detail::scheduler::run(boost::system::error_code&) [clone .isra.0] ()
#10 0x000055b2bc748fb8 in boost::asio::detail::posix_thread::funcboost::asio::thread_pool::thread_function::run() ()
#11 0x000055b2bc74636f in boost_asio_detail_posix_thread_function ()
#12 0x00007f3fb3ea8044 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#13 0x00007f3fb3f2861c in ?? () from /lib/x86_64-linux-gnu/libc.so.6

from tilemaker.

i recalculated with the current master, great work, good improvement :)

All of the locations mentioned above are now looking good but unfortunately i found one problem area near Trieste.
#11.09/45.6796/13.7192

cannot say if its related to the South Uist / simplify effect mentioned in #597 or if it is still an edge-case in clipping

from tilemaker.

Thanks. Out of interest, is that with Maplibre GL Native or JS?

from tilemaker.

this is both and in inspect i see this effect in raw data

In Stockholm i have an effect only in raster/maplibre-native so i didn't report it first (vector and inspect data seem to be ok in this case on first sight)
#15/59.3232/18.0935

from tilemaker.

Thanks - I'll take a closer look!

from tilemaker.

3e90739 should fix the Trieste issue. It appears to be a rarely triggered bug in boost::geometry::union_.

I then tried with Stockholm on Native (iOS) and that seemed ok to me - let me know how you find it.

from tilemaker.

Unfortunately doesnt fix the problem :(
Doesnt behave exactly like before so it changed something but still having the effects - and some of the problems mentioned before near Cadiz, Copenhagen, Athens , Venice are also back (same regions as before but different zoom levels)
does the fix maybe not apply to all zoom levels equally (the outer.size > 15) or is this another effect ?

from tilemaker.

Gah. How can running correct more often break things. boost::geometry really hacks me off sometimes.

from tilemaker.

If I remove the outer.size check so that the line just reads

geom::correct(new_p);

and additionally replace #define BOOST_GEOMETRY_INCLUDE_SELF_TURNS in include/geom.h with #define BOOST_GEOMETRY_NO_ROBUSTNESS, then the results are as follows:

• Athens (--bbox 23.69,37.37,24.61,38.07): pass
• Boston (--bbox -70.86,41.68,-69.86,42.31): error at z5 (missing ocean area)
• Cadiz (--bbox -6.32,36.45,-6.16,36.58): pass
• Hebrides (--bbox -7.84,56.73,-6.09,58.55): minor error at z6 (small corner cut on South Uist)
• Shetlands (--bbox -1.78,59.84,-0.69,60.86): error at z9 (southern part of Bressay flooded)
• Stockholm (--bbox 17.97,59.3,18.16,59.36): pass
• Trieste (--bbox 13.45,45.59,13.84,45.8): pass
• Venice (--bbox 12.23,45.33,12.47,45.51): pass

(Tested with both Maplibre GL JS and Native. All using coastline-only data except Venice which is coastline+OSM.)

So I'm tempted to go with that and then look at Boston (in particular) when time permits as that's the most egregious outstanding issue.

does the fix maybe not apply to all zoom levels equally (the outer.size > 15) or is this another effect ?

The outer.size check was a small optimisation not to bother correcting the smallest polygons. The remaining issue isn't reliably correlated to any particular zoom level - essentially, as we scale and simplify polygons for use at the smaller zoom levels, this has the possibility of introducing self-intersections. For example, these are from the Boston coastline at z6:

from tilemaker.

#611 should fix the issues (famous last words).

from tilemaker.

great improvement
atm i have the Athens effect in z8 (#8.09/37.831/23.972) and a very small in Venice with islands parts cutted off (#10.12/45.4166/12.2092) but both only on one zoom level and only Athens directly visible
All others are fixed, but i will calculate a planet the next days so we can test more on our side

At the moment i would suggest merging the PR 611 as it fixes nearly all issues the current master has for all users
For those last issues (without being deep in the code) i have the feeling we try to workaround effects of boost libs, maybe we should file an issue there ?

from tilemaker.

That's great! Thanks for your help in identifying these issues.

I've merged #611. Do drop any screenshots here if you spot issues.

One of the changes with #611 is that it will now pro-actively drop very small polygons at lower zoom levels. This reduces tile sizes and rendering time, and removes artefacts and complexity that could cause rendering issues. The side effect is that you may occasionally see a polygon being 'clipped' if only a small amount of it is in one tile - for example, if a z6 tile is mostly land but has a very small bit of ocean polygon at one corner, that bit of ocean might be dropped. I have some ideas on what we can do about that, but it's a nice-to-have rather than an essential.

If I can drill down the boost::geometry issues to a couple of reproducible testcases then I'll file issues accordingly. I think often we're in edge case territory, where self-intersecting geometries are causing more drastic failures than you would expect.

from tilemaker.