Merge pull request #2383 from MapQuest/min-dist-compatibility

3.x -> 2.3.x compatibility for minimum-distance

bindings/python/mapnik_text_placement.cpp

@@ -425,8 +425,9 @@ void export_text_placement()
         .def_readwrite("label_spacing", &text_symbolizer_properties::label_spacing)
         .def_readwrite("label_position_tolerance", &text_symbolizer_properties::label_position_tolerance)
         .def_readwrite("avoid_edges", &text_symbolizer_properties::avoid_edges)
-        .def_readwrite("minimum_distance", &text_symbolizer_properties::minimum_distance)
+        .def_readwrite("text_margin", &text_symbolizer_properties::text_margin)
         .def_readwrite("repeat_distance", &text_symbolizer_properties::repeat_distance)
+        .def_readwrite("minimum_distance", &text_symbolizer_properties::minimum_distance)
         .def_readwrite("minimum_padding", &text_symbolizer_properties::minimum_padding)
         .def_readwrite("minimum_path_length", &text_symbolizer_properties::minimum_path_length)
         .def_readwrite("maximum_angle_char_delta", &text_symbolizer_properties::max_char_angle_delta)

include/mapnik/label_collision_detector.hpp

@@ -161,19 +161,19 @@ public:
         return true;
     }
 
-    bool has_placement(box2d<double> const& box, double minimum_distance)
+    bool has_placement(box2d<double> const& box, double margin)
     {
-        box2d<double> const& minimum_box = (minimum_distance > 0
+        box2d<double> const& margin_box = (margin > 0
-                                               ? box2d<double>(box.minx() - minimum_distance, box.miny() - minimum_distance,
+                                               ? box2d<double>(box.minx() - margin, box.miny() - margin,
-                                                               box.maxx() + minimum_distance, box.maxy() + minimum_distance)
+                                                               box.maxx() + margin, box.maxy() + margin)
                                                : box);
 
-        tree_t::query_iterator itr = tree_.query_in_box(minimum_box);
+        tree_t::query_iterator itr = tree_.query_in_box(margin_box);
         tree_t::query_iterator end = tree_.query_end();
 
         for (;itr != end; ++itr)
         {
-            if (itr->box.intersects(minimum_box))
+            if (itr->box.intersects(margin_box))
             {
                 return false;
             }
@@ -181,24 +181,27 @@ public:
         return true;
     }
 
-    bool has_placement(box2d<double> const& box, double minimum_distance, mapnik::value_unicode_string const& text, double repeat_distance)
+    bool has_placement(box2d<double> const& box, double margin, mapnik::value_unicode_string const& text, double repeat_distance)
     {
-        box2d<double> const& minimum_box = (minimum_distance > 0
+        // Don't bother with any of the repeat checking unless the repeat distance is greater than the margin
-                                               ? box2d<double>(box.minx() - minimum_distance, box.miny() - minimum_distance,
+        if (repeat_distance <= margin) {
-                                                               box.maxx() + minimum_distance, box.maxy() + minimum_distance)
+            return has_placement(box, margin);
+        }
+
+        box2d<double> repeat_box(box.minx() - repeat_distance, box.miny() - repeat_distance,
+                                 box.maxx() + repeat_distance, box.maxy() + repeat_distance);
+
+        box2d<double> const& margin_box = (margin > 0
+                                               ? box2d<double>(box.minx() - margin, box.miny() - margin,
+                                                               box.maxx() + margin, box.maxy() + margin)
                                                : box);
 
-        box2d<double> const& repeat_box = (repeat_distance > 0
+        tree_t::query_iterator itr = tree_.query_in_box(repeat_box);
-                                               ? box2d<double>(box.minx() - repeat_distance, box.miny() - repeat_distance,
-                                                               box.maxx() + repeat_distance, box.maxy() + repeat_distance)
-                                               : box);
-
-        tree_t::query_iterator itr = tree_.query_in_box(repeat_distance > minimum_distance ? repeat_box : minimum_box);
         tree_t::query_iterator end = tree_.query_end();
 
         for ( ;itr != end; ++itr)
         {
-            if (itr->box.intersects(minimum_box) || (text == itr->text && itr->box.intersects(repeat_box)))
+            if (itr->box.intersects(margin_box) || (text == itr->text && itr->box.intersects(repeat_box)))
             {
                 return false;
             }

include/mapnik/text/placement_finder.hpp

@@ -72,7 +72,7 @@ private:
     // Adjusts user defined spacing to place an integer number of labels.
     double get_spacing(double path_length, double layout_width) const;
     // Checks for collision.
-    bool collision(box2d<double> const& box, const value_unicode_string &repeat_key = "") const;
+    bool collision(box2d<double> const& box, const value_unicode_string &repeat_key, bool line_placement) const;
     // Adds marker to glyph_positions and to collision detector. Returns false if there is a collision.
     bool add_marker(glyph_positions_ptr glyphs, pixel_position const& pos) const;
     // Maps upright==auto, left_only and right_only to left,right to simplify processing.

include/mapnik/text/text_properties.hpp

@@ -137,6 +137,7 @@ struct text_properties_expressions
     symbolizer_base::value_type label_spacing = 0.0;
     symbolizer_base::value_type label_position_tolerance = 0.0;
     symbolizer_base::value_type avoid_edges = false;
+    symbolizer_base::value_type text_margin = 0.0;
     symbolizer_base::value_type repeat_distance = 0.0;
     symbolizer_base::value_type minimum_distance = 0.0;
     symbolizer_base::value_type minimum_padding = 0.0;
@@ -180,6 +181,7 @@ struct MAPNIK_DECL text_symbolizer_properties
     // distance the label can be moved on the line to fit, if 0 the default is used
     double label_position_tolerance;
     bool avoid_edges;
+    double text_margin;
     double repeat_distance;
     double minimum_distance;
     double minimum_padding;

src/text/placement_finder.cpp

@@ -121,7 +121,7 @@ bool placement_finder::find_point_placement(pixel_position const& pos)
         bbox.re_center(layout_center.x, layout_center.y);
 
         /* For point placements it is faster to just check the bounding box. */
-        if (collision(bbox, layouts_.text())) return false;
+        if (collision(bbox, layouts_.text(), false)) return false;
 
         if (layout.num_lines()) bboxes.push_back(std::move(bbox));
 
@@ -244,7 +244,7 @@ bool placement_finder::single_line_placement(vertex_cache &pp, text_upright_e or
                 cluster_offset.y -= rot.sin * glyph.advance();
 
                 box2d<double> bbox = get_bbox(layout, glyph, pos, rot);
-                if (collision(bbox, layouts_.text())) return false;
+                if (collision(bbox, layouts_.text(), true)) return false;
                 bboxes.push_back(std::move(bbox));
                 glyphs->push_back(glyph, pos, rot);
             }
@@ -311,9 +311,20 @@ double placement_finder::get_spacing(double path_length, double layout_width) co
     return path_length / num_labels;
 }
 
-bool placement_finder::collision(const box2d<double> &box, const value_unicode_string &repeat_key) const
+bool placement_finder::collision(const box2d<double> &box, const value_unicode_string &repeat_key, bool line_placement) const
 {
-    if (!detector_.extent().intersects(box)
+    double text_margin, repeat_distance;
+    if (line_placement)
+    {
+        text_margin = info_.properties.text_margin * scale_factor_;
+        repeat_distance = (info_.properties.repeat_distance != 0 ? info_.properties.repeat_distance : info_.properties.minimum_distance) * scale_factor_;
+    }
+    else
+    {
+        text_margin = (info_.properties.text_margin != 0 ? info_.properties.text_margin : info_.properties.minimum_distance) * scale_factor_;
+        repeat_distance = info_.properties.repeat_distance * scale_factor_;
+    }
+    return !detector_.extent().intersects(box)
                ||
            (info_.properties.avoid_edges && !extent_.contains(box))
                ||
@@ -321,15 +332,9 @@ bool placement_finder::collision(const box2d<double> &box, const value_unicode_s
             !extent_.contains(box + (scale_factor_ * info_.properties.minimum_padding)))
                ||
            (!info_.properties.allow_overlap &&
-            ((repeat_key.length() == 0 && !detector_.has_placement(box, info_.properties.minimum_distance * scale_factor_))
+               ((repeat_key.length() == 0 && !detector_.has_placement(box, text_margin))
                    ||
-             (repeat_key.length() > 0  && !detector_.has_placement(box, info_.properties.minimum_distance * scale_factor_,
+               (repeat_key.length() > 0 && !detector_.has_placement(box, text_margin, repeat_key, repeat_distance))));
-                                                                   repeat_key, info_.properties.repeat_distance * scale_factor_))))
-        )
-    {
-        return true;
-    }
-    return false;
 }
 
 void placement_finder::set_marker(marker_info_ptr m, box2d<double> box, bool marker_unlocked, pixel_position const& marker_displacement)
@@ -348,7 +353,7 @@ bool placement_finder::add_marker(glyph_positions_ptr glyphs, pixel_position con
     box2d<double> bbox = marker_box_;
     bbox.move(real_pos.x, real_pos.y);
     glyphs->set_marker(marker_, real_pos);
-    if (collision(bbox, layouts_.text())) return false;
+    if (collision(bbox, layouts_.text(), false)) return false;
     detector_.insert(bbox);
     return true;
 }

src/text/text_properties.cpp

@@ -44,6 +44,7 @@ text_symbolizer_properties::text_symbolizer_properties()
       label_spacing(0.0),
       label_position_tolerance(0.0),
       avoid_edges(false),
+      text_margin(0.0),
       repeat_distance(0.0),
       minimum_distance(0.0),
       minimum_padding(0.0),
@@ -63,6 +64,7 @@ void text_symbolizer_properties::evaluate_text_properties(feature_impl const& fe
     label_spacing = util::apply_visitor(extract_value<value_double>(feature,attrs), expressions.label_spacing);
     label_position_tolerance = util::apply_visitor(extract_value<value_double>(feature,attrs), expressions.label_position_tolerance);
     avoid_edges = util::apply_visitor(extract_value<value_bool>(feature,attrs), expressions.avoid_edges);
+    text_margin = util::apply_visitor(extract_value<value_double>(feature,attrs), expressions.text_margin);
     repeat_distance = util::apply_visitor(extract_value<value_double>(feature,attrs), expressions.repeat_distance);
     minimum_distance = util::apply_visitor(extract_value<value_double>(feature,attrs), expressions.minimum_distance);
     minimum_padding = util::apply_visitor(extract_value<value_double>(feature,attrs), expressions.minimum_padding);
@@ -112,11 +114,25 @@ formatting::node_ptr text_symbolizer_properties::format_tree() const
 
 void text_symbolizer_properties::text_properties_from_xml(xml_node const& node)
 {
+    // The options 'text-margin' and 'repeat-distance' replace 'minimum-distance'.
+    // Only allow one or the other to be defined here.
+    if (node.has_attribute("text-margin") || node.has_attribute("repeat-distance"))
+    {
+        if (node.has_attribute("minimum-distance"))
+        {
+            throw config_error(std::string("Cannot use deprecated option minimum-distance with "
+                                           "new options text-margin and repeat-distance."));
+        }
+        set_property_from_xml<value_double>(expressions.text_margin, "text-margin", node);
+        set_property_from_xml<value_double>(expressions.repeat_distance, "repeat-distance", node);
+    }
+    else
+    {
+        set_property_from_xml<value_double>(expressions.minimum_distance, "minimum-distance", node);
+    }
     set_property_from_xml<label_placement_e>(expressions.label_placement, "placement", node);
     set_property_from_xml<value_double>(expressions.label_spacing, "spacing", node);
     set_property_from_xml<value_double>(expressions.label_position_tolerance, "label-position-tolerance", node);
-    set_property_from_xml<value_double>(expressions.repeat_distance, "repeat-distance", node);
-    set_property_from_xml<value_double>(expressions.minimum_distance, "minimum-distance", node);
     set_property_from_xml<value_double>(expressions.minimum_padding, "minimum-padding", node);
     set_property_from_xml<value_double>(expressions.minimum_path_length, "minimum-path-length", node);
     set_property_from_xml<boolean_type>(expressions.avoid_edges, "avoid-edges", node);
@@ -158,6 +174,10 @@ void text_symbolizer_properties::to_xml(boost::property_tree::ptree &node,
     {
         serialize_property("spacing", expressions.label_spacing, node);
     }
+    if (!(expressions.text_margin == dfl.expressions.text_margin) || explicit_defaults)
+    {
+        serialize_property("text-margin", expressions.text_margin, node);
+    }
     if (!(expressions.repeat_distance == dfl.expressions.repeat_distance) || explicit_defaults)
     {
         serialize_property("repeat-distance", expressions.repeat_distance, node);
@@ -207,6 +227,8 @@ void text_symbolizer_properties::add_expressions(expression_set & output) const
     if (is_expression(expressions.label_spacing)) output.insert(util::get<expression_ptr>(expressions.label_spacing));
     if (is_expression(expressions.label_position_tolerance)) output.insert(util::get<expression_ptr>(expressions.label_position_tolerance));
     if (is_expression(expressions.avoid_edges)) output.insert(util::get<expression_ptr>(expressions.avoid_edges));
+    if (is_expression(expressions.text_margin)) output.insert(util::get<expression_ptr>(expressions.text_margin));
+    if (is_expression(expressions.repeat_distance)) output.insert(util::get<expression_ptr>(expressions.repeat_distance));
     if (is_expression(expressions.minimum_distance)) output.insert(util::get<expression_ptr>(expressions.minimum_distance));
     if (is_expression(expressions.minimum_padding)) output.insert(util::get<expression_ptr>(expressions.minimum_padding));
     if (is_expression(expressions.minimum_path_length)) output.insert(util::get<expression_ptr>(expressions.minimum_path_length));

tests/visual_tests/grids/repeat-labels-1-750-250-1.0-grid-reference.json

@@ -21,7 +21,7 @@
   "                                                                                                                                                                                            ", 
   "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!", 
   "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!", 
-  "              !!!!                 !         !!!!                 !         !!!!!                     !     !!!!                           !!!!                          !!!!!              ", 
+  "                           !                                                                          !                                                                          !          ", 
   "                                                                                                                                                                                            ", 
   "                                                                                                                                                                                            ", 
   "                                                                                                                                                                                            ", 
@@ -34,10 +34,10 @@
   "                                                                                                                                                                                            ", 
   "                                                                                                                                                                                            ", 
   "                                                                                                                                                                                            ", 
-  "              $$$$                           $$$$  $                        $$$$$                           $$$$              $            $$$$               $           $$$$              ", 
+  "                                                                                                                                                                                            ", 
   "$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$", 
   "$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$$", 
-  "                              $                               $                                    $                                     $                                                  ", 
+  "                                                                                                                                                                                            ", 
   "                                                                                                                                                                                            ", 
   "                                                                                                                                                                                            ", 
   "                                                                                                                                                                                            ", 

tests/visual_tests/styles/repeat-labels-1.xml

@@ -18,14 +18,14 @@
     <Style name="shield">
         <Rule>
             <Filter>[type] = 'motorway'</Filter>
-            <ShieldSymbolizer minimum-distance="0" spacing="120" placement="line" face-name="DejaVu Sans Book" size="10" fill="white" file="../../data/svg/rect.svg" transform="scale(0.75)">[ref]</ShieldSymbolizer>
+            <ShieldSymbolizer minimum-distance="50" spacing="120" placement="line" face-name="DejaVu Sans Book" size="10" fill="white" file="../../data/svg/rect.svg" transform="scale(0.75)">[ref]</ShieldSymbolizer>
         </Rule>
     </Style>
 
     <Style name="text">
         <Rule>
             <Filter>[type] != 'poi'</Filter>
-            <TextSymbolizer minimum-distance="0" spacing="70" placement="line" face-name="DejaVu Sans Book" size="12">[name]</TextSymbolizer>
+            <TextSymbolizer minimum-distance="50" spacing="70" placement="line" face-name="DejaVu Sans Book" size="12">[name]</TextSymbolizer>
         </Rule>
     </Style>
 

tests/visual_tests/styles/repeat-labels-2.xml

@@ -18,14 +18,14 @@
     <Style name="shield">
         <Rule>
             <Filter>[type] = 'motorway'</Filter>
-            <ShieldSymbolizer minimum-distance="60" spacing="120" placement="line" face-name="DejaVu Sans Book" size="10" fill="white" file="../../data/svg/rect.svg" transform="scale(0.75)">[ref]</ShieldSymbolizer>
+            <ShieldSymbolizer text-margin="60" spacing="120" placement="line" face-name="DejaVu Sans Book" size="10" fill="white" file="../../data/svg/rect.svg" transform="scale(0.75)">[ref]</ShieldSymbolizer>
         </Rule>
     </Style>
 
     <Style name="text">
         <Rule>
             <Filter>[type] != 'poi'</Filter>
-            <TextSymbolizer minimum-distance="50" spacing="70" placement="line" face-name="DejaVu Sans Book" size="12">[name]</TextSymbolizer>
+            <TextSymbolizer text-margin="50" spacing="70" placement="line" face-name="DejaVu Sans Book" size="12">[name]</TextSymbolizer>
         </Rule>
     </Style>
 

tests/visual_tests/styles/repeat-labels-3.xml

@@ -18,14 +18,14 @@
     <Style name="shield">
         <Rule>
             <Filter>[type] = 'motorway'</Filter>
-            <ShieldSymbolizer minimum-distance="0" repeat-distance="60" spacing="120" placement="line" face-name="DejaVu Sans Book" size="10" fill="white" file="../../data/svg/rect.svg" transform="scale(0.75)">[ref]</ShieldSymbolizer>
+            <ShieldSymbolizer text-margin="0" repeat-distance="60" spacing="120" placement="line" face-name="DejaVu Sans Book" size="10" fill="white" file="../../data/svg/rect.svg" transform="scale(0.75)">[ref]</ShieldSymbolizer>
         </Rule>
     </Style>
 
     <Style name="text">
         <Rule>
             <Filter>[type] != 'poi'</Filter>
-            <TextSymbolizer minimum-distance="0" repeat-distance="40" spacing="70" placement="line" face-name="DejaVu Sans Book" size="12">[name]</TextSymbolizer>
+            <TextSymbolizer text-margin="0" repeat-distance="40" spacing="70" placement="line" face-name="DejaVu Sans Book" size="12">[name]</TextSymbolizer>
         </Rule>
     </Style>
 

tests/visual_tests/styles/repeat-labels-4.xml

@@ -18,14 +18,14 @@
     <Style name="shield">
         <Rule>
             <Filter>[type] = 'motorway'</Filter>
-            <ShieldSymbolizer minimum-distance="10" repeat-distance="60" spacing="120" placement="line" face-name="DejaVu Sans Book" size="10" fill="white" file="../../data/svg/rect.svg" transform="scale(0.75)">[ref]</ShieldSymbolizer>
+            <ShieldSymbolizer text-margin="10" repeat-distance="60" spacing="120" placement="line" face-name="DejaVu Sans Book" size="10" fill="white" file="../../data/svg/rect.svg" transform="scale(0.75)">[ref]</ShieldSymbolizer>
         </Rule>
     </Style>
 
     <Style name="text">
         <Rule>
             <Filter>[type] != 'poi'</Filter>
-            <TextSymbolizer minimum-distance="3" repeat-distance="40" spacing="70" placement="line" face-name="DejaVu Sans Book" size="12">[name]</TextSymbolizer>
+            <TextSymbolizer text-margin="3" repeat-distance="40" spacing="70" placement="line" face-name="DejaVu Sans Book" size="12">[name]</TextSymbolizer>
         </Rule>
     </Style>