Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Group letters and halo in a label in exported SVG #2229

Open
Zverik opened this Issue Apr 29, 2014 · 19 comments

Comments

Projects
None yet
7 participants

Zverik commented Apr 29, 2014

Ideally all symbolizers should produce not an array of objects, but a group with them. An outstanding case is TextSymbolyzer: it produces a lot of individual letters and paths for halo. I had to write a script to group those, but it would be nice if mapnik did it.

Contributor

tomhughes commented Apr 29, 2014

The problem is that mapnik currently outputs SVG via cairo, and cairo has no support for grouping so there is not much mapnik can do.

When the native SVG renderer is complete no doubt it will do what you want.

Zverik commented Apr 29, 2014

So #1917 does not actually work?

Contributor

tomhughes commented Apr 30, 2014

It probably does, yes, but that is using the native svg renderer which, as I understand it, is not complete yet and might not therefore be able to render everything that the agg and cairo renderers can.

Zverik commented Apr 30, 2014

Is it possible to include some markers in exported SVG objects that indicate symbolizers (and hopefully layers) those objects are on? E.g. symbolizer="t1231", or as id? This would allow for easier postprocessing, maybe with grouping.

Owner

springmeyer commented May 7, 2014

Yes, <g id="layer name"> is done in the native svg_renderer if I recall correctly. Its also easy to extend. I recommend you try it out. The limitations are that it does not yet support text and is not bound from python (so you have to currently write a bit of c++ to access it). But otherwise works well and has promise.

Owner

springmeyer commented Jul 29, 2014

patch to support text at https://gist.github.com/springmeyer/2778d7c64b301c9e2aa5. Just needs updated to latest master.

hannesj commented Jul 1, 2015

Hi @springmeyer,

Sorry to bulge in on an old issue, but I was just wondering if you have any plans to returning to that patch? Would be really nice to get text rendered on the SVG output and the patch doesn't look that overwhelming.

Owner

springmeyer commented Feb 1, 2016

Noting that @gnzzz worked on this and his work is the most developed at this point: gnzzz/mapnik@e5d03c6.

@hannesj I've not had time to integrate either of these patches yet and don't see myself having time in the future. But willing to help someone else get this done.

gnzzz commented Feb 1, 2016

Yes, that was working at the time but I put it aside while the changes to data structure was being done, preparing for version 3 I think. Since I put it aside then I fell off with it but I've been meaning to make time to push through those changes at least.

Owner

springmeyer commented Feb 1, 2016

@gnzzz great, let me know if you need a hand with anything.

jc101 commented Feb 3, 2016

I was going to have a look at this as I'm trying to create some minimal sized svg tiles and the Cairo renderer has very inefficient properties that I can't postprocess out. In fact, I was attempting to merge in the changes from the 2014 patch when I noticed the recent updated posts about the @gnzzz work!

I'm guessing that as the latter work is more developed it would make more sense to start from this point rather than continuing with the patch file? I'm noticing that there seems to be quite a few changes to the code since the 2014 patch was first created.

Owner

springmeyer commented Feb 3, 2016

I'm noticing that there seems to be quite a few changes to the code since the 2014 patch was first created.

Yes, significant. Changes for the better, but the patch would need to be manually adapted.

jc101 commented Feb 15, 2016

I think I've adapted the @gnzzz work (which is very similar to the @springmeyer patch) and managed to get the code to compile with the (almost) latest codebase. I'm getting a surprise introduced error not obviously related to the text rendering though which has stumped me. I was wondering if others had seen something similar that might point me to a place to investigate.

Basically, without the text processing incorporated this code at the start of svg renderer works:

boost::optional<color> const& bgcolor = map.background();
 if(bgcolor)
{
        // generate background color as a rectangle that spans the whole image.
        svg::rect_output_attributes bg_attributes(0, 0, common_.width_, common_.height_, *bgcolor);
        generator_.generate_rect(bg_attributes);
}

When I incorporate the text symbolizer additions, the definition of bg_attributes in the code above now fails with an access violation writing error in mapnik::color:to_hex_string(). The call stack is shown below. What I don't understand is why this happens since it's not obvious to me how the addition of the text output attributes can affect the rect output attributes or the boost karma stuff.

mapnik.dll!boost::spirit::karma::detail::buffer_sink::~buffer_sink() Line 206
mapnik.dll!boost::spirit::karma::detail::right_align_generate<boost::spirit::karma::detail::output_iterator<std::back_insert_iterator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > >,boost::mpl::int_<3>,boost::spirit::unused_type>,boost::spirit::unused_type const ,boost::spirit::unused_type,boost::spirit::unused_type,boost::spirit::karma::action<boost::spirit::karma::any_uint_generator<unsigned int,boost::spirit::unused_type,boost::spirit::unused_type,16>,boost::phoenix::actor<boost::proto::exprns_::basic_expr<boost::proto::tagns_::tag::assign,boost::proto::argsns_::list2<boost::proto::exprns_::expr<boost::proto::tagns_::tag::terminal,boost::proto::argsns_::term<boost::spirit::argument<0> >,0>,boost::phoenix::actor<boost::proto::exprns_::basic_expr<boost::proto::tagns_::tag::terminal,boost::proto::argsns_::term<unsigned char>,0> > >,2> > >,boost::spirit::karma::literal_char<boost::spirit::char_encoding::standard,boost::spirit::unused_type,1> >(boost::spirit::karma::detail::output_iterator<std::back_insert_iterator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > >,boost::mpl::int_<3>,boost::spirit::unused_type> & sink, const boost::spirit::unused_type & ctx, const boost::spirit::unused_type & d, const boost::spirit::unused_type & attr, const boost::spirit::karma::action<boost::spirit::karma::any_uint_generator<unsigned int,boost::spirit::unused_type,boost::spirit::unused_type,16>,boost::phoenix::actor<boost::proto::exprns_::basic_expr<boost::proto::tagns_::tag::assign,boost::proto::argsns_::list2<boost::proto::exprns_::expr<boost::proto::tagns_::tag::terminal,boost::proto::argsns_::term<boost::spirit::argument<0> >,0>,boost::phoenix::actor<boost::proto::exprns_::basic_expr<boost::proto::tagns_::tag::terminal,boost::proto::argsns_::term<unsigned char>,0> > >,2> > > & e, const unsigned int width, const boost::spirit::karma::literal_char<boost::spirit::char_encoding::standard,boost::spirit::unused_type,1> & p) Line 118
mapnik.dll!??@97ee3de34c8c790c4ab25829b3df5827@(const boost::fusion::cons<boost::spirit::karma::literal_char<boost::spirit::char_encoding::standard,boost::spirit::unused_type,1>,boost::fusion::cons<boost::spirit::karma::padding_right_alignment<boost::spirit::karma::action<boost::spirit::karma::any_uint_generator<unsigned int,boost::spirit::unused_type,boost::spirit::unused_type,16>,boost::phoenix::actor<boost::proto::exprns_::basic_expr<boost::proto::tagns_::tag::assign,boost::proto::argsns_::list2<boost::proto::exprns_::expr<boost::proto::tagns_::tag::terminal,boost::proto::argsns_::term<boost::spirit::argument<0> >,0>,boost::phoenix::actor<boost::proto::exprns_::basic_expr<boost::proto::tagns_::tag::terminal,boost::proto::argsns_::term<unsigned char>,0> > >,2> > >,boost::spirit::karma::literal_char<boost::spirit::char_encoding::standard,boost::spirit::unused_type,1>,int>,boost::fusion::cons<boost::spirit::karma::padding_right_alignment<boost::spirit::karma::action<boost::spirit::karma::any_uint_generator<unsigned int,boost::spirit::unused_type,boost::spirit::unused_type,16>,boost::phoenix::actor<boost::proto::exprns_::basic_expr<boost::proto::tagns_::tag::assign,boost::proto::argsns_::list2<boost::proto::exprns_::expr<boost::proto::tagns_::tag::terminal,boost::proto::argsns_::term<boost::spirit::argument<0> >,0>,boost::phoenix::actor<boost::proto::exprns_::basic_expr<boost::proto::tagns_::tag::terminal,boost::proto::argsns_::term<unsigned char>,0> > >,2> > >,boost::spirit::karma::literal_char<boost::spirit::char_encoding::standard,boost::spirit::unused_type,1>,int>,boost::fusion::cons<boost::spirit::karma::padding_right_alignment<boost::spirit::karma::action<boost::spirit::karma::any_uint_generator<unsigned int,boost::spirit::unused_type,boost::spirit::unused_type,16>,boost::phoenix::actor<boost::proto::exprns_::basic_expr<boost::proto::tagns_::tag::assign,boost::proto::argsns_::list2<boost::proto::exprns_::expr<boost::proto::tagns_::tag::terminal,boost::proto::argsns_::term<boost::spirit::argument<0> >,0>,boost::phoenix::actor<boost::proto::exprns_::basic_expr<boost::proto::tagns_::tag::terminal,boost::proto::argsns_::term<unsigned char>,0> > >,2> > >,boost::spirit::karma::literal_char<boost::spirit::char_encoding::standard,boost::spirit::unused_type,1>,int>,boost::fusion::cons<boost::spirit::karma::semantic_predicate,boost::fusion::cons<boost::spirit::karma::padding_right_alignment<boost::spirit::karma::action<boost::spirit::karma::any_uint_generator<unsigned int,boost::spirit::unused_type,boost::spirit::unused_type,16>,boost::phoenix::actor<boost::proto::exprns_::basic_expr<boost::proto::tagns_::tag::assign,boost::proto::argsns_::list2<boost::proto::exprns_::expr<boost::proto::tagns_::tag::terminal,boost::proto::argsns_::term<boost::spirit::argument<0> >,0>,boost::phoenix::actor<boost::proto::exprns_::basic_expr<boost::proto::tagns_::tag::terminal,boost::proto::argsns_::term<unsigned char>,0> > >,2> > >,boost::spirit::karma::literal_char<boost::spirit::char_encoding::standard,boost::spirit::unused_type,1>,int>,boost::fusion::nil_> > > > > > & seq, boost::spirit::karma::detail::fail_function<boost::spirit::karma::detail::output_iterator<std::back_insert_iterator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > >,boost::mpl::int_<3>,boost::spirit::unused_type>,boost::spirit::unused_type const ,boost::spirit::unused_type> f, boost::fusion::forward_traversal_tag __formal) Line 53 
mapnik.dll!mapnik::color::to_hex_string() Line 94

Any ideas on where to start debugging would be very helpful.

Contributor

lightmare commented Feb 15, 2016

That looks like a destructor called on a nonexistent object, which is pretty weird for function-local stuff. Does it occur if you disable inlining functions? I'm missing something in the call stack, looking into spirit 2.5.3 I'd expect right_align_generate -> ~enable_buffering -> ~buffer_sink -> tidy

Oh, and what's the extra line you have in color.cpp? The error should be pointing at line 93.

jc101 commented Feb 16, 2016

Thanks for the suggestions.

I tried disabling inline functions by recompiling the mapnik solution with an /Ob0 compile flag (I was forced to disable optimizations too because of this). This seemed to do the trick as the error I was seeing went away.

On color.cpp, I don't have additional lines, the debugger (in Visual Studio - I'm on Windows) stopped at line 94 (return str;). I also checked the call stack again before recompiling and I think I noted it down correctly.

I don't know if this is the underlying cause of the problem but between the two compilations of Mapnik there are a large number of returning address of local variable or temporary warnings about some of the boost header files, including in the phoenix parts, when inline functions were enabled. I hadn't considered it a problem before as my original compiled Mapnik worked fine until now. This is an example:

.....\mapnik-gyp\mapnik-sdk\include\boost\phoenix\core\meta_grammar.hpp(74): warning C4172: returning address of local variable or temporary
Contributor

lightmare commented Feb 16, 2016

On color.cpp, I don't have additional lines, the debugger (in Visual Studio - I'm on Windows) stopped at line 94 (return str;). I also checked the call stack again before recompiling and I think I noted it down correctly.

I know you did, I meant that some functions were missing due to inlining. The fact that it pointed at return str but trace contained karma::detail::right_align_generate shows you can't trust line numbers or stack trace when functions are inlined :) It's difficult to judge whether the compiler got some return-value-optimization wrong, or there's another problem with to_hex_string which only manifests itself when inlining/optimizations are enabled.

However the warning

returning address of local variable or temporary

... should be taken seriously. It may not be spirit/phoenix` fault but rather how it's used -- I guess that's preceded by a wall of "in file included from ... while instantiating ..." -- you could cut the grammar to make error messages shorter:

    karma::generate(sink,
                    // begin grammar
                    '#'
                    << right_align(2,'0')[hex[_1 = red()]]
                    //<< right_align(2,'0')[hex[_1 = green()]]
                    //<< right_align(2,'0')[hex[_1 = blue()]]
                    //<< eps(alpha() < 255) <<  right_align(2,'0')[hex [_1 = alpha()]]
                    // end grammar

... and then go backwards from the warning to see what triggers it.

jc101 commented Feb 19, 2016

After a lot of checking I think I've discovered that I directly caused the error in a pretty stupid way: when I was modifying the patch, I had left in a BOOST_SPIRIT_UNICODE pragma in value.hpp which, when removed, stopped the access violation when inlining was renabled.

I'm still seeing the compiler warning errors mind which I guess is not good. Anyway, the svg generation seems to be working correctly now so sorry for wasting your time.

hannesj commented Feb 25, 2016

@jc101 Do you have the code rebased on the recent master available somewhere?

jc101 commented Feb 27, 2016

I have a patch here based on a recent version of the codebase. It updates the work carried out by @artemp and @gnzzz to include text symbolizer output. I've also added basic point symbolizer support (which requires further work as I've recently spotted a problem). Please note that I don't code in cpp so forgive any code irregularities!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment