mapbox / mapnik-vector-tile

Mapnik implemention of Mapbox Vector Tile specification
BSD 3-Clause "New" or "Revised" License
553 stars 117 forks source link

MultiPoint geometries only draw one point #62

Closed e-n-f closed 9 years ago

e-n-f commented 10 years ago

@springmeyer, as we just discussed on chat, I have been trying to use multipoint geometries in Mapbox Studio.

As a test, I uploaded this GeoJSON file to mapbox.com as map ID enf.tqgmbo6r.

It looks like each MultiPoint draws as a single point at the centroid of the points within each feature within each tile rather than drawing all the points, regardless of how marker-multi-policy is set:

screen shot 2014-10-01 at 5 32 01 pm

As individual points (enf.6vjqncdi) it looks like this:

screen shot 2014-10-01 at 5 35 03 pm

but the file is twice as large with each point in its own feature.

pnorman commented 10 years ago

I can definitely confirm this an issue - I've run into it using the API directly, not in MapBox Studio.

My testcase:

Saved as test/single_multipoint.xml

<Map
    srs="+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over"
    maximum-extent="-20037508.34,-20037508.34,20037508.34,20037508.34">
  <Layer name="point" srs="+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0.0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs +over">
    <Datasource>
      <Parameter name="type">csv</Parameter>
      <Parameter name="inline">
id|name|wkt
1|null island|MultiPoint((-1000000 0), (0 0), (1000000 0))
      </Parameter>
    </Datasource>
  </Layer>
</Map>
void test_multipoint() {
/* This test uses a map file with a CSV source with a multipoint and checks
 * the resulting vector tile at 0/0/0
 */
  mapnik::Map map;
  avecado::tile tile;
  mapnik::load_map(map, "test/single_multipoint.xml");
  map.resize(tile_size, tile_size);
  map.zoom_to_box(bbox);
  avecado::make_vector_tile(tile, path_multiplier, map, buffer_size, scale_factor,
                            offset_x, offset_y, tolerance, image_format,
                            scaling_method, scale_denominator, boost::none);
  mapnik::vector::tile result;
  result.ParseFromString(tile.get_data());
  test::assert_equal(result.layers_size(), 1, "Wrong number of layers");
  mapnik::vector::tile_layer layer = result.layers(0);

  // Query the layer with mapnik. See https://github.com/mapbox/mapnik-vector-tile/blob/2e3e2c28/test/vector_tile.cpp#L236
  mapnik::vector::tile_datasource ds(layer, 0, 0, 0, tile_size);

  mapnik::query qq = mapnik::query(bbox);
  qq.add_property_name("name");
  mapnik::featureset_ptr fs;
  fs = ds.features(qq);
  mapnik::feature_ptr feat = fs->next();
  std::string json = feature_to_geojson(*feat);
  test::assert_equal(json, blank, "Wrong JSON");
}

The geojson contains a Point, not a MultiPoint In this case, avecado::make_vector_tile just returns painted() from a mapnik::vector::processor<mapnik::vector::backend_pbf>, after setting up stuff.

pnorman commented 10 years ago

@ericfischer did you have occasion to test with MULTILINESTRINGs or MULTIPOLYGONs?

e-n-f commented 10 years ago

@pnorman They both worked fine for me when I tried, although the multipolygon only gets one label even if you ask for marker-multi-policy: each.

pnorman commented 10 years ago

the multipolygon only gets one label even if you ask for marker-multi-policy: each

Given how polygons interact with tile boundaries, that's probably not an issue for common usages, but may be for other consumption of vector tiles.

I'll see what I get back with vector tiles from a datasource with the other MULTI* geometries, so I'll test and serialize the results to something human-readable.

springmeyer commented 10 years ago

@ericfischer as discussed on chat, I think a good solution is to add a new mapnik symbolizer for fast rendering of multipoints. Let's call it the dot symbolizer for now. I've stubbed out:

After you've got Mapnik to a point you'd like to test then we'd together create new node-mapnik packages via a dots branch (https://github.com/mapnik/node-mapnik/compare/dots) and then point Mapbox studio at it like:

diff --git a/package.json b/package.json
index 81b2f62..0ca8dd3 100644
--- a/package.json
+++ b/package.json
@@ -31,8 +31,8 @@
     "js-yaml": "https://github.com/mapbox/js-yaml/tarball/scalar-styles",
     "fstream": "0.1.x",
     "tar": "0.1.x",
-    "mapnik": "1.4.17",
-    "mapnik-reference": "~6.0.1",
+    "mapnik": "https://github.com/mapnik/node-mapnik/tarball/dots",
+    "mapnik-reference": "https://github.com/mapnik/mapnik/-reference/tarball/dots",
     "carto": "0.14.0",
     "speedometer": "0.1.2",
     "tilelive": "5.2.3",
e-n-f commented 10 years ago

Thank you @springmeyer! It will probably be a few days before I am able to get started on this but I am looking forward to it.

springmeyer commented 9 years ago

tl;dr: Closing this, next actions at https://github.com/mapnik/mapnik/issues/2611.

To recap:

pnorman commented 9 years ago

Does #98 and multipoints not existing in the spec change this?

springmeyer commented 9 years ago

@pnorman multipoints are supported in the spec and always have been: they are type POINT and encoded as multiple move_to commands rather than just one (for a single point).