Merge pull request #101 from mapbox/polygon-clipping-fix

Polygon clipping fix

Author: artemp

.travis.yml

@@ -35,18 +35,10 @@ matrix:
 
     ## ** Builds that are published **
 
-    # linux cfi build node v6/release
+    # linux cfi build node v10/release
     - os: linux
       env: BUILDTYPE=release TOOLSET=cfi CXXFLAGS="-flto -fsanitize=cfi -fvisibility=hidden" LDFLAGS="-flto -fsanitize=cfi"
-      node_js: 6
-    # linux publishable node v6/release
-    - os: linux
-      env: BUILDTYPE=release
-      node_js: 6
-    # linux publishable node v6/debug
-    - os: linux
-      env: BUILDTYPE=debug
-      node_js: 6
+      node_js: 10
     # linux publishable node v8/release
     - os: linux
       env: BUILDTYPE=release
@@ -63,16 +55,6 @@ matrix:
     - os: linux
       env: BUILDTYPE=debug
       node_js: 10
-    # osx publishable node v6/release
-    - os: osx
-      osx_image: xcode9.2
-      env: BUILDTYPE=release
-      node_js: 6
-    # osx publishable node v6/debug
-    - os: osx
-      osx_image: xcode9.2
-      env: BUILDTYPE=debug
-      node_js: 6
     # osx publishable node v8/release
     - os: osx
       osx_image: xcode9.2
@@ -85,7 +67,7 @@ matrix:
       node_js: 10
     - os: linux
       env: BUILDTYPE=debug TOOLSET=asan
-      node_js: 6
+      node_js: 10
       sudo: required
       # Overrides `install` to set up custom asan flags
       install:
@@ -107,8 +89,8 @@ matrix:
 
     # g++ build (default builds all use clang++)
     - os: linux
-      env: BUILDTYPE=debug CXX="g++-6" CC="gcc-6" LINK="g++-6" AR="ar" NM="nm"
-      node_js: 6
+      env: BUILDTYPE=debug CXX="g++-6" CC="gcc-6" LINK="g++-6" AR="ar" NM="nm" CXXFLAGS="-fext-numeric-literals"
+      node_js: 10
       addons:
         apt:
           sources:
@@ -124,7 +106,7 @@ matrix:
     # Coverage build
     - os: linux
       env: BUILDTYPE=debug CXXFLAGS="--coverage" LDFLAGS="--coverage"
-      node_js: 6
+      node_js: 10
       # Overrides `script` to publish coverage data to codecov
       script:
         - export PATH=$(pwd)/mason_packages/.link/bin/:${PATH}
@@ -151,7 +133,7 @@ matrix:
     # Clang tidy build
     - os: linux
       env: CLANG_TIDY
-      node_js: 6
+      node_js: 10
       # Overrides `install` to avoid initializing clang toolchain
       install:
         # First run the clang-tidy target

src/feature_builder.hpp

@@ -220,43 +220,40 @@ struct overzoomed_feature_builder
     {
         std::vector<detail::annotated_ring<CoordinateType>> rings;
         vtzero::decode_polygon_geometry(feature.geometry(), detail::polygon_handler<CoordinateType>(rings, dx_, dy_, zoom_factor_));
-        if (!rings.empty())
+        std::vector<mapbox::geometry::polygon<CoordinateType>> polygons;
+        bool process = false;
+        for (auto const& r : rings)
+        {
+            if (r.second == vtzero::ring_type::outer)
+            {
+                auto extent = mapbox::geometry::envelope(r.first);
+                process = boost::geometry::intersects(extent, bbox_);
+                if (process) polygons.emplace_back(); // start new polygon
+            }
+            if (process && r.first.size() > 3)
+            {
+                polygons.back().push_back(std::move(r.first));
+            }
+        }
+        if (!polygons.empty())
         {
             vtzero::polygon_feature_builder feature_builder{layer_builder_};
             feature_builder.copy_id(feature);
             bool valid = false;
-            bool process = false;
-            for (auto& r : rings)
+            for (auto const& poly : polygons)
             {
-                if (r.second == vtzero::ring_type::outer)
-                {
-                    auto extent = mapbox::geometry::envelope(r.first);
-                    process = boost::geometry::intersects(extent, bbox_);
-                }
-                if (process)
+                std::vector<mapbox::geometry::polygon<coordinate_type>> result;
+                boost::geometry::intersection(poly, bbox_, result);
+                for (auto const& p : result)
                 {
-                    std::vector<mapbox::geometry::polygon<coordinate_type>> result;
-                    if (r.second == vtzero::ring_type::inner) boost::geometry::reverse(r.first);
-                    // ^^ reverse inner rings before clipping as we're dealing with a disassembled polygon
-                    boost::geometry::intersection(r.first, bbox_, result);
-                    for (auto const& p : result)
+                    for (auto const& ring : p)
                     {
-                        for (auto const& ring : p)
+                        if (ring.size() > 3)
                         {
-                            if (ring.size() > 3)
-                            {
-                                valid = true;
-                                feature_builder.add_ring(static_cast<unsigned>(ring.size()));
-                                if (r.second == vtzero::ring_type::outer)
-                                {
-                                    std::for_each(ring.begin(), ring.end(), [&feature_builder](auto const& pt) { feature_builder.set_point(static_cast<int>(pt.x), static_cast<int>(pt.y)); });
-                                }
-                                else
-                                {
-                                    // apply points in reverse to preserve original winding order of inner rings
-                                    std::for_each(ring.rbegin(), ring.rend(), [&feature_builder](auto const& pt) { feature_builder.set_point(static_cast<int>(pt.x), static_cast<int>(pt.y)); });
-                                }
-                            }
+                            valid = true;
+                            feature_builder.add_ring(static_cast<unsigned>(ring.size()));
+                            std::for_each(ring.begin(), ring.end(),
+                                          [&feature_builder](auto const& pt) { feature_builder.set_point(static_cast<int>(pt.x), static_cast<int>(pt.y)); });
                         }
                     }
                 }

test/fixtures/polygons-with-holes-4-13-6.mvt

@@ -129,3 +129,39 @@ test('[composite] composite and overzooming success polygons - overzooming polyg
     assert.end();
   });
 });
+
+test('[composite] composite and overzooming polygons with holes', function(assert) {
+  const zxy = { z:8, x:221, y:99 };
+  const parent = { z:4, x:13, y:6 };
+
+  const tiles = [
+    { buffer: fs.readFileSync('./test/fixtures/polygons-with-holes-4-13-6.mvt'),  z: parent.z, x: parent.x, y: parent.y }
+  ];
+
+  composite(tiles, zxy, {buffer_size:4080}, (err, vtBuffer) => {
+    assert.notOk(err);
+    assert.equal(vtBuffer.length, 848);
+    const outputInfo = vtinfo(vtBuffer);
+    // one feature
+    assert.equal(outputInfo.layers.polygons.feature.length, 1);
+    var feature = outputInfo.layers.polygons.feature(0);
+    var geojson = feature.toGeoJSON(zxy.x,zxy.y,zxy.z);
+    var coords = geojson.geometry.coordinates;
+    // two polygons
+    assert.equal(coords.length,2);
+    // first polygons 8 rings
+    assert.equal(coords[0].length, 8);
+    assert.equal(coords[0][0].length, 95);
+    assert.equal(coords[0][1].length, 25);
+    assert.equal(coords[0][2].length, 23);
+    assert.equal(coords[0][3].length, 18);
+    assert.equal(coords[0][4].length, 26);
+    assert.equal(coords[0][5].length, 18);
+    assert.equal(coords[0][6].length, 14);
+    assert.equal(coords[0][7].length, 16);
+    // second polygon 1 ring
+    assert.equal(coords[1].length,1);
+    assert.equal(coords[1][0].length, 6);
+    assert.end();
+  });
+});