mapnik / mapnik

Mapnik is an open source toolkit for developing mapping applications
http://mapnik.org
GNU Lesser General Public License v2.1
3.68k stars 826 forks source link

Predictable offsetting on polygons #2851

Closed springmeyer closed 9 years ago

springmeyer commented 9 years ago

2834 introduces a problematic back compatibility problem for cartographers using line-offset on polygons because it will potentially flip the side of the polygon that the offset occurs on.

This may be a serious blocker for people upgrading to Mapnik 3.0.0.

Let's brainstorm solutions...

refs https://github.com/gravitystorm/openstreetmap-carto/issues/481 (a bug report for #1382 but also proof of people using offsetting on polygons)

pnorman commented 9 years ago

Just for a graphical reference, 2.2.0 image 3.0.0-rc2 image

Both OpenStreetMap Carto and osm.fr make heavy use of this kind of style

springmeyer commented 9 years ago

@pnorman - thanks, and can you explain how you think it is working currently? Basically v2.2.0 does not change the winding/orientation of the polygons, so it works.... but do you find that the orientation of polygons is consistent coming out of PostGIS? If so, do you know how they are consistent? Is ST_ForceRHR being called on the postgis layers for example?

pnorman commented 9 years ago

@pnorman - thanks, and can you explain how you think it is working currently? Basically v2.2.0 does not change the winding/orientation of the polygons, so it works.... but do you find that the orientation of polygons is consistent coming out of PostGIS? If so, do you know how they are consistent? Is ST_ForceRHR being called on the postgis layers for example?

There is no geometry manipulation going on in PostGIS, everything is directly as it comes out of the DB. I've never worried about winding order when developing stylesheets.

I can see if I can produce an example which has a reasonable number of points, not the 100+ that most nature reserves have.

pnorman commented 9 years ago

I imported a DB I could freely mess around with and turned all buildings into nature reserves with UPDATE planet_osm_polygon SET building = NULL, leisure = 'nature_reserve' WHERE building IS NOT NULL;

Every building rendered as an inside-out reserve.

Dumping out from psql shows that one I looked at is in the RHR

SELECT 
    ST_AsText(way),
    ST_AsText(ST_ForceRHR(way)),
    ST_AsText(ST_Reverse(ST_ForceRHR(way)))
  FROM planet_osm_polygon WHERE osm_id = 107269688;
-[ RECORD 1 ]---------------------------------------------------------------------------------------------------------------------------------
st_astext | POLYGON((-13682920.16 6311493.11,-13682904.41 6311505.97,-13682889.44 6311487.65,-13682905.18 6311474.79,-13682920.16 6311493.11))
st_astext | POLYGON((-13682920.16 6311493.11,-13682904.41 6311505.97,-13682889.44 6311487.65,-13682905.18 6311474.79,-13682920.16 6311493.11))
st_astext | POLYGON((-13682920.16 6311493.11,-13682905.18 6311474.79,-13682889.44 6311487.65,-13682904.41 6311505.97,-13682920.16 6311493.11))

Doing a check with

SELECT 
    DISTINCT ST_AsText(way) = ST_AsText(ST_ForceRHR(way)) AS IsRHR
  FROM planet_osm_polygon;

finds all polygons in the polygon table are RHR.

If I flip a building with UPDATE planet_osm_polygon SET way=ST_Reverse(way) WHERE osm_id = 107269688; then it, to my considerable surprise, still renders inside out.

flippmoke commented 9 years ago

boost::geometry::correct will always enforce ccw on outer ring if the y axis is upward (which it will be for lat lon coordinates). I think honestly it might be for the best if we just have this as something known for people to fix. I would assume that it would make all offsets consistent for every type of data? It would make it easier going forward to provide instructions on how offset works?

pnorman commented 9 years ago

2.2.0 is consistent and 3.0.0-rc2 is consistent (on a given coordinate system). They just don't agree with each other. If I flip which side it tries to draw on, then it will break the style on 2.2.0.

flippmoke commented 9 years ago

@pnorman I thought that in 2.2.0, they were not consistent, and depended on the winding order of the source data.

pnorman commented 9 years ago

@pnorman I thought that in 2.2.0, they were not consistent, and depended on the winding order of the source data.

After testing, yes, this is correct. But the standard osm2pgsql toolchain has a consistent winding order, and with that winding order, it ends up reversed in 3.0

One workaround would be to reverse the side in the style and change the SQL to

(SELECT
    ST_Reverse(way) AS way,
    name,
    tourism
  FROM planet_osm_polygon
  WHERE (tourism = 'theme_park'
    OR tourism = 'zoo')
    AND way && !bbox!
) AS tourism_boundary

This is a bit of an ugly hack.

flippmoke commented 9 years ago

@pnorman Which way is the line-offset "negative" and "positive". "negative" = inward? "positive" = outward?

pnorman commented 9 years ago

@pnorman Which way is the line-offset "negative" and "positive". "negative" = inward? "positive" = outward?

https://github.com/gravitystorm/openstreetmap-carto/blob/e557cb91a7bbe15c935cbd9ef6601c74330fb402/landcover.mss#L626-L648

They have a negative line-offset to get a line farther inside than normal. This is consistent with ST_Buffer, where negative shrinks. I'd just flip 3.0.0 behavior. It will get it consistent with ST_Buffer (positive being bigger) and get it consistent with osm2pgsql + Mapnik 2.2.

It does mean that anyone using a different data loading toolchain which loads in polygons with a left-hand rule will have to flip their offset and use ST_ForceRHR, but I believe that is fewer than who are using osm2pgsql and offsets.