diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ef6fc8c0..5042d8f87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ For a complete change history, see the git log. ## Unreleased +- Added support for controlling rendering behavior of markers on multi-geometries `marker-multi-policy` (#1555,#1573) + - Add DebugSymbolizer (#1366) diff --git a/bindings/python/mapnik_markers_symbolizer.cpp b/bindings/python/mapnik_markers_symbolizer.cpp index 08519976f..fb0f44d1c 100644 --- a/bindings/python/mapnik_markers_symbolizer.cpp +++ b/bindings/python/mapnik_markers_symbolizer.cpp @@ -89,6 +89,12 @@ void export_markers_symbolizer() .value("LINE_PLACEMENT",mapnik::MARKER_LINE_PLACEMENT) ; + mapnik::enumeration_("marker_multi_policy") + .value("EACH",mapnik::MARKER_EACH_MULTI) + .value("WHOLE",mapnik::MARKER_WHOLE_MULTI) + .value("LARGEST",mapnik::MARKER_LARGEST_MULTI) + ; + class_("MarkersSymbolizer", init<>("Default Markers Symbolizer - circle")) .def (init("")) @@ -143,6 +149,10 @@ void export_markers_symbolizer() &markers_symbolizer::get_marker_placement, &markers_symbolizer::set_marker_placement, "Set/get the marker placement") + .add_property("multi_policy", + &markers_symbolizer::get_marker_multi_policy, + &markers_symbolizer::set_marker_multi_policy, + "Set/get the marker multi geometry rendering policy") .add_property("comp_op", &markers_symbolizer::comp_op, &markers_symbolizer::set_comp_op, diff --git a/include/mapnik/geom_util.hpp b/include/mapnik/geom_util.hpp index 7e90a7625..b3eeee7bd 100644 --- a/include/mapnik/geom_util.hpp +++ b/include/mapnik/geom_util.hpp @@ -329,6 +329,70 @@ bool centroid(PathType & path, double & x, double & y) return true; } +// Compute centroid over a set of paths +template +bool centroid_geoms(Iter start, Iter end, double & x, double & y) +{ + double x0 = 0.0; + double y0 = 0.0; + double x1 = 0.0; + double y1 = 0.0; + double start_x = x0; + double start_y = y0; + + bool empty = true; + + double atmp = 0.0; + double xtmp = 0.0; + double ytmp = 0.0; + unsigned count = 1; + + while (start!=end) + { + typename Iter::value_type & path = *start++; + path.rewind(0); + unsigned command = path.vertex(&x0, &y0); + if (command == SEG_END) continue; + empty = false; + + while (SEG_END != (command = path.vertex(&x1, &y1))) + { + double dx0 = x0 - start_x; + double dy0 = y0 - start_y; + double dx1 = x1 - start_x; + double dy1 = y1 - start_y; + double ai = dx0 * dy1 - dx1 * dy0; + atmp += ai; + xtmp += (dx1 + dx0) * ai; + ytmp += (dy1 + dy0) * ai; + x0 = x1; + y0 = y1; + ++count; + } + } + + if ( empty ) return false; + + if (count <= 2) { + x = (start_x + x0) * 0.5; + y = (start_y + y0) * 0.5; + return true; + } + + if (atmp != 0) + { + x = (xtmp/(3*atmp)) + start_x; + y = (ytmp/(3*atmp)) + start_y; + } + else + { + x = x0; + y = y0; + } + + return true; +} + template bool hit_test(PathType & path, double x, double y, double tol) { diff --git a/include/mapnik/marker_helpers.hpp b/include/mapnik/marker_helpers.hpp index 9677753ca..fd1a923d8 100644 --- a/include/mapnik/marker_helpers.hpp +++ b/include/mapnik/marker_helpers.hpp @@ -398,6 +398,68 @@ void setup_transform_scaling(agg::trans_affine & tr, box2d const& bbox, } } +// Apply markers to a feature with multiple geometries +template +void apply_markers_multi(feature_impl & feature, Converter& converter, markers_symbolizer const& sym) +{ + std::size_t geom_count = feature.paths().size(); + if (geom_count == 1) + { + converter.apply(feature.paths()[0]); + } + else if (geom_count > 1) + { + marker_multi_policy_e multi_policy = sym.get_marker_multi_policy(); + marker_placement_e placement = sym.get_marker_placement(); + if (placement == MARKER_POINT_PLACEMENT && + multi_policy == MARKER_WHOLE_MULTI) + { + double x, y; + if (label::centroid_geoms(feature.paths().begin(), feature.paths().end(), x, y)) + { + geometry_type pt(Point); + pt.move_to(x, y); + // unset any clipping since we're now dealing with a point + converter.template unset(); + converter.apply(pt); + } + } + else if ((placement == MARKER_POINT_PLACEMENT || placement == MARKER_INTERIOR_PLACEMENT) && + multi_policy == MARKER_LARGEST_MULTI) + { + // Only apply to path with largest envelope area + // TODO: consider using true area for polygon types + double maxarea = 0; + geometry_type* largest = 0; + BOOST_FOREACH(geometry_type & geom, feature.paths()) + { + const box2d& env = geom.envelope(); + double area = env.width() * env.height(); + if (area > maxarea) + { + maxarea = area; + largest = &geom; + } + } + if (largest) + { + converter.apply(*largest); + } + } + else + { + if (multi_policy != MARKER_EACH_MULTI && placement != MARKER_POINT_PLACEMENT) + { + MAPNIK_LOG_WARN(marker_symbolizer) << "marker_multi_policy != 'each' has no effect with marker_placement != 'point'"; + } + BOOST_FOREACH(geometry_type & path, feature.paths()) + { + converter.apply(path); + } + } + } +} + } #endif //MAPNIK_MARKER_HELPERS_HPP diff --git a/include/mapnik/markers_symbolizer.hpp b/include/mapnik/markers_symbolizer.hpp index 1dd832de6..ddcb6a8f2 100644 --- a/include/mapnik/markers_symbolizer.hpp +++ b/include/mapnik/markers_symbolizer.hpp @@ -46,6 +46,15 @@ enum marker_placement_enum { DEFINE_ENUM( marker_placement_e, marker_placement_enum ); +enum marker_multi_policy_enum { + MARKER_EACH_MULTI, // each component in a multi gets its marker + MARKER_WHOLE_MULTI, // consider all components of a multi as a whole + MARKER_LARGEST_MULTI, // only the largest component of a multi gets a marker + marker_multi_policy_enum_MAX +}; + +DEFINE_ENUM( marker_multi_policy_e, marker_multi_policy_enum ); + struct MAPNIK_DECL markers_symbolizer : public symbolizer_with_image, public symbolizer_base { @@ -74,6 +83,8 @@ public: boost::optional get_stroke() const; void set_marker_placement(marker_placement_e marker_p); marker_placement_e get_marker_placement() const; + void set_marker_multi_policy(marker_multi_policy_e marker_p); + marker_multi_policy_e get_marker_multi_policy() const; private: expression_ptr width_; expression_ptr height_; @@ -86,6 +97,7 @@ private: boost::optional opacity_; boost::optional stroke_; marker_placement_e marker_p_; + marker_multi_policy_e marker_mp_; }; } diff --git a/include/mapnik/vertex_converters.hpp b/include/mapnik/vertex_converters.hpp index ed57a5a03..38498060a 100644 --- a/include/mapnik/vertex_converters.hpp +++ b/include/mapnik/vertex_converters.hpp @@ -347,6 +347,16 @@ struct vertex_converter : private boost::noncopyable disp_.vec_[index]=1; } + template + void unset() + { + typedef typename boost::mpl::find::type iter; + typedef typename boost::mpl::end::type end; + std::size_t index = boost::mpl::distance::value - 1; + if (index < disp_.vec_.size()) + disp_.vec_[index]=0; + } + detail::dispatcher disp_; }; diff --git a/src/agg/process_markers_symbolizer.cpp b/src/agg/process_markers_symbolizer.cpp index 078dfd5cd..966d81d6d 100644 --- a/src/agg/process_markers_symbolizer.cpp +++ b/src/agg/process_markers_symbolizer.cpp @@ -26,6 +26,7 @@ #include #include +#include #include #include #include @@ -136,10 +137,7 @@ void agg_renderer::process(markers_symbolizer const& sym, } converter.template set(); //always transform if (sym.smooth() > 0.0) converter.template set(); // optional smooth converter - BOOST_FOREACH(geometry_type & geom, feature.paths()) - { - converter.apply(geom); - } + apply_markers_multi(feature, converter, sym); } else { @@ -172,10 +170,7 @@ void agg_renderer::process(markers_symbolizer const& sym, } converter.template set(); //always transform if (sym.smooth() > 0.0) converter.template set(); // optional smooth converter - BOOST_FOREACH(geometry_type & geom, feature.paths()) - { - converter.apply(geom); - } + apply_markers_multi(feature, converter, sym); } } else // raster markers @@ -207,11 +202,7 @@ void agg_renderer::process(markers_symbolizer const& sym, } converter.template set(); //always transform if (sym.smooth() > 0.0) converter.template set(); // optional smooth converter - - BOOST_FOREACH(geometry_type & geom, feature.paths()) - { - converter.apply(geom); - } + apply_markers_multi(feature, converter, sym); } } } diff --git a/src/cairo_renderer.cpp b/src/cairo_renderer.cpp index c44171da5..ced6f7cb3 100644 --- a/src/cairo_renderer.cpp +++ b/src/cairo_renderer.cpp @@ -1706,10 +1706,7 @@ void cairo_renderer_base::process(markers_symbolizer const& sym, } converter.set(); //always transform if (sym.smooth() > 0.0) converter.set(); // optional smooth converter - BOOST_FOREACH(geometry_type & geom, feature.paths()) - { - converter.apply(geom); - } + apply_markers_multi(feature, converter, sym); } else { @@ -1734,10 +1731,7 @@ void cairo_renderer_base::process(markers_symbolizer const& sym, } converter.set(); //always transform if (sym.smooth() > 0.0) converter.set(); // optional smooth converter - BOOST_FOREACH(geometry_type & geom, feature.paths()) - { - converter.apply(geom); - } + apply_markers_multi(feature, converter, sym); } } else // raster markers @@ -1767,10 +1761,7 @@ void cairo_renderer_base::process(markers_symbolizer const& sym, } converter.set(); //always transform if (sym.smooth() > 0.0) converter.set(); // optional smooth converter - BOOST_FOREACH(geometry_type & geom, feature.paths()) - { - converter.apply(geom); - } + apply_markers_multi(feature, converter, sym); } } } diff --git a/src/grid/process_markers_symbolizer.cpp b/src/grid/process_markers_symbolizer.cpp index d21a23d03..e4325d2fc 100644 --- a/src/grid/process_markers_symbolizer.cpp +++ b/src/grid/process_markers_symbolizer.cpp @@ -163,10 +163,7 @@ void grid_renderer::process(markers_symbolizer const& sym, } converter.template set(); //always transform if (sym.smooth() > 0.0) converter.template set(); // optional smooth converter - BOOST_FOREACH(geometry_type & geom, feature.paths()) - { - converter.apply(geom); - } + apply_markers_multi(feature, converter, sym); } else { @@ -208,10 +205,7 @@ void grid_renderer::process(markers_symbolizer const& sym, } converter.template set(); //always transform if (sym.smooth() > 0.0) converter.template set(); // optional smooth converter - BOOST_FOREACH(geometry_type & geom, feature.paths()) - { - converter.apply(geom); - } + apply_markers_multi(feature, converter, sym); } } else // raster markers @@ -256,10 +250,7 @@ void grid_renderer::process(markers_symbolizer const& sym, } converter.template set(); //always transform if (sym.smooth() > 0.0) converter.template set(); // optional smooth converter - BOOST_FOREACH(geometry_type & geom, feature.paths()) - { - converter.apply(geom); - } + apply_markers_multi(feature, converter, sym); } } } diff --git a/src/load_map.cpp b/src/load_map.cpp index f9df8a8b1..883b6458a 100644 --- a/src/load_map.cpp +++ b/src/load_map.cpp @@ -1027,6 +1027,10 @@ void map_parser::parse_markers_symbolizer(rule & rule, xml_node const& node) marker_placement_e placement = node.get_attr("placement",sym.get_marker_placement()); sym.set_marker_placement(placement); + + marker_multi_policy_e mpolicy = node.get_attr("multi-policy",sym.get_marker_multi_policy()); + sym.set_marker_multi_policy(mpolicy); + parse_symbolizer_base(sym, node); rule.append(sym); } diff --git a/src/markers_symbolizer.cpp b/src/markers_symbolizer.cpp index e992ef59e..cf9d47e2a 100644 --- a/src/markers_symbolizer.cpp +++ b/src/markers_symbolizer.cpp @@ -37,6 +37,15 @@ static const char * marker_placement_strings[] = { IMPLEMENT_ENUM( marker_placement_e, marker_placement_strings ) +static const char * marker_multi_policy_strings[] = { + "each", + "whole", + "largest", + "" +}; + +IMPLEMENT_ENUM( marker_multi_policy_e, marker_multi_policy_strings ) + markers_symbolizer::markers_symbolizer() : symbolizer_with_image(parse_path("shape://ellipse")), symbolizer_base(), @@ -46,7 +55,10 @@ markers_symbolizer::markers_symbolizer() allow_overlap_(false), spacing_(100.0), max_error_(0.2), - marker_p_(MARKER_POINT_PLACEMENT) { } + marker_p_(MARKER_POINT_PLACEMENT), + // TODO: consider defaulting to MARKER_WHOLE_MULTI, + // for backward compatibility with 2.0.0 + marker_mp_(MARKER_EACH_MULTI) { } markers_symbolizer::markers_symbolizer(path_expression_ptr const& filename) : symbolizer_with_image(filename), @@ -57,7 +69,10 @@ markers_symbolizer::markers_symbolizer(path_expression_ptr const& filename) allow_overlap_(false), spacing_(100.0), max_error_(0.2), - marker_p_(MARKER_POINT_PLACEMENT) { } + marker_p_(MARKER_POINT_PLACEMENT), + // TODO: consider defaulting to MARKER_WHOLE_MULTI, + // for backward compatibility with 2.0.0 + marker_mp_(MARKER_EACH_MULTI) { } markers_symbolizer::markers_symbolizer(markers_symbolizer const& rhs) : symbolizer_with_image(rhs), @@ -71,7 +86,8 @@ markers_symbolizer::markers_symbolizer(markers_symbolizer const& rhs) fill_(rhs.fill_), fill_opacity_(rhs.fill_opacity_), stroke_(rhs.stroke_), - marker_p_(rhs.marker_p_) {} + marker_p_(rhs.marker_p_), + marker_mp_(rhs.marker_mp_) {} void markers_symbolizer::set_ignore_placement(bool ignore_placement) { @@ -173,4 +189,14 @@ marker_placement_e markers_symbolizer::get_marker_placement() const return marker_p_; } +void markers_symbolizer::set_marker_multi_policy(marker_multi_policy_e marker_mp) +{ + marker_mp_ = marker_mp; +} + +marker_multi_policy_e markers_symbolizer::get_marker_multi_policy() const +{ + return marker_mp_; +} + } diff --git a/src/save_map.cpp b/src/save_map.cpp index dfc163a75..722cf699d 100644 --- a/src/save_map.cpp +++ b/src/save_map.cpp @@ -311,6 +311,10 @@ public: { set_attr( sym_node, "placement", sym.get_marker_placement() ); } + if ( sym.get_marker_multi_policy() != dfl.get_marker_multi_policy() || explicit_defaults_ ) + { + set_attr( sym_node, "multi-policy", sym.get_marker_multi_policy() ); + } if (sym.get_image_transform()) { std::string tr_str = sym.get_image_transform_string(); diff --git a/src/xml_tree.cpp b/src/xml_tree.cpp index 821f92ade..647b979d5 100644 --- a/src/xml_tree.cpp +++ b/src/xml_tree.cpp @@ -486,6 +486,7 @@ compile_get_attr(std::string); compile_get_attr(filter_mode_e); compile_get_attr(point_placement_e); compile_get_attr(marker_placement_e); +compile_get_attr(marker_multi_policy_e); compile_get_attr(pattern_alignment_e); compile_get_attr(line_rasterizer_e); compile_get_attr(colorizer_mode); diff --git a/tests/cpp_tests/label_algo_test.cpp b/tests/cpp_tests/label_algo_test.cpp index d43651afc..cf7a0d414 100644 --- a/tests/cpp_tests/label_algo_test.cpp +++ b/tests/cpp_tests/label_algo_test.cpp @@ -31,6 +31,9 @@ int main( int, char*[] ) BOOST_TEST( x == 25 ); BOOST_TEST( y == 25 ); + // TODO - centroid and interior should be equal but they appear not to be (check largest) + // MULTIPOLYGON(((-52 40,-60 32,-68 40,-60 48,-52 40)),((-60 50,-80 30,-100 49.9999999999999,-80.0000000000001 70,-60 50)),((-52 60,-60 52,-68 60,-60 68,-52 60))) + if (!::boost::detail::test_errors()) { std::clog << "C++ label algorithms: \x1b[1;32m✓ \x1b[0m\n"; #if BOOST_VERSION >= 104600 diff --git a/tests/python_tests/object_test.py b/tests/python_tests/object_test.py index 95b92c4a1..5024e0cd3 100644 --- a/tests/python_tests/object_test.py +++ b/tests/python_tests/object_test.py @@ -209,6 +209,7 @@ def test_markers_symbolizer(): eq_(p.fill_opacity,None) eq_(p.filename,'shape://ellipse') eq_(p.placement,mapnik.marker_placement.POINT_PLACEMENT) + eq_(p.multi_policy,mapnik.marker_multi_policy.EACH) eq_(p.fill,None) eq_(p.ignore_placement,False) eq_(p.spacing,100) @@ -239,10 +240,14 @@ def test_markers_symbolizer(): p.allow_overlap = True p.opacity = 0.5 p.fill_opacity = 0.5 + p.placement = mapnik.marker_placement.LINE_PLACEMENT + p.multi_policy = mapnik.marker_multi_policy.WHOLE eq_(p.allow_overlap, True) eq_(p.opacity, 0.5) eq_(p.fill_opacity, 0.5) + eq_(p.multi_policy,mapnik.marker_multi_policy.WHOLE) + eq_(p.placement,mapnik.marker_placement.LINE_PLACEMENT) #https://github.com/mapnik/mapnik/issues/1285 #https://github.com/mapnik/mapnik/issues/1427 diff --git a/tests/visual_tests/data/marker-multi-policy.csv b/tests/visual_tests/data/marker-multi-policy.csv new file mode 100644 index 000000000..9599215bc --- /dev/null +++ b/tests/visual_tests/data/marker-multi-policy.csv @@ -0,0 +1,3 @@ +i|wkt +1|MULTIPOLYGON(((-10 -50,-21.7157287525381 -78.2842712474619,-49.9999999999999 -90,-78.2842712474618 -78.284271247462,-90 -50.0000000000001,-78.284271247462 -21.7157287525382,-50.0000000000002 -10,-21.7157287525383 -21.7157287525379,-10 -50)),((90 -50,78.2842712474619 -78.2842712474619,50.0000000000001 -90,21.7157287525382 -78.284271247462,10 -50.0000000000001,21.715728752538 -21.7157287525382,49.9999999999998 -10,78.2842712474617 -21.7157287525379,90 -50)),((90 50,50.0000000000001 10,10 49.9999999999999,49.9999999999998 90,90 50))) +2|MULTIPOLYGON(((-52 40,-60 32,-68 40,-60 48,-52 40)),((-60 50,-80 30,-100 49.9999999999999,-80.0000000000001 70,-60 50)),((-52 60,-60 52,-68 60,-60 68,-52 60))) diff --git a/tests/visual_tests/images/marker-multi-policy-600-reference.png b/tests/visual_tests/images/marker-multi-policy-600-reference.png new file mode 100644 index 000000000..0233625a5 Binary files /dev/null and b/tests/visual_tests/images/marker-multi-policy-600-reference.png differ diff --git a/tests/visual_tests/styles/marker-multi-policy.xml b/tests/visual_tests/styles/marker-multi-policy.xml new file mode 100644 index 000000000..1ba2a2bb7 --- /dev/null +++ b/tests/visual_tests/styles/marker-multi-policy.xml @@ -0,0 +1,36 @@ + + + + + + + boundary + each + whole + largest + + csv + ../data/marker-multi-policy.csv + | + + + diff --git a/tests/visual_tests/test.py b/tests/visual_tests/test.py index 0f9fc0632..9c579c8a6 100755 --- a/tests/visual_tests/test.py +++ b/tests/visual_tests/test.py @@ -27,6 +27,7 @@ files = [ {'name': "lines-2", 'sizes': sizes_few_square}, {'name': "lines-3", 'sizes': sizes_few_square}, {'name': "lines-shield", 'sizes': sizes_few_square}, + {'name': "marker-multi-policy", 'sizes':[(600,400)]}, {'name': "simple-E"}, {'name': "simple-NE"}, {'name': "simple-NW"},