maplibre-native icon indicating copy to clipboard operation
maplibre-native copied to clipboard

Labels getting clipped in tile mode (node version)

Open acalcutt opened this issue 2 years ago • 15 comments

I mentioned this issue in https://github.com/maplibre/maplibre-gl-native/issues/279 , but did not want to pollute that thread with an unrelated problem.

In my testing of the node version of maplibre-gl-native in my branch of tileserver-gl I have noticed labels getting clipped more than in the mapbox-gl-native version.

For example, in the maplibre-gl-native version it is consistently cutting off labels all over the place like this maplibre

Where in the maptiler version using mapbox-gl-native v5.02, does not have this issue mapbox

Looking at several similar threads I found these related mapbox issues https://github.com/mapbox/mapbox-gl-native/issues/15805 https://github.com/mapbox/mapbox-gl-native/issues/14011 https://github.com/mapbox/mapbox-gl-native/pull/15880 The first article in this list mentions this was improved in v5.0.2, which I did find the change they mentioned here https://github.com/mapbox/mapbox-gl-native/commit/d500b67d4a73ccbfbc59b78a3277538b01d1cb5e. the padding was increased when in tile mode to help the labels to help with the trimming (a change which still exists, slightly rewritten). Also, in tileserver-gl they added the code to use tile mode here https://github.com/maptiler/tileserver-gl/commit/5048388d1fbfce3f983e6ee0338bafaa90e947ce

Looking though the related code in https://github.com/maplibre/maplibre-gl-native/blob/main/src/mbgl/text/collision_index.cpp#L26-L50 it seems like the changes that added padding in tile mode are still there and looking in my tileserver-gl code the tile mode option is still set ( https://github.com/acalcutt/tileserver-gl/blob/main/src/serve_rendered.js#L574 )

From this, to me, it seems like it should be showing the labels correct like in the second screenshot, but for some reason it is not.

One idea I had is maybe it is ignoring the tile mode switch? I'm guessing that would be this code in the node binding code?

Does anyone have any other idea or tips that could help? I am told there is no issue with labels in the android version, which Is why I was thinking something with the node mode switch.

acalcutt avatar May 19 '22 03:05 acalcutt

Thanks for reporting this @acalcutt Is there a way to reproduce this cropping problem with the linux native renderer and the demotiles?

wipfli avatar Jun 04 '22 16:06 wipfli

Is the any information on using the linux native renderer? like at least switches mbgl-render needs?

Basically I would want to try an recreate this view, where it trims "United States" https://tiles.wifidb.net/styles/demotiles/?raster#6/39.470/-98.701 image

acalcutt avatar Jun 04 '22 18:06 acalcutt

I tried various zoom levels, like xvfb-run -a ./mbgl-render -z 6 -y 39.436 -x -101 --style style.json --output out6.png

but I wasn't able to see any clipping. I always assumed this was where the label was at the edge of the tile. so I think to test this you would need two tiles that are side by side where the label gets clipped off. also, I'm not sure this mbgl-render can do tile mode, where it overdraws a bit to help with the labels.

acalcutt avatar Jun 04 '22 19:06 acalcutt

I've made a application to test this issue here https://github.com/acalcutt/maplibre-label-clipping-test ( Note, this is made to be run in node 10 to be compatible with the mapbox-gl-native release )

Test 1 - With maplibre-gl-native git clone https://github.com/acalcutt/maplibre-label-clipping-test.git cd maplibre-label-clipping-test npm i xvfb-run -a node index.js 2 images are generated, which look like this. I added the red boxes to show where the issue is. ml_sel

Test 2 - With mapbox-gl-native Edit index.js. uncomment line 8 (let mbgl = require('@mapbox/mapbox-gl-native');) comment line 9 (//let mbgl = require('@acalcutt/maplibre-gl-native');) xvfb-run -a node index.js 2 images are generated, which look like this. mb_sel

Compare the maplibre version to the mapbox version. In the maplibre version, the "North" in "North Adams", which is right on the edge, is missing. However in the mapbox version, it is there as expected. mlmbcombined

acalcutt avatar Jun 05 '22 03:06 acalcutt

I also tried with

wget https://raw.githubusercontent.com/maplibre/demotiles/gh-pages/style.json
./build/bin/mbgl-render --style style.json --output out.png --zoom 6 --lat 39.470 --lon -103

but always got correctly clipped labels...

wipfli avatar Jun 05 '22 07:06 wipfli

Hm interesting, is the diff between mapbox-gl-native and maplibre-gl-native large? When was their last common commit?

wipfli avatar Jun 05 '22 07:06 wipfli

The diff is pretty large between the mapbox last version.

My assumption right now is this is related to the mode switch. If I go remove this line (mode: 'tile',) https://github.com/acalcutt/maplibre-label-clipping-test/blob/main/index.js#L147 , with the mapbox version, it produces the same output as maplibre.

The code for tile mode has been refactored a bit, since mapbox 5.0.2...maybe there was some mistake with that https://github.com/maplibre/maplibre-gl-native/blob/main/src/mbgl/text/collision_index.cpp#L26-L50 or maybe something reading the mode setting? https://github.com/maplibre/maplibre-gl-native/blob/main/platform/node/src/node_map.cpp#L1403-L1409 1 2

acalcutt avatar Jun 05 '22 10:06 acalcutt

I've done a few thing to try and track this down...non successful so far.

1.) I tried setting viewportPaddingDefault to 1024, like is supposed to happen in tile mode to see if maybe the tile mode switch was not working. However even at 1024 the label clipping still occurred.

2.) I tried reverting back to the first maplibre node release in cfcbc62fadd72224aac242c516a5a86510a1d102 , but the issue seemed to be happening in that release also

3.) I tried going back really far to a mapbox release that mentioned tile mode b84449e35fd38788fbbbe9c67e338e8317cd7150 , but unfortunately I couldn't get it to build. I used the instruction for that release, but I assume it needs an older GCC since I was getting syntax errors.

I've been trying to test this with https://www.npmjs.com/package/@acalcutt/maplibre-gl-native-test since I wasn't sure how to use an unreleased version of maplibre-gl-native with node. does anybody know if that is possible (to use the node package without releasing it?)

acalcutt avatar Jun 07 '22 11:06 acalcutt

Maybe you can install it from a local folder https://stackoverflow.com/questions/8088795/installing-a-local-module-using-npm

wipfli avatar Jun 07 '22 13:06 wipfli

Have you made any progress @acalcutt ? I'm seeing a fair amount of clipping and overlaps.

pbnsilva avatar Jun 26 '22 18:06 pbnsilva

Unfortunately I haven't figured anything else. I haven't had much time to look though since my real job has kept me really busy this month.

My last test leads me to believe maybe it isn't tile mode padding like I was originally thinking.

acalcutt avatar Jun 26 '22 19:06 acalcutt

Thanks for putting in the work. I'll do a bit of digging myself this week.

pbnsilva avatar Jun 27 '22 08:06 pbnsilva

I updated @acalcutt/[email protected] to include these render fixes. unfortunately it doesn't look like they fixed the issue here.

I was hopeful since the fixes were in the same files... but somewhat expected it since I had tried to revert https://github.com/maplibre/maplibre-gl-native/pull/270 before and it didn't fix this issue.

I also updated the test app in this thread to use the latest node release and confirmed it didn't help this issue.

acalcutt avatar Jul 05 '22 13:07 acalcutt

I will say I need to look at these render test in https://github.com/maplibre/maplibre-gl-native/pull/351#issuecomment-1174168866

'render-tests/text-variable-anchor/all-anchors-tile-map-mode' looks like it tests this.

acalcutt avatar Jul 05 '22 13:07 acalcutt

Maybe also look in the ignored tests.

If I run

cmake . -B build -G Ninja -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_C_COMPILER=gcc-10 -DCMAKE_CXX_COMPILER=g++-10
cmake --build build -j $(nproc 2>/dev/null || sysctl -n hw.ncpu 2>/dev/null)
./build/mbgl-render-test-runner --manifestPath metrics/linux-clang8-release-style.json &> render-test.log
sed -i 's/\x1b\[[0-9;]*m//g' render-test.log
cat render-test.log | grep "* ignore"

I get these ignored tests:

* ignore render-tests/symbol-cross-fade/chinese (https://github.com/mapbox/mapbox-gl-native/issues/10619)
* ignore render-tests/real-world/bangkok (https://github.com/mapbox/mapbox-gl-native/issues/10412)
* ignore render-tests/real-world/sanfrancisco (https://github.com/mapbox/mapbox-gl-native/issues/10412)
* ignore render-tests/real-world/chicago (https://github.com/mapbox/mapbox-gl-native/issues/10412)
* ignore render-tests/tilejson-bounds/overwrite-bounds (started failing after https://github.com/mapbox/mapbox-gl-native/pull/16091)
* ignore render-tests/fill-extrusion-vertical-gradient/default (https://github.com/mapbox/mapbox-gl-js/pull/9202)
* ignore render-tests/fill-extrusion-vertical-gradient/false (https://github.com/mapbox/mapbox-gl-js/pull/9202)
* ignore render-tests/mixed-zoom/z10-z11 (https://github.com/mapbox/mapbox-gl-native/issues/10397)
* ignore render-tests/projection/perspective (https://github.com/mapbox/mapbox-gl-js/pull/9202)
* ignore render-tests/runtime-styling/set-style-glyphs (started failing after https://github.com/mapbox/mapbox-gl-native/pull/16091)
* ignore render-tests/fill-extrusion-geometry/linestring (https://github.com/mapbox/mapbox-gl-native/pull/14240)
* ignore render-tests/regressions/mapbox-gl-js#6806 (pending https://github.com/mapbox/mapbox-gl-js/pull/6812)
* ignore render-tests/regressions/mapbox-gl-js#5740 (https://github.com/mapbox/mapbox-gl-native/issues/10619)
* ignore render-tests/regressions/mapbox-gl-js#6706 (https://github.com/mapbox/mapbox-gl-native/issues/10619)
* ignore render-tests/regressions/mapbox-gl-js#7271 (https://github.com/mapbox/mapbox-gl-native/issues/12888)
* ignore render-tests/regressions/mapbox-gl-js#2762 (https://github.com/mapbox/mapbox-gl-native/issues/10619)
* ignore render-tests/regressions/mapbox-gl-js#5642 (Failing with mbgl-render-test)
* ignore render-tests/regressions/mapbox-gl-native#7357 (https://github.com/mapbox/mapbox-gl-native/issues/7357)
* ignore render-tests/regressions/mapbox-gl-js#5982 (https://github.com/mapbox/mapbox-gl-native/issues/10619)
* ignore render-tests/regressions/mapbox-gl-js#2769 (https://github.com/mapbox/mapbox-gl-native/issues/10619)
* ignore render-tests/regressions/mapbox-gl-js#7066 (Failing with mbgl-render-test)
* ignore render-tests/regressions/mapbox-gl-js#2467 (https://github.com/mapbox/mapbox-gl-native/issues/10619)
* ignore render-tests/symbol-sort-key/placement-tile-boundary-right-then-left (https://github.com/mapbox/mapbox-gl-js/pull/9054)
* ignore render-tests/text-max-width/zero-width-point-placement (https://github.com/mapbox/mapbox-gl-native/issues/15648)
* ignore render-tests/extent/1024-circle (needs investigation)
* ignore render-tests/extent/1024-symbol (https://github.com/mapbox/mapbox-gl-js/pull/9202)
* ignore render-tests/icon-text-fit/text-variable-anchor-overlap (https://github.com/mapbox/mapbox-gl-native/issues/15809)
* ignore render-tests/zoomed-fill/negative-zoom (https://github.com/mapbox/mapbox-gl-native/issues/16019)
* ignore render-tests/geojson/inline-linestring-fill (current behavior is arbitrary)
* ignore render-tests/debug/padding/ease-to-no-distort (https://github.com/mapbox/mapbox-gl-native/issues/16212)
* ignore render-tests/debug/padding/set-padding (https://github.com/mapbox/mapbox-gl-native/issues/16212)
* ignore render-tests/debug/padding/ease-to-right-distort (https://github.com/mapbox/mapbox-gl-native/issues/16212)
* ignore render-tests/debug/padding/ease-to-left-distort (https://github.com/mapbox/mapbox-gl-native/issues/16212)
* ignore render-tests/debug/padding/ease-to-btm-distort (https://github.com/mapbox/mapbox-gl-native/issues/16212)
* ignore render-tests/debug/padding/ease-to-top-distort (https://github.com/mapbox/mapbox-gl-native/issues/16212)
* ignore render-tests/debug/collision (https://github.com/mapbox/mapbox-gl-native/issues/3841)
* ignore render-tests/debug/tile-overscaled (https://github.com/mapbox/mapbox-gl-native/issues/3841)
* ignore render-tests/debug/overdraw (https://github.com/mapbox/mapbox-gl-native/issues/15638)
* ignore render-tests/debug/raster (https://github.com/mapbox/mapbox-gl-native/issues/15510)
* ignore render-tests/debug/tile (https://github.com/mapbox/mapbox-gl-native/issues/3841)
* ignore render-tests/line-pattern/opacity (Flaky on Linux: https://github.com/mapbox/mapbox-gl-native/issues/15320)
* ignore render-tests/line-pattern/with-dasharray (https://github.com/mapbox/mapbox-gl-js/pull/9189)
* ignore render-tests/fill-opacity/zoom-and-property-function-pattern (Flaky on Linux: https://github.com/mapbox/mapbox-gl-native/issues/15322)
* ignore render-tests/raster-masking/overlapping-zoom (https://github.com/mapbox/mapbox-gl-native/issues/10195)
* ignore render-tests/text-size/nan (https://github.com/mapbox/mapbox-gl-native/issues/16020)
* ignore render-tests/background-color/colorSpace-hcl (needs issue)
* ignore render-tests/background-color/transition (https://github.com/mapbox/mapbox-gl-native/issues/10619)
* ignore render-tests/line-translate/literal (Flaky on Linux: https://github.com/mapbox/mapbox-gl-native/issues/14859)
* ignore render-tests/tile-mode/streets-v11 (ignored due to flaky metrics results)
* ignore render-tests/fill-pattern/update-feature-state (https://github.com/mapbox/mapbox-gl-native/issues/15895)
* ignore render-tests/fill-pattern/literal (Flaky on Linux: https://github.com/mapbox/mapbox-gl-native/issues/14423)
* ignore render-tests/fill-pattern/zoomed (Flaky on Linux: https://github.com/mapbox/mapbox-gl-native/issues/14768)
* ignore render-tests/fill-pattern/opacity (Flaky on Linux: https://github.com/mapbox/mapbox-gl-native/issues/14870)
* ignore render-tests/fill-extrusion-translate/default (https://github.com/mapbox/mapbox-gl-js/pull/9202)
* ignore render-tests/fill-extrusion-pattern/feature-expression (https://github.com/mapbox/mapbox-gl-js/issues/3327)
* ignore render-tests/fill-extrusion-pattern/literal (https://github.com/mapbox/mapbox-gl-js/issues/3327)
* ignore render-tests/fill-extrusion-pattern/opacity (https://github.com/mapbox/mapbox-gl-js/issues/3327)
* ignore render-tests/fill-extrusion-pattern/function (https://github.com/mapbox/mapbox-gl-js/issues/3327)
* ignore render-tests/fill-extrusion-pattern/function-2 (https://github.com/mapbox/mapbox-gl-js/issues/3327)
* ignore render-tests/fill-extrusion-pattern/tile-buffer (https://github.com/mapbox/mapbox-gl-js/issues/3327)
* ignore render-tests/fill-extrusion-pattern/@2x (https://github.com/mapbox/mapbox-gl-js/issues/3327)
* ignore render-tests/feature-state/promote-id-fill-extrusion (https://github.com/mapbox/mapbox-gl-js/pull/9202)
* ignore render-tests/feature-state/promote-id-line (https://github.com/mapbox/mapbox-gl-js/pull/9202)
* ignore render-tests/feature-state/promote-id-fill (https://github.com/mapbox/mapbox-gl-js/pull/9202)
* ignore render-tests/feature-state/promote-id-circle (https://github.com/mapbox/mapbox-gl-js/pull/9202)
* ignore render-tests/feature-state/promote-id-symbol (https://github.com/mapbox/mapbox-gl-js/pull/9202)
* ignore render-tests/fill-extrusion-base/property-function (https://github.com/mapbox/mapbox-gl-js/pull/9202)
* ignore render-tests/fill-extrusion-base/zoom-and-property-function (https://github.com/mapbox/mapbox-gl-js/pull/9202)
* ignore render-tests/collator/default (Some test platforms don't resolve 'en' locale)
* ignore render-tests/collator/resolved-locale (Some test platforms don't resolve 'en' locale)
* ignore render-tests/fill-extrusion-opacity/default (https://github.com/mapbox/mapbox-gl-js/pull/9202)
* ignore render-tests/fill-extrusion-opacity/literal (https://github.com/mapbox/mapbox-gl-js/pull/9202)
* ignore render-tests/fill-extrusion-opacity/function (https://github.com/mapbox/mapbox-gl-js/pull/9202)
* ignore query-tests/regressions/mapbox-gl-js#7883 (https://github.com/mapbox/mapbox-gl-native/issues/14585)
* ignore query-tests/regressions/mapbox-gl-js#8999 (https://github.com/mapbox/mapbox-gl-js/pull/9138)
* ignore query-tests/fill-translate/multiple-layers (https://github.com/mapbox/mapbox-gl-native/issues/12701)
* ignore query-tests/fill-extrusion/sort-rotated (https://github.com/mapbox/mapbox-gl-native/issues/13139)
* ignore query-tests/fill-extrusion/sort (https://github.com/mapbox/mapbox-gl-native/issues/13139)
* ignore query-tests/fill-extrusion/top-in (https://github.com/mapbox/mapbox-gl-native/issues/13139)
* ignore query-tests/fill-extrusion/sort-concave-outer (https://github.com/mapbox/mapbox-gl-native/issues/13139)
* ignore query-tests/fill-extrusion/box-in (https://github.com/mapbox/mapbox-gl-native/issues/13139)
* ignore query-tests/fill-extrusion/sort-concave-inner (https://github.com/mapbox/mapbox-gl-native/issues/13139)
* ignore query-tests/fill-extrusion/side-in (https://github.com/mapbox/mapbox-gl-native/issues/13139)
* ignore query-tests/fill-extrusion/base-in (https://github.com/mapbox/mapbox-gl-native/issues/13139)
* ignore query-tests/geometry/polygon (needs investigation)
* ignore query-tests/geometry/multipolygon (needs investigation)
* ignore query-tests/geometry/multilinestring (needs investigation)
* ignore query-tests/evaluated/line-width (Port https://github.com/mapbox/mapbox-gl-js/pull/9198)
* ignore query-tests/fill-extrusion-translate/multiple-layers (https://github.com/mapbox/mapbox-gl-native/issues/12701)

wipfli avatar Jul 05 '22 14:07 wipfli

I think I may have figured this out, but I am still testing to be sure.

For some reason in platform/node/src/node_map.cpp , the map "withMapMode" was hardcoded to "mbgl::MapMode::Static" and not using the "mode" variable L1424 and L645

acalcutt avatar Aug 27 '22 22:08 acalcutt

It worked! So far I have tested in my tileserver-gl and in the label clipping test above and it seems like it is fixed

This should be fixed in my npm package "@acalcutt/maplibre-gl-native": "5.0.25

This is what the two images from my label test look like now, no longer clipping labels 1

acalcutt avatar Aug 27 '22 22:08 acalcutt

Really nice! Do you know if we can run the render test via the node platform?

wipfli avatar Aug 28 '22 07:08 wipfli

Fixed by #415

birkskyum avatar Aug 28 '22 08:08 birkskyum

@wipfli unfortunately I am not aware of how to run the render tests for node, or if there are any.

acalcutt avatar Aug 28 '22 11:08 acalcutt

@wipfli

To do the basic tests the build does in linux, it would be something like ubuntu 20.04

install prereqs from https://github.com/acalcutt/maplibre-gl-native/tree/main/platform/linux
git clone --recurse-submodules https://github.com/maplibre/maplibre-gl-native.git
npm ci --ignore-scripts
cmake . -B build -G Ninja -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_C_COMPILER=gcc-10 -DCMAKE_CXX_COMPILER=g++-10
cmake --build build -j $(nproc 2>/dev/null || sysctl -n hw.ncpu 2>/dev/null)
xvfb-run --auto-servernum npm test

however, looking in the package.json there are some other tests that didn't seem to work for me on my test vm

    "test-memory": "node --expose-gc platform/node/test/memory.test.js",
    "test-expressions": "node -r esm platform/node/test/expression.test.js",
    "test-render": "node -r esm platform/node/test/render.test.js",
    "test-query": "node -r esm platform/node/test/query.test.js",

Edit: the error test-render gets after installing d3 is

/opt/test6/maplibre-gl-native/maplibre-gl-js/test/integration/lib/harness.js:1
Error [ERR_REQUIRE_ESM]: require() of ES Module /opt/test6/maplibre-gl-native/node_modules/d3/src/index.js not supported.
Instead change the require of index.js in null to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (/opt/test6/maplibre-gl-native/maplibre-gl-js/test/integration/lib/harness.js:1)
    at Generator.next (<anonymous>)
    at Object.<anonymous> (/opt/test6/maplibre-gl-native/maplibre-gl-js/test/integration/lib/render.js:1)
    at Generator.next (<anonymous>)
    at Object.<anonymous> (/opt/test6/maplibre-gl-native/platform/node/test/render.test.js:1)
    at Generator.next (<anonymous>) {
  code: 'ERR_REQUIRE_ESM'
}

seems like it runs the render test from the js version?

acalcutt avatar Aug 28 '22 15:08 acalcutt

Ah interesting. The render test fixtures are indeed in the maplibre-gl-js submodule. Maybe even the test harness from GL JS is used for node?

wipfli avatar Aug 29 '22 10:08 wipfli