From ee018dc589f76b01c67f39cdac3346743d02d09b Mon Sep 17 00:00:00 2001
From: Matt Amos <zerebubuth@gmail.com>
Date: Thu, 5 Dec 2013 18:39:09 +0000
Subject: [PATCH] Factor common fields out of cairo renderer.

This involved making the freetype engine stack allocated and
accessible via a `shared_ptr` because `FT_Face` elements stored
in it are referenced from cairo structures which persist beyond
the lifetime of the `cairo_renderer` object.

Hopefully this can be resolved in a future commit.
---
 include/mapnik/cairo_renderer.hpp  |  12 +--
 include/mapnik/renderer_common.hpp |   4 +-
 src/cairo_context.cpp              |  24 +++++
 src/cairo_renderer.cpp             | 149 ++++++++++-------------------
 src/renderer_common.cpp            |   3 +-
 5 files changed, 83 insertions(+), 109 deletions(-)

diff --git a/include/mapnik/cairo_renderer.hpp b/include/mapnik/cairo_renderer.hpp
index e95233c80..f839c340b 100644
--- a/include/mapnik/cairo_renderer.hpp
+++ b/include/mapnik/cairo_renderer.hpp
@@ -37,6 +37,7 @@
 #include <mapnik/cairo_context.hpp>
 #include <mapnik/pixel_position.hpp>
 #include <mapnik/ctrans.hpp>    // for CoordTransform
+#include <mapnik/renderer_common.hpp>
 
 // cairo
 #include <cairo.h>
@@ -128,7 +129,7 @@ public:
 
     inline double scale_factor() const
     {
-        return scale_factor_;
+        return common_.scale_factor_;
     }
 
     void render_marker(pixel_position const& pos,
@@ -140,15 +141,8 @@ public:
 protected:
     Map const& m_;
     cairo_context context_;
-    unsigned width_;
-    unsigned height_;
-    double scale_factor_;
-    CoordTransform t_;
-    std::shared_ptr<freetype_engine> font_engine_;
-    face_manager<freetype_engine> font_manager_;
+    renderer_common common_;
     cairo_face_manager face_manager_;
-    std::shared_ptr<label_collision_detector4> detector_;
-    box2d<double> query_extent_;
     void setup(Map const& m);
 };
 
diff --git a/include/mapnik/renderer_common.hpp b/include/mapnik/renderer_common.hpp
index 9eac7dafc..ecf90d007 100644
--- a/include/mapnik/renderer_common.hpp
+++ b/include/mapnik/renderer_common.hpp
@@ -50,7 +50,9 @@ struct renderer_common
     unsigned width_;
     unsigned height_;
     double scale_factor_;
-    freetype_engine font_engine_;
+    // TODO: dirty hack for cairo renderer, figure out how to remove this
+    std::shared_ptr<freetype_engine> shared_font_engine_;
+    freetype_engine &font_engine_;
     face_manager<freetype_engine> font_manager_;
     box2d<double> query_extent_;
     CoordTransform t_;
diff --git a/src/cairo_context.cpp b/src/cairo_context.cpp
index 61ee817fe..d30f54861 100644
--- a/src/cairo_context.cpp
+++ b/src/cairo_context.cpp
@@ -499,4 +499,28 @@ void cairo_context::add_text(glyph_positions_ptr path,
     }
 
 }
+
+cairo_face_manager::cairo_face_manager(std::shared_ptr<freetype_engine> font_engine)
+  : font_engine_(font_engine)
+{
+}
+
+cairo_face_ptr cairo_face_manager::get_face(face_ptr face)
+{
+    cairo_face_cache::iterator itr = cache_.find(face);
+    cairo_face_ptr entry;
+
+    if (itr != cache_.end())
+    {
+        entry = itr->second;
+    }
+    else
+    {
+        entry = std::make_shared<cairo_face>(font_engine_, face);
+        cache_.insert(std::make_pair(face, entry));
+    }
+
+    return entry;
+}
+
 }
diff --git a/src/cairo_renderer.cpp b/src/cairo_renderer.cpp
index 74c0b07bc..e334713d3 100644
--- a/src/cairo_renderer.cpp
+++ b/src/cairo_renderer.cpp
@@ -94,31 +94,6 @@ struct cairo_save_restore
     cairo_context & context_;
 };
 
-
-cairo_face_manager::cairo_face_manager(std::shared_ptr<freetype_engine> engine)
-    : font_engine_(engine)
-{
-}
-
-cairo_face_ptr cairo_face_manager::get_face(face_ptr face)
-{
-    cairo_face_cache::iterator itr = cache_.find(face);
-    cairo_face_ptr entry;
-
-    if (itr != cache_.end())
-    {
-        entry = itr->second;
-    }
-    else
-    {
-        entry = std::make_shared<cairo_face>(font_engine_, face);
-        cache_.insert(std::make_pair(face, entry));
-    }
-
-    return entry;
-}
-
-
 cairo_renderer_base::cairo_renderer_base(Map const& m,
                                          cairo_ptr const& cairo,
                                          double scale_factor,
@@ -126,16 +101,8 @@ cairo_renderer_base::cairo_renderer_base(Map const& m,
                                          unsigned offset_y)
     : m_(m),
       context_(cairo),
-      width_(m.width()),
-      height_(m.height()),
-      scale_factor_(scale_factor),
-      t_(m.width(),m.height(),m.get_current_extent(),offset_x,offset_y),
-      font_engine_(std::make_shared<freetype_engine>()),
-      font_manager_(*font_engine_),
-      face_manager_(font_engine_),
-      detector_(std::make_shared<label_collision_detector4>(
-                  box2d<double>(-m.buffer_size(), -m.buffer_size(),
-                                m.width() + m.buffer_size(), m.height() + m.buffer_size())))
+      common_(m, offset_x, offset_y, m.width(), m.height(), scale_factor),
+      face_manager_(common_.shared_font_engine_)
 {
     setup(m);
 }
@@ -148,16 +115,8 @@ cairo_renderer_base::cairo_renderer_base(Map const& m,
                                          unsigned offset_y)
     : m_(m),
       context_(cairo),
-      width_(req.width()),
-      height_(req.height()),
-      scale_factor_(scale_factor),
-      t_(req.width(),req.height(),req.extent(),offset_x,offset_y),
-      font_engine_(std::make_shared<freetype_engine>()),
-      font_manager_(*font_engine_),
-      face_manager_(font_engine_),
-      detector_(std::make_shared<label_collision_detector4>(
-                  box2d<double>(-m.buffer_size(), -m.buffer_size(),
-                                m.width() + m.buffer_size(), m.height() + m.buffer_size())))
+      common_(req, offset_x, offset_y, req.width(), req.height(), scale_factor),
+      face_manager_(common_.shared_font_engine_)
 {
     setup(m);
 }
@@ -170,14 +129,8 @@ cairo_renderer_base::cairo_renderer_base(Map const& m,
                                          unsigned offset_y)
     : m_(m),
       context_(cairo),
-      width_(m.width()),
-      height_(m.height()),
-      scale_factor_(scale_factor),
-      t_(m.width(),m.height(),m.get_current_extent(),offset_x,offset_y),
-      font_engine_(std::make_shared<freetype_engine>()),
-      font_manager_(*font_engine_),
-      face_manager_(font_engine_),
-      detector_(detector)
+      common_(m, offset_x, offset_y, m.width(), m.height(), scale_factor, detector),
+      face_manager_(common_.shared_font_engine_)
 {
     MAPNIK_LOG_DEBUG(cairo_renderer) << "cairo_renderer_base: Scale=" << m.scale();
 }
@@ -236,8 +189,8 @@ void cairo_renderer_base::setup(Map const& map)
             if ( w > 0 && h > 0)
             {
                 // repeat background-image both vertically and horizontally
-                unsigned x_steps = unsigned(std::ceil(width_/double(w)));
-                unsigned y_steps = unsigned(std::ceil(height_/double(h)));
+                unsigned x_steps = unsigned(std::ceil(common_.width_/double(w)));
+                unsigned y_steps = unsigned(std::ceil(common_.height_/double(h)));
                 for (unsigned x=0;x<x_steps;++x)
                 {
                     for (unsigned y=0;y<y_steps;++y)
@@ -257,7 +210,7 @@ void cairo_renderer_base::setup(Map const& map)
 void cairo_renderer_base::start_map_processing(Map const& map)
 {
     MAPNIK_LOG_DEBUG(cairo_renderer) << "cairo_renderer_base: Start map processing bbox=" << map.get_current_extent();
-    box2d<double> bounds = t_.forward(t_.extent());
+    box2d<double> bounds = common_.t_.forward(common_.t_.extent());
     context_.rectangle(bounds.minx(), bounds.miny(), bounds.maxx(), bounds.maxy());
     context_.clip();
 }
@@ -284,9 +237,9 @@ void cairo_renderer_base::start_layer_processing(layer const& lay, box2d<double>
 
     if (lay.clear_label_cache())
     {
-        detector_->clear();
+        common_.detector_->clear();
     }
-    query_extent_ = query_extent;
+    common_.query_extent_ = query_extent;
 }
 
 void cairo_renderer_base::end_layer_processing(layer const&)
@@ -326,7 +279,7 @@ void cairo_renderer_base::process(polygon_symbolizer const& sym,
     typedef boost::mpl::vector<clip_poly_tag,transform_tag,affine_transform_tag,simplify_tag,smooth_tag> conv_types;
     vertex_converter<box2d<double>, cairo_context, polygon_symbolizer,
                      CoordTransform, proj_transform, agg::trans_affine, conv_types>
-        converter(query_extent_,context_,sym,t_,prj_trans,tr,1.0);
+        converter(common_.query_extent_,context_,sym,common_.t_,prj_trans,tr,1.0);
 
     if (prj_trans.equal() && clip) converter.set<clip_poly_tag>(); //optional clip (default: true)
     converter.set<transform_tag>(); //always transform
@@ -402,7 +355,7 @@ void cairo_renderer_base::process(building_symbolizer const& sym,
                 faces->line_to(std::get<2>(seg), std::get<3>(seg) + height);
                 faces->line_to(std::get<0>(seg), std::get<1>(seg) + height);
 
-                path_type faces_path(t_, *faces, prj_trans);
+                path_type faces_path(common_.t_, *faces, prj_trans);
                 context_.set_color(fill.red()  * 0.8 / 255.0, fill.green() * 0.8 / 255.0,
                                   fill.blue() * 0.8 / 255.0, fill.alpha() * opacity / 255.0);
                 context_.add_path(faces_path);
@@ -433,14 +386,14 @@ void cairo_renderer_base::process(building_symbolizer const& sym,
                 }
             }
 
-            path_type path(t_, *frame, prj_trans);
+            path_type path(common_.t_, *frame, prj_trans);
             context_.set_color(fill.red()  * 0.8 / 255.0, fill.green() * 0.8/255.0,
                               fill.blue() * 0.8 / 255.0, fill.alpha() * opacity / 255.0);
-            context_.set_line_width(scale_factor_);
+            context_.set_line_width(common_.scale_factor_);
             context_.add_path(path);
             context_.stroke();
 
-            path_type roof_path(t_, *roof, prj_trans);
+            path_type roof_path(common_.t_, *roof, prj_trans);
             context_.set_color(fill, opacity);
             context_.add_path(roof_path);
             context_.fill();
@@ -477,30 +430,30 @@ void cairo_renderer_base::process(line_symbolizer const& sym,
     context_.set_line_join(stroke_join);
     context_.set_line_cap(stroke_cap);
     context_.set_miter_limit(miterlimit);
-    context_.set_line_width(width * scale_factor_);
+    context_.set_line_width(width * common_.scale_factor_);
     if (dash)
     {
-        context_.set_dash(*dash, scale_factor_);
+        context_.set_dash(*dash, common_.scale_factor_);
     }
 
     agg::trans_affine tr;
     if (geom_transform) { evaluate_transform(tr, feature, *geom_transform); }
 
-    box2d<double> clipping_extent = query_extent_;
+    box2d<double> clipping_extent = common_.query_extent_;
     if (clip)
     {
-        double padding = (double)(query_extent_.width()/width_);
+        double padding = (double)(common_.query_extent_.width()/common_.width_);
         double half_stroke = width/2.0;
         if (half_stroke > 1)
             padding *= half_stroke;
         if (std::fabs(offset) > 0)
             padding *= std::fabs(offset) * 1.2;
-        padding *= scale_factor_;
+        padding *= common_.scale_factor_;
         clipping_extent.pad(padding);
     }
     vertex_converter<box2d<double>, cairo_context, line_symbolizer,
                      CoordTransform, proj_transform, agg::trans_affine, conv_types>
-        converter(clipping_extent,context_,sym,t_,prj_trans,tr,scale_factor_);
+        converter(clipping_extent,context_,sym,common_.t_,prj_trans,tr,common_.scale_factor_);
 
     if (clip) converter.set<clip_line_tag>(); // optional clip (default: true)
     converter.set<transform_tag>(); // always transform
@@ -645,7 +598,7 @@ void cairo_renderer_base::render_marker(pixel_position const& pos,
         if (vmarker)
         {
             agg::trans_affine marker_tr = tr;
-            marker_tr *=agg::trans_affine_scaling(scale_factor_);
+            marker_tr *=agg::trans_affine_scaling(common_.scale_factor_);
             agg::pod_bvector<svg::path_attributes> const & attributes = vmarker->attributes();
             render_vector_marker(context_, pos, *vmarker, attributes, marker_tr, opacity, recenter);
         }
@@ -659,7 +612,7 @@ void cairo_renderer_base::render_marker(pixel_position const& pos,
         agg::trans_affine marker_tr;
         marker_tr *= agg::trans_affine_translation(-cx,-cy);
         marker_tr *= tr;
-        marker_tr *= agg::trans_affine_scaling(scale_factor_);
+        marker_tr *= agg::trans_affine_scaling(common_.scale_factor_);
         marker_tr *= agg::trans_affine_translation(pos.x,pos.y);
         context_.add_image(marker_tr, **marker.get_bitmap_data(), opacity);
     }
@@ -701,7 +654,7 @@ void cairo_renderer_base::process(point_symbolizer const& sym,
 
         agg::trans_affine_translation recenter(-center.x, -center.y);
         agg::trans_affine recenter_tr = recenter * tr;
-        box2d<double> label_ext = bbox * recenter_tr * agg::trans_affine_scaling(scale_factor_);
+        box2d<double> label_ext = bbox * recenter_tr * agg::trans_affine_scaling(common_.scale_factor_);
 
         for (std::size_t i = 0; i < feature.num_geometries(); ++i)
         {
@@ -722,15 +675,15 @@ void cairo_renderer_base::process(point_symbolizer const& sym,
             }
 
             prj_trans.backward(x, y, z);
-            t_.forward(&x, &y);
+            common_.t_.forward(&x, &y);
             label_ext.re_center(x,y);
             if (allow_overlap ||
-                detector_->has_placement(label_ext))
+                common_.detector_->has_placement(label_ext))
             {
                 render_marker(pixel_position(x,y),**marker, tr, opacity);
 
                 if (!ignore_placement)
-                    detector_->insert(label_ext);
+                    common_.detector_->insert(label_ext);
             }
         }
     }
@@ -742,10 +695,10 @@ void cairo_renderer_base::process(shield_symbolizer const& sym,
 {
     text_symbolizer_helper helper(
             sym, feature, prj_trans,
-            width_, height_,
-            scale_factor_,
-            t_, font_manager_, *detector_,
-            query_extent_);
+            common_.width_, common_.height_,
+            common_.scale_factor_,
+            common_.t_, common_.font_manager_, *common_.detector_,
+            common_.query_extent_);
 
     cairo_save_restore guard(context_);
     composite_mode_e comp_op = get<composite_mode_e>(sym, keys::comp_op, feature, src_over);
@@ -764,7 +717,7 @@ void cairo_renderer_base::process(shield_symbolizer const& sym,
                           opacity);
         }
 
-        context_.add_text(glyphs, face_manager_, font_manager_, scale_factor_);
+        context_.add_text(glyphs, face_manager_, common_.font_manager_, common_.scale_factor_);
     }
 }
 
@@ -798,7 +751,7 @@ void cairo_renderer_base::process(line_pattern_symbolizer const& sym,
 
     pattern.set_extend(CAIRO_EXTEND_REPEAT);
     pattern.set_filter(CAIRO_FILTER_BILINEAR);
-    context_.set_line_width(height * scale_factor_);
+    context_.set_line_width(height * common_.scale_factor_);
 
     for (std::size_t i = 0; i < feature.num_geometries(); ++i)
     {
@@ -807,8 +760,8 @@ void cairo_renderer_base::process(line_pattern_symbolizer const& sym,
         if (geom.size() > 1)
         {
             clipped_geometry_type clipped(geom);
-            clipped.clip_box(query_extent_.minx(),query_extent_.miny(),query_extent_.maxx(),query_extent_.maxy());
-            path_type path(t_,clipped,prj_trans);
+            clipped.clip_box(common_.query_extent_.minx(),common_.query_extent_.miny(),common_.query_extent_.maxx(),common_.query_extent_.maxy());
+            path_type path(common_.t_,clipped,prj_trans);
 
             double length(0);
             double x0(0), y0(0);
@@ -903,7 +856,7 @@ void cairo_renderer_base::process(polygon_pattern_symbolizer const& sym,
     typedef boost::mpl::vector<clip_poly_tag,transform_tag,affine_transform_tag,simplify_tag,smooth_tag> conv_types;
     vertex_converter<box2d<double>, cairo_context, polygon_pattern_symbolizer,
                      CoordTransform, proj_transform, agg::trans_affine, conv_types>
-        converter(query_extent_,context_,sym,t_,prj_trans,tr, scale_factor_);
+        converter(common_.query_extent_,context_,sym,common_.t_,prj_trans,tr, common_.scale_factor_);
 
     if (prj_trans.equal() && clip) converter.set<clip_poly_tag>(); //optional clip (default: true)
     converter.set<transform_tag>(); //always transform
@@ -944,7 +897,7 @@ void cairo_renderer_base::process(raster_symbolizer const& sym,
 
         box2d<double> target_ext = box2d<double>(source->ext_);
         prj_trans.backward(target_ext, PROJ_ENVELOPE_POINTS);
-        box2d<double> ext = t_.forward(target_ext);
+        box2d<double> ext = common_.t_.forward(target_ext);
         int start_x = static_cast<int>(std::floor(ext.minx()+.5));
         int start_y = static_cast<int>(std::floor(ext.miny()+.5));
         int end_x = static_cast<int>(std::floor(ext.maxx()+.5));
@@ -1219,7 +1172,7 @@ void cairo_renderer_base::process(markers_symbolizer const& sym,
 
     context_.set_operator(comp_op);
 
-    agg::trans_affine tr = agg::trans_affine_scaling(scale_factor_);
+    agg::trans_affine tr = agg::trans_affine_scaling(common_.scale_factor_);
 
     if (!filename.empty())
     {
@@ -1252,15 +1205,15 @@ void cairo_renderer_base::process(markers_symbolizer const& sym,
                     build_ellipse(sym, feature, marker_ellipse, svg_path);
                     svg_attributes_type attributes;
                     bool result = push_explicit_style( (*stock_vector_marker)->attributes(), attributes, sym);
-                    agg::trans_affine marker_tr = agg::trans_affine_scaling(scale_factor_);
+                    agg::trans_affine marker_tr = agg::trans_affine_scaling(common_.scale_factor_);
                     if (img_transform) { evaluate_transform(marker_tr, feature, *img_transform); }
                     box2d<double> new_bbox = marker_ellipse.bounding_box();
 
                     dispatch_type dispatch(context_, marker_ellipse, result?attributes:(*stock_vector_marker)->attributes(),
-                                           *detector_, sym, new_bbox, marker_tr, scale_factor_);
+                                           *common_.detector_, sym, new_bbox, marker_tr, common_.scale_factor_);
                     vertex_converter<box2d<double>, dispatch_type, markers_symbolizer,
                                      CoordTransform, proj_transform, agg::trans_affine, conv_types>
-                        converter(query_extent_, dispatch, sym, t_, prj_trans, marker_tr, scale_factor_);
+                        converter(common_.query_extent_, dispatch, sym, common_.t_, prj_trans, marker_tr, common_.scale_factor_);
 
                     if (clip && feature.paths().size() > 0) // optional clip (default: true)
                     {
@@ -1282,10 +1235,10 @@ void cairo_renderer_base::process(markers_symbolizer const& sym,
                     bool result = push_explicit_style( (*stock_vector_marker)->attributes(), attributes, sym);
 
                     dispatch_type dispatch(context_, **stock_vector_marker, result?attributes:(*stock_vector_marker)->attributes(),
-                                           *detector_, sym, bbox, tr, scale_factor_);
+                                           *common_.detector_, sym, bbox, tr, common_.scale_factor_);
                     vertex_converter<box2d<double>, dispatch_type, markers_symbolizer,
                                      CoordTransform, proj_transform, agg::trans_affine, conv_types>
-                        converter(query_extent_, dispatch, sym, t_, prj_trans, tr, scale_factor_);
+                        converter(common_.query_extent_, dispatch, sym, common_.t_, prj_trans, tr, common_.scale_factor_);
 
                     if (clip && feature.paths().size() > 0) // optional clip (default: true)
                     {
@@ -1311,11 +1264,11 @@ void cairo_renderer_base::process(markers_symbolizer const& sym,
                 if ( marker )
                 {
                     dispatch_type dispatch(context_, *marker,
-                                           *detector_, sym, bbox, tr, scale_factor_);
+                                           *common_.detector_, sym, bbox, tr, common_.scale_factor_);
 
                     vertex_converter<box2d<double>, dispatch_type, markers_symbolizer,
                                      CoordTransform, proj_transform, agg::trans_affine, conv_types>
-                        converter(query_extent_, dispatch, sym, t_, prj_trans, tr, scale_factor_);
+                        converter(common_.query_extent_, dispatch, sym, common_.t_, prj_trans, tr, common_.scale_factor_);
 
                     if (clip && feature.paths().size() > 0) // optional clip (default: true)
                     {
@@ -1342,10 +1295,10 @@ void cairo_renderer_base::process(text_symbolizer const& sym,
 {
     text_symbolizer_helper helper(
             sym, feature, prj_trans,
-            width_, height_,
-            scale_factor_,
-            t_, font_manager_, *detector_,
-            query_extent_);
+            common_.width_, common_.height_,
+            common_.scale_factor_,
+            common_.t_, common_.font_manager_, *common_.detector_,
+            common_.query_extent_);
 
     cairo_save_restore guard(context_);
     composite_mode_e comp_op = get<composite_mode_e>(sym, keys::comp_op, feature, src_over);
@@ -1355,7 +1308,7 @@ void cairo_renderer_base::process(text_symbolizer const& sym,
     placements_list const &placements = helper.get();
     for (glyph_positions_ptr glyphs : placements)
     {
-        context_.add_text(glyphs, face_manager_, font_manager_, scale_factor_);
+        context_.add_text(glyphs, face_manager_, common_.font_manager_, common_.scale_factor_);
     }
 }
 
diff --git a/src/renderer_common.cpp b/src/renderer_common.cpp
index 024688c12..9a3e4b475 100644
--- a/src/renderer_common.cpp
+++ b/src/renderer_common.cpp
@@ -32,7 +32,8 @@ renderer_common::renderer_common(unsigned width, unsigned height, double scale_f
    : width_(width),
      height_(height),
      scale_factor_(scale_factor),
-     font_engine_(),
+     shared_font_engine_(std::make_shared<freetype_engine>()),
+     font_engine_(*shared_font_engine_),
      font_manager_(font_engine_),
      query_extent_(),
      t_(t),