move rule cache off style objects as styles should be immutable - amends 1c4b67ba98 - refs #916

include/mapnik/feature_style_processor.hpp

@@ -39,6 +39,7 @@ class layer;
 class projection;
 class proj_transform;
 class feature_type_style;
+class rule_cache;
 
 enum eAttributeCollectionPolicy
 {
@@ -77,11 +78,11 @@ private:
      */
     void render_style(layer const& lay,
                       Processor & p,
-                      feature_type_style* style,
+                      feature_type_style const* style,
+                      rule_cache const& rules,
                       std::string const& style_name,
                       featureset_ptr features,
-                      proj_transform const& prj_trans,
+                      proj_transform const& prj_trans);
-                      double scale_denom);
 
     Map const& m_;
     double scale_factor_;

include/mapnik/feature_style_processor_impl.hpp

@@ -38,6 +38,7 @@
 #include <mapnik/box2d.hpp>
 #include <mapnik/layer.hpp>
 #include <mapnik/rule.hpp>
+#include <mapnik/rule_cache.hpp>
 #include <mapnik/attribute_collector.hpp>
 #include <mapnik/expression_evaluator.hpp>
 #include <mapnik/utils.hpp>
@@ -136,7 +137,6 @@ struct has_process
         );
 };
 
-
 template <typename Processor>
 feature_style_processor<Processor>::feature_style_processor(Map const& m, double scale_factor)
     : m_(m), scale_factor_(scale_factor)
@@ -361,10 +361,11 @@ void feature_style_processor<Processor>::apply_to_layer(layer const& lay, Proces
                                m_.height()/qh);
 
     query q(layer_ext,res,scale_denom,m_.get_current_extent());
-    std::vector<feature_type_style*> active_styles;
+    std::vector<feature_type_style const*> active_styles;
     attribute_collector collector(names);
     double filt_factor = 1.0;
     directive_collector d_collector(filt_factor);
+    boost::ptr_vector<rule_cache> rule_caches;
 
     // iterate through all named styles collecting active styles and attribute names
     BOOST_FOREACH(std::string const& style_name, style_names)
@@ -379,12 +380,14 @@ void feature_style_processor<Processor>::apply_to_layer(layer const& lay, Proces
             continue;
         }
 
-        std::vector<rule> const& rules=(*style).get_rules();
+        std::vector<rule> const& rules = style->get_rules();
         bool active_rules = false;
+        std::auto_ptr<rule_cache> rc(new rule_cache);
         BOOST_FOREACH(rule const& r, rules)
         {
             if (r.active(scale_denom))
             {
+                rc->add_rule(r);
                 active_rules = true;
                 if (ds->type() == datasource::Vector)
                 {
@@ -395,7 +398,8 @@ void feature_style_processor<Processor>::apply_to_layer(layer const& lay, Proces
         }
         if (active_rules)
         {
-            active_styles.push_back(const_cast<feature_type_style*>(&(*style)));
+            rule_caches.push_back(rc);
+            active_styles.push_back(&(*style));
         }
     }
 
@@ -419,7 +423,7 @@ void feature_style_processor<Processor>::apply_to_layer(layer const& lay, Proces
         }
 
         // Update filter_factor for all enabled raster layers.
-        BOOST_FOREACH (feature_type_style * style, active_styles)
+        BOOST_FOREACH (feature_type_style const* style, active_styles)
         {
             BOOST_FOREACH(rule const& r, style->get_rules())
             {
@@ -464,10 +468,11 @@ void feature_style_processor<Processor>::apply_to_layer(layer const& lay, Proces
                         // We're at a value boundary, so render what we have
                         // up to this point.
                         int i = 0;
-                        BOOST_FOREACH (feature_type_style * style, active_styles)
+                        BOOST_FOREACH (feature_type_style const* style, active_styles)
                         {
-                            render_style(lay, p, style, style_names[i++],
+                            render_style(lay, p, style, rule_caches[i], style_names[i],
-                                         cache.features(q), prj_trans, scale_denom);
+                                         cache.features(q), prj_trans);
+                            i++;
                         }
                         cache.clear();
                     }
@@ -476,10 +481,11 @@ void feature_style_processor<Processor>::apply_to_layer(layer const& lay, Proces
                 }
 
                 int i = 0;
-                BOOST_FOREACH (feature_type_style * style, active_styles)
+                BOOST_FOREACH (feature_type_style const* style, active_styles)
                 {
-                    render_style(lay, p, style, style_names[i++],
+                    render_style(lay, p, style, rule_caches[i], style_names[i],
-                                 cache.features(q), prj_trans, scale_denom);
+                                 cache.features(q), prj_trans);
+                    i++;
                 }
             }
         }
@@ -496,20 +502,22 @@ void feature_style_processor<Processor>::apply_to_layer(layer const& lay, Proces
                 }
             }
             int i = 0;
-            BOOST_FOREACH (feature_type_style * style, active_styles)
+            BOOST_FOREACH (feature_type_style const* style, active_styles)
             {
-                render_style(lay, p, style, style_names[i++],
+                render_style(lay, p, style, rule_caches[i], style_names[i],
-                             cache.features(q), prj_trans, scale_denom);
+                             cache.features(q), prj_trans);
+                i++;
             }
         }
         // We only have a single style and no grouping.
         else
         {
             int i = 0;
-            BOOST_FOREACH (feature_type_style * style, active_styles)
+            BOOST_FOREACH (feature_type_style const* style, active_styles)
             {
-                render_style(lay, p, style, style_names[i++],
+                render_style(lay, p, style, rule_caches[i], style_names[i],
-                             ds->features(q), prj_trans, scale_denom);
+                             ds->features(q), prj_trans);
+                i++;
             }
         }
     }
@@ -526,11 +534,11 @@ template <typename Processor>
 void feature_style_processor<Processor>::render_style(
     layer const& lay,
     Processor & p,
-    feature_type_style* style,
+    feature_type_style const* style,
+    rule_cache const& rc,
     std::string const& style_name,
     featureset_ptr features,
-    proj_transform const& prj_trans,
+    proj_transform const& prj_trans)
-    double scale_denom)
 {
     p.start_style_processing(*style);
     if (!features)
@@ -560,7 +568,7 @@ void feature_style_processor<Processor>::render_style(
         bool do_else = true;
         bool do_also = false;
 
-        BOOST_FOREACH(rule * r, style->get_if_rules(scale_denom) )
+        BOOST_FOREACH(rule const* r, rc.get_if_rules() )
         {
             expression_ptr const& expr=r->get_filter();
             value_type result = boost::apply_visitor(evaluate<Feature,value_type>(*feature),*expr);
@@ -589,13 +597,14 @@ void feature_style_processor<Processor>::render_style(
                 if (style->get_filter_mode() == FILTER_FIRST)
                 {
                     // Stop iterating over rules and proceed with next feature.
+                    do_also=false;
                     break;
                 }
             }
         }
         if (do_else)
         {
-            BOOST_FOREACH( rule * r, style->get_else_rules(scale_denom) )
+            BOOST_FOREACH( rule const* r, rc.get_else_rules() )
             {
 #if defined(RENDERING_STATS)
                 feat_processed = true;
@@ -617,7 +626,7 @@ void feature_style_processor<Processor>::render_style(
         }
         if (do_also)
         {
-            BOOST_FOREACH( rule * r, style->get_also_rules(scale_denom) )
+            BOOST_FOREACH( rule const* r, rc.get_also_rules() )
             {
 #if defined(RENDERING_STATS)
                 feat_processed = true;

include/mapnik/feature_type_style.hpp

@@ -50,7 +50,6 @@ enum filter_mode_enum {
 DEFINE_ENUM( filter_mode_e, filter_mode_enum );
 
 typedef std::vector<rule> rules;
-typedef std::vector<rule*> rule_ptrs;
 
 class MAPNIK_DECL feature_type_style
 {
@@ -62,11 +61,6 @@ private:
     std::vector<filter::filter_type> direct_filters_;
     // comp-op
     boost::optional<composite_mode_e> comp_op_;
-    // The rule_ptrs vectors are only valid for the scale_denom_validity_.
-    double scale_denom_validity_;
-    rule_ptrs if_rules_;
-    rule_ptrs else_rules_;
-    rule_ptrs also_rules_;
     float opacity_;
 public:
     feature_type_style();
@@ -77,9 +71,6 @@ public:
 
     void add_rule(rule const& rule);
     rules const& get_rules() const;
-    rule_ptrs const& get_if_rules(double scale_denom);
-    rule_ptrs const& get_else_rules(double scale_denom);
-    rule_ptrs const& get_also_rules(double scale_denom);
     rules& get_rules_nonconst();
     
     bool active(double scale_denom) const;
@@ -100,9 +91,6 @@ public:
 
     ~feature_type_style() {}
 
-private:
-    void update_rule_cache(double scale_denom);
-
 };
 }
 

include/mapnik/rule_cache.hpp

@@ -0,0 +1,95 @@
+/*****************************************************************************
+ *
+ * This file is part of Mapnik (c++ mapping toolkit)
+ *
+ * Copyright (C) 2013 Artem Pavlenko
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ *****************************************************************************/
+
+#ifndef MAPNIK_RULE_CACHE_HPP
+#define MAPNIK_RULE_CACHE_HPP
+
+// mapnik
+#include <mapnik/rule.hpp>
+#include <mapnik/feature_type_style.hpp>
+
+// boost
+#include <boost/foreach.hpp>
+
+// stl
+#include <vector>
+
+namespace mapnik
+{
+
+class rule_cache
+{
+public:
+    typedef std::vector<rule const*> rule_ptrs;
+    rule_cache()
+     : if_rules_(),
+       else_rules_(),
+       also_rules_(),
+       active_(false) {}
+
+    bool active()
+    {
+        return active_;
+    }
+
+    void add_rule(rule const& r)
+    {
+        if (r.has_else_filter())
+        {
+            else_rules_.push_back(&r);
+        }
+        else if (r.has_also_filter())
+        {
+            also_rules_.push_back(&r);
+        }
+        else
+        {
+            if_rules_.push_back(&r);
+        }
+        active_ = true;
+    }
+
+    rule_ptrs const& get_if_rules() const
+    {
+        return if_rules_;
+    }
+    
+    rule_ptrs const& get_else_rules() const
+    {
+        return else_rules_;
+    }
+    
+    rule_ptrs const& get_also_rules() const
+    {
+        return also_rules_;
+    }
+
+private:
+    rule_ptrs if_rules_;
+    rule_ptrs else_rules_;
+    rule_ptrs also_rules_;
+    bool active_;
+};
+
+}
+
+#endif // MAPNIK_RULE_CACHE_HPP

src/feature_type_style.cpp

@@ -42,7 +42,6 @@ feature_type_style::feature_type_style()
 : filter_mode_(FILTER_ALL),
     filters_(),
     direct_filters_(),
-    scale_denom_validity_(-1),
     opacity_(1.0f)
 {}
 
@@ -51,7 +50,6 @@ feature_type_style::feature_type_style(feature_type_style const& rhs, bool deep_
       filters_(rhs.filters_),
       direct_filters_(rhs.direct_filters_),
       comp_op_(rhs.comp_op_),
-      scale_denom_validity_(-1),
       opacity_(rhs.opacity_)
 {
     if (!deep_copy) {
@@ -72,7 +70,6 @@ feature_type_style& feature_type_style::operator=(feature_type_style const& rhs)
     filters_ = rhs.filters_;
     direct_filters_ = rhs.direct_filters_;
     comp_op_ = rhs.comp_op_;
-    scale_denom_validity_ = -1;
     opacity_= rhs.opacity_;
     return *this;
 }
@@ -80,7 +77,6 @@ feature_type_style& feature_type_style::operator=(feature_type_style const& rhs)
 void feature_type_style::add_rule(rule const& rule)
 {
     rules_.push_back(rule);
-    scale_denom_validity_ = -1;
 }
 
 rules const& feature_type_style::get_rules() const
@@ -155,59 +151,5 @@ float feature_type_style::get_opacity() const
     return opacity_;
 }
 
-void feature_type_style::update_rule_cache(double scale_denom)
-{
-    if_rules_.clear();
-    else_rules_.clear();
-    also_rules_.clear();
-
-    BOOST_FOREACH(rule & r, rules_)
-    {
-        if (r.active(scale_denom))
-        {
-            if (r.has_else_filter())
-            {
-                else_rules_.push_back(&r);
-            }
-            else if (r.has_also_filter())
-            {
-                also_rules_.push_back(&r);
-            }
-            else
-            {
-                if_rules_.push_back(&r);
-            }
-        }
-    }
-
-    scale_denom_validity_ = scale_denom;
-}
-
-rule_ptrs const& feature_type_style::get_if_rules(double scale_denom)
-{
-    if (scale_denom_validity_ != scale_denom)
-    {
-        update_rule_cache(scale_denom);
-    }
-    return if_rules_;
-}
-
-rule_ptrs const& feature_type_style::get_else_rules(double scale_denom)
-{
-    if (scale_denom_validity_ != scale_denom)
-    {
-        update_rule_cache(scale_denom);
-    }
-    return else_rules_;
-}
-
-rule_ptrs const& feature_type_style::get_also_rules(double scale_denom)
-{
-    if (scale_denom_validity_ != scale_denom)
-    {
-        update_rule_cache(scale_denom);
-    }
-    return also_rules_;
-}
 
 }