sij/mapnik
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

improve handling of format properties - refs #2516

Browse source 
- changes evaluated_format_properties_ptr to unique_ptr
 - refactors group symbolizer helper to keep scope by
   having each "thunk" own its helper
 - glyph_info can now be noncopyable
 - format node children now need an owner for their
   evaluated_format_ptr so the enclosing parent layout
   now allocates and holds pointers in a deque
 - plus other noncopyable/move constructor fixes
This commit is contained in:
Dane Springmeyer 2014-10-12 21:41:59 -07:00
parent a39cb0aaf8
commit 885a98f9c1
19 changed files with 96 additions and 100 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

34
include/mapnik/renderer_common/process_group_symbolizer.hpp
View file

@ -97,7 +97,7 @@ struct virtual_renderer_common : private mapnik::noncopyable
// stores all the arguments necessary to re-render this point
// symbolizer at a later time.
struct point_render_thunk
struct point_render_thunk : noncopyable
{
pixel_position pos_;
marker_ptr marker_;
@ -108,22 +108,36 @@ struct point_render_thunk
point_render_thunk(pixel_position const& pos, marker const& m,
agg::trans_affine const& tr, double opacity,
composite_mode_e comp_op);
point_render_thunk(point_render_thunk && rhs)
: pos_(std::move(rhs.pos_)),
marker_(std::move(rhs.marker_)),
tr_(std::move(rhs.tr_)),
opacity_(std::move(rhs.opacity_)),
comp_op_(std::move(rhs.comp_op_)) {}
};
struct text_render_thunk
using helper_ptr = std::unique_ptr<text_symbolizer_helper>;
struct text_render_thunk : noncopyable
{
// need to keep these around, annoyingly, as the glyph_position
// struct keeps a pointer to the glyph_info, so we have to
// ensure the lifetime is the same.
placements_list placements_;
std::shared_ptr<std::vector<glyph_info> > glyphs_;
// helper is stored here in order
// to keep in scope the text rendering structures
helper_ptr helper_;
placements_list const& placements_;
double opacity_;
composite_mode_e comp_op_;
halo_rasterizer_enum halo_rasterizer_;
text_render_thunk(placements_list const& placements,
text_render_thunk(helper_ptr && helper,
double opacity, composite_mode_e comp_op,
halo_rasterizer_enum halo_rasterizer);
text_render_thunk(text_render_thunk && rhs)
: helper_(std::move(rhs.helper_)),
placements_(std::move(rhs.placements_)),
opacity_(std::move(rhs.opacity_)),
comp_op_(std::move(rhs.comp_op_)),
halo_rasterizer_(std::move(rhs.halo_rasterizer_)) {}
};
// Variant type for render thunks to allow us to re-render them
@ -131,7 +145,7 @@ struct text_render_thunk
using render_thunk = util::variant<point_render_thunk,
text_render_thunk>;
using render_thunk_ptr = std::shared_ptr<render_thunk>;
using render_thunk_ptr = std::unique_ptr<render_thunk>;
using render_thunk_list = std::list<render_thunk_ptr>;
// Base class for extracting the bounding boxes associated with placing
@ -163,7 +177,7 @@ struct render_thunk_extractor : public util::static_visitor<>
}
private:
void extract_text_thunk(text_symbolizer_helper & helper, text_symbolizer const& sym) const;
void extract_text_thunk(helper_ptr && helper, text_symbolizer const& sym) const;
box2d<double> & box_;
render_thunk_list & thunks_;

2
include/mapnik/text/evaluated_format_properties_ptr.hpp
View file

@ -28,7 +28,7 @@
namespace mapnik { namespace detail {
struct evaluated_format_properties;
}
using evaluated_format_properties_ptr = std::shared_ptr<detail::evaluated_format_properties>;
using evaluated_format_properties_ptr = std::unique_ptr<detail::evaluated_format_properties>;
}
#endif // EVALUATED_FORMAT_PROPERTIES_PTR_HPP

2
include/mapnik/text/formatting/base.hpp
View file

@ -53,7 +53,7 @@ public:
virtual ~node() {}
virtual void to_xml(boost::property_tree::ptree & xml) const = 0;
static node_ptr from_xml(xml_node const& xml, fontset_map const& fontsets);
virtual void apply(evaluated_format_properties_ptr p, feature_impl const& feature, attributes const& vars, text_layout & output) const = 0;
virtual void apply(evaluated_format_properties_ptr const& p, feature_impl const& feature, attributes const& vars, text_layout & output) const = 0;
virtual void add_expressions(expression_set & output) const = 0;
};
} //ns formatting

2
include/mapnik/text/formatting/format.hpp
View file

@ -41,7 +41,7 @@ class MAPNIK_DECL format_node: public node
public:
void to_xml(boost::property_tree::ptree & xml) const;
static node_ptr from_xml(xml_node const& xml, fontset_map const& fontsets);
virtual void apply(evaluated_format_properties_ptr p, feature_impl const& feature, attributes const& vars, text_layout & output) const;
virtual void apply(evaluated_format_properties_ptr const& p, feature_impl const& feature, attributes const& vars, text_layout & output) const;
virtual void add_expressions(expression_set & output) const;
void set_child(node_ptr child);

2
include/mapnik/text/formatting/layout.hpp
View file

@ -35,7 +35,7 @@ class MAPNIK_DECL layout_node: public node
public:
void to_xml(boost::property_tree::ptree &xml) const;
static node_ptr from_xml(xml_node const& xml, fontset_map const& fontsets);
virtual void apply(evaluated_format_properties_ptr p, feature_impl const& feature, attributes const& vars, text_layout &output) const;
virtual void apply(evaluated_format_properties_ptr const& p, feature_impl const& feature, attributes const& vars, text_layout &output) const;
virtual void add_expressions(expression_set &output) const;
void set_child(node_ptr child);
node_ptr get_child() const;

2
include/mapnik/text/formatting/list.hpp
View file

@ -40,7 +40,7 @@ class MAPNIK_DECL list_node: public node {
public:
list_node() : node(), children_() {}
virtual void to_xml(boost::property_tree::ptree &xml) const;
virtual void apply(evaluated_format_properties_ptr p, feature_impl const& feature, attributes const& vars, text_layout &output) const;
virtual void apply(evaluated_format_properties_ptr const& p, feature_impl const& feature, attributes const& vars, text_layout &output) const;
virtual void add_expressions(expression_set &output) const;
void push_back(node_ptr n);

2
include/mapnik/text/formatting/text.hpp
View file

@ -39,7 +39,7 @@ public:
text_node(std::string text): node(), text_(parse_expression(text)) {}
void to_xml(boost::property_tree::ptree &xml) const;
static node_ptr from_xml(xml_node const& xml, fontset_map const& fontsets);
virtual void apply(evaluated_format_properties_ptr p, feature_impl const& feature, attributes const& vars, text_layout &output) const;
virtual void apply(evaluated_format_properties_ptr const& p, feature_impl const& feature, attributes const& vars, text_layout &output) const;
virtual void add_expressions(expression_set &output) const;
void set_text(expression_ptr text);

26
include/mapnik/text/glyph_info.hpp
View file

@ -38,11 +38,13 @@ using face_ptr = std::shared_ptr<font_face>;
struct glyph_info : noncopyable
{
glyph_info(unsigned g_index, unsigned c_index)
glyph_info(unsigned g_index,
unsigned c_index,
evaluated_format_properties_ptr const& f)
: glyph_index(g_index),
char_index(c_index),
format(f),
face(nullptr),
format(),
unscaled_ymin(0.0),
unscaled_ymax(0.0),
unscaled_advance(0.0),
@ -52,8 +54,8 @@ struct glyph_info : noncopyable
glyph_info(glyph_info && rhs)
: glyph_index(std::move(rhs.glyph_index)),
char_index(std::move(rhs.char_index)),
format(rhs.format), // take ref
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)),
@ -61,27 +63,11 @@ struct glyph_info : noncopyable
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;
evaluated_format_properties_ptr const& format;
face_ptr face;
evaluated_format_properties_ptr format;
double unscaled_ymin;
double unscaled_ymax;
double unscaled_advance;

3
include/mapnik/text/harfbuzz_shaper.hpp
View file

@ -126,11 +126,10 @@ static void shape_text(text_line & line,
auto const& pos = positions[i];
auto const& glyph = glyphs[i];
unsigned char_index = glyph.cluster;
glyph_info g(glyph.codepoint,char_index);
glyph_info g(glyph.codepoint,char_index,text_item.format_);
if (face->glyph_dimensions(g))
{
g.face = face;
g.format = text_item.format_;
g.scale_multiplier = size / face->get_face()->units_per_EM;
//Overwrite default advance with better value provided by HarfBuzz
g.unscaled_advance = pos.x_advance;

19
include/mapnik/text/itemizer.hpp
View file

@ -47,18 +47,23 @@ struct text_item : noncopyable
unsigned e,
UScriptCode sc,
UBiDiDirection d,
evaluated_format_properties_ptr f)
evaluated_format_properties_ptr const& f)
: start(s),
end(e),
script(sc),
dir(d),
format_(f) {}
text_item( text_item && ) = default;
text_item( text_item && rhs)
: start(std::move(rhs.start)),
end(std::move(rhs.end)),
script(std::move(rhs.script)),
dir(std::move(rhs.dir)),
format_(rhs.format_) {}
unsigned start; // First char (UTF16 offset)
unsigned end; // Char _after_ the last char (UTF16 offset)
UScriptCode script;
UBiDiDirection dir;
evaluated_format_properties_ptr format_;
evaluated_format_properties_ptr const& format_;
};
// This class splits text into parts which all have the same
@ -70,7 +75,7 @@ class text_itemizer
{
public:
text_itemizer();
void add_text(value_unicode_string const& str, evaluated_format_properties_ptr format);
void add_text(value_unicode_string const& str, evaluated_format_properties_ptr const& format);
std::list<text_item> const& itemize(unsigned start=0, unsigned end=0);
void clear();
value_unicode_string const& text() const { return text_; }
@ -79,15 +84,15 @@ public:
std::pair<unsigned, unsigned> line(unsigned i) const;
unsigned num_lines() const;
private:
template<typename T> struct run
template<typename T> struct run : noncopyable
{
run(T const& data, unsigned start, unsigned end)
: start(start), end(end), data(data){}
: start(start), end(end), data(data) {}
unsigned start;
unsigned end;
T data;
};
using format_run_t = run<evaluated_format_properties_ptr>;
using format_run_t = run<evaluated_format_properties_ptr const&>;
using direction_run_t = run<UBiDiDirection>;
using script_run_t = run<UScriptCode>;
using format_run_list = std::list<format_run_t>;

15
include/mapnik/text/text_layout.hpp
View file

@ -37,6 +37,7 @@
//stl
#include <vector>
#include <deque>
#include <memory>
#include <map>
#include <utility>
@ -49,6 +50,12 @@ class text_layout;
using text_layout_ptr = std::shared_ptr<text_layout>;
using text_layout_vector = std::vector<text_layout_ptr>;
// this is a std::deque to ensure pointers stay valid as a deque
// "never invalidates pointers or references to the rest of the elements"
// http://en.cppreference.com/w/cpp/container/deque
// If this were a vector this test would crash:
// python tests/visual_tests/test.py text-expressionformat-color
using child_format_ptrs = std::deque<evaluated_format_properties_ptr>;
class text_layout
{
@ -66,7 +73,7 @@ public:
formatting::node_ptr tree);
// Adds a new text part. Call this function repeatedly to build the complete text.
void add_text(mapnik::value_unicode_string const& str, evaluated_format_properties_ptr format);
void add_text(mapnik::value_unicode_string const& str, evaluated_format_properties_ptr const& format);
// Returns the complete text stored in this layout.
mapnik::value_unicode_string const& text() const;
@ -114,6 +121,7 @@ public:
inline horizontal_alignment_e horizontal_alignment() const { return halign_; }
pixel_position alignment_offset() const;
double jalign_offset(double line_width) const;
evaluated_format_properties_ptr & new_child_format_ptr(evaluated_format_properties_ptr const& p);
private:
void break_line(std::pair<unsigned, unsigned> && line_limits);
@ -167,6 +175,11 @@ private:
// children
text_layout_vector child_layout_list_;
// take ownership of evaluated_format_properties_ptr of any format children
// in order to keep them in scope
// NOTE: this must not be a std::vector - see note above about child_format_ptrs
child_format_ptrs format_ptrs_;
};
class layout_container

2
src/group/group_symbolizer_helper.cpp
View file

@ -64,7 +64,7 @@ pixel_position_list const& group_symbolizer_helper::get()
for (auto const& geom : geometries_to_process_)
{
// TODO to support clipped geometries this needs to use
// vertext_converters
// vertex_converters
using path_type = transform_path_adapter<view_transform,geometry_type>;
path_type path(t_, *geom, prj_trans_);
find_line_placements(path);

60
src/renderer_common/process_group_symbolizer.cpp
View file

@ -24,6 +24,7 @@
#include <mapnik/renderer_common/process_group_symbolizer.hpp>
#include <mapnik/renderer_common/process_point_symbolizer.hpp>
#include <mapnik/text/glyph_info.hpp>
#include <mapnik/make_unique.hpp>
namespace mapnik {
@ -35,44 +36,14 @@ point_render_thunk::point_render_thunk(pixel_position const& pos, marker const&
{}
text_render_thunk::text_render_thunk(placements_list const& placements,
text_render_thunk::text_render_thunk(helper_ptr && helper,
double opacity, composite_mode_e comp_op,
halo_rasterizer_enum halo_rasterizer)
: placements_(), glyphs_(std::make_shared<std::vector<glyph_info> >()),
opacity_(opacity), comp_op_(comp_op), halo_rasterizer_(halo_rasterizer)
{
std::vector<glyph_info> & glyph_vec = *glyphs_;
size_t glyph_count = 0;
for (glyph_positions_ptr positions : placements)
{
glyph_count += std::distance(positions->begin(), positions->end());
}
glyph_vec.reserve(glyph_count);
for (glyph_positions_ptr positions : placements)
{
glyph_positions_ptr new_positions = std::make_shared<glyph_positions>();
new_positions->reserve(std::distance(positions->begin(), positions->end()));
glyph_positions & new_pos = *new_positions;
new_pos.set_base_point(positions->get_base_point());
if (positions->marker())
{
new_pos.set_marker(positions->marker(), positions->marker_pos());
}
for (glyph_position const& pos : *positions)
{
// 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);
}
placements_.push_back(new_positions);
}
}
: helper_(std::move(helper)),
placements_(helper_->get()),
opacity_(opacity),
comp_op_(comp_op),
halo_rasterizer_(halo_rasterizer) {}
render_thunk_extractor::render_thunk_extractor(box2d<double> & box,
render_thunk_list & thunks,
@ -94,7 +65,7 @@ void render_thunk_extractor::operator()(point_symbolizer const& sym) const
[&](pixel_position const& pos, marker const& marker,
agg::trans_affine const& tr, double opacity) {
point_render_thunk thunk(pos, marker, tr, opacity, comp_op);
thunks_.push_back(std::make_shared<render_thunk>(std::move(thunk)));
thunks_.push_back(std::make_unique<render_thunk>(std::move(thunk)));
});
update_box();
@ -103,38 +74,37 @@ void render_thunk_extractor::operator()(point_symbolizer const& sym) const
void render_thunk_extractor::operator()(text_symbolizer const& sym) const
{
box2d<double> clip_box = clipping_extent_;
text_symbolizer_helper helper(
helper_ptr helper = std::make_unique<text_symbolizer_helper>(
sym, feature_, vars_, prj_trans_,
common_.width_, common_.height_,
common_.scale_factor_,
common_.t_, common_.font_manager_, *common_.detector_,
clip_box, agg::trans_affine());
extract_text_thunk(helper, sym);
extract_text_thunk(std::move(helper), sym);
}
void render_thunk_extractor::operator()(shield_symbolizer const& sym) const
{
box2d<double> clip_box = clipping_extent_;
text_symbolizer_helper helper(
helper_ptr helper = std::make_unique<text_symbolizer_helper>(
sym, feature_, vars_, prj_trans_,
common_.width_, common_.height_,
common_.scale_factor_,
common_.t_, common_.font_manager_, *common_.detector_,
clip_box, agg::trans_affine());
extract_text_thunk(helper, sym);
extract_text_thunk(std::move(helper), sym);
}
void render_thunk_extractor::extract_text_thunk(text_symbolizer_helper & helper, text_symbolizer const& sym) const
void render_thunk_extractor::extract_text_thunk(helper_ptr && helper, text_symbolizer const& sym) const
{
double opacity = get<double>(sym, keys::opacity, feature_, common_.vars_, 1.0);
composite_mode_e comp_op = get<composite_mode_e>(sym, keys::comp_op, feature_, common_.vars_, src_over);
halo_rasterizer_enum halo_rasterizer = get<halo_rasterizer_enum>(sym, keys::halo_rasterizer, feature_, common_.vars_, HALO_RASTERIZER_FULL);
placements_list const& placements = helper.get();
text_render_thunk thunk(placements, opacity, comp_op, halo_rasterizer);
thunks_.push_back(std::make_shared<render_thunk>(thunk));
text_render_thunk thunk(std::move(helper), opacity, comp_op, halo_rasterizer);
thunks_.push_back(std::make_unique<render_thunk>(std::move(thunk)));
update_box();
}

5
src/text/formatting/format.cpp
View file

@ -24,6 +24,7 @@
#include <mapnik/debug.hpp>
#include <mapnik/feature.hpp>
#include <mapnik/symbolizer.hpp>
#include <mapnik/text/text_layout.hpp>
#include <mapnik/text/formatting/format.hpp>
#include <mapnik/text/properties_util.hpp>
#include <mapnik/ptree_helpers.hpp>
@ -103,10 +104,10 @@ node_ptr format_node::from_xml(xml_node const& xml, fontset_map const& fontsets)
}
void format_node::apply(evaluated_format_properties_ptr p, feature_impl const& feature, attributes const& attrs, text_layout &output) const
void format_node::apply(evaluated_format_properties_ptr const& p, feature_impl const& feature, attributes const& attrs, text_layout &output) const
{
evaluated_format_properties_ptr new_properties = std::make_shared<detail::evaluated_format_properties>(*p);
evaluated_format_properties_ptr & new_properties = output.new_child_format_ptr(p);
if (text_size) new_properties->text_size = util::apply_visitor(extract_value<value_double>(feature,attrs), *text_size);
if (character_spacing) new_properties->character_spacing = util::apply_visitor(extract_value<value_double>(feature,attrs), *character_spacing);
if (line_spacing) new_properties->line_spacing = util::apply_visitor(extract_value<value_double>(feature,attrs), *line_spacing);

2
src/text/formatting/layout.cpp
View file

@ -83,7 +83,7 @@ node_ptr layout_node::from_xml(xml_node const& xml, fontset_map const& fontsets)
return n;
}
void layout_node::apply(evaluated_format_properties_ptr p, feature_impl const& feature, attributes const& vars, text_layout & parent) const
void layout_node::apply(evaluated_format_properties_ptr const& p, feature_impl const& feature, attributes const& vars, text_layout & parent) const
{
text_layout_properties new_properties(parent.get_layout_properties());
if (dx) new_properties.dx = *dx;

2
src/text/formatting/list.cpp
View file

@ -42,7 +42,7 @@ void list_node::to_xml(boost::property_tree::ptree & xml) const
}
void list_node::apply(evaluated_format_properties_ptr p, feature_impl const& feature, attributes const& vars, text_layout & output) const
void list_node::apply(evaluated_format_properties_ptr const& p, feature_impl const& feature, attributes const& vars, text_layout & output) const
{
for (node_ptr const& node : children_)
{

2
src/text/formatting/text.cpp
View file

@ -50,7 +50,7 @@ node_ptr text_node::from_xml(xml_node const& xml, fontset_map const& fontsets)
return std::make_shared<text_node>(xml.get_value<expression_ptr>());
}
void text_node::apply(evaluated_format_properties_ptr p, feature_impl const& feature, attributes const& vars, text_layout &output) const
void text_node::apply(evaluated_format_properties_ptr const& p, feature_impl const& feature, attributes const& vars, text_layout &output) const
{
mapnik::value_unicode_string text_str = util::apply_visitor(evaluate<feature_impl,value_type,attributes>(feature,vars), *text_).to_unicode();
if (p->text_transform == UPPERCASE)

2
src/text/itemizer.cpp
View file

@ -37,7 +37,7 @@ text_itemizer::text_itemizer()
forced_line_breaks_.push_back(0);
}
void text_itemizer::add_text(mapnik::value_unicode_string const& str, evaluated_format_properties_ptr format)
void text_itemizer::add_text(mapnik::value_unicode_string const& str, evaluated_format_properties_ptr const& format)
{
unsigned start = text_.length();
text_ += str;

12
src/text/text_layout.cpp
View file

@ -27,6 +27,7 @@
#include <mapnik/feature.hpp>
#include <mapnik/symbolizer.hpp>
#include <mapnik/text/harfbuzz_shaper.hpp>
#include <mapnik/make_unique.hpp>
// ICU
#include <unicode/brkiter.h>
@ -107,7 +108,7 @@ text_layout::text_layout(face_manager_freetype & font_manager,
lines_(),
layout_properties_(layout_defaults),
properties_(properties),
format_(std::make_shared<detail::evaluated_format_properties>())
format_(std::make_unique<detail::evaluated_format_properties>())
{
double dx = util::apply_visitor(extract_value<value_double>(feature,attrs), layout_properties_.dx);
double dy = util::apply_visitor(extract_value<value_double>(feature,attrs), layout_properties_.dy);
@ -153,7 +154,7 @@ text_layout::text_layout(face_manager_freetype & font_manager,
}
}
void text_layout::add_text(mapnik::value_unicode_string const& str, evaluated_format_properties_ptr format)
void text_layout::add_text(mapnik::value_unicode_string const& str, evaluated_format_properties_ptr const& format)
{
itemizer_.add_text(str, format);
}
@ -163,6 +164,13 @@ void text_layout::add_child(text_layout_ptr const& child_layout)
child_layout_list_.push_back(child_layout);
}
evaluated_format_properties_ptr & text_layout::new_child_format_ptr(evaluated_format_properties_ptr const& p)
{
format_ptrs_.emplace_back(std::make_unique<detail::evaluated_format_properties>(*p));
return format_ptrs_.back();
}
mapnik::value_unicode_string const& text_layout::text() const
{
return itemizer_.text();