refactor pass

include/mapnik/text/formatting/base.hpp

@@ -47,7 +47,7 @@ public:
     virtual void to_xml(boost::property_tree::ptree &xml) const = 0;
     static node_ptr from_xml(xml_node const& xml);
     virtual void apply(char_properties_ptr p, feature_impl const& feature, attributes const& vars, text_layout &output) const = 0;
-    virtual void add_expressions(expression_set &output) const = 0;
+    virtual void add_expressions(expression_set &output) const;// = 0;
 };
 } //ns formatting
 } //ns mapnik

include/mapnik/text/text_properties.hpp

@@ -95,7 +95,6 @@ struct MAPNIK_DECL text_layout_properties
     vertical_alignment_e valign;
     std::function<pixel_position(double,double)> displacement_evaluator_ =
         [](double dx, double dy) { return pixel_position(dx,dy);};
-
 };
 
 class text_layout;

src/text/formatting/base.cpp

@@ -49,9 +49,14 @@ node_ptr node::from_xml(xml_node const& xml)
     }
     else
     {
-        return node_ptr();
+        return nullptr;
     }
 }
 
+void node::add_expressions(expression_set & /*output*/) const
+{
+    //Do nothing by default
+}
+
 } //ns formatting
 } //ns mapnik

src/text/placement_finder.cpp

@@ -64,27 +64,19 @@ placement_finder::placement_finder(feature_impl const& feature,
 
 bool placement_finder::next_position()
 {
-    if (!valid_)
+    if (info_.next())
     {
-        MAPNIK_LOG_WARN(placement_finder) << "next_position() called while last call already returned false!\n";
-        return false;
-    }
-    if (!info_.next())
-    {
-        valid_ = false;
-        return false;
-    }
-
         text_layout_ptr layout = std::make_shared<text_layout>(font_manager_, scale_factor_, info_.properties.layout_defaults);
-    info_.properties.process(*layout, feature_, attr_);
         layout->evaluate_properties(feature_, attr_);
-
+        info_.properties.process(*layout, feature_, attr_);
-    layouts_.clear();
+        layouts_.clear(); // FIXME !!!!
         layouts_.add(layout);
         layouts_.layout();
-
         return true;
     }
+    MAPNIK_LOG_WARN(placement_finder) << "next_position() called while last call already returned false!\n";
+    return false;
+}
 
 text_upright_e placement_finder::simplify_upright(text_upright_e upright, double angle) const
 {

src/text/placements/list.cpp

@@ -38,12 +38,8 @@ bool text_placement_info_list::next()
     }
     else
     {
-        if (state  >= parent_->list_.size() + 1) return false;
+        if (state  > parent_->list_.size()) return false;
         properties = parent_->list_[state-1];
-        properties.layout_defaults.displacement_evaluator_ = [](double dx, double dy)
-            {
-                return pixel_position(dx,dy);
-            };
     }
     ++state;
     return true;
@@ -51,10 +47,13 @@ bool text_placement_info_list::next()
 
 text_symbolizer_properties & text_placements_list::add()
 {
-    if (list_.size()) {
+    if (list_.size())
+    {
         text_symbolizer_properties & last = list_.back();
-        list_.push_back(last); //Preinitialize with old values
+        list_.push_back(last); //Preinitialize with old values FIXME
-    } else {
+    }
+    else
+    {
         list_.push_back(defaults);
     }
     return list_.back();
@@ -89,27 +88,26 @@ unsigned text_placements_list::size() const
     return list_.size();
 }
 
+
 text_placements_ptr text_placements_list::from_xml(xml_node const &xml, fontset_map const & fontsets)
 {
     using boost::property_tree::ptree;
-    auto list = std::make_shared<text_placements_list>();
+    text_placements_list *list = new text_placements_list;
-
+    text_placements_ptr ptr = text_placements_ptr(list);
     list->defaults.from_xml(xml, fontsets);
-    for (auto const& node : xml)
+    xml_node::const_iterator itr = xml.begin();
+    xml_node::const_iterator end = xml.end();
+    for( ;itr != end; ++itr)
     {
-        if (node.is_text() || !node.is("Placement")) continue;
+        if (itr->is_text() || !itr->is("Placement")) continue;
         text_symbolizer_properties &p = list->add();
-        p.format = std::make_shared<char_properties>(*(p.format)); //Make a deep copy <-- FIXME
+        p.format = std::make_shared<char_properties>(*(p.format)); //Make a deep copy -- FIXME
-
-        //p.layout_defaults = std::make_shared<text_layout_properties>(*(p.layout_defaults));
         //TODO: This needs a real copy constructor for text_symbolizer_properties
-        p.from_xml(node, fontsets);
+        p.from_xml(*itr, fontsets);
-
+       //TODO: if (strict_ && !p.format.fontset.size())
-//TODO: if (strict_ &&
-//                !p.format.fontset.size())
        //         ensure_font_face(p.format.face_name);
     }
-    return list;
+    return ptr;
 }
 
 } //ns mapnik

src/text/text_properties.cpp

@@ -58,11 +58,8 @@ text_symbolizer_properties::text_symbolizer_properties()
 void text_symbolizer_properties::process(text_layout & output, feature_impl const& feature, attributes const& vars) const
 {
     output.clear();
-    if (tree_) {
+    if (tree_) tree_->apply(format, feature, vars, output);
-        tree_->apply(format, feature, vars, output);
+    else MAPNIK_LOG_WARN(text_properties) << "text_symbolizer_properties can't produce text: No formatting tree!";
-    } else {
-        MAPNIK_LOG_WARN(text_properties) << "text_symbolizer_properties can't produce text: No formatting tree!";
-    }
 }
 
 void text_symbolizer_properties::set_format_tree(formatting::node_ptr tree)