Inconsistent collision management between Agg rendering and Cairo

[dmarteau]

We have noticed a big difference how text conflicts are handled in AGG compared to Cairo, but it appears that in some cases the AGG renderer doesn't not care about buffer values and thus lead to problems when rendering tiles (there could be not text at all along the edge of the tiles).

you can see the difference between the two rendering of the same template here (Mapnik 2.0.x):
AGG: http://www.tiikoni.com/tis/view/?id=f38780d
CAIRO: http://www.tiikoni.com/tis/view/?id=b21cf43

We use a buffer of 384, in the case of AGG, changing the size of the buffer has no effects.

For mapnik 2.0.x, the following patch fix the problem (the patch has been tested):

--- a/src/agg/process_text_symbolizer.cpp
+++ b/src/agg/process_text_symbolizer.cpp
@@ -109,8 +109,9 @@ void agg_renderer<T>::process(text_symbolizer const& 
sym,
          ren.set_halo_radius(sym.get_halo_radius() * scale_factor_);
          ren.set_opacity(sym.get_text_opacity());

-        box2d<double> dims(0,0,width_,height_);
-        placement_finder<label_collision_detector4> finder(detector_,dims);
+        //XXX be consistent with cairo rendering
+        //box2d<double> dims(0,0,width_,height_);
+        placement_finder<label_collision_detector4> finder(detector_);

          string_info info(text);

[springmeyer]

nice catch. I'll need to create a testcase that replicates this before considering backporting. I'm headed out offline for a week, so I am unlikely to get to this today. But, thanks for reporting and posting a fix!

[springmeyer]

planning on getting to this in the coming week.

[artemp]

closing as text_symbolizer processing has beed re-implemented and I see identical text positioning with recent master

[springmeyer]

re-opening, as this is just relevant to the 2.0.2 release (not master). taking off the mustfix label so we don't confuse with the master push to 2.1.x.

[springmeyer]

Okay, finally looking at this. I'm not able to create a testcast against 2.0.2 that can actually demonstrate the issue. So, this is not likely in 2.0.2. In fact it is looking like the changes made to master in #1283 will be reverted because I don't think this is a bug.

I started digging through history and found: 6713df5, which points to the reasoning behind sending this separate dims - basically that we need to check against the unbuffered map extent for avoid-edges to work correctly.

So, @dmarteau - do you have avoid-edges set in your XML? You should not likely have it set. Also, is it possible for you to create a reduced testcase?

[dmarteau]

In our test case, we use exactly the same xml. The only thing that was changed was in one case we use agg and in the other case we use cairo. The above patch has fixed the huge difference we had between cairo rendering and agg rendering.
I will try to find a reduced test case (but as usual time is missing...)

regards

[dmarteau]

Good catch, avoid-edge was set in our xml, we have removed it and the adquation between the agg and cairo is must better, but theres is still some discrepencies that I can't explain for now.
But, IMHO, I think that there still a problem froml the fact that starting with the same xml (with avoid-edges) we have a so big differences between agg and cairo and I would suggest that the problem does not come from the agg renderer as I thought in the beginning, but lies in the cairo text renderer which does not support 'avoid-edges'.

regards,

[springmeyer]

yes, the problem does appear to be in the cairo renderer - in the sense that it does not keep info on the unbuffered map width/height and does not properly use this for the avoid-edges check.

[springmeyer]

Done in 6a6a48b, problem, to recap, was that avoid-edges and minimum-padding serve to avoid labels or shields ever being placed near the map edges, so we need to do the intersection checks using the unbuffered map extent. This was not happening consistenly, but does now in the 2.0.x branch, which will become the 2.0.2 release. closing.

For similar consistency fixes in master (aka. 2.1.0-pre) see #1283