Marker rendering broken with 'placement:point' on line geometries #1425

Closed
springmeyer opened this Issue Aug 21, 2012 · 5 comments

Projects

None yet

3 participants

@springmeyer
Member

This blows apart because our label_position::centroid algorithm does not work for lines. This is the same regression as reported in #1350 for shields (see also #740). Except it is more problematic: the vertex_converters template usage prevents the ability to know the type of the geometry at https://github.com/mapnik/mapnik/blob/master/include/mapnik/marker_helpers.hpp#L84-99, which makes implementing the same fix from #1350 impossible.

Obviously the geometry type could change after processing by the converters, so trying to pass through the original geometry type is not a perfect solution.

@lightmare
Collaborator

Assuming 2 points, the while loop in centroid() runs exactly once, with:

(x0,y0)==(start_x,start_y)
=> dx0 == dy0 == 0
=> ai == 0
=> atmp == xtmp == ytmp == 0
which in the end returns the second point

How about this?

if (count <= 2) { // will also do the same as before for (count == 1)
    x = (start_x + x0) * 0.5;
    y = (start_y + y0) * 0.5;
    return true;
}
@springmeyer
Member

@lightmare looks good to me, but I'm noticing it breaks a dozen or so of the visual tests. /cc @herm

@springmeyer
Member

ah, sorry, no just 4 tests:

Visual text rendering summary: Failed: 52 different pixels:
        /tmp/mapnik-visual-images/lines-shield-800-agg.png (actual)
        tests/visual_tests/images/lines-shield-800-reference.png (expected)
Failed: 52 different pixels:
        /tmp/mapnik-visual-images/lines-shield-600-agg.png (actual)
        tests/visual_tests/images/lines-shield-600-reference.png (expected)
Failed: 52 different pixels:
        /tmp/mapnik-visual-images/lines-shield-400-agg.png (actual)
        tests/visual_tests/images/lines-shield-400-reference.png (expected)
Failed: 79 different pixels:
        /tmp/mapnik-visual-images/lines-shield-200-agg.png (actual)
        tests/visual_tests/images/lines-shield-200-reference.png (expected)
@springmeyer springmeyer pushed a commit that referenced this issue Aug 22, 2012
Dane Springmeyer start a label algorithm c++ test - refs #1425 da1f126
@strk
strk commented Dec 21, 2012

I think this was fixed by 3e938cb right ?

@springmeyer
Member

yes, exactly, thanks @strk - closing - refs #1604, #1350, #1607 and #1625

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