mapnik/mapnik — pull request #1243 by lightmare (2012-06-06 20:23:56+00:00)
Title: data-driven transform expressions, implements #664

I'm not sure whether cherry-picking relevant commits from our master branch onto a fresh mapnik/master branch was the right way to do this - if there's a better way I can scratch this request and make another.

There are a lot of changes in this patch, some of which are not necessary to deliver the functionality. I will describe those first, and try to explain why I included them.

* `agg::trans_affine::identity` - not necessary. Without it, `converter_traits::conv_type` can't get rid of `trans_` member, unless there is some other global trans_affine instance it can pass to conv_transform constructor.

* `namespace value_adl_barrier` - not necessary. Avoids expressions like `a / b` where neither `a` nor `b` are of type `mapnik::value` (and don't have a matching `operator/` defined) from being compiled as `value(a) / value(b)`. Described in more detail in #1228.

* `agg_renderer::debug_draw_box()` - not necessary at all, I left it there because it helped me a lot when debugging collision detection, and I believe it can be helpful to others as well.

* `agg_renderer::render_marker()` - this method was called from several places like this `render_marker(pixel_position(x-0.5*w, y-0.5*h), ...)`. Inside the function, the half-size offset was eventually reverted by `mtx.translate(pos.x+0.5*w, pos.y+0.5*h)` - of course it worked fine, but in my opinion didn't make much sense, because `(x-0.5*w, y-0.5*h)` are the coordinates of the top left corner only if the marker is not transformed, otherwise it's adding pears to apples. So I changed the calls to `render_marker(pixel_position(x, y), ...)`, but didn't go any further - I considered changing `shield_symbolizer_helper::get_marker_position` to return center coordinates, but then I'd have to change other renderers accordingly, which would be a bit too many opportunities for new bugs. Plus I wasn't sure whether you'd agree that using center coordinates would be clearer.

* `attribute_collector` - changed into a template taking container type as a parameter (similar to `path_processor::collect_

`) - not very useful, but at the time it allowed me to have an instance in `transform_processor::attribute_collector

` with only a forward declaration.

* `vertex_converter` - changed the order of template parameters `T` and `P` to match the order in the constructor and in `args_type`, and added `A` for `trans_affine`.

* `process_markers_symbolizer.cpp` - some time ago we sort of fixed #1119, that is included. It's not ideal, because in `process()` we compute the extent of the transformed marker, and `markers_placement` then slides this box along the path. To make it completely correct (if I'm not mistaken), `markers_placement` would have to take the original marker bounding box and transformation, transform and remember all four corners (not their bounding box) and for each placement along the path apply local transformation (position and angle on the path) upon those four pre-transformed corners and use the bounding box of that for collision detection.

Now to the real thing. I took the svg transform grammar, replaced every occurence of double_ with expr (taken from within expression_grammar), wrote a set of transform node types, an attribute_collector and an evaluator that applies the stored transform list. Simple ;-)

Some transform expressions have optional parameters, for example `rotate(angle [cx cy])`. I store each parsed transform expression in a distinct node type for later re-creating the expression string, with each parameter in a member of type expr_node (optionals set to value_null). I could've stored each optional parameter in a `boost::optional` and in fact rewrote it that way - but then I realized that the former (with value_null meaning "not set") allows you to write `transform="rotate([angle] [cx] [cy])"` in your style, and it will simply work if you have nulls in cx, cy.

Python binding set_svg_transform still uses plain mapnik::svg::parse_transform. It should be easy to make it use the new parser, I just didn't have it constantly in mind.