From f3eba54078c0f8f839b8bab99f123e972e32b7e8 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Sun, 12 Oct 2014 14:24:57 -0700 Subject: [PATCH] noncopyable glyph_info - refs #2516 --- include/mapnik/text/glyph_info.hpp | 37 +++++++++++++++++-- include/mapnik/text/harfbuzz_shaper.hpp | 2 +- include/mapnik/text/text_line.hpp | 2 +- .../process_group_symbolizer.cpp | 3 +- src/text/text_line.cpp | 4 +- 5 files changed, 39 insertions(+), 9 deletions(-) diff --git a/include/mapnik/text/glyph_info.hpp b/include/mapnik/text/glyph_info.hpp index f8b7559e0..039147e44 100644 --- a/include/mapnik/text/glyph_info.hpp +++ b/include/mapnik/text/glyph_info.hpp @@ -25,6 +25,7 @@ //mapnik #include #include +#include #include #include @@ -35,23 +36,52 @@ namespace mapnik class font_face; using face_ptr = std::shared_ptr; -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(), - format() {} + offset() {} + 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; } diff --git a/include/mapnik/text/harfbuzz_shaper.hpp b/include/mapnik/text/harfbuzz_shaper.hpp index cb0ed8496..bb123b63d 100644 --- a/include/mapnik/text/harfbuzz_shaper.hpp +++ b/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); diff --git a/include/mapnik/text/text_line.hpp b/include/mapnik/text/text_line.hpp index fea4e35e9..f23fd121e 100644 --- a/include/mapnik/text/text_line.hpp +++ b/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); diff --git a/src/renderer_common/process_group_symbolizer.cpp b/src/renderer_common/process_group_symbolizer.cpp index 27022f4fb..db734c1a1 100644 --- a/src/renderer_common/process_group_symbolizer.cpp +++ b/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); } diff --git a/src/text/text_line.cpp b/src/text/text_line.cpp index b463497d5..e1da37d0f 100644 --- a/src/text/text_line.cpp +++ b/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)