mapnik / mapnik

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

adaptive line smoothing #2715

Open zdila opened 9 years ago

zdila commented 9 years ago

Currently line smoothing doesn't take into account the angle of line break and it smoothes lines with any angle. This is not good for smoothing eg. roads, because road in L shape should not be smoothed at the break.

Old osmarender was using script "lines2curves.pl" to smooth lines and it performed well with this and other usecases. It did not smooth eg. orthogonally broken line or if angle was more than 90. Greater the angle was, the "smoothing strength" was higher. Even there was no smooth factor configurable and lines could be smooth or not smooth, the results were very good.

Please add simmilar feature to mapnik, it will make smoothing useable on various ways. It can be a new parameter to keeb backward compatibility.

You can check http://www.freemap.sk/ which still uses osmarender-like rendering and has this neat feature.

springmeyer commented 9 years ago

Okay, thanks will keep in mind.

der-stefan commented 9 years ago

A similar behavior can be see with smoothed contour lines: http://opentopomap.org/#marker=16/48.70559/10.75969

Contour lines should - of course - never intersect. These artefacts in the shown map are artefacts due to this 400m plain around there. But as smoothing only accepts one fixed value, it is a compromise for smoothing the 90m SRTM contour lines.

zdila commented 5 years ago

Is agg_vcgen_smooth_poly1.cpp a correct file to implement it? I could try to do it even I am no C++ developer.

I think this feature is very important for beautiful maps.

talaj commented 5 years ago

Yes, that's the file. It's a vertex converter which is chained with other converters in vertex_converters.

I've updated the documentation concerning development of Mapnik a bit, it may come in handy: docs/contributing.md

mboeringa commented 5 years ago

@zdila Maybe my remarks here https://github.com/gravitystorm/openstreetmap-carto/issues/3551#issuecomment-452436841 and here https://github.com/gravitystorm/openstreetmap-carto/issues/3551#issuecomment-452453207 are relevant too.

zdila commented 5 years ago

@mboeringa thanks for info, but I would use smoothing on non-simplified ways

mboeringa commented 5 years ago

@mboeringa thanks for info, but I would use smoothing on non-simplified ways

@zdila , the break-up of road network connectivity is, if I am right, not related to simplification, but merely the consequence of the "corner cutting" properties of the smoothing algorithm. This means you will be confronted with the same issues as I illustrated in the first link I posted.

zdila commented 5 years ago

In that link he mentions thant he uses simplification and smoothing at SQL level. I don't plan to use it that way. And I expect smoothed lines in mapnik to go via nodes which define that line.

mboeringa commented 5 years ago

That person is me.

Yes, I used this at the SQL level, but I see no inherent reason why Mapnik doesn't suffer from the same issue, as the problem is that line elements in OpenStreetMap that do not end at junctions, but go across it, get "corner cut" by a smoothing algorithm, meaning the line is likely no longer connected to the junction. This is a generic issue with smoothing road networks that do not have strict junction / edge topology.

But maybe I haven't explained the issue clearly enough, and this image might help:

afbeelding

zdila commented 5 years ago

the problem is that line elements in OpenStreetMap that do not end at junctions, but go across it, get "corner cut" by a smoothing algorithm

Such smoothing alghorithm is wrong :-) I'll check it if mapnik's smoothing does this cross-cutting and if yes then it should be imho fixed.

As I mentioned in issue description, for default map at www.freemap.sk which is based on SVG we use lines2curves.pl postprocessing script (from tiles@home project) which smoothes some line categories without this problem and makes the map look much nicer.

mboeringa commented 5 years ago

Such smoothing alghorithm is wrong :-) I'll check it if mapnik's smoothing does this cross-cutting and if yes then it should be imho fixed.

This is not necessarily wrong, it is the consequence of database records essentially being "independent". Unless you define a PostGIS Topology or some kind of "network dataset" in a GIS, any record is on its own. There is no inherent knowledge of connecting line elements stored in the database.

Of course, you could do a rather costly "intersection" type selection in PostGIS, enhanced of course by the the presence of a spatial index, and then determine which lines connect, but this wouldn't be generic, as it would still require knowledge about what connecting lines actually define the network by certain attributes (e.g. "highways").

You wouldn't want to connect a railway / waterway with your road network just because they share a node (which is perfectly possible in OSM).

I therefore think this issue is not easily solved (except PostGIS Topology) in a "generic" way that would be suitable for a PostGIS function fix.

As I suggested in the other thread, possibly the better solution would be to cut the red line in two at the junction node, so as to create two separate records. But this should preferably be done at the import stage, e.g. in osm2pgsql, as that is when full membership of nodes-in-ways or nodes-in-relations is being processed.

This is not yet possible in osm2pgsql though.

zdila commented 5 years ago

I expect one OSM way with 10 nodes to be represented as one linestring in PostGIS defined by 10 points. Such line would be fetched by mapnik and smoothed the way that resulting line would still go via all 10 points.

mboeringa commented 5 years ago

I expect one OSM way with 10 nodes to be represented as one linestring in PostGIS defined by 10 points. Such line would be fetched by mapnik and smoothed the way that resulting line would still go via all 10 points.

I can't speak for Mapnik's smoothing algorithm, but this is certainly not the case for PostGIS's "ST_ChaikinSmoothing" SQL function that I used.

This is a fast and simple algorithm, but explicitly drops existing intermediate nodes, and replaces them with new nodes. See here: https://postgis.net/docs/ST_ChaikinSmoothing.html and especially also here: http://www.idav.ucdavis.edu/education/CAGDNotes/Chaikins-Algorithm/Chaikins-Algorithm.html

Only the end nodes are retained.

zdila commented 5 years ago

As I mentioned in https://github.com/mapnik/mapnik/issues/2715#issuecomment-452644946 I don't plan to do simplification or smoothing at PostGIS level.

mboeringa commented 5 years ago

Only the end nodes are retained.

I need to amend this: you also need to explicitly specify - as I did in my testing - the preserveEndPoints input parameter to the ST_ChaikinSmoothing PostGIS SQL function if you want to retain end points, because the algorithm itself does not maintain them

As I mentioned in #2715 (comment) I don't plan to do simplification or smoothing at PostGIS level.

Yes, but do post whether you see Mapnik suffering from the same issue. I think it likely will, but you can test it.

zdila commented 5 years ago

I've managed to implement it. So far this is the diff:

diff --git a/deps/agg/src/agg_vcgen_smooth_poly1.cpp b/deps/agg/src/agg_vcgen_smooth_poly1.cpp
index eec9ca576..a5287cc59 100644
--- a/deps/agg/src/agg_vcgen_smooth_poly1.cpp
+++ b/deps/agg/src/agg_vcgen_smooth_poly1.cpp
@@ -91,10 +91,19 @@ void vcgen_smooth_poly1::calculate(const vertex_dist& v0,
     double xm2 = v1.x + (v3.x - v1.x) * k2;
     double ym2 = v1.y + (v3.y - v1.y) * k2;

-    m_ctrl1_x = v1.x + m_smooth_value * (v2.x - xm1);
-    m_ctrl1_y = v1.y + m_smooth_value * (v2.y - ym1);
-    m_ctrl2_x = v2.x + m_smooth_value * (v1.x - xm2);
-    m_ctrl2_y = v2.y + m_smooth_value * (v1.y - ym2);
+    double dot1 = (v0.x - v1.x) * (v2.x - v1.x) + (v0.y - v1.y) * (v2.y - v1.y);
+    double dot2 = (v1.x - v2.x) * (v3.x - v2.x) + (v1.y - v2.y) * (v3.y - v2.y);
+
+    double a1 = std::acos(dot1 / (v0.dist * v1.dist));
+    double a2 = std::acos(dot2 / (v1.dist * v2.dist));
+
+    double s1 = a1 < pi / 2 ? 0 : ((a1 - pi / 2) / (pi / 2));
+    double s2 = a2 < pi / 2 ? 0 : ((a2 - pi / 2) / (pi / 2));
+
+    m_ctrl1_x = v1.x + s1 * m_smooth_value * (v2.x - xm1);
+    m_ctrl1_y = v1.y + s1 * m_smooth_value * (v2.y - ym1);
+    m_ctrl2_x = v2.x + s2 * m_smooth_value * (v1.x - xm2);
+    m_ctrl2_y = v2.y + s2 * m_smooth_value * (v1.y - ym2);
 }

I think we should have both behaviours of line smoothing. For this we could introduce new attribute like smooth-adaptive or "hack" existing one by eg. providing negative value for "adaptive" smoothing. What do you think?

Also the result is still behind osmarender's lines2curves.pl as that perl script prevents smoothing nodes which are shared by more than a single way (or used in the same way more than once). Do you think this feature is implementable for mapnik? Maybe other way to do it could be to write PostGIS query that would split linestrings at such points.

Some samples:

Without smoothing: image

With default smoothing: image

With "adaptive" smoothing: image

zdila commented 5 years ago

Any opinions about this feature from core developers?

talaj commented 5 years ago

@zdila I like both the idea and the implementation. Great work!

I think it should be fine to introduce it under smooth-adaptive option. If you want I can do the integration work.

zdila commented 5 years ago

If you want I can do the integration work.

@talaj thanks! that would be great.

zdila commented 5 years ago

BTW smooth-adaptive could be boolean value to prevent something like <LineSymbolizer smooth="0.5" smooth-adaptive="0.8" .... So it would be <LineSymbolizer smooth="0.5" smooth-adaptive="true" ....

Also I am not 100% sure about the terminology. That "adaptive" word was my naming of this feature and maybe there exists some official naming for this.

talaj commented 5 years ago

https://github.com/mapnik/mapnik/pull/4031 was merged, smooth-algorithm="adaptive" works on master now.

zdila commented 5 years ago

Cool, thanks. I am looging forward to seeing it in npm Mapnik package :).

Next step could be to prevent smoothing of nodes where more than one way is attached. But this could go to different issue. What do you think?