Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Commit

Permalink
Restore point order in polygons that don't require fixup
Browse files Browse the repository at this point in the history
wagyu algorithm setup and checking if polygon fixup is required is expensive as much as fixing it - so we continue running wagyu for all geojson and, if attempt to restore original point order after wagyu.  Even with no polygon fixup, starting point in rings is changed.  This causes a disparity between GL-JS and GL-native, visible in implementation of fill-extrusion pattern.

Fixes: #15268
  • Loading branch information
astojilj committed Aug 6, 2019
1 parent acfcd5c commit 131ce55
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 8 deletions.
8 changes: 1 addition & 7 deletions platform/node/test/ignores.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,7 @@
"render-tests/debug/tile": "https://github.com/mapbox/mapbox-gl-native/issues/3841",
"render-tests/debug/tile-overscaled": "https://github.com/mapbox/mapbox-gl-native/issues/3841",
"render-tests/extent/1024-circle": "needs investigation",
"render-tests/fill-extrusion-pattern/@2x": "https://github.com/mapbox/mapbox-gl-js/issues/3327",
"render-tests/fill-extrusion-pattern/function": "https://github.com/mapbox/mapbox-gl-js/issues/3327",
"render-tests/fill-extrusion-pattern/function-2": "https://github.com/mapbox/mapbox-gl-js/issues/3327",
"render-tests/fill-extrusion-pattern/literal": "https://github.com/mapbox/mapbox-gl-js/issues/3327",
"render-tests/fill-extrusion-pattern/opacity": "https://github.com/mapbox/mapbox-gl-js/issues/3327",
"render-tests/fill-extrusion-pattern/feature-expression": "https://github.com/mapbox/mapbox-gl-js/issues/3327",
"render-tests/fill-extrusion-pattern/tile-buffer": "https://github.com/mapbox/mapbox-gl-js/issues/3327",
"render-tests/fill-extrusion-pattern/tile-buffer": "https://github.com/mapbox/mapbox-gl-js/issues/4403",
"render-tests/fill-pattern/literal": "FLAKY: do not re-enable without extensive testing - https://github.com/mapbox/mapbox-gl-native/issues/14423",
"render-tests/fill-pattern/opacity": "FLAKY: do not re-enable without extensive testing - https://github.com/mapbox/mapbox-gl-native/issues/14870",
"render-tests/fill-pattern/update-feature-state": "skip - port https://github.com/mapbox/mapbox-gl-js/pull/6263 - needs issue",
Expand Down
23 changes: 22 additions & 1 deletion src/mbgl/tile/geometry_tile_data.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,27 @@ static GeometryCollection toGeometryCollection(MultiPolygon<int16_t>&& multipoly
return result;
}

// Attempts to restore original point order in rings:
// see https://github.com/mapbox/mapbox-gl-native/issues/15268.
static GeometryCollection restorePointOrder(GeometryCollection&& rings, const GeometryCollection& originalRings) {
if (rings.size() != originalRings.size()) {
// No sense restoring starting point if polygons are changed.
return std::move(rings);
}

int i = 0;
for (auto& ring : rings) {
const auto originalRing = originalRings[i++];
auto item = find(ring.begin(), ring.end() - 1, originalRing[0]);
if (item != ring.end()) {
rotate(ring.begin(), item, ring.end() - 1);
// Close the ring.
ring.back() = ring.front();
}
}
return std::move(rings);
}

GeometryCollection fixupPolygons(const GeometryCollection& rings) {
using namespace mapbox::geometry::wagyu;

Expand All @@ -48,7 +69,7 @@ GeometryCollection fixupPolygons(const GeometryCollection& rings) {
MultiPolygon<int16_t> multipolygon;
clipper.execute(clip_type_union, multipolygon, fill_type_even_odd, fill_type_even_odd);

return toGeometryCollection(std::move(multipolygon));
return restorePointOrder(toGeometryCollection(std::move(multipolygon)), rings);
}

std::vector<GeometryCollection> classifyRings(const GeometryCollection& rings) {
Expand Down

0 comments on commit 131ce55

Please sign in to comment.