Support any projection #29

Nakaner · 2018-01-10T16:17:29Z

This is work in progress.

This pull request aims to make it possible to render maps in any projection Proj4 supports.

This is work in progress.

Zverik · 2018-01-10T16:51:41Z

Thanks for working on this!
Please try not to complicate the source code too much :)

Nakaner · 2018-01-10T17:23:53Z

@Zverik wrote:

Please try not to complicate the source code too much :)

Test cases are welcome. My personal use of Nik4 is limited to --bbox, --ppi, --zoom and --factor. I have not used everything around paper size, fitting to layers and routes, paddings and margins.

Projection does only project geographical coordinates but ProjTransform takes also care of datum shifts

Nakaner · 2018-01-15T12:38:23Z

--center and --fit-layers works now (including a bug fix)

Nakaner · 2018-01-15T15:30:23Z

@Zverik Is it ok to disable the generation of OZI explorer files if the new option --projection <value> is used? If this option is missing, the default (EPSG:3857) is assumed.

Zverik · 2018-01-15T16:27:41Z

Yes, I guess so. OziExplorer supports many projections, but converting their names would be non-trivial.

Nakaner · 2018-02-12T10:10:06Z

@Zverik I think that this pull request could be merged. Or are there any issues I should fix?

Zverik · 2018-02-12T10:22:12Z

nik4.py

-		lbbox = layer.envelope().inverse(p).forward(p3857)
+		layer_proj = mapnik.Projection(layer.srs)
+		box_trans = mapnik.ProjTransform(layer_proj, proj_target)
+		lbbox = box_trans.forward(mapnik.Box2d.from_string('995585 6291591 998380 6293955'))


What do these numbers mean? You are not using layer.envelope() now, as far as I see.

Oh yeah, my stupidity. I fixed it.

Zverik · 2018-02-12T10:23:01Z

nik4.py

@@ -199,6 +209,8 @@ def add_fonts(path):
 	parser.add_argument('--add-layers', help='Map layers to include, comma-separated')
 	parser.add_argument('--hide-layers', help='Map layers to hide, comma-separated')

+	parser.add_argument('-P', '--projection', help='EPSG code as EPSG:1234 or Proj4 string', default='+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0 +k=1.0 +units=m +nadgrids=@null +no_defs +over')


This string is used three times in the code — could you make it a constant please?

Zverik · 2018-02-12T10:24:04Z

nik4.py

@@ -219,6 +231,9 @@ def add_fonts(path):
 	bbox = None
 	rotate = not options.norotate

+        if options.ozi and options.projection:
+                raise Exception('ozi map file output is only supported for Web Mercator (EPSG:3857). Please remove --projection.')


Since options.projection has a default value, it is always set. Maybe do not set default and check for None here?

Zverik · 2018-02-12T10:56:21Z

nik4.py

+
+	# output projection
+	if options.projection.lower().startswith('epsg:'):
+		proj_target = mapnik.Projection('+init={}'.format(options.projection.lower()))


If the code understands epsg numbers, why not initialize projections above with epsg:3857 and epsg:4326?

I don't understand what you mean. If --projection epsg:1234 was used, I initialise mapnik.Projection with +init=epsg:1234. But if a Proj4 string was specified, I have to initialise mapnik.Projection with the string. It is not possible to just use mapnik.Projection("epsg:4326"). You have to prefix it with +init= but I decided not to force users to always type +init= if they want to use --projection.

No, I mean first lines in the file, EPSG_4326 and such. Why expand these projections when you can just do +init=epsg:4326?

If I remember correctly, I had an issue with Mapnik some time ago. When I only specified +init=epsg:1234 it failed. But it worked when I used the full Proj4 string.

Okay, let's leave that as is.

Zverik · 2018-02-12T11:02:42Z

@Nakaner sorry I didn't review it earlier. There are some issues, I think.

Zverik · 2018-02-14T13:46:38Z

nik4.py

@@ -219,6 +233,9 @@ def add_fonts(path):
 	bbox = None
 	rotate = not options.norotate

+        if options.ozi and (options.projection.lower() != 'epsg:3857' or options.projection != EPSG_3857):


I think it should be ... and ... and ...

Zverik · 2018-02-14T13:47:33Z

nik4.py

@@ -199,6 +211,8 @@ def add_fonts(path):
 	parser.add_argument('--add-layers', help='Map layers to include, comma-separated')
 	parser.add_argument('--hide-layers', help='Map layers to hide, comma-separated')

+	parser.add_argument('-P', '--projection', help='EPSG code as EPSG:1234 or Proj4 string', default=EPSG_3857)


What do you think of omitting EPSG: for numeric EPSG projections? E.g. using -P 1234 in this case?

I don't have a strong opinion on that. I used "EPSG:1234" because ogr2ogr uses it. But I am aware that the command line user interface of ogr2ogr is – well – strange. Having "EPSG:" at the beginning makes it easy to decide whether I have to prefix it with +init= before I hand it over to Mapnik, or not.

To me, it would just look better. Users won't type espg by mistake, like I always do, and you would need to do only .isdigit() instead of .lower().startswith().

Zverik · 2018-02-14T13:48:56Z

nik4.py

-p3857 = mapnik.Projection('+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0 +k=1.0 +units=m +nadgrids=@null +no_defs +over')
-p4326 = mapnik.Projection('+proj=longlat +ellps=WGS84 +datum=WGS84 +no_defs')
-transform = mapnik.ProjTransform(p4326, p3857)
+proj_target = mapnik.Projection(EPSG_3857)


proj_target and transform are always set below, so maybe they are not needed here now?

You are right. I fixed it.

support any projection

4e097e1

This is work in progress.

Nakaner added 2 commits January 11, 2018 12:15

use correct methods for transformation

54937d4

use ProjTransform, not Projection for transformations

bb793cd

Projection does only project geographical coordinates but ProjTransform takes also care of datum shifts

Nakaner changed the title ~~[WIP] Support any projection~~ Support any projection Jan 15, 2018

disable ozi map output if --projection is used

ebff0c4

Zverik requested changes Feb 12, 2018

View reviewed changes

Nakaner added 2 commits February 14, 2018 10:38

remove forgotten debugging stuff

29f7e64

remove repeated Proj4 strings by variables

00f4b0b

Zverik reviewed Feb 14, 2018

View reviewed changes

--projection has a default value and will therefore not be None

baad3f7

Nakaner force-pushed the any-projection branch from dda4dcf to baad3f7 Compare February 14, 2018 14:05

Nakaner added 2 commits February 14, 2018 15:20

remove unnecessary early definition of variables

30e50f8

don't expect a prefix EPSG: before a EPSG code

da68a76

Zverik approved these changes Apr 14, 2018

View reviewed changes

Zverik merged commit 3980c3c into Zverik:master Apr 14, 2018

Zverik mentioned this pull request Apr 17, 2018

Support alternative coordinate reference systems in .wld files #26

Closed

Nakaner deleted the any-projection branch June 25, 2018 15:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support any projection #29

Support any projection #29

Nakaner commented Jan 10, 2018

Zverik commented Jan 10, 2018

Nakaner commented Jan 10, 2018 •

edited

Loading

Nakaner commented Jan 15, 2018

Nakaner commented Jan 15, 2018

Zverik commented Jan 15, 2018

Nakaner commented Feb 12, 2018

Zverik Feb 12, 2018

Nakaner Feb 14, 2018

Zverik Feb 12, 2018

Nakaner Feb 14, 2018

Zverik Feb 12, 2018

Nakaner Feb 14, 2018

Zverik Feb 12, 2018

Nakaner Feb 14, 2018

Zverik Feb 14, 2018

Nakaner Feb 14, 2018

Zverik Feb 14, 2018

Zverik commented Feb 12, 2018

Zverik Feb 14, 2018

Zverik Feb 14, 2018

Nakaner Feb 14, 2018

Zverik Feb 14, 2018

Nakaner Feb 14, 2018

Zverik Feb 14, 2018

Nakaner Feb 14, 2018

Support any projection #29

Support any projection #29

Conversation

Nakaner commented Jan 10, 2018

Zverik commented Jan 10, 2018

Nakaner commented Jan 10, 2018 • edited Loading

Nakaner commented Jan 15, 2018

Nakaner commented Jan 15, 2018

Zverik commented Jan 15, 2018

Nakaner commented Feb 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zverik commented Feb 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nakaner commented Jan 10, 2018 •

edited

Loading