Raster reprojection

[klokan]

Implementation of the raster reprojection with triangulation with Proj4js as described and discussed in #3785.

The raster reprojection is triggered automatically when projection in ol.View differs from projection of a raster source such as WMTS or WMS.

Examples:

• Raster reprojection demo
• OpenStreetMap to WGS84 reprojection
• Reprojection with EPSG.io database search
• Image reprojection

Tutorial for use is being prepared at:
https://github.com/klokantech/ol3/wiki/Raster-Reprojection-Tutorial
and to be pushed to official tutorials.

We may add an example with UK's Ordnance Survey OpenSpace API tiles as well.

There are related tickets to some implementation issues:

• Stitch lines visible between triangles for semi-transparent sources Raster reprojection stitch lines visible between triangles for semi-transparent sources #4123
• Source resolution Raster reprojection source resolution #4124

[bill-chadwick]

Great to see this PR.

It must represent a large amount of hard work.

[marcjansen]

Typo: An augmented

[petrsloup]

This is actually not a typo: A is the name of the parameter.

[marcjansen]

Sure, got it.

[marcjansen]

Awesome work, Petr! Thank you so much! I added some minor comments, which you may want to address.

Other people can surely review this better than me, but I have the impression that we have never been closer to raster reprojection in OpenLayers since 2009 (you remember, right?). Excited when we will get this in.

[bill-chadwick]

I think control of the Overlay Map opacity would add a bit extra to your first demo.

I am seeing the image, or parts of it, disappear when zooming in and panning around on the image reprojection demo using Firefox 40.0.3, operation on Chrome seems better (both Win 7).

[petrsloup]

@bill-chadwick The image reprojection does not sometimes work optimally especially on low-RAM and/or HiDPI devices, where creating large canvas can lead to strange behavior during rendering.
There is, however, no simple way around this at the moment (the image-based sources are always expected to return a single raster -- no tiling is possible).

I addressed the comments (thanks @marcjansen) and rebased on latest master (+ added some commits to follow the latest changes -- removed calls to goog.math.clamp and goog.isDef).

[ahocevar]

Thanks again for this great work. I'm now starting to review.

It would be great if you could rebase the pull request one more time. Please also check the code for any occurrences of the goog.* functions that we stopped using in the meantime: goog.fx.easing, goog.string.remove, goog.string.startsWith, goog.string.trim(), goog.dom.appendChild, goog.isNull, goog.isDefAndNotNull, goog.object.get, goog.functions.constant, goog.object.getKeys, goog.nullFunction, goog.array.*, goog.now, goog.math.clamp, goog.isDef, goog.object.remove and goog.object.containsKey. If any of those are there, please replace them.

[petrsloup]

@ahocevar Rebased and removed calls to all goog.* functions that you mentioned.

[ahocevar]

Thanks @petrsloup, reviewing now.

[ahocevar]

Sorry for the comments noise so far. I realised that it's better to review the final result instead of individual commits, because some of my concerns are addressed in later commits. Continuing with the final result now...

[ahocevar]

Couldn't hurt to be a bit more verbose and call this ol.DEFAULT_RASTER_REPROJECTION_ERROR_THRESHOLD .

[ahocevar]

Very impressive work. I have added a few remarks (some of which may be obsolete, because I started the review on a per-commit basis).

The only major concern I have is wrapX handling. It adds a lot of complexity, which I think can be avoided when making proper use of tile ranges, and getTileCoordForTileUrlFunction instead of getTileCoord. It may make sense to set up a quick hangout where you walk me through the problem set, so I can better judge if I'm just missing something, or if there is really a huge potential for simplification here.

[bill-chadwick]

I can't think of many use cases for raster reprojection that will involve
wrap X. I expect it to mostly be used to warp local, non-global projection
mapping to web mercator or lat/lon and vice versa.

Are there some use cases involving wrap X that the rest of you can think of?
On 14 Oct 2015 22:43, "Andreas Hocevar" notifications@github.com wrote:

Very impressive work. I have added a few remarks (some of which may be
obsolete, because I started the review on a per-commit basis).

The only major concern I have is wrapX handling. It adds a lot of
complexity, which I think can be avoided when making proper use of tile
ranges, and getTileCoordForTileUrlFunction instead of getTileCoord. It may
make sense to set up a quick hangout where you walk me through the problem
set, so I can better judge if I'm just missing something, or if there is
really a huge potential for simplification here.

—
Reply to this email directly or view it on GitHub
#4122 (comment).

[bill-chadwick]

I guess there are some text book demos comparing different global
projections that would need wrap X.
On 15 Oct 2015 08:53, "Bill Chadwick" bill.chadwick2@googlemail.com wrote:

I can't think of many use cases for raster reprojection that will involve
wrap X. I expect it to mostly be used to warp local, non-global projection
mapping to web mercator or lat/lon and vice versa.

Are there some use cases involving wrap X that the rest of you can think
of?
On 14 Oct 2015 22:43, "Andreas Hocevar" notifications@github.com wrote:

Very impressive work. I have added a few remarks (some of which may be
obsolete, because I started the review on a per-commit basis).

The only major concern I have is wrapX handling. It adds a lot of
complexity, which I think can be avoided when making proper use of tile
ranges, and getTileCoordForTileUrlFunction instead of getTileCoord. It may
make sense to set up a quick hangout where you walk me through the problem
set, so I can better judge if I'm just missing something, or if there is
really a huge potential for simplification here.

—
Reply to this email directly or view it on GitHub
#4122 (comment).

[ahocevar]

Are there some use cases involving wrap X that the rest of you can think of?

There are some. Reproject from EPSG:3857 to EPSG:3338 for example.

[ahocevar]

I think this is where we have potential for simplification: You could calculate the source tile range instead of the source extent, and use source.getTileCoordForTileUrlFunction() to get tile extents.

Or did you choose this approach because you wanted to support reprojecting global ImageStatic sources to local projections that cross the date line?

[ahocevar]

Even if you keep this method, what would happen if your left bound is always smaller than the right bound as it should be? I think it would save you some special case handling when working with the extent you get returned from this method.

diff --git a/src/ol/reproj/triangulation.js b/src/ol/reproj/triangulation.js
index dc95b77..9b47f31 100644
--- a/src/ol/reproj/triangulation.js
+++ b/src/ol/reproj/triangulation.js
@@ -283,38 +283,12 @@ ol.reproj.Triangulation.prototype.calculateSourceExtent = function() {

   var extent = ol.extent.createEmpty();

-  if (this.wrapsXInSource_) {
-    // although only some of the triangles are crossing the dateline,
-    // all coordinates need to be "shifted" to be positive
-    // to properly calculate the extent (and then possibly shifted back)
-
-    this.triangles_.forEach(function(triangle, i, arr) {
-      goog.asserts.assert(this.sourceWorldWidth_);
-      var src = triangle.source;
-      ol.extent.extendCoordinate(extent,
-          [goog.math.modulo(src[0][0], this.sourceWorldWidth_), src[0][1]]);
-      ol.extent.extendCoordinate(extent,
-          [goog.math.modulo(src[1][0], this.sourceWorldWidth_), src[1][1]]);
-      ol.extent.extendCoordinate(extent,
-          [goog.math.modulo(src[2][0], this.sourceWorldWidth_), src[2][1]]);
-    }, this);
-
-    var sourceProjExtent = this.sourceProj_.getExtent();
-    var right = sourceProjExtent[2];
-    if (extent[0] > right) {
-      extent[0] -= this.sourceWorldWidth_;
-    }
-    if (extent[2] > right) {
-      extent[2] -= this.sourceWorldWidth_;
-    }
-  } else {
-    this.triangles_.forEach(function(triangle, i, arr) {
-      var src = triangle.source;
-      ol.extent.extendCoordinate(extent, src[0]);
-      ol.extent.extendCoordinate(extent, src[1]);
-      ol.extent.extendCoordinate(extent, src[2]);
-    });
-  }
+  this.triangles_.forEach(function(triangle, i, arr) {
+    var src = triangle.source;
+    ol.extent.extendCoordinate(extent, src[0]);
+    ol.extent.extendCoordinate(extent, src[1]);
+    ol.extent.extendCoordinate(extent, src[2]);
+  });

   this.trianglesSourceExtent_ = extent;
   return extent;

[tsauerwein]

Would it make sense to have opt_errorThreshold and opt_renderEdges also for ol.reproj.Image?

[petrsloup]

Adding it only to ol.reproj.Image would not be very useful -- it would have to be added also to ol.source.Image and passed from all the subclasses, but I don't think it's worth the extra code:
opt_renderEdges is used for debugging only and it's probably enough to have this in tiled sources.
I also think that opt_errorThreshold is kind of "advanced" parameter for fine-tuning and nobody needs it when displaying image sources.
But if you think otherwise, I don't have any problem adding these (or just the opt_errorThreshold).

[tsauerwein]

No, because it is only for debugging I think it is not necessary.

[ahocevar]

I'm done with my review @petrsloup. Now please also answer @tsauerwein's questions and try to address his concerns. @tsauerwein, please also add a note when you're done with your review.

[tsauerwein]

Do you have an example where you would need that many source tiles to reproject a single tile?

[petrsloup]

The resolution of the source data is determined in the center of the reprojected tile and is constant for the whole tile.
If the projection behaves significantly different in the center and at the edges of the tile, the number of required tiles can be very high.

Another case (probably easier to achieve):
This can also happen when the tile grid has more than 100 tiles on the least-detailed zoom level and the reprojection needs to render the whole extent. (It would have to download and process all the tiles, potentially crashing the browser.)

[tsauerwein]

I am done with my review. I only added a couple of questions and minor suggestions. Thanks for this impressive work!

[ahocevar]

Some metrics on build size:

• Current master:
  
  uncompressed: 479959 bytes, compressed: 142186 bytes
• This branch without ol.ENABLE_RASTER_REPROJECTION:
  
  uncompressed: 482251 bytes, compressed: 142599 bytes
• This branch with ol.ENABLE_RASTER_REPROJECTION:
  
  uncompressed: 490031 bytes, compressed: 145353 bytes

[petrsloup]

Most of the build size without ol.ENABLE_RASTER_REPROJECTION is caused by reprojectionErrorThreshold constructor options and setRenderReprojectionEdges methods (exported to the api) on the sources.

[tsauerwein]

Thanks for the clarifications, @petrsloup!

[marcjansen]

What a nice discussion here. Great collaboration, guys!

Can we ship it ?!?

[ahocevar]

Can we ship it ?!?

Yes we can :-)

[marcjansen]

Nice indeed! Thanks to everyone involved.

[petrsloup]

Thanks for the comments and remarks, everyone! Great to see this merged!

[bartvde]

w00t - great to see this in! Thanks everyone.

Sent from my iPhone

On 16 okt. 2015, at 21:47, Andreas Hocevar notifications@github.com wrote:

Merged #4122.

—
Reply to this email directly or view it on GitHub.

[klokan]

Cool. Thanks!

[ximex]

Thanks! This is a huge improvement!