geotrellis / geotrellis-contrib

GeoTrellis functionality related to the core.
Apache License 2.0
5 stars 11 forks source link

GDALRasterSource reproject with only targetCRS #235

Closed echeipesh closed 5 years ago

echeipesh commented 5 years ago

This is possibly an issue for clarification. When using GDALRasterSource and GDALWarpOptions my expectation is that only targetCRS would be required because that's how it works with gdalwarp. However, I find that indeed both sourceCRS and targetCRS are required.

Example (using geotrellis 3.0.0-SNAPSHOT and latest version of this lib):

scala> import geotrellis.raster.gdal._
import geotrellis.raster.gdal._

scala> val rs = new GDALRasterSource("/Users/eugene/tmp/ppp_prj_2019_DJI.tif")
rs: geotrellis.raster.gdal.GDALRasterSource = GDALRasterSource(/Users/eugene/tmp/ppp_prj_2019_DJI.tif,GDALWarpOptions(-of VRT -ovr AUTO))

scala> val rs = new GDALRasterSource("/Users/eugene/tmp/ppp_prj_2019_DJI.tif", GDALWarpOptions(targetCRS = Some(geotrellis.proj4.WebMercator)))
rs: geotrellis.raster.gdal.GDALRasterSource = GDALRasterSource(/Users/eugene/tmp/ppp_prj_2019_DJI.tif,GDALWarpOptions(-of VRT -ovr AUTO))

scala> val rs = new GDALRasterSource("/Users/eugene/tmp/ppp_prj_2019_DJI.tif", GDALWarpOptions(sourceCRS = Some(geotrellis.proj4.LatLng), targetCRS = Some(geotrellis.proj4.WebMercator)))
rs: geotrellis.raster.gdal.GDALRasterSource = GDALRasterSource(/Users/eugene/tmp/ppp_prj_2019_DJI.tif,GDALWarpOptions(-of VRT -s_srs +proj=longlat +datum=WGS84 +no_defs  -t_srs +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 +wktext +no_defs  -ovr AUTO))

scrolling to the right we can see that options are identical between just opening the file and providing targetCRS.

I'm guessing this is for a reason, but I'm unclear on what the reason is. Also its a little counter-intuitive so I think its worth an issue. The intention here is to have a fully lazy reprojection because call to GDALRasterSource.reproject will query the underlying GDAL Dataset for the source CRS in order to set it in the options. This is specifically what I would like to avoid in a scenario I'm working on.

https://github.com/geotrellis/geotrellis-contrib/blob/c67e35765df420f9e94169463749c81d6bd60802/gdal/src/main/scala/geotrellis/contrib/vlm/gdal/GDALRasterSource.scala#L115-L116

pomadchin commented 5 years ago

Looks like an oversight / typo. It can also be the legacy from the experience of working with usual GDAL bindings, where derived warped datasets can loose some of the metadata.

https://github.com/geotrellis/geotrellis-contrib/blob/master/gdal/src/main/scala/geotrellis/contrib/vlm/gdal/GDALWarpOptions.scala#L171-L173

The logic should be changed to

(sourceCRS, targetCRS) match {
  case (Some(source), Some(target)) => if(source != target) List("-s_srs", source.toProj4String, "-t_srs", target.toProj4String)
     else Nil
  case (Some(source), _) => List("-s_srs", source.toProj4String)
  case (_, Some(target)) => List("-t_srs", target.toProj4String)
}

And if tests won't fail, it is a good enough fix and it alings with an intuitive logic.

pomadchin commented 5 years ago

Resolved here https://github.com/locationtech/geotrellis/pull/3053