New module @turf/line-offset #729

rowanwins · 2017-05-10T23:40:41Z

New Module `@turf/line-offset`

Adds a new lineOffset module as per this issue. Basically takes an input line and returns a new line offset by the distance.

Geometry Support

LineString => LineString
MultiLineString => MultiLineString

Geometry NOT supported

Point => LineString (alternative @turf/circle?)
MultiPoint => MultiLineString (alternative @turf/circle?)
Polygon => LineString (alternative @turf/buffer / @turf/transform-scale)
MultiPolygon => MultiLineString (alternative @turf/buffer / @turf/transform-scale)
FeatureCollection/GeometryCollection (might contain mixed geometries and cause errors)

To-Do

Start and end points of the resulting line shall not be connected.
Equidistance offset (similar to issue Apply equidistance @turf/buffer to polygons #660 / Improved/fixed @turf/buffer using equidistant buffers #718)

JSDocs

/**
 * Takes a {@link LineString|line} and returns a {@link LineString|line} at offset by the specified distance.
 *
 * @name lineOffset
 * @param {Geometry|Feature<LineString>} line input line
 * @param {number} offset distance to offset the line (can be of negative value)
 * @param {string} [units=kilometers] can be degrees, radians, miles, kilometers, inches, yards, meters
 * @returns {Feature<LineString>} Line offset from the input line
 * @example
 * var line = {
 *   "type": "Feature",
 *   "properties": {},
 *   "geometry": {
 *     "type": "LineString",
 *     "coordinates": [[-83, 30], [-84, 36], [-78, 41]]
 *   }
 * };
 *
 * var offsetLine = turf.lineOffset(line, 2, 'miles');
 *
 * //addToMap
 * var addToMap = [offsetLine, line]
 */

Examples

Use a meaningful title for the pull request. Include the name of the package modified.
Have read How To Contribute.
Run npm test at the sub modules where changes have occurred.
Run npm run lint to ensure code style at the turf module level.

DenisCarriere · 2017-05-10T23:56:47Z

@rowanwins Can you provide more information in your Initial Description, that way when we reference this PR we have all the information available for the module at the top.

Use this as a rough example: #700

And for some reason the benchmark isn't printing results?

You need to upgrade the benchmark version (v1.0.0 has issues with ES6 syntax).

$ yarn add --dev tape benchmark

DenisCarriere · 2017-05-11T00:00:00Z

packages/turf-line-offset/bench.js

+  }
+};
+
+var route = JSON.parse(fs.readFileSync(__dirname + '/test/fixtures/route.geojson'));


Try to make the new benchmark.js files as minimalistic as possible, use a few of the latest modules as examples:
https://github.com/Turfjs/turf/blob/0cfbda88456a89c7cd988ac17afd6056a42f59a8/packages/turf-rhumb-distance/bench.js

Doh didn't push the right benchmark file!

DenisCarriere · 2017-05-11T00:01:06Z

packages/turf-line-offset/index.d.ts

+/**
+ * http://turfjs.org/docs/#lineOffset
+ */
+declare function lineOffset(line: LineString | GeoJSON.LineString, distance: number, units?: string): LineString;


Recently I've been adding the Units types pulled from @turf/helpers.
https://github.com/Turfjs/turf/blob/0cfbda88456a89c7cd988ac17afd6056a42f59a8/packages/turf-rhumb-distance/index.d.ts

DenisCarriere · 2017-05-11T00:02:26Z

packages/turf-line-offset/index.js

+ * Takes a {@link LineString|line} and returns a {@link LineString|line} at offset by the specified distance.
+ *
+ * @name lineOffset
+ * @param {Feature<LineString>|Geometry<LineString>} line input line


You can group the JSDoc types together

/** * @param {Geometry|Feature<LineString>} line input line

DenisCarriere · 2017-05-11T00:07:46Z

packages/turf-line-offset/index.js

+            seg2Coords[0][1] = int.y;
+        }
+    });
+    var finalCoords = segments.map(function (segment) {


Might be worth using forEach or for loop since it will be significantly faster than using map (~30-40% faster - Use benchmark to get an exact %)

could be faster, although the benchmark is performing 600,000 operations per second so we're talking about shaving milli-milli-milli-mill seconds :) Can probably be tackled in the overall refactoring of those loops

I'll have a look at the refactoring, sometimes it's not worth it, but sometimes it's an easy change for a pretty significant performance increase. Not a deal breaker whatsoever

DenisCarriere · 2017-05-11T00:11:28Z

packages/turf-line-offset/index.js

+    coordEach(line, function (currentCoords, currentIndex) {
+        if (currentIndex !== coords.length - 1) {
+            var outCoords = processSegment(currentCoords, coords[currentIndex + 1], offsetDegrees);
+            segments.push(outCoords);


Iterating over all coords, then iterating over all segments, then iterating over segments again.

Looks like this could be done by only iterating your coordinates once.

You'll need to convert this code block into it's own method and use a callback to prevent the double iterations.

I can tackle this if you don't feel comfortable using callbacks

Yeah it is a bit messy at the moment, I'll probably wont get a chance to look at it for another few nights so if you've got time and want to tackle it go nuts :)

Will do :) I'll have a look at it more tomorrow, it's a good starting point so far!
+1

DenisCarriere · 2017-05-11T00:13:06Z

packages/turf-line-offset/index.js

+        return segment[0];
+    });
+    finalCoords.push(segments[segments.length - 1][1]);
+    return lineString(finalCoords);


Should we also translate the properties from the input LineString?

DenisCarriere · 2017-05-11T00:13:23Z

packages/turf-line-offset/index.js

+    return lineString(finalCoords);
+};
+
+// Inspiration taken from http://stackoverflow.com/questions/2825412/draw-a-parallel-line


👍 Nice reference

DenisCarriere · 2017-05-11T00:14:59Z

packages/turf-line-offset/package.json

+    "offset"
+  ],
+  "author": "Turf Authors",
+  "license": "MIT",


Add yourself as contributors.

DenisCarriere · 2017-05-11T00:15:34Z

packages/turf-line-offset/package.json

+  },
+  "homepage": "https://github.com/Turfjs/turf",
+  "devDependencies": {
+    "benchmark": "^1.0.0",


Upgrade benchmark & tape

I'll upgrade all devDependencies in TurfJS

DenisCarriere · 2017-05-11T00:19:39Z

packages/turf-line-offset/package.json

+    "@turf/helpers": "^4.1.0",
+    "@turf/invariant": "^4.2.0",
+    "@turf/meta": "^4.2.0",
+    "intersection": "0.0.1"


I'd drop intersection in favor of using @turf/line-intersect.

intersection works out where lines would intersect based on their direction, even if they don't actually intersect, eg it works irrespective of length. See this interactive example.

Oooohh, yes that's completely different, I wonder if it's worth including that logic directly in Turf. I'm not a big fan of external dependencies, especially if it can be refactored in less than 30-50 LOC

yeah it probably could be included, I had a bit of a look at the source code and it's not huge but it was enough of a hassle that I thought I'd pull in the external first just to test the concept first.

DenisCarriere · 2017-05-11T00:21:08Z

packages/turf-line-offset/index.js

+        if (index !== segments.length - 1) {
+            var seg2Coords = segments[index + 1];
+            var seg1 = {start: {x: segment[0][0], y: segment[0][1]}, end: {x: segment[1][0], y: segment[1][1]}};
+            var seg2 = {start: {x: seg2Coords[0][0], y: seg2Coords[0][1]}, end: {x: seg2Coords[1][0], y: seg2Coords[1][1]}};


Pretty sure this can be handle by @turf/line-intersect.
We could include Array<Array<number>> support in line-intersect (geometry.coordinates)

See comment above about how the intersect module differs from line-intersect

+1 yep makes sense

DenisCarriere · 2017-05-11T00:24:40Z

packages/turf-line-offset/package.json

+  "devDependencies": {
+    "benchmark": "^1.0.0",
+    "tape": "^3.5.0",
+    "@turf/helpers": "^4.1.0"


Missing write-json-file & load-json-file devDependencies

DenisCarriere · 2017-05-11T02:30:49Z

To make the tests pass you might need to wrap the results with @turf/truncate since the error is coming from coordinates being slightly off of 0.000001.

rowanwins · 2017-05-11T13:43:22Z

Gday @DenisCarriere

Have just updated the whole looping approach and its now much tidier I think.

Only thing is the benchmark results look like they are slower than before but in reality I think it's now twice as fast, either a) my laptop is running on battery and goes slower, or b) I changed a benchmark test parameter. Anyway I've run the same benchmark test on both variations on the code this evening and the latest commit is twice as fast as the old.

Cheers

DenisCarriere · 2017-05-11T19:11:11Z

Woot! Passing tests, one of the reasons why I decided to build @turf/truncate, it was a total pain having 0.000001 offset causing the deepEqual to fail.

…sts.

rowanwins · 2017-05-12T01:13:42Z

Updated to support lines with only 1 segment and lines that are completely straight (horizontal and vertical).

rowanwins · 2017-05-12T01:17:06Z

PS Turns out that having my battery plugged in does effect the performance of my benchmark tests!

linestring-feature with changer === 1M ops/sec
linestring-feature with battery === 300K ops/sec

DenisCarriere · 2017-05-12T04:37:59Z

👍 Yea the benchmark results are very dependent on CPU performance, but it's a good gage of performance. Also closing 10+ Google Chrome browser windows help 🤓

DenisCarriere · 2017-05-17T01:18:03Z

@rowanwins Just an update on this PR, once we get a few other geometries supported this will be ready to merge.

Added Geometry Support details above

rowanwins · 2017-05-17T03:22:15Z

Gday @DenisCarriere

It did just cross my mind that this could conceivable be used as a replacement for a buffer module, particularly for polys... Not sure how performance would go but worth a quick trial.

DenisCarriere · 2017-05-17T04:22:26Z

Dropping Polygon or MultiPolygon support since this algorithm is not designed for this type of offset. Below are examples of attempts using line-offset on Polygons:

Red: Input
Blue: Results

1000 miles - Inner rings become larger than outer rings

-500 miles - Doesn't make any sense

Start & End vertices are disconnected (might be worth looking into for LineString)

CC: @rowanwins

DenisCarriere · 2017-05-17T04:25:47Z

be used as a replacement for a buffer module, particularly for polys

I thought so too! Minus the crazy outputs! Lets stick with LineStrings on this module.

I really like it so far, but not with Polygons.

DenisCarriere · 2017-05-17T04:33:53Z

@rowanwins Added MultiLineString support, I'm ready to merge this if ever you are 👍 with it.

DenisCarriere · 2017-05-17T04:41:13Z

@rowanwins Another issue that can be addressed at a later time is the Equidistance offset, same issue we had with @turf/buffer #718.

Issue in Northern Latitudes

50km offset north/south directions, 12km offset west/east direction

https://github.com/Turfjs/turf/blob/bc34c926a11a424992b9e08643603fa81d54329f/packages/turf-line-offset/test/out/northern-line.geojson

rowanwins · 2017-05-17T05:47:46Z

merge away. I might have a tinker with the polygon issue because im sure the code is there, just might need a bit of refactoring

stebogit · 2017-05-17T08:31:59Z

@rowanwins @DenisCarriere I believe the output with (Multi)Polygons is actually the correct/expected result for this module, as it creates a parallel (and elongated/shortened) line alongside the original one. A Polygon just happens to be a closed line, however the module I think should not connect the start and end points of the resulting line.
If that was the actual need, then it would probably be more appropriate a scale transformation (#747) of the Polygon.
Ergo, I'd say is correct applying this module only to (opened) lines.

DenisCarriere · 2017-05-17T15:21:15Z

the module I think should not connect the start and end points of the resulting line.

👍 Agreed

is correct applying this module only to (opened) lines.

Agreed, only supporting LineString would be best for this module.

stebogit · 2017-05-17T16:02:02Z

@rowanwins @DenisCarriere I believe these formulas might help to correct the distortion here and in other modules.

DenisCarriere · 2017-05-17T17:07:41Z

these formulas might help to correct the distortion here

That's what was used for @turf/buffer.
https://github.com/Turfjs/turf/blob/master/packages/turf-buffer/index.js#L117-L163

rowanwins · 2017-05-17T23:45:05Z

Hi @DenisCarriere & @stebogit

One thought on the whole distortion issue, my understanding is that projections have been designed differently, some projections are designed for preserving shape, while others are designed to preserve area etc etc

With the lines @DenisCarriere references above we're making that judgement for ppl as to what the best projection is (one that preserves shape). D3 actually supports a bunch which are useful for different purposes.

Rather than us forcing an choice on people perhaps we should be getting them to pass in their data with the projection they deem appropriate?

Thoughts?

stebogit · 2017-05-18T06:50:42Z

I personally don't have experience with projections different from (Web)Mercator (i.e. Google Maps).
I'm wondering also how many users use Turf in a non-Mercator map, being Pseudo-Mercator the

"de facto standard for Web mapping applications"

and Turf.js a JavaScript (i.e. web) library.

Just my thought.
I'd be genuinely curious to know about interesting applications in other environments with different projections.

DenisCarriere · 2017-05-18T07:18:17Z

Rather than us forcing an choice on people perhaps we should be getting them to pass in their data with the projection they deem appropriate?

👎 TurfJS shouldn't be providing projection support.

@w8r explained it well in #660 (comment).

The most important point is that you have to do the geometrical calculations in the R2 space where they make sense, so abstract equidistant does the trick.

DenisCarriere · 2017-05-18T07:25:15Z

@rowanwins Using that equidistance projection trick will solve the offset being different. This isn't a projection "visual effect", the offset is legitimately different (12km & 50km).

DenisCarriere · 2017-05-24T18:53:45Z

Going to merge this since it's 95% complete, only tiny issue is situations up north (can be done in the next release or a new PR).

@rowanwins @stebogit

rowanwins · 2017-05-24T23:14:27Z

Gday @DenisCarriere

Just checking back in on the projection discussion. I wasn't proposing for turf to support projections, there is already proj4js to do that. I just wanted to check that by forcing a projection during a process like buffer, we weren't setting ourselves up for trouble. It's been way too long since I thought about datums and projections so apologies for the misuse of terminology etc :)

DenisCarriere · 2017-05-25T03:53:48Z

Agree we should do the same as the buffer, however we should make the project/reproject process as a module or external dependency.

rowanwins added 2 commits May 11, 2017 09:36

Initial commit of lineOffset module

f9adf2b

Fix linting

d8e1f4b

DenisCarriere reviewed May 11, 2017

View reviewed changes

Fis dependencies and jsdocs

8be7b24

Updated looping approach. Fix test truncate.

275f29f

DenisCarriere added the new-module label May 11, 2017

DenisCarriere assigned rowanwins May 11, 2017

DenisCarriere added this to the 4.4.0 milestone May 11, 2017

rowanwins added 2 commits May 12, 2017 11:09

Add handling for straight lines and lines with only 1 segment. Inc te…

5e08ff5

…sts.

Test for a horizontal line for completeness

7153dd8

DenisCarriere added 2 commits May 12, 2017 00:53

Add Readme docs & update JSDocs

a420455

Remove local intersection module

d4a3d2b

DenisCarriere self-assigned this May 17, 2017

DenisCarriere added 3 commits May 16, 2017 21:56

Add prevent input mutation tests

1176d01

Separate intersection.js

34378db

Add switch statement for Geometry expansion

4199f1a

Add MultiLineString support

bc34c92

stebogit mentioned this pull request May 17, 2017

New module proposal: Transform #747

Closed

6 tasks

added line-concave test; renamed single-segment-only

a82fd5b

rowanwins mentioned this pull request May 22, 2017

Initial commit to fix the output for multipolys and polys with holes w8r/martinez#18

Closed

DenisCarriere merged commit 67ee226 into master May 24, 2017

DenisCarriere deleted the lineOffset branch May 24, 2017 18:54

This was referenced Jun 5, 2017

Implement parallel_offset #368

Closed

New minor release! Turf 4.4.0 🎉 #778

Closed

New module @turf/line-offset #729

New module @turf/line-offset #729

Conversation

rowanwins commented May 10, 2017 • edited by DenisCarriere Loading

New Module @turf/line-offset

Geometry Support

Geometry NOT supported

To-Do

JSDocs

Examples

DenisCarriere commented May 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DenisCarriere May 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DenisCarriere commented May 11, 2017

rowanwins commented May 11, 2017

DenisCarriere commented May 11, 2017

rowanwins commented May 12, 2017

rowanwins commented May 12, 2017

DenisCarriere commented May 12, 2017

DenisCarriere commented May 17, 2017 • edited Loading

rowanwins commented May 17, 2017

DenisCarriere commented May 17, 2017 • edited Loading

DenisCarriere commented May 17, 2017 • edited Loading

DenisCarriere commented May 17, 2017 • edited Loading

DenisCarriere commented May 17, 2017

Issue in Northern Latitudes

rowanwins commented May 17, 2017

stebogit commented May 17, 2017

DenisCarriere commented May 17, 2017

stebogit commented May 17, 2017 • edited Loading

DenisCarriere commented May 17, 2017 • edited Loading

rowanwins commented May 17, 2017

stebogit commented May 18, 2017

DenisCarriere commented May 18, 2017

DenisCarriere commented May 18, 2017 • edited Loading

DenisCarriere commented May 24, 2017 • edited Loading

rowanwins commented May 24, 2017

DenisCarriere commented May 25, 2017

rowanwins commented May 10, 2017 •

edited by DenisCarriere

Loading

New Module `@turf/line-offset`

DenisCarriere May 11, 2017 •

edited

Loading

DenisCarriere commented May 17, 2017 •

edited

Loading

DenisCarriere commented May 17, 2017 •

edited

Loading

DenisCarriere commented May 17, 2017 •

edited

Loading

DenisCarriere commented May 17, 2017 •

edited

Loading

stebogit commented May 17, 2017 •

edited

Loading

DenisCarriere commented May 17, 2017 •

edited

Loading

DenisCarriere commented May 18, 2017 •

edited

Loading

DenisCarriere commented May 24, 2017 •

edited

Loading