Backward compatibility: point marker placement for lines means on the line in 2.0 and centroid in 2.1 #1604

Closed
strk opened this Issue Nov 28, 2012 · 6 comments

Projects

None yet

2 participants

@strk
Contributor
strk commented Nov 28, 2012

Sounds like a bug, if placement="point" is specified for a marker against a line, the marker gets placed on the line centroid with 2.1.x. It used to be on the line in 2.0.x.

Note that placement="interior" doesn't change the fact, if that was the intention.

Here's an XML test:

<Map>
<Style name="carto_tests" filter-mode="first" >
  <Rule>
    <LineSymbolizer stroke="#000000" />
    <MarkersSymbolizer placement="point" marker-type="ellipse" />
  </Rule>
</Style>
<Layer name="carto_tests">
    <StyleName>carto_tests</StyleName>
    <Datasource>
       <Parameter name="type"><![CDATA[postgis]]></Parameter>
       <Parameter name="dbname"><![CDATA[cartodb_dev_user_2_db]]></Parameter>
       <Parameter name="geometry_field"><![CDATA[g]]></Parameter>
       <Parameter name="table"><![CDATA[(SELECT $$LINESTRING(-10  0, 0 20, 10  0)$$::geometry as g) as f]]></Parameter>
    </Datasource>
  </Layer>
</Map>
@strk
Contributor
strk commented Nov 28, 2012

Culprit here seems to be the implementation of interior_position in geom_util.hpp:

template <typename PathType>
bool interior_position(PathType & path, double & x, double & y)
{
    // start with the centroid
    if (!label::centroid(path, x,y))
        return false;

    // if we are not a polygon, or the default is within the polygon we are done
    if (hit_test(path,x,y,0.001))
        return true;

Why are we done if we're not a polygon ?!

@strk
Contributor
strk commented Nov 28, 2012

Is there a unit test to add for testing interior_position ?

@strk
Contributor
strk commented Nov 28, 2012

I think 2.0.x used middle_point for lines, can we tell if a geometry is a polygon or a line now ? how ?

@springmeyer
Member

See also #1350 (which handled this same problem/regression with shields).

Yes, 2.0.x used middle_point.

No, interior is not intended to force a middle point. I guess it could be made to however, but I've never considered that.

Right, we can no longer "know" geometry type once working with arbitrary paths. This is the case with markers because of the code used to place them markers_rasterizer_dispatch in marker_helpers.hpp. So, the hack used to fix #1350 does not work (and should not work long term for shields, once they too start supporting more transformations). So, my best sense of a solution is at #1607.

As far as unit tests, because this code is not exposed in python, we need to write them in C++. I've started a place to do that at https://github.com/mapnik/mapnik/blob/master/tests/cpp_tests/label_algo_test.cpp

@springmeyer springmeyer pushed a commit that closed this issue Nov 29, 2012
Dane Springmeyer use middle_point placement algorithm for marker POINT placement on li…
…nes - closes #1604 - refs #1350 and refs #1607
629d768
@strk
Contributor
strk commented Nov 29, 2012

Can this be backported to 2.1.x ? It's a backward compatibility issue

@springmeyer
Member

Yes. //cc @artemp to make sure he saw this approach.

On Nov 29, 2012, at 1:34 AM, strk notifications@github.com wrote:

Can this be backported to 2.1.x ? It's a backward compatibility issue


Reply to this email directly or view it on GitHub.

@springmeyer springmeyer pushed a commit that referenced this issue Dec 3, 2012
Dane Springmeyer close #1548 with explict handling of point geometries and amend 629d768
… for cairo/grid renderers to ensure proper placement on lines (refs #1604,#1350,#1607)
7d408d5
@strk strk pushed a commit to strk/mapnik that referenced this issue Dec 10, 2012
Dane Springmeyer + Sandro Santilli Use middle_point placement for marker POINT placement on lines
REFS #1604, #1350, #1607 and #1625
3e938cb
@strk strk pushed a commit to strk/mapnik that referenced this issue Dec 10, 2012
Sandro Santilli Add visual test for point placement marker on lines (#1604) 33dcd5a
@strk strk pushed a commit to strk/mapnik that referenced this issue Dec 10, 2012
Dane Springmeyer + Sandro Santilli Fix line placement of markers on points (#1548) 664e05b
@PetrDlouhy PetrDlouhy added a commit to PetrDlouhy/mapnik that referenced this issue Aug 22, 2013
@PetrDlouhy Dane Springmeyer + PetrDlouhy modify agg conv classes to support type() member - closes #1607 refs #… 539e133
@PetrDlouhy PetrDlouhy added a commit to PetrDlouhy/mapnik that referenced this issue Aug 22, 2013
@PetrDlouhy Dane Springmeyer + PetrDlouhy use middle_point placement algorithm for marker POINT placement on li…
…nes - closes #1604 - refs #1350 and refs #1607
e1fdced
@PetrDlouhy PetrDlouhy added a commit to PetrDlouhy/mapnik that referenced this issue Aug 22, 2013
@PetrDlouhy Dane Springmeyer + PetrDlouhy close #1548 with explict handling of point geometries and amend 629d768
… for cairo/grid renderers to ensure proper placement on lines (refs #1604,#1350,#1607)
62818d8
@PetrDlouhy PetrDlouhy added a commit to PetrDlouhy/mapnik that referenced this issue Aug 22, 2013
@PetrDlouhy Dane Springmeyer + PetrDlouhy add a visual test for marker middle point placement on lines - refs #… 37bcd8f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment