timwaters / mapwarper

free open source public map georeferencer, georectifier and warper
http://mapwarper.net
MIT License
196 stars 80 forks source link

Incorrect warping. Control Points not positioned accordingly #159

Closed derMart closed 6 years ago

derMart commented 6 years ago

It seems, the warping algorithm is incorrect. See the image below for the map http://mapwarper.net/maps/29903 warp_error

The control points on the left image should exactly match the historical map on the right side, but they are not.

timwaters commented 6 years ago

Thanks for taking the time to write this issue.

You can change the algorithm in the control panel. under advanced options. image

Try selecting thin plate spline and see if that helps. Otherwise the default choice is to use a polynomial method.

derMart commented 6 years ago

Thank you very much. Yes, the TPS is working as expected. Although, even if there are more than 25 control points, the automated method chooses a second order polynomial. From the descriptions I would assumed this to be 3rd order. As I haven't seen the advanced options in the first place, and as a novice user (like me in this case) I would expect the control points to match, why not choosing TPS as default in any case?

timwaters commented 6 years ago

TPS only really works well when the points are many and all spread out, and often fails for most maps.

I am not sure how the gdalwarp program selects the method to use. From what I've found the numbers shown are the minimum amount of points needed for the method. One would expect 25 points to choose 3rd polynomial, yes!

edits: https://github.com/OSGeo/gdal/blob/master/gdal/alg/gdal_nrgcrs.c#L94 I think this is where it choses. It should based on this choose a 3rd polynomial solution for your map.

timwaters commented 6 years ago

okay so apparently this is the correct location for that logic.

https://github.com/OSGeo/gdal/blob/master/gdal/alg/gdal_crs.c#L209

Note the comment: /for now we avoid 3rd order since it is unstable/

So we should expect 3rd polynomials to be used, but by default they won't be... I think I'll make a new issue for this.

timwaters commented 6 years ago

refs #160