From 131ce55ade6765e883d45a5ee2ba7d65d9aaeee0 Mon Sep 17 00:00:00 2001 From: Aleksandar Stojiljkovic Date: Tue, 6 Aug 2019 13:32:16 +0300 Subject: [PATCH 1/2] Restore point order in polygons that don't require fixup 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: https://github.com/mapbox/mapbox-gl-native/issues/15268 --- platform/node/test/ignores.json | 8 +------- src/mbgl/tile/geometry_tile_data.cpp | 23 ++++++++++++++++++++++- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/platform/node/test/ignores.json b/platform/node/test/ignores.json index 5814114a155..b23978cac40 100644 --- a/platform/node/test/ignores.json +++ b/platform/node/test/ignores.json @@ -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", diff --git a/src/mbgl/tile/geometry_tile_data.cpp b/src/mbgl/tile/geometry_tile_data.cpp index 5320df6893a..7f4c3dbeb0e 100644 --- a/src/mbgl/tile/geometry_tile_data.cpp +++ b/src/mbgl/tile/geometry_tile_data.cpp @@ -36,6 +36,27 @@ static GeometryCollection toGeometryCollection(MultiPolygon&& 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; @@ -48,7 +69,7 @@ GeometryCollection fixupPolygons(const GeometryCollection& rings) { MultiPolygon 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 classifyRings(const GeometryCollection& rings) { From e4d35d8de5035a64de68da99ab2628ebd0faa19d Mon Sep 17 00:00:00 2001 From: Aleksandar Stojiljkovic Date: Tue, 6 Aug 2019 15:16:57 +0300 Subject: [PATCH 2/2] Run node render tests on macos-release bot, in addition to mbgl-render-tests run on linux --- Makefile | 7 +++++++ circle.yml | 30 ++---------------------------- 2 files changed, 9 insertions(+), 28 deletions(-) diff --git a/Makefile b/Makefile index 5a127471c9f..b67844a5487 100644 --- a/Makefile +++ b/Makefile @@ -559,6 +559,13 @@ test-node: node npm run test-query npm run test-expressions +.PHONY: test-node-all +test-node-all: node + npm test + npm run test-query + npm run test-expressions + npm run test-render + #### Android targets ########################################################### MBGL_ANDROID_ABIS = arm-v7;armeabi-v7a diff --git a/circle.yml b/circle.yml index e7d50421c4a..b84df89e2c5 100644 --- a/circle.yml +++ b/circle.yml @@ -54,7 +54,6 @@ workflows: branches: ignore: /.*/ - macos-debug - - macos-render-tests - qt5-linux-gcc5-release - qt5-macos-debug nightly: @@ -372,7 +371,7 @@ commands: steps: - run: name: Run node tests - command: make test-node + command: make test-node-all run-node-linux-tests: parameters: @@ -387,14 +386,6 @@ commands: xvfb-run --server-args="-screen 0 1024x768x24" \ logbt -- apitrace trace --api=egl -v make test-node - run-macos-render-tests: - steps: - - run: - name: Run render tests (mbgl-render-test) - command: | - build/mbgl-render-test --recycle-map --shuffle - no_output_timeout: 2m - run-linux-render-tests: parameters: node_version: @@ -784,6 +775,7 @@ jobs: - save-dependencies - run-node-macos-tests - publish-node-package + - upload-render-tests - collect-xcode-build-logs - upload-xcode-build-logs @@ -1177,24 +1169,6 @@ jobs: - collect-xcode-build-logs - upload-xcode-build-logs -# ------------------------------------------------------------------------------ - macos-render-tests: - macos: - xcode: "10.3.0" - environment: - BUILDTYPE: RelWithDebInfo - HOMEBREW_NO_AUTO_UPDATE: 1 - HOMEBREW_NO_INSTALL_CLEANUP: 1 - JOBS: 2 - steps: - - install-macos-dependencies - - install-dependencies - - configure-cmake - - build-mbgl-render-test - - save-dependencies - - run-macos-render-tests - - upload-render-tests - # ------------------------------------------------------------------------------ qt5-linux-gcc5-release: docker: