Permalink
Browse files

+ avoid copying

  • Loading branch information...
1 parent 6e23b64 commit de71db7d388cd3af7f99b18a984871cf7ab754f8 @artemp artemp committed May 31, 2012
Showing with 8 additions and 8 deletions.
  1. +8 −8 include/mapnik/vertex_converters.hpp
@@ -105,7 +105,7 @@ struct converter_traits<T, mapnik::clip_line_tag>
template <typename Args>
static void setup(geometry_type & geom, Args & args)
{
- typename boost::mpl::at<Args,boost::mpl::int_<0> >::type box = boost::fusion::at_c<0>(args);
+ typename boost::mpl::at<Args,boost::mpl::int_<0> >::type const& box = boost::fusion::at_c<0>(args);
geom.clip_box(box.minx(),box.miny(),box.maxx(),box.maxy());
}
};
@@ -120,7 +120,7 @@ struct converter_traits<T, mapnik::dash_tag>
template <typename Args>
static void setup(geometry_type & geom, Args & args)
{
- typename boost::mpl::at<Args,boost::mpl::int_<2> >::type sym = boost::fusion::at_c<2>(args);
+ typename boost::mpl::at<Args,boost::mpl::int_<2> >::type const& sym = boost::fusion::at_c<2>(args);
double scale_factor = boost::fusion::at_c<5>(args);
stroke const& stroke_ = sym.get_stroke();
dash_array const& d = stroke_.get_dash_array();
@@ -144,7 +144,7 @@ struct converter_traits<T, mapnik::stroke_tag>
template <typename Args>
static void setup(geometry_type & geom, Args & args)
{
- typename boost::mpl::at<Args,boost::mpl::int_<2> >::type sym = boost::fusion::at_c<2>(args);
+ typename boost::mpl::at<Args,boost::mpl::int_<2> >::type const& sym = boost::fusion::at_c<2>(args);
stroke const& stroke_ = sym.get_stroke();
set_join_caps(stroke_,geom);
geom.generator().miter_limit(stroke_.get_miterlimit());
@@ -163,7 +163,7 @@ struct converter_traits<T,mapnik::clip_poly_tag>
template <typename Args>
static void setup(geometry_type & geom, Args & args)
{
- typename boost::mpl::at<Args,boost::mpl::int_<0> >::type box = boost::fusion::at_c<0>(args);
+ typename boost::mpl::at<Args,boost::mpl::int_<0> >::type const& box = boost::fusion::at_c<0>(args);
geom.clip_box(box.minx(),box.miny(),box.maxx(),box.maxy());
}
};
@@ -178,8 +178,8 @@ struct converter_traits<T,mapnik::transform_tag>
template <typename Args>
static void setup(geometry_type & geom, Args & args)
{
- typename boost::mpl::at<Args,boost::mpl::int_<3> >::type tr = boost::fusion::at_c<3>(args);
- typename boost::mpl::at<Args,boost::mpl::int_<4> >::type prj_trans = boost::fusion::at_c<4>(args);
+ typename boost::mpl::at<Args,boost::mpl::int_<3> >::type const& tr = boost::fusion::at_c<3>(args);
+ typename boost::mpl::at<Args,boost::mpl::int_<4> >::type const& prj_trans = boost::fusion::at_c<4>(args);
geom.set_proj_trans(prj_trans);
geom.set_trans(tr);
}
@@ -202,7 +202,7 @@ struct converter_traits<T,mapnik::affine_transform_tag>
template <typename Args>
static void setup(geometry_type & geom, Args & args)
{
- typename boost::mpl::at<Args,boost::mpl::int_<2> >::type sym = boost::fusion::at_c<2>(args);
+ typename boost::mpl::at<Args,boost::mpl::int_<2> >::type const& sym = boost::fusion::at_c<2>(args);
boost::array<double,6> const& m = sym.get_transform();
geom.trans_.load_from(&m[0]);
}
@@ -217,7 +217,7 @@ struct converter_traits<T,mapnik::offset_transform_tag>
template <typename Args>
static void setup(geometry_type & geom, Args & args)
{
- typename boost::mpl::at<Args,boost::mpl::int_<2> >::type sym = boost::fusion::at_c<2>(args);
+ typename boost::mpl::at<Args,boost::mpl::int_<2> >::type const& sym = boost::fusion::at_c<2>(args);
geom.set_offset(sym.offset());
}
};

6 comments on commit de71db7

Contributor

lightmare replied May 31, 2012

generates this error with gcc:

include/mapnik/vertex_converters.hpp: In instantiation of ‘static void mapnik::detail::converter_traits<T, mapnik::clip_poly_tag>::setup(mapnik::detail::converter_traits<T, mapnik::clip_poly_tag>::geometry_type&, const Args&) [with Args = boost::fusion::vector<const mapnik::box2d<double>&, mapnik::cairo_context&, const mapnik::polygon_symbolizer&, const mapnik::CoordTransform&, const mapnik::proj_transform&, double, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_>; T = agg::conv_clip_polygon<mapnik::geometry<double, mapnik::vertex_vector> >; mapnik::detail::converter_traits<T, mapnik::clip_poly_tag>::geometry_type = agg::conv_clip_polygon<mapnik::geometry<double, mapnik::vertex_vector> >]’:
...
include/mapnik/vertex_converters.hpp +166 :73: error: forming reference to reference type ‘boost::fusion::extension::value_at_impl<boost::fusion::vector_tag>::apply<boost::fusion::vector<const mapnik::box2d<double>&, mapnik::cairo_context&, const mapnik::polygon_symbolizer&, const mapnik::CoordTransform&, const mapnik::proj_transform&, double, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_>, mpl_::int_<0> >::type {aka const mapnik::box2d<double>&}’

There should be no copying, since args_type already stores references:

    typedef typename boost::fusion::vector
    <
    bbox_type const&,
    rasterizer_type&,
    symbolizer_type const&,
    trans_type const&,
    proj_trans_type const&,
    double //scale-factor
    > args_type;

I think adding a const& here is potentially dangerous, you're making a reference to something which might be passed by value (depends entirely on Args)

template <int N, typename Sequence>
typename result_of::at_c<Sequence, N>::type
at_c(Sequence& seq);

Proper syntax is therefore:

typename boost::fusion::result_of::at_c<Args,0>::type box = boost::fusion::at_c<0>(args);
// or alternatively
typename boost::mpl::at_c<Args,0>::type box = boost::fusion::at_c<0>(args);
Owner

artemp replied May 31, 2012

@lightmare - oops, boost::mpl::at_c<Args,0>::type is already reference. good catch, thanks!

Owner

artemp replied May 31, 2012

@lightmare

"... I think adding a const& here is potentially dangerous, you're making a reference to something which might be passed by value (depends entirely on Args).." - do you mean passing Args by const& ?

Contributor

lightmare replied May 31, 2012

no, I was referring to this patch alone, I meant "whether boost::fusion::at_c<0>(args) returns a value or a reference depends entirely on the type Args, thus there should be nothing but foo::type on the left side" ;)

passing Args by const& ref in the other patch is imo ok

Owner

artemp replied May 31, 2012

cool, reverted in 404ddf3

Owner

springmeyer replied Jun 4, 2012

hey @lightmare - did you see #1237 - would be interested in to know what time zone you are on and if you could join the call?

Please sign in to comment.