noncopyable glyph_info - refs #2516

include/mapnik/text/glyph_info.hpp

@@ -25,6 +25,7 @@
 //mapnik
 #include <mapnik/text/evaluated_format_properties_ptr.hpp>
 #include <mapnik/pixel_position.hpp>
+#include <mapnik/noncopyable.hpp>
 
 #include <memory>
 #include <cmath>
@@ -35,23 +36,52 @@ namespace mapnik
 class font_face;
 using face_ptr = std::shared_ptr<font_face>;
 
-struct glyph_info
+struct glyph_info : noncopyable
 {
     glyph_info(unsigned g_index, unsigned c_index)
         : glyph_index(g_index),
           char_index(c_index),
           face(nullptr),
+          format(),
           unscaled_ymin(0.0),
           unscaled_ymax(0.0),
           unscaled_advance(0.0),
           unscaled_line_height(0.0),
           scale_multiplier(1.0),
-          offset(),
+          offset() {}
-          format() {}
+    glyph_info(glyph_info && rhs)
+        : glyph_index(std::move(rhs.glyph_index)),
+          char_index(std::move(rhs.char_index)),
+          face(std::move(rhs.face)), // shared_ptr move just ref counts, right?
+          format(std::move(rhs.format)), // shared_ptr move just ref counts, right?
+          unscaled_ymin(std::move(rhs.unscaled_ymin)),
+          unscaled_ymax(std::move(rhs.unscaled_ymax)),
+          unscaled_advance(std::move(rhs.unscaled_advance)),
+          unscaled_line_height(std::move(rhs.unscaled_line_height)),
+          scale_multiplier(std::move(rhs.scale_multiplier)),
+          offset(std::move(rhs.offset)) {}
+
+    // copying a glyph_info is not ideal, so we
+    // require an explicit copy constructor
+    inline glyph_info clone() const
+    {
+        glyph_info g(glyph_index,char_index);
+        g.face = face;
+        g.format = format;
+        g.unscaled_ymin = unscaled_ymin;
+        g.unscaled_ymax = unscaled_ymax;
+        g.unscaled_advance = unscaled_advance;
+        g.unscaled_line_height = unscaled_line_height;
+        g.scale_multiplier = scale_multiplier;
+        g.offset = offset;
+        return g;
+    }
+
     unsigned glyph_index;
     // Position in the string of all characters i.e. before itemizing
     unsigned char_index;
     face_ptr face;
+    evaluated_format_properties_ptr format;
     double unscaled_ymin;
     double unscaled_ymax;
     double unscaled_advance;
@@ -60,7 +90,6 @@ struct glyph_info
     double unscaled_line_height;
     double scale_multiplier;
     pixel_position offset;
-    evaluated_format_properties_ptr format;
 
     double ymin() const { return unscaled_ymin * 64.0 * scale_multiplier; }
     double ymax() const { return unscaled_ymax * 64.0 * scale_multiplier; }

include/mapnik/text/harfbuzz_shaper.hpp

@@ -135,7 +135,7 @@ static void shape_text(text_line & line,
                     double tmp_height = g.height();
                     if (tmp_height > max_glyph_height) max_glyph_height = tmp_height;
                     width_map[char_index] += g.advance();
-                    line.add_glyph(g, scale_factor);
+                    line.add_glyph(std::move(g), scale_factor);
                 }
             }
             line.update_max_char_height(max_glyph_height);

include/mapnik/text/text_line.hpp

@@ -46,7 +46,7 @@ public:
     text_line( text_line && rhs);
 
     // Append glyph.
-    void add_glyph(glyph_info const& glyph, double scale_factor_);
+    void add_glyph(glyph_info && glyph, double scale_factor_);
 
     // Preallocate memory.
     void reserve(glyph_vector::size_type length);

src/renderer_common/process_group_symbolizer.cpp

@@ -64,7 +64,8 @@ text_render_thunk::text_render_thunk(placements_list const& placements,
 
         for (glyph_position const& pos : *positions)
         {
-            glyph_vec.push_back(pos.glyph);
+            // TODO - could we store pointers to glyph_info instead to avoid copy?
+            glyph_vec.push_back(pos.glyph.clone());
             new_pos.emplace_back(glyph_vec.back(), pos.pos, pos.rot);
         }
 

src/text/text_line.cpp

@@ -45,7 +45,7 @@ text_line::text_line( text_line && rhs)
       last_char_(std::move(rhs.last_char_)),
       first_line_(std::move(rhs.first_line_)) {}
 
-void text_line::add_glyph(glyph_info const& glyph, double scale_factor_)
+void text_line::add_glyph(glyph_info && glyph, double scale_factor_)
 {
     line_height_ = std::max(line_height_, glyph.line_height() + glyph.format->line_spacing);
     double advance = glyph.advance();
@@ -58,7 +58,7 @@ void text_line::add_glyph(glyph_info const& glyph, double scale_factor_)
         // Only add character spacing if the character is not a zero-width part of a cluster.
         width_ += advance + glyphs_.back().format->character_spacing  * scale_factor_;
     }
-    glyphs_.push_back(glyph);
+    glyphs_.emplace_back(std::move(glyph));
 }
 
 void text_line::reserve(glyph_vector::size_type length)