Modify

Opened 2 years ago

Closed 2 years ago

Last modified 23 months ago

#12186 closed enhancement (fixed)

extend projection support in josm core

Reported by: bastiK Owned by: team
Priority: normal Milestone: 16.02
Component: Core Version:
Keywords: projection Cc: wiktorn

Description

We have essentially all the ingredients for a fully fledged projection library, but only a modest number of projections in our database.

It should be possible to include a lot more without too much work.

I've tried the current epsg file of the proj.4 project (5006 entries), here are some preliminary results:

  • At the moment we should be able to support 4503 of 5006 entries. The rest doesn't work (right now), because:
    • unsupported base projection ("code"="nr. of entries"): aea=31, laea=16, cea=2, cass=20, poly=5, merc=19, nzmg=1, stere=38, krovak=4, omerc=20, eqc=5
    • relies on NAD27 datum, which requires a shift database for conversion: 204 entries
    • relies on various other shift databases (+geoidgrids=*): 5 entries
    • +proj=geocent "projection" converts to 3D (X,Y,Z) - not useful in JOSM: 138 entries
  • 4466 of 4503 potentially supported entries have been tested against proj.4 and work correctly
    • 15 of the remaining 37 entries are untested, because I couldn't extract bounds information for them (EPSG:4296, EPSG:21817, EPSG:23853, EPSG:23886, EPSG:29118, EPSG:29119, EPSG:29120, EPSG:29121, EPSG:29177, EPSG:29178, EPSG:29179, EPSG:29180, EPSG:29635, EPSG:29636, EPSG:31461)
    • for the rest (22 entries) there is a mismatch between JOSM and proj.4. (EPSG:2155, EPSG:2156, EPSG:2577, EPSG:2636, EPSG:2694, EPSG:2754, EPSG:3143, EPSG:3389, EPSG:3390, EPSG:3460, EPSG:3477, EPSG:3851, EPSG:5925, EPSG:5930, EPSG:5935, EPSG:6086, EPSG:6093, EPSG:6120, EPSG:6122, EPSG:6403, EPSG:26940, EPSG:26979)
      There are two reasons for this:
      1. Problems when the central meridian is near 180 degrees longitude.
      2. The Lambert Conformal Conic projection breaks when two of the standard parallels are the same.

Attachments (4)

TemplatedWMSTileSource.patch (5.7 KB) - added by bastiK 2 years ago.
transmerc-bounds.png (67.0 KB) - added by bastiK 2 years ago.
polar-stereographic-josm.png (310.1 KB) - added by bastiK 2 years ago.
errors.PNG (63.9 KB) - added by Don-vip 2 years ago.

Download all attachments as: .zip

Change History (122)

comment:1 Changed 2 years ago by bastiK

In 9104/josm:

add more datums and ellipsoids (see #12186)

comment:2 Changed 2 years ago by bastiK

In 9105/josm:

add support for proj.4 parameter +pm=* - prime meridian (see #12186)

comment:3 Changed 2 years ago by bastiK

In [9106]:

do not throw an error when datum is missing, return NullDatum instead of CentricDatum
(to match the proj.4 implementation)

comment:4 Changed 2 years ago by bastiK

In [9112]:
applied #12185 - support latitude of natural origin for Transverse Mercator projection
(imports pieces of code from the Geotools project)

comment:5 Changed 2 years ago by bastiK

In 9116/josm:

fix projection near 180 degree meridian (see #12186)

comment:6 Changed 2 years ago by bastiK

Update: I've excluded deprecated projections (330 entries) and fixed the 180 degrees issue. All projections that we should be able to support seem to be working now (4200 entries in total).

One remaining issue is the bounds data. For testing, I've taken it from the the EPSG database (proprietary). Either we have to live without bounds or try to calculate "mathematical bounds", i.e. find parameter limits where the formulas for the base projection still gives reasonable results.

Last edited 2 years ago by bastiK (previous) (diff)

comment:7 Changed 2 years ago by Don-vip

Milestone: 15.12

comment:8 Changed 2 years ago by bastiK

In [9118]:

more stable zoom&move for extreme scales and unusual world bounds

comment:9 Changed 2 years ago by bastiK

In 9121/josm:

typo in [9118] (see #12186)

comment:10 Changed 2 years ago by bastiK

In 9122/josm:

fix possible division by 0 (see #12186)

comment:11 Changed 2 years ago by bastiK

In 9123/josm:

make sure the scale ("circum") is never 0 (but positive) (see #12186)

comment:12 Changed 2 years ago by bastiK

In 9124/josm:

guess resonable projection bounds, when they haven't been specified (see #12186)

comment:13 Changed 2 years ago by bastiK

In 9125/josm:

see #12186 - add projection parameter vunits (ignored)

comment:14 Changed 2 years ago by bastiK

In 9126/josm:

see #12186 - rework loading of projection definitions
to make it accessible from outside

comment:15 Changed 2 years ago by bastiK

Summary: extent projection support in josm coreextend projection support in josm core

comment:16 Changed 2 years ago by bastiK

In 9127/josm:

see #12186 - simplify method return type

comment:17 Changed 2 years ago by bastiK

In 9128/josm:

see #12186 - rework projection parameters loading
so it can be accessed from outside

comment:18 Changed 2 years ago by bastiK

In 9130/josm:

see #12186 - move projection data files to separate directory

comment:19 Changed 2 years ago by bastiK

In 9131/josm:

see #12186 - move one more file

comment:20 Changed 2 years ago by bastiK

In 9133/josm:

see #12186 - add more projections
The list of projections is now generated during the build process:
scripts/BuildEpsgList.java runs and creates data/projection/custom-epsg which is generated by combining data_nodist/projection/josm-epsg (a list maintained by the JOSM team) and data_nodist/projection/epsg (an upstream list from the proj.4 project, git: 3795cdf)
The ant task epsg cares for this.

comment:21 Changed 2 years ago by bastiK

In [9133], I've added a java file that is compiled & run at build time to create a clean list from two sources.

Now that I think about it, we could simply ship both lists and select the right entries at runtime.

Advantages:

  • Simpler build process (as before)
  • Easier for Linux-distributors to remove the proj.4 epsg file from the JOSM package and simply refer to the corresponding file from the proj.4 package, e.g. /usr/share/proj/epsg.

Disadvantage:

  • We ship entries that are never used (about 1000) wasting ~15kB in size of the .jar file.

Any opinions?

comment:22 Changed 2 years ago by bastiK

Resolution: fixed
Status: newclosed

Okay, I leave it like this.

Stats:

epsg:
     [java] loaded 182 entries from data_nodist/projection/josm-epsg
     [java] loaded 5006 entries from data_nodist/projection/epsg
     [java] 
     [java] some entries from proj.4 have not been included:
     [java]  * already in the maintained JOSM list: 182 entries
     [java]  * deprecated: 330 entries
     [java]  * using +proj=geocent, which is 3D (X,Y,Z) and not useful in JOSM: 138 entries
     [java]  * unsupported base projection: 299 entries
     [java]    in particular: {aea=31, laea=16, cea=2, cass=20, poly=5, merc=19, nzmg=1, stere=38, krovak=4, omerc=20, eqc=5}
     [java]  * requires data file for datum conversion: 209 entries
     [java] 
     [java] written 182 entries from data_nodist/projection/josm-epsg
     [java] written 4020 entries from data_nodist/projection/epsg

TODO:

  • add more base projections
  • convenient way for the user to download datum shift database files when required

comment:24 Changed 2 years ago by bastiK

In 9139/josm:

see #12186 - more conservative bounds for transverse mercator

comment:25 Changed 2 years ago by bastiK

In 9140/josm:

see #12186 - actually test what is intended to be tested

comment:26 Changed 2 years ago by Don-vip

org.openstreetmap.josm.data.imagery.TemplatedWMSTileSourceTest.testEPSG3006_withoutbounds is failing

comment:27 in reply to:  26 Changed 2 years ago by bastiK

Cc: wiktorn added

Replying to Don-vip:

org.openstreetmap.josm.data.imagery.TemplatedWMSTileSourceTest.testEPSG3006_withoutbounds is failing

I think it is a problem of TemplatedWMSTileSource.initProjection: The world bounds is the area between two meridians. Taking the lat/lon world bounds max and min will give you the poles, so nothing useful. The new method Projection.getWorldBoundsBoxEastNorth can be used to get an area in east-north space that covers the valid area. But I suppose this would break the WMS-cache?

comment:28 Changed 2 years ago by wiktorn

Looking into this.

comment:29 Changed 2 years ago by wiktorn

As far as I've checked for all the tiles that are tested with our regression tests maintain the same tile numbers, so the probability of cache breakage is low.

But even after moving to Projection.getWorldBoundsBoxEastNorth I see failures in this tests...

comment:30 Changed 2 years ago by wiktorn

In 9167/josm:

  • Move TemplatedWMSTileSource to Projection.getWorldBoundsBoxEastNorth
  • fix failing tests - move to EastNorth computations instead of LatLon,

which might be non-square, which lead to trouble checking, whether point was
within the bounds or not

See #12186

Changed 2 years ago by bastiK

comment:31 in reply to:  29 ; Changed 2 years ago by bastiK

Replying to wiktorn:

As far as I've checked for all the tiles that are tested with our regression tests maintain the same tile numbers, so the probability of cache breakage is low.

The tile numbers should be different, as the reference point is changed.

I tested this with Imagery "Bavaria 2m" (location 48.22038087911696, 11.672173475465426) and projection
+proj=tmerc +lon_0=12 +x_0=4500000 +ellps=bessel +nadgrids=BETA2007.gsb +units=m +axis=neu +bounds=9.5,-85,14.5,85 +wmssrs=EPSG:31468.
When switching from r9166 to r9167, the images get re-downloaded, so the cache indices seem to be shifted.

I've attached a patch, that should fix this, by restoring the old reference point and using negative tile numbers to the left of this point. The tiles are downloaded, but I cannot get tiles with index < 0 to display on screen...

comment:32 in reply to:  31 ; Changed 2 years ago by wiktorn

Replying to bastiK:

The tile numbers should be different, as the reference point is changed.

Do you know projections are affected?

I tested this with Imagery "Bavaria 2m" (location 48.22038087911696, 11.672173475465426) and projection
+proj=tmerc +lon_0=12 +x_0=4500000 +ellps=bessel +nadgrids=BETA2007.gsb +units=m +axis=neu +bounds=9.5,-85,14.5,85 +wmssrs=EPSG:31468.
When switching from r9166 to r9167, the images get re-downloaded, so the cache indices seem to be shifted.

Redownloading doesn't seem that far of the concern as shifted/misaligned imagery. I'll give it a try and check, what's the real difference.

I've attached a patch, that should fix this, by restoring the old reference point and using negative tile numbers to the left of this point. The tiles are downloaded, but I cannot get tiles with index < 0 to display on screen...

How come it can be, that it's old reference point, and we have negative tile numbers? Something doesn't match to me :-)

Responsibility for lack of display of negative tile indexes are lines of code in AbstractTileSourceLayer.getTile:881 - maybe it should check here for getTile*Min(zoom) here.

Though the general problem here is, that whatever the solution is, if we make some changes to projections (change world bounds), we will always need users to clear the cache - there is no stable point, like LatLon(0, 0) or EastNorth(0, 0), that would give stable conversion no matter the change.

Thus for future - I see two possible solutions:
# have a specific version number on projection code and use it as a key in tile cache
# have a upgrade task that will clear the cache, when current version is higher or equal X, and previous version that was running, was smaller than X

Neither gets my immediate buy-in.

comment:33 Changed 2 years ago by bastiK

In 9176/josm:

restore old reference point for wms tiling (see #12186)
use negative tile indices for tiles left to this point

Changed 2 years ago by bastiK

Attachment: transmerc-bounds.png added

comment:34 in reply to:  32 ; Changed 2 years ago by bastiK

Thanks for pointing me to AbstractTileSourceLayer.getTile, I think the patch is working now.

Here is an example (+proj=tmerc +lon_0=12 +ellps=bessel +bounds=0.5,-70,23.5,70):


In white are the normal lat/lon world bounds. Green is the area that was supported before (it takes the corners of the lat/lon box and draws a rectangle in projected space). This does not work - what one actually wants is the red rectangle (getWorldBoundsBoxEastNorth).

Replying to wiktorn:

Replying to bastiK:

The tile numbers should be different, as the reference point is changed.

Do you know projections are affected?

See above.

When switching from r9166 to r9167, the images get re-downloaded, so the cache indices seem to be shifted.

Redownloading doesn't seem that far of the concern as shifted/misaligned imagery. I'll give it a try and check, what's the real difference.

Redownloading shows that the cache isn't used, one reason for this would be renumbering of the (internal) tile index. This is only visibly to the user, when you happen to find the place where the old tiles are relocated to.

I've attached a patch, that should fix this, by restoring the old reference point and using negative tile numbers to the left of this point. The tiles are downloaded, but I cannot get tiles with index < 0 to display on screen...

How come it can be, that it's old reference point, and we have negative tile numbers? Something doesn't match to me :-)

Previously, we have only dealt with projections/bounds, where the red and green box, as shown in the example, are roughly the same. But now I have added reasonable default bounds for projections without explicit bounds. Basically the lat/lon box is taken as large as the formulas allow. Then these strange shapes for the world bounds cannot be avoided.

So what has negative tile numbers now (everything left to the green box) wasn't even supported before [9167].

Though the general problem here is, that whatever the solution is, if we make some changes to projections (change world bounds), we will always need users to clear the cache - there is no stable point, like LatLon(0, 0) or EastNorth(0, 0), that would give stable conversion no matter the change.

It may be possible to add a method that returns a natural center for each projection, so a change of bounds wouldn't invalidate the cache.

But I think the current solution (top-left corner of the lat/lon bounds) is good enough for now. (The corner of getWorldBoundsBoxEastNorth would not be a good reference point as it is calculated approximately and the details of this calculation may change in future.)

Thus for future - I see two possible solutions:
# have a specific version number on projection code and use it as a key in tile cache
# have a upgrade task that will clear the cache, when current version is higher or equal X, and previous version that was running, was smaller than X

Neither gets my immediate buy-in.

Maybe save the reference point (as lat/lon) along with the projection code - then clear the cache when it has changed?

comment:35 in reply to:  34 ; Changed 2 years ago by wiktorn

Replying to bastiK:

Thanks for pointing me to AbstractTileSourceLayer.getTile, I think the patch is working now.

Here is an example (+proj=tmerc +lon_0=12 +ellps=bessel +bounds=0.5,-70,23.5,70):

Great example, thanks.

Thus for future - I see two possible solutions:
# have a specific version number on projection code and use it as a key in tile cache
# have a upgrade task that will clear the cache, when current version is higher or equal X, and previous version that was running, was smaller than X

Neither gets my immediate buy-in.

Maybe save the reference point (as lat/lon) along with the projection code - then clear the cache when it has changed?

After some thought, using the center as a reference will result in different offsets depending on where someone checks. Using corners as reference points will result in smaller number of places, where imagery might be aligned properly, so it should alert users that something is wrong.

I'll think how to incorporate indexing parameters along the cache.

comment:36 in reply to:  35 Changed 2 years ago by bastiK

Replying to wiktorn:

Thus for future - I see two possible solutions:
# have a specific version number on projection code and use it as a key in tile cache
# have a upgrade task that will clear the cache, when current version is higher or equal X, and previous version that was running, was smaller than X

Neither gets my immediate buy-in.

Maybe save the reference point (as lat/lon) along with the projection code - then clear the cache when it has changed?

After some thought, using the center as a reference will result in different offsets depending on where someone checks. Using corners as reference points will result in smaller number of places, where imagery might be aligned properly, so it should alert users that something is wrong.

I'm not sure, I understand correctly, are you talking about center/corner of the screen, i.e. the location the user has currently loaded?

Now that I think about it, it's perfectly fine to use EastNorth(0, 0) as reference point. Even if it is not within projection bounds, this does not matter, as it is just used to fix the position of the (0,0)-tile. The minimum and maximum tile indices can then be calculated at runtime (as is done at the moment).

comment:37 Changed 2 years ago by bastiK

If there is more on the topic of WMS reference points, I would suggest to open a new ticket.

comment:38 Changed 2 years ago by Don-vip

Does this ticket allow us to deprecate proj4j plugin?

comment:39 Changed 2 years ago by Don-vip

also, is #4477 still relevant?

comment:40 in reply to:  39 Changed 2 years ago by bastiK

Replying to Don-vip:

Does this ticket allow us to deprecate proj4j plugin?

Not yet, the plugin has more base projections, see plugins/proj4j/src/org/osgeo/proj4j/proj.

But only 10 or so (that we don't have already) are actually used in the big EPSG list, so I guess most of the rest is historic and can be dropped. We can add those 10 to JOSM core and deprecate the plugin.

Replying to Don-vip:

also, is #4477 still relevant?

Closed.

comment:41 Changed 2 years ago by Don-vip

In 9333/josm:

see #12186, #11924 - update projection regression test file for Java 9

generated on Windows with:

java version "9-ea"
Java(TM) SE Runtime Environment (build 9-ea+99-2015-12-23-183325.javare.4146.nc)
Java HotSpot(TM) 64-Bit Server VM (build 9-ea+99-2015-12-23-183325.javare.4146.nc, mixed mode)

Changed 2 years ago by bastiK

comment:42 Changed 2 years ago by bastiK

Projections are fun! :)

Countries of the earth from the top:

(Polar Stereographic projection)

comment:43 Changed 2 years ago by simon04

Wow, awesome :))

Does this solve #2212?

comment:44 Changed 2 years ago by bastiK

Some issues remain, but it may be a partial solution. It's upside down along the 180th meridian so surely it won't work with Bing background. :)

comment:45 Changed 2 years ago by Don-vip

This is indeed awesome, can't wait to test it :)

comment:46 Changed 2 years ago by bastiK

In 9419/josm:

add Albers Equal Area Projection and Polar Stereographic Projection (see #12186)
(imports pieces of code from the Geotools project)

comment:47 Changed 2 years ago by Don-vip

In 9426/josm:

see #12186 - checkstyle

comment:48 Changed 2 years ago by Don-vip

In 9428/josm:

see #12186 - fix some javadoc warnings

comment:49 in reply to:  48 ; Changed 2 years ago by Don-vip

@bastiK: can you please complete javadoc of AbstractProj.tsfn() method? (missing tags for parameters and return value)

comment:50 Changed 2 years ago by bastiK

In 9431/josm:

see #2212, see #12186 - improve display for polar projections

comment:51 Changed 2 years ago by bastiK

In 9432/josm:

see #12186 - javadoc

comment:52 in reply to:  49 Changed 2 years ago by bastiK

Replying to Don-vip:

@bastiK: can you please complete javadoc of AbstractProj.tsfn() method? (missing tags for parameters and return value)

No idea what this method does, but sure can do. :)

comment:53 Changed 2 years ago by Don-vip

I was thinking you would :)

comment:54 Changed 2 years ago by bastiK

In 9532/josm:

see #12186 - add Oblique Mercator projection
(imports pieces of code from the Geotools project)

comment:55 Changed 2 years ago by Don-vip

In 9534/josm:

see #12186 - checkstyle

comment:56 Changed 2 years ago by Don-vip

Will you add HotineObliqueMercator? There's a javadoc warning because of unknown reference, but that's not really a warning if the projection is coming :)

However the javadoc for AbstractProj.cphi2() must be completed (missing parameter and return type).

comment:57 Changed 2 years ago by bastiK

In 9535/josm:

see #12186 - update tests

comment:58 in reply to:  56 Changed 2 years ago by bastiK

Replying to Don-vip:

Will you add HotineObliqueMercator? There's a javadoc warning because of unknown reference, but that's not really a warning if the projection is coming :)

It is there, but not as a separate class.

comment:59 Changed 2 years ago by bastiK

In 9549/josm:

see #12186 - add Cassini-Soldner Projection
(imports pieces of code from the Geotools project)

comment:60 Changed 2 years ago by Don-vip

In 9555/josm:

see #12186 - checkstyle

comment:61 Changed 2 years ago by bastiK

In 9558/josm:

always normalize longitude before projection and after inverse projection (see #12186)

comment:62 Changed 2 years ago by bastiK

In 9559/josm:

auto-detect poles in view (see #12186)

comment:63 Changed 2 years ago by bastiK

In 9560/josm:

added Lambert Azimuthal Equal Area projection (see #12186)
(imports pieces of code from the Geotools project)

comment:64 Changed 2 years ago by Don-vip

In 9562/josm:

see #12186 - checkstyle

comment:65 Changed 2 years ago by Don-vip

Milestone: 15.1216.02
Resolution: fixed
Status: closedreopened

some errors in unit tests:



Last edited 2 years ago by Don-vip (previous) (diff)

Changed 2 years ago by Don-vip

Attachment: errors.PNG added

comment:66 Changed 2 years ago by bastiK

In 9565/josm:

add 2 standard parallel & non-spherical variants for Mercator projection (see #12186)
(imports pieces of code from the Geotools project)

comment:67 Changed 2 years ago by stoecker

Maybe would should from now on finally call it GIS tool instead of OSM editor? :-)

comment:68 Changed 2 years ago by bastiK

In 9567/josm:

epsg script: list missing projections only for entries that are not excluded already for another reason (see #12186)

comment:69 Changed 2 years ago by bastiK

comment:70 in reply to:  66 Changed 2 years ago by bastiK

Replying to bastiK:

In 9565/josm:

add 2 standard parallel & non-spherical variants for Mercator projection (see #12186)
(imports pieces of code from the Geotools project)

Note that this removes the base projection josm:smerc. It is superseded by merc, but parameters have to be adapted when switching. I'm not aware that this string can get anywhere in the user configuration (except textbox history of custom projections), so there should be no reason to keep legacy for backwards compatibility.

comment:71 in reply to:  67 Changed 2 years ago by bastiK

Replying to stoecker:

Maybe would should from now on finally call it GIS tool instead of OSM editor? :-)

Yeah, but a decent GIS tool should support image warping (#7427) ... :)

comment:72 in reply to:  69 Changed 2 years ago by bastiK

Replying to bastiK:

In [9568]:

update to EPSG v8.8
(see ​https://github.com/OSGeo/proj.4/commit/832329a9a)

Stats before:

epsg:
     [java] loaded 199 entries from data_nodist/projection/josm-epsg
     [java] loaded 5006 entries from data_nodist/projection/epsg
     [java] 
     [java] some entries from proj.4 have not been included:
     [java]  * already in the maintained JOSM list: 199 entries
     [java]  * deprecated: 330 entries
     [java]  * using +proj=geocent, which is 3D (X,Y,Z) and not useful in JOSM: 138 entries
     [java]  * unsupported base projection: 13 entries
     [java]    in particular: {cea=2, poly=4, nzmg=1, krovak=4, eqc=2}
     [java]  * requires data file for datum conversion: 209 entries
     [java] 
     [java] written 199 entries from data_nodist/projection/josm-epsg
     [java] written 4132 entries from data_nodist/projection/epsg

Stats after:

epsg:
     [java] loaded 207 entries from data_nodist/projection/josm-epsg
     [java] loaded 5432 entries from data_nodist/projection/epsg
     [java] 
     [java] some entries from proj.4 have not been included:
     [java]  * already in the maintained JOSM list: 207 entries
     [java]  * deprecated: 341 entries
     [java]  * using +proj=geocent, which is 3D (X,Y,Z) and not useful in JOSM: 148 entries
     [java]  * unsupported base projection: 13 entries
     [java]    in particular: {cea=2, poly=4, nzmg=1, krovak=4, eqc=2}
     [java]  * requires data file for datum conversion: 210 entries
     [java] 
     [java] written 207 entries from data_nodist/projection/josm-epsg
     [java] written 4528 entries from data_nodist/projection/epsg

comment:73 Changed 2 years ago by stoecker

Shouldn't it be "written ... entries to"?

comment:74 Changed 2 years ago by bastiK

No, the output is data/projection/custom-epsg which is generated during build by combining 2 sources.

comment:75 Changed 2 years ago by Don-vip

In 9573/josm:

see #12186 - checkstyle

comment:76 Changed 2 years ago by bastiK

In 9577/josm:

use proj.4 id for Clarke 1880 IGN ellipsoid (see #12186)

comment:77 Changed 2 years ago by bastiK

In [9578]:

update carthage datum (following proj.4)

comment:78 Changed 2 years ago by bastiK

In 9579/josm:

fix bounds that are too large and make tests fail (see #12186)

comment:79 Changed 2 years ago by bastiK

In 9580/josm:

update hermannskogel datum (followin proj.4) - see #12186

comment:80 Changed 2 years ago by bastiK

In 9581/josm:

update regression test (see #12186)

comment:81 Changed 2 years ago by bastiK

In 9597/josm:

remove GRS80 as datum code (see #12186)

comment:82 Changed 2 years ago by bastiK

In 9598/josm:

fix bounds (see #12186)

comment:83 Changed 2 years ago by bastiK

In 9600/josm:

fix Coverity Scan warning (see #12186)

comment:84 Changed 2 years ago by bastiK

In 9608/josm:

change default unit to meter (see #12186)

comment:85 in reply to:  84 ; Changed 2 years ago by bastiK

Replying to bastiK:

In 9608/josm:

change default unit to meter (see #12186)

@wiktorn: I need this default value for reference test, does it work with WMTS?

comment:86 in reply to:  85 ; Changed 2 years ago by wiktorn

Replying to bastiK:

Replying to bastiK:

In 9608/josm:

change default unit to meter (see #12186)

@wiktorn: I need this default value for reference test, does it work with WMTS?

I'm not sure what is really the default (see https://github.com/OSGeo/proj.4/issues/284). But if all projections will properly specify this value, then I do not have a problem with that. Take for example EPSG:4326 - does it specify, that it unit is degrees?

What I see, is that proj.4 documentations said, that default is "meter", but a lot of projections use unit=m (why, if it's default), and all degree based projections that I know, did not specify unit=degree. I recognized that as documentation error. But if it's not, then there is a need to properly redefine some of the projections.

You can check your changes with Geoportal ortophoto WMTS in Poland. It supports 4326.

comment:87 Changed 2 years ago by bastiK

In 9609/josm:

rewrite of ProjectionRefTest - now covers all projections (see #12186)

comment:88 in reply to:  86 ; Changed 2 years ago by bastiK

Replying to wiktorn:

Replying to bastiK:

@wiktorn: I need this default value for reference test, does it work with WMTS?

I'm not sure what is really the default (see https://github.com/OSGeo/proj.4/issues/284). But if all projections will properly specify this value, then I do not have a problem with that. Take for example EPSG:4326 - does it specify, that it unit is degrees?

I think the default is to take whatever comes out of the projection algorithm, without any factor. For a normal projection, the calculation is done on a unit-sphere and afterwards multiplied by the earth radius (given in meter) so the result is in meter. For lonlat this is different and the result will be in degrees. Degrees is a unit of angle and not a length unit, so technically it cannot be converted to meter (at least not globally).

What I see, is that proj.4 documentations said, that default is "meter", but a lot of projections use unit=m (why, if it's default),

A lot of default values are given explicitly, I think it's okay.

You can check your changes with Geoportal ortophoto WMTS in Poland. It supports 4326.

Thanks, I'll have a look.

comment:89 in reply to:  88 ; Changed 2 years ago by wiktorn

Replying to bastiK:

I think the default is to take whatever comes out of the projection algorithm, without any factor. For a normal projection, the calculation is done on a unit-sphere and afterwards multiplied by the earth radius (given in meter) so the result is in meter. For lonlat this is different and the result will be in degrees. Degrees is a unit of angle and not a length unit, so technically it cannot be converted to meter (at least not globally).

metersPerUnit is only valid for conversions along great circle. I understand, that projection units define also some conversion multipliers for metersPerUnit. But maybe following approach will be OK (together with your recent changes):

  • src/org/openstreetmap/josm/data/projection/CustomProjection.java

    ### Eclipse Workspace Patch 1.0
    #P JOSM
    diff --git src/org/openstreetmap/josm/data/projection/CustomProjection.java src/org/openstreetmap/josm/data/projection/CustomProjection.java
    index 79ae1be..751617e 100644
     
    2626import org.openstreetmap.josm.data.projection.datum.WGS84Datum;
    2727import org.openstreetmap.josm.data.projection.proj.ICentralMeridianProvider;
    2828import org.openstreetmap.josm.data.projection.proj.IPolar;
     29import org.openstreetmap.josm.data.projection.proj.LonLat;
    2930import org.openstreetmap.josm.data.projection.proj.Mercator;
    3031import org.openstreetmap.josm.data.projection.proj.Proj;
    3132import org.openstreetmap.josm.data.projection.proj.ProjParameters;
     
    279280                } else {
    280281                    Main.warn("No metersPerUnit found for: " + s);
    281282                }
     283            } else if (this.proj instanceof LonLat) {
     284                // if there is no units= in projection specification and projection is based on lonlat, then use
     285                // METER_PER_UNIT_DEGREE as default
     286                this.metersPerUnit = METER_PER_UNIT_DEGREE;
    282287            }
    283288            s = parameters.get(Param.to_meter.key);
    284289            if (s != null) {
    285290                this.metersPerUnit = parseDouble(s, Param.to_meter.key);

comment:90 Changed 2 years ago by Don-vip

In 9612/josm:

see #12186 - checkstyle

comment:91 in reply to:  88 Changed 2 years ago by stoecker

You can check your changes with Geoportal ortophoto WMTS in Poland. It supports 4326.

Thanks, I'll have a look.

I'd say some of the related unit tests fail due to this, some others due to rounding errors because of +/- 180, both recent changes.

comment:92 Changed 2 years ago by wiktorn

In 9619/josm:

Finish removal of tile-size workaround code started in [9617].

Now WMTS TileSources with other than 256px tile size should work properly.
See #12437, #12186

Fix name of "valid-georeference", so it will be properly loaded from preferences

comment:93 Changed 2 years ago by bastiK

In 9628/josm:

fix WMTS with EPSG:4326 broken in [9608] (see #12186)

comment:94 in reply to:  89 ; Changed 2 years ago by bastiK

Replying to wiktorn:

metersPerUnit is only valid for conversions along great circle. I understand, that projection units define also some conversion multipliers for metersPerUnit. But maybe following approach will be OK (together with your recent changes):
[...]

Yes, done in [9628].

Btw., how did you get this constant in CustomProjection?

    private static final double METER_PER_UNIT_DEGREE = 2 * Math.PI * 6370997 / 360;

It lines up perfectly with the Polish Geoportal, but I cannot find any reference to this number.

Looking at the Well-known scale set examples in the WMTS documentation, it should actually be 6378137 (major axis of WGS84) instead of 6370997 (which could be a spherical earth radius).

I tried the server Kartverket which also offers EPSG:4326. It does in fact only line up with 6378137 but not the current value.

comment:95 Changed 2 years ago by bastiK

3 test failures remaining which are related to this ticket. They go away by changing

        assertEquals(expected.getLon(), result.lon(), 1e-4);

to

        assertEquals(LatLon.normalizeLon(expected.getLon()), LatLon.normalizeLon(result.lon()), 1e-4);

but is there something similar that needs to be fixed in the imagery code as well?

comment:96 Changed 2 years ago by stoecker

I also thought about that. But also your fix sounds not totally right. Correct would be to check the result from normalize(result-expected) against 0 (even if that means the error message is less helpful). Otherwise it is still unstable.

comment:97 in reply to:  96 Changed 2 years ago by bastiK

Replying to stoecker:

I also thought about that. But also your fix sounds not totally right. Correct would be to check the result from normalize(result-expected) against 0 (even if that means the error message is less helpful). Otherwise it is still unstable.

Right, this is better.

comment:98 in reply to:  94 ; Changed 2 years ago by wiktorn

Replying to bastiK:

Btw., how did you get this constant in CustomProjection?

    private static final double METER_PER_UNIT_DEGREE = 2 * Math.PI * 6370997 / 360;

It lines up perfectly with the Polish Geoportal, but I cannot find any reference to this number.

If I remember correctly, I took it from cs2cs -lu (or something similar - I write just from my memory). But to make this constant less magic, I deducted, that it should be somehow related to 2*PI*x/360. And I came with 6370997. I'll provide exact reference tonight once I get access to my environment.

Looking at the Well-known scale set examples in the WMTS documentation, it should actually be 6378137 (major axis of WGS84) instead of 6370997 (which could be a spherical earth radius).

I tried the server Kartverket which also offers EPSG:4326. It does in fact only line up with 6378137 but not the current value.

Ok, maye it's worth to find third service supporting EPSG:4326 to perform arbitrage :-)

comment:99 in reply to:  98 Changed 2 years ago by wiktorn

Replying to wiktorn:

Replying to bastiK:

Btw., how did you get this constant in CustomProjection?

    private static final double METER_PER_UNIT_DEGREE = 2 * Math.PI * 6370997 / 360;

It lines up perfectly with the Polish Geoportal, but I cannot find any reference to this number.

I took it from here:
https://github.com/openlayers/ol3/blob/master/src/ol/proj/proj.js#L48

And sphere radius is defined here:
https://github.com/openlayers/ol3/blob/master/src/ol/sphere/normal.js#L11

As 6370997 - hence this value.

As it was working with Polish Geoportal I took this as a good value, I haven't found proper reference value in specifications.

comment:100 Changed 2 years ago by wiktorn

In 9630/josm:

Document source of METER_PER_UNIT_DEGREE. See #12186

comment:101 Changed 2 years ago by bastiK

Yes, the specification is very unspecific about this. But in Table E.1 they have a scale denominator of 500e6 and a pixel size in degrees of 1.25764139776733. This corresponds to an earth radius of

0.28e-3 * 360 / (2 * pi) * 500e6 / 1.25764139776733 = 6378137.0,

i.e. the WGS84 major axis length, i.e. equatorial radius.

I think OL3 also uses the value 6378137.0: See wmtstilegrid.js, proj.test.js, epsg4326projection.js and wgs84sphere.js

comment:102 Changed 2 years ago by Don-vip

in LambertAzimuthalEqualArea.invproject() / case OBLIQUE, the assignments of variable q are without effect.

comment:103 Changed 2 years ago by wiktorn

Indeed. I use also this value to create tile matrix for WMS and using METER_PER_UNIT_DEGREE = 6378137.0 I get tiles, that are Y borders are aligned with TMS, what was the goal.

I've also created sample OpenLayer comparision of WMTS and WMS using EPSG:4326 of Geoportal:
http://precel.vink.pl/w/osm/test/

(WMS is on the left, WMTS on the right). And it looks like (at least using OL as reference implementation) that we have EPSG:4326 broken here.

The only problem I have is that ESRI implemtation doesn't show this problem when using EPSG:4326.

I tried to add Kartenverket to this ESRI viewer, but it didn't work at all. It broken in different way, than JOSM is now in tile index computations.

I'll try to take this into discussion with Polish Geoportal. We will see where it will go.

comment:104 Changed 2 years ago by wiktorn

I'll also cross-check QGIS tonight, how does it handle this two WMTS services.

comment:105 Changed 2 years ago by bastiK

In 9636/josm:

remove unnecessary assignment (see #12186)

comment:106 in reply to:  102 Changed 2 years ago by bastiK

Replying to Don-vip:

in LambertAzimuthalEqualArea.invproject() / case OBLIQUE, the assignments of variable q are without effect.

Good catch, reported upstream!

comment:107 Changed 2 years ago by Don-vip

you can thank coverity :)

comment:108 Changed 2 years ago by wiktorn

In 9642/josm:

Correct METER_PER_UNIT_DEGREE to match values in OpenLayers and QGIS.

After some investigation in the code, it looks like both OpenLayers and QGIS use
this value for calculations how many meters fit within one degree. This is also
in line with WMTS specificationi (thanks: bastiK).

Warning: this will break (probably broken) Geoportal 2 in Poland WMTS service
(and alike), when using EPSG:4326 projection. The problem is already reported to
Geoportal administrator in Poland.

See: #12186

comment:109 Changed 2 years ago by Don-vip

Still some projections to add, or everything's here now?

comment:110 Changed 2 years ago by bastiK

In 9767/josm:

update tests for [9558] and [9642] (see #12186, see #12514)

comment:111 Changed 2 years ago by bastiK

In 9790/josm:

use intended units for east/north coordinates (see #12186)

When east/north coordinates are not in meter, but
feet, ... convert them to correct units.
Currently they are always stored in meters or degrees.
This makes no difference to JOSM internally, but affects
services like WMS/WMTS.
Only relevant for projections with +units=... or +to_meter=...
parameter set to non-default value.

comment:112 Changed 2 years ago by bastiK

Wiktor, I'm not sure if WMS code needs to be updated, do you know a server one could use for testing?

Last edited 2 years ago by bastiK (previous) (diff)

comment:113 Changed 2 years ago by wiktorn

My guess is that it would affect only WMTS, as WMS doesn't use scale denomintors / units from projection (other than to create artificial tile grid).

But I do not know any WMTS using such projection.

comment:114 Changed 2 years ago by bastiK

Isn't WMS bbox given in projected coordinates?

comment:115 Changed 2 years ago by Don-vip

still something to do?

comment:116 Changed 2 years ago by bastiK

Resolution: fixed
Status: reopenedclosed

I'd like to change / add a few more things, but it is fine for now.

comment:117 Changed 23 months ago by Don-vip

Ok. Can we deprecate proj4j plugin or does it still provide some projections on its own?

comment:118 Changed 23 months ago by Don-vip

In 9908/josm:

see #12186, #11924 - update projection regression test file for Java 9

generated on Windows with:

java version "9-ea"
Java(TM) SE Runtime Environment (build 9-ea+107-2016-02-24-182456.javare.4520.nc)
Java HotSpot(TM) 64-Bit Server VM (build 9-ea+107-2016-02-24-182456.javare.4520.nc, mixed mode)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.