New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Euclidean distance tiles #1552

Merged
merged 14 commits into from Jul 15, 2016

Conversation

Projects
None yet
2 participants
@jpolchlo
Contributor

jpolchlo commented Jun 15, 2016

This pull request addresses issue #941 by creating a Euclidean distance field from a set of points. The operation creates a Voronoi diagram from the input points and rasterizes each resulting polygonal cell to an output DoubleArrayTile using a simple distance function from the point that generated the Voronoi cell.

A note on complexity: this method builds a Delaunay triangulation in O(n log n) time, generates the Voronoi diagram in O(n) time, and the rasterization function applied to each pixel is O(1). Thus, the expectation is that the performance of this module ought to be fairly reasonable.

@jpolchlo jpolchlo changed the title from [WIP] Euclidean distance tiles to Euclidean distance tiles Jun 16, 2016

import geotrellis.vector.voronoi._
import scala.math.sqrt
object EuclideanDistanceTile {

This comment has been minimized.

@lossyrob

lossyrob Jun 20, 2016

Member

This should be moved out of root package; geotrellis.raster.distance seems general enough to be a home for it.

Needs MethodExtensions[Traversable[Point]], and the corresponding machinery.

def fillFn(base: Point)(c: Int, r: Int): Unit = {
val (x,y) = re.gridToMap(c,r)
tile.setDouble(c,r,sqrt((x-base.x)*(x-base.x) + (y-base.y)*(y-base.y)))

This comment has been minimized.

@lossyrob

lossyrob Jun 20, 2016

Member

Formatting: spaces after comma. Spaces between operands.
Everywhere else we give the argument the names col and row, should follow that pattern (and generally use the longer names for clarity, and to fit the rest of the library, e.g. "points" instead of "pts", "rasterExtent" instead of "re")

def det3 (a11: Double, a12: Double, a13: Double,
a21: Double, a22: Double, a23: Double,
a31: Double, a32: Double, a33: Double): Double = {
val m = MatrixUtils.createRealMatrix(Array(Array(a11,a12,a13),Array(a21,a22,a23),Array(a31,a32,a33)))

This comment has been minimized.

@lossyrob

lossyrob Jun 20, 2016

Member

format: comma spacing

// m.setEntry(0, 1, a12)
// m.setEntry(0, 2, a13)
// m.setEntry(1, 0, a21)
// m.setEntry(1, 1, a22)

This comment has been minimized.

@lossyrob

lossyrob Jun 20, 2016

Member

Shouldn't keep around debug comments in finished code.

}
//println(s"Inserting $e as $vs [using $idx as key]")
faceIncidentToVertex += (e.vert -> e, e.next.vert -> e.next, e.next.next.vert -> e.next.next)
//println(s"Adding to faceIncidentToVertex: ${e.vert -> e}, ${e.next.vert -> e.next}, ${e.next.next.vert -> e.next.next}")

This comment has been minimized.

@lossyrob

lossyrob Jun 20, 2016

Member

Debug printlns should be removed. If there's a specific reason for it to be around (e.g. this is always the information that is needed when this breaks, so kept for perpetuity) then it should be commented on and kept, but by default all debug lines and comments should be removed.

case (Some(x),_,_,_) => (x,0)
case (_,Some(x),_,_) => (x,1)
case (_,_,Some(x),_) => (x,2)
case (_,_,_,Some(x)) => (x,3)

This comment has been minimized.

@lossyrob

lossyrob Jun 20, 2016

Member

These magic numbers probably deserve some comments explaining what they are

@@ -0,0 +1,97 @@
package geotrellis.vector.voronoi
class HalfEdge[V,F](val vert: V, var flip: HalfEdge[V,F], var next: HalfEdge[V,F], var face: Option[F]) {

This comment has been minimized.

@lossyrob

lossyrob Jun 28, 2016

Member

Polygon method?

* A method to generate the Voronoi cell corresponding to the point in verts(i). Note that if
* verts(i) is not distinct, this function may raise an exception.
*/
def mkVoronoiCell(i: Int): Polygon = {

This comment has been minimized.

@lossyrob

lossyrob Jun 28, 2016

Member

can drop the mk

@lossyrob

This comment has been minimized.

Member

lossyrob commented Jul 15, 2016

@jpolchlo this cannot be merged. Please update it to master.

@lossyrob lossyrob merged commit 2a74a3b into locationtech:master Jul 15, 2016

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

@lossyrob lossyrob added this to the 1.0 milestone Oct 18, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment