From a9f58c70b46794a3ce39801518216da83911893d Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Sun, 12 Oct 2014 16:07:26 -0700 Subject: [PATCH] shuffle complexity into layout constructor - should reduce mistakes in refactoring - gives top level ownership of evaluated_format_properties_ptr to a layout node (#2516) - not sure ^^ if this actually keeps it in scope enough for rendering? - moves data transformation functions off of text_symbolizer_properties --- .../mapnik/group/group_symbolizer_helper.hpp | 2 +- include/mapnik/text/symbolizer_helpers.hpp | 2 +- include/mapnik/text/text_layout.hpp | 33 ++-- include/mapnik/text/text_properties.hpp | 5 +- src/text/formatting/layout.cpp | 17 +- src/text/placement_finder.cpp | 17 +- src/text/symbolizer_helpers.cpp | 8 +- src/text/text_layout.cpp | 147 +++++++++++------- src/text/text_properties.cpp | 37 +---- 9 files changed, 146 insertions(+), 122 deletions(-) diff --git a/include/mapnik/group/group_symbolizer_helper.hpp b/include/mapnik/group/group_symbolizer_helper.hpp index f40291214..20b97c778 100644 --- a/include/mapnik/group/group_symbolizer_helper.hpp +++ b/include/mapnik/group/group_symbolizer_helper.hpp @@ -75,7 +75,7 @@ public: inline text_symbolizer_properties const& get_properties() const { - return placement_->properties; + return info_ptr_->properties; } pixel_position_list const& get(); diff --git a/include/mapnik/text/symbolizer_helpers.hpp b/include/mapnik/text/symbolizer_helpers.hpp index 3344183e2..8e13e7793 100644 --- a/include/mapnik/text/symbolizer_helpers.hpp +++ b/include/mapnik/text/symbolizer_helpers.hpp @@ -96,7 +96,7 @@ protected: mutable std::list::iterator point_itr_; // Use point placement. Otherwise line placement is used. mutable bool point_placement_; - text_placement_info_ptr placement_; + text_placement_info_ptr info_ptr_; evaluated_text_properties_ptr text_props_; }; diff --git a/include/mapnik/text/text_layout.hpp b/include/mapnik/text/text_layout.hpp index 3cafc51cd..44aac1cf0 100644 --- a/include/mapnik/text/text_layout.hpp +++ b/include/mapnik/text/text_layout.hpp @@ -57,7 +57,13 @@ public: using const_iterator = line_vector::const_iterator; using child_iterator = text_layout_vector::const_iterator; - text_layout(face_manager_freetype & font_manager, double scale_factor, text_layout_properties const& properties); + text_layout(face_manager_freetype & font_manager, + feature_impl const& feature, + attributes const& attrs, + double scale_factor, + text_symbolizer_properties const& properties, + text_layout_properties const& layout_defaults, + 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); @@ -99,7 +105,8 @@ public: inline text_layout_vector const& get_child_layouts() const { return child_layout_list_; } inline face_manager_freetype & get_font_manager() const { return font_manager_; } inline double get_scale_factor() const { return scale_factor_; } - inline text_layout_properties const& get_layout_properties() const { return properties_; } + inline text_symbolizer_properties const& get_default_text_properties() const { return properties_; } + inline text_layout_properties const& get_layout_properties() const { return layout_properties_; } inline rotation const& orientation() const { return orientation_; } inline pixel_position const& displacement() const { return displacement_; } @@ -108,8 +115,6 @@ public: pixel_position alignment_offset() const; double jalign_offset(double line_width) const; - void evaluate_properties(feature_impl const& feature, attributes const& attr); - private: void break_line(std::pair && line_limits); void break_line_icu(std::pair && line_limits); @@ -118,11 +123,11 @@ private: void clear_cluster_widths(unsigned first, unsigned last); void init_auto_alignment(); - //input + // input face_manager_freetype & font_manager_; double scale_factor_; - //processing + // processing text_itemizer itemizer_; // Maps char index (UTF-16) to width. If multiple glyphs map to the same char the sum of all widths is used // note: this probably isn't the best solution. it would be better to have an object for each cluster, but @@ -132,13 +137,19 @@ private: double height_; unsigned glyphs_count_; - //output + // output line_vector lines_; - //text layout properties - text_layout_properties properties_; + // layout properties (owned by text_layout) + text_layout_properties layout_properties_; - //alignments + // text symbolizer properties (owned by placement_finder's 'text_placement_info' (info_) which is owned by symbolizer_helper + text_symbolizer_properties const& properties_; + + // format properties (owned by text_layout) + evaluated_format_properties_ptr format_; + + // alignments vertical_alignment_e valign_; horizontal_alignment_e halign_; justify_alignment_e jalign_; @@ -154,7 +165,7 @@ private: pixel_position displacement_ = {0,0}; box2d bounds_; - //children + // children text_layout_vector child_layout_list_; }; diff --git a/include/mapnik/text/text_properties.hpp b/include/mapnik/text/text_properties.hpp index 8b1f3f6f0..a22f0486f 100644 --- a/include/mapnik/text/text_properties.hpp +++ b/include/mapnik/text/text_properties.hpp @@ -196,11 +196,10 @@ struct MAPNIK_DECL text_symbolizer_properties // Save all values to XML ptree (but does not create a new parent node!). void to_xml(boost::property_tree::ptree & node, bool explicit_defaults, text_symbolizer_properties const& dfl) const; - // Takes a feature and produces formatted text as output. - // The output object has to be created by the caller and passed in for thread safety. - void process(text_layout & output, feature_impl const& feature, attributes const& vars) const; // Sets new format tree. void set_format_tree(formatting::node_ptr tree); + // Get format tree. + formatting::node_ptr format_tree() const; // Get a list of all expressions used in any placement. // This function is used to collect attributes. void add_expressions(expression_set & output) const; diff --git a/src/text/formatting/layout.cpp b/src/text/formatting/layout.cpp index 91c3cc1a6..62f83708b 100644 --- a/src/text/formatting/layout.cpp +++ b/src/text/formatting/layout.cpp @@ -83,9 +83,9 @@ 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 & output) const +void layout_node::apply(evaluated_format_properties_ptr p, feature_impl const& feature, attributes const& vars, text_layout & parent) const { - text_layout_properties new_properties(output.get_layout_properties()); + text_layout_properties new_properties(parent.get_layout_properties()); if (dx) new_properties.dx = *dx; if (dy) new_properties.dy = *dy; if (text_ratio) new_properties.text_ratio = *text_ratio; @@ -100,9 +100,14 @@ void layout_node::apply(evaluated_format_properties_ptr p, feature_impl const& f if (jalign) new_properties.jalign = *jalign; // starting a new offset child with the new displacement value - text_layout_ptr child_layout = std::make_shared(output.get_font_manager(), output.get_scale_factor(), new_properties); - child_layout->evaluate_properties(feature,vars); - + // we pass a null format tree since this is not the parent but a child + text_layout_ptr child_layout = std::make_shared(parent.get_font_manager(), + feature, + vars, + parent.get_scale_factor(), + parent.get_default_text_properties(), + new_properties, + formatting::node_ptr()); // process contained format tree into the child node if (child_) { @@ -112,7 +117,7 @@ void layout_node::apply(evaluated_format_properties_ptr p, feature_impl const& f { MAPNIK_LOG_WARN(format) << "Useless layout node: Contains no text"; } - output.add_child(child_layout); + parent.add_child(child_layout); } void layout_node::set_child(node_ptr child) diff --git a/src/text/placement_finder.cpp b/src/text/placement_finder.cpp index 3cb60ad7f..9c12e9658 100644 --- a/src/text/placement_finder.cpp +++ b/src/text/placement_finder.cpp @@ -64,19 +64,26 @@ bool placement_finder::next_position() { if (info_.next()) { - text_layout_ptr layout = std::make_shared(font_manager_, scale_factor_, info_.properties.layout_defaults); - layout->evaluate_properties(feature_, attr_); - move_dx_ = layout->displacement().x; - // TODO: this call is needed (text-bug1533) ?? + // parent layout, has top-level ownership of a new evaluated_format_properties_ptr (TODO is this good enough to stay in scope???) + // but does not take ownership of the text_symbolizer_properties (info_.properties) + text_layout_ptr layout = std::make_shared(font_manager_, + feature_, + attr_, + scale_factor_, + info_.properties, + info_.properties.layout_defaults, + info_.properties.format_tree()); + // TODO: why is this call needed? // https://github.com/mapnik/mapnik/issues/2525 text_props_ = evaluate_text_properties(info_.properties,feature_,attr_); - info_.properties.process(*layout, feature_, attr_); // Note: this clear call is needed when multiple placements are tried // like with placement-type="simple|list" if (!layouts_.empty()) layouts_.clear(); // Note: multiple layouts_ may result from this add() call layouts_.add(layout); layouts_.layout(); + // cache a few values for use elsewhere in placement finder + move_dx_ = layout->displacement().x; horizontal_alignment_ = layout->horizontal_alignment(); return true; } diff --git a/src/text/symbolizer_helpers.cpp b/src/text/symbolizer_helpers.cpp index e448efe42..7842c0cbf 100644 --- a/src/text/symbolizer_helpers.cpp +++ b/src/text/symbolizer_helpers.cpp @@ -58,8 +58,8 @@ base_symbolizer_helper::base_symbolizer_helper( dims_(0, 0, width, height), query_extent_(query_extent), scale_factor_(scale_factor), - placement_(get(sym_, keys::text_placements_)->get_placement_info(scale_factor)), - text_props_(evaluate_text_properties(placement_->properties,feature_,vars_)) + info_ptr_(get(sym_, keys::text_placements_)->get_placement_info(scale_factor)), + text_props_(evaluate_text_properties(info_ptr_->properties,feature_,vars_)) { initialize_geometries(); if (!geometries_to_process_.size()) return; // FIXME - bad practise @@ -183,7 +183,7 @@ text_symbolizer_helper::text_symbolizer_helper( DetectorT &detector, box2d const& query_extent, agg::trans_affine const& affine_trans) : base_symbolizer_helper(sym, feature, vars, prj_trans, width, height, scale_factor, t, query_extent), - finder_(feature, vars, detector, dims_, *placement_, font_manager, scale_factor), + finder_(feature, vars, detector, dims_, *info_ptr_, font_manager, scale_factor), adapter_(finder_,false), converter_(query_extent_, adapter_, sym_, t, prj_trans, affine_trans, feature, vars, scale_factor) { @@ -275,7 +275,7 @@ text_symbolizer_helper::text_symbolizer_helper( view_transform const& t, FaceManagerT & font_manager, DetectorT & detector, box2d const& query_extent, agg::trans_affine const& affine_trans) : base_symbolizer_helper(sym, feature, vars, prj_trans, width, height, scale_factor, t, query_extent), - finder_(feature, vars, detector, dims_, *placement_, font_manager, scale_factor), + finder_(feature, vars, detector, dims_, *info_ptr_, font_manager, scale_factor), adapter_(finder_,true), converter_(query_extent_, adapter_, sym_, t, prj_trans, affine_trans, feature, vars, scale_factor) { diff --git a/src/text/text_layout.cpp b/src/text/text_layout.cpp index a65288697..89499720b 100644 --- a/src/text/text_layout.cpp +++ b/src/text/text_layout.cpp @@ -51,12 +51,52 @@ static void rotated_box2d(box2d & box, rotation const& rot, pixel_positi box.init(center.x - half_width, center.y - half_height, center.x + half_width, center.y + half_height); } +pixel_position evaluate_displacement(double dx, double dy, directions_e dir) +{ + switch (dir) + { + case EXACT_POSITION: + return pixel_position(dx,dy); + break; + case NORTH: + return pixel_position(0,-std::abs(dy)); + break; + case EAST: + return pixel_position(std::abs(dx),0); + break; + case SOUTH: + return pixel_position(0,std::abs(dy)); + break; + case WEST: + return pixel_position(-std::abs(dx),0); + break; + case NORTHEAST: + return pixel_position(std::abs(dx),-std::abs(dy)); + break; + case SOUTHEAST: + return pixel_position(std::abs(dx),std::abs(dy)); + break; + case NORTHWEST: + return pixel_position(-std::abs(dx),-std::abs(dy)); + break; + case SOUTHWEST: + return pixel_position(-std::abs(dx),std::abs(dy)); + break; + } +} + pixel_position pixel_position::rotate(rotation const& rot) const { return pixel_position(x * rot.cos - y * rot.sin, x * rot.sin + y * rot.cos); } -text_layout::text_layout(face_manager_freetype & font_manager, double scale_factor, text_layout_properties const& properties) +text_layout::text_layout(face_manager_freetype & font_manager, + feature_impl const& feature, + attributes const& attrs, + double scale_factor, + text_symbolizer_properties const& properties, + text_layout_properties const& layout_defaults, + formatting::node_ptr tree) : font_manager_(font_manager), scale_factor_(scale_factor), itemizer_(), @@ -65,7 +105,53 @@ text_layout::text_layout(face_manager_freetype & font_manager, double scale_fact height_(0.0), glyphs_count_(0), lines_(), - properties_(properties) {} + layout_properties_(layout_defaults), + properties_(properties), + format_(std::make_shared()) + { + double dx = util::apply_visitor(extract_value(feature,attrs), layout_properties_.dx); + double dy = util::apply_visitor(extract_value(feature,attrs), layout_properties_.dy); + displacement_ = evaluate_displacement(dx,dy, layout_properties_.dir); + std::string wrap_str = util::apply_visitor(extract_value(feature,attrs), layout_properties_.wrap_char); + if (!wrap_str.empty()) wrap_char_ = wrap_str[0]; + wrap_width_ = util::apply_visitor(extract_value(feature,attrs), layout_properties_.wrap_width); + double angle = util::apply_visitor(extract_value(feature,attrs), layout_properties_.orientation); + orientation_.init(angle * M_PI/ 180.0); + wrap_before_ = util::apply_visitor(extract_value(feature,attrs), layout_properties_.wrap_before); + repeat_wrap_char_ = util::apply_visitor(extract_value(feature,attrs), layout_properties_.repeat_wrap_char); + rotate_displacement_ = util::apply_visitor(extract_value(feature,attrs), layout_properties_.rotate_displacement); + valign_ = util::apply_visitor(extract_value(feature,attrs),layout_properties_.valign); + halign_ = util::apply_visitor(extract_value(feature,attrs),layout_properties_.halign); + jalign_ = util::apply_visitor(extract_value(feature,attrs),layout_properties_.jalign); + + // Takes a feature and produces formatted text as output. + if (tree) + { + format_properties const& format_defaults = properties_.format_defaults; + format_->text_size = util::apply_visitor(extract_value(feature,attrs), format_defaults.text_size); + format_->character_spacing = util::apply_visitor(extract_value(feature,attrs), format_defaults.character_spacing); + format_->line_spacing = util::apply_visitor(extract_value(feature,attrs), format_defaults.line_spacing); + format_->text_opacity = util::apply_visitor(extract_value(feature,attrs), format_defaults.text_opacity); + format_->halo_opacity = util::apply_visitor(extract_value(feature,attrs), format_defaults.halo_opacity); + format_->halo_radius = util::apply_visitor(extract_value(feature,attrs), format_defaults.halo_radius); + format_->fill = util::apply_visitor(extract_value(feature,attrs), format_defaults.fill); + format_->halo_fill = util::apply_visitor(extract_value(feature,attrs), format_defaults.halo_fill); + format_->text_transform = util::apply_visitor(extract_value(feature,attrs), format_defaults.text_transform); + format_->face_name = format_defaults.face_name; + format_->fontset = format_defaults.fontset; + format_->ff_settings = util::apply_visitor(extract_value(feature,attrs), format_defaults.ff_settings); + // Turn off ligatures if character_spacing > 0. + if (format_->character_spacing > .0 && format_->ff_settings.count() == 0) + { + format_->ff_settings.append(font_feature_liga_off); + } + tree->apply(format_, feature, attrs, *this); + } + else + { + MAPNIK_LOG_WARN(text_properties) << "text_symbolizer_properties can't produce text: No formatting tree!"; + } + } void text_layout::add_text(mapnik::value_unicode_string const& str, evaluated_format_properties_ptr format) { @@ -341,63 +427,6 @@ void text_layout::shape_text(text_line & line) harfbuzz_shaper::shape_text(line, itemizer_, width_map_, font_manager_, scale_factor_); } -pixel_position evaluate_displacement(double dx, double dy, directions_e dir) -{ - switch (dir) - { - case EXACT_POSITION: - return pixel_position(dx,dy); - break; - case NORTH: - return pixel_position(0,-std::abs(dy)); - break; - case EAST: - return pixel_position(std::abs(dx),0); - break; - case SOUTH: - return pixel_position(0,std::abs(dy)); - break; - case WEST: - return pixel_position(-std::abs(dx),0); - break; - case NORTHEAST: - return pixel_position(std::abs(dx),-std::abs(dy)); - break; - case SOUTHEAST: - return pixel_position(std::abs(dx),std::abs(dy)); - break; - case NORTHWEST: - return pixel_position(-std::abs(dx),-std::abs(dy)); - break; - case SOUTHWEST: - return pixel_position(-std::abs(dx),std::abs(dy)); - break; - } -} - -void text_layout::evaluate_properties(feature_impl const& feature, attributes const& attrs) -{ - // dx,dy - double dx = util::apply_visitor(extract_value(feature,attrs), properties_.dx); - double dy = util::apply_visitor(extract_value(feature,attrs), properties_.dy); - displacement_ = evaluate_displacement(dx,dy, properties_.dir); - std::string wrap_str = util::apply_visitor(extract_value(feature,attrs), properties_.wrap_char); - if (!wrap_str.empty()) wrap_char_ = wrap_str[0]; - wrap_width_ = util::apply_visitor(extract_value(feature,attrs), properties_.wrap_width); - - double angle = util::apply_visitor(extract_value(feature,attrs), properties_.orientation); - orientation_.init(angle * M_PI/ 180.0); - - wrap_before_ = util::apply_visitor(extract_value(feature,attrs), properties_.wrap_before); - repeat_wrap_char_ = util::apply_visitor(extract_value(feature,attrs), properties_.repeat_wrap_char); - rotate_displacement_ = util::apply_visitor(extract_value(feature,attrs), properties_.rotate_displacement); - - valign_ = util::apply_visitor(extract_value(feature,attrs),properties_.valign); - halign_ = util::apply_visitor(extract_value(feature,attrs),properties_.halign); - jalign_ = util::apply_visitor(extract_value(feature,attrs),properties_.jalign); - -} - void text_layout::init_auto_alignment() { if (valign_ == V_AUTO) diff --git a/src/text/text_properties.cpp b/src/text/text_properties.cpp index b6b0428a0..2126b8ecd 100644 --- a/src/text/text_properties.cpp +++ b/src/text/text_properties.cpp @@ -68,43 +68,16 @@ text_symbolizer_properties::text_symbolizer_properties() format_defaults(), tree_() {} -void text_symbolizer_properties::process(text_layout & output, feature_impl const& feature, attributes const& attrs) const -{ - output.clear(); - - if (tree_) - { - //evaluate format properties - evaluated_format_properties_ptr format = std::make_shared(); - format->text_size = util::apply_visitor(extract_value(feature,attrs), format_defaults.text_size); - format->character_spacing = util::apply_visitor(extract_value(feature,attrs), format_defaults.character_spacing); - format->line_spacing = util::apply_visitor(extract_value(feature,attrs), format_defaults.line_spacing); - format->text_opacity = util::apply_visitor(extract_value(feature,attrs), format_defaults.text_opacity); - format->halo_opacity = util::apply_visitor(extract_value(feature,attrs), format_defaults.halo_opacity); - format->halo_radius = util::apply_visitor(extract_value(feature,attrs), format_defaults.halo_radius); - format->fill = util::apply_visitor(extract_value(feature,attrs), format_defaults.fill); - format->halo_fill = util::apply_visitor(extract_value(feature,attrs), format_defaults.halo_fill); - format->text_transform = util::apply_visitor(extract_value(feature,attrs), format_defaults.text_transform); - format->face_name = format_defaults.face_name; - format->fontset = format_defaults.fontset; - - format->ff_settings = util::apply_visitor(extract_value(feature,attrs), format_defaults.ff_settings); - // Turn off ligatures if character_spacing > 0. - if (format->character_spacing > .0 && format->ff_settings.count() == 0) - { - format->ff_settings.append(font_feature_liga_off); - } - - tree_->apply(format, feature, attrs, output); - } - else MAPNIK_LOG_WARN(text_properties) << "text_symbolizer_properties can't produce text: No formatting tree!"; -} - void text_symbolizer_properties::set_format_tree(formatting::node_ptr tree) { tree_ = tree; } +formatting::node_ptr text_symbolizer_properties::format_tree() const +{ + return tree_; +} + void text_symbolizer_properties::text_properties_from_xml(xml_node const& node) { // The options 'margin' and 'repeat-distance' replace 'minimum-distance'.