-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Add Rhumb Line Support to Polygon and Polyline Geometries #7492
Changes from 22 commits
f473014
ba3a6dc
0248d9e
f90b6d4
5be47b2
7c60e88
d9db99a
6362be5
4c2141d
9cbc248
851a21e
39e6d95
739eb52
62b28a8
2745a2d
0f4e088
9ee96b7
a47ebd1
1dcdbe2
1883350
6584a8f
b2e70ed
81b2ecb
ce9f96b
96d25d5
763aa70
5f7a7ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,11 +13,13 @@ define([ | |
'./defineProperties', | ||
'./Ellipsoid', | ||
'./EllipsoidGeodesic', | ||
'./EllipsoidRhumbLine', | ||
'./EncodedCartesian3', | ||
'./GeographicProjection', | ||
'./Geometry', | ||
'./GeometryAttribute', | ||
'./IntersectionTests', | ||
'./LineType', | ||
'./Matrix3', | ||
'./Plane', | ||
'./Quaternion', | ||
|
@@ -38,11 +40,13 @@ define([ | |
defineProperties, | ||
Ellipsoid, | ||
EllipsoidGeodesic, | ||
EllipsoidRhumbLine, | ||
EncodedCartesian3, | ||
GeographicProjection, | ||
Geometry, | ||
GeometryAttribute, | ||
IntersectionTests, | ||
LineType, | ||
Matrix3, | ||
Plane, | ||
Quaternion, | ||
|
@@ -80,6 +84,7 @@ define([ | |
* @param {Number} [options.width=1.0] The screen space width in pixels. | ||
* @param {Number} [options.granularity=9999.0] The distance interval in meters used for interpolating options.points. Defaults to 9999.0 meters. Zero indicates no interpolation. | ||
* @param {Boolean} [options.loop=false] Whether during geometry creation a line segment will be added between the last and first line positions to make this Polyline a loop. | ||
* @param {LineType} [options.lineType=LineType.GEODESIC] The type of line the polyline segments must follow. Valid options are {@link LineType.GEODESIC} and {@link LineType.RHUMB}. | ||
* | ||
* @exception {DeveloperError} At least two positions are required. | ||
* | ||
|
@@ -104,6 +109,9 @@ define([ | |
if ((!defined(positions)) || (positions.length < 2)) { | ||
throw new DeveloperError('At least two positions are required.'); | ||
} | ||
if (defined(options.lineType) && options.lineType === LineType.STRAIGHT) { | ||
throw new DeveloperError('Only Geodesic and Rhumb line types are supported.'); | ||
} | ||
//>>includeEnd('debug'); | ||
|
||
/** | ||
|
@@ -130,6 +138,13 @@ define([ | |
*/ | ||
this.loop = defaultValue(options.loop, false); | ||
|
||
/** | ||
* The type of path the polyline must follow. Valid options are {@link LineType.GEODESIC} and {@link LineType.RHUMB}. | ||
* @type {LineType} | ||
* @default LineType.GEODESIC | ||
*/ | ||
this.lineType = defaultValue(options.lineType, LineType.GEODESIC); | ||
|
||
this._ellipsoid = Ellipsoid.WGS84; | ||
|
||
// MapProjections can't be packed, so store the index to a known MapProjection. | ||
|
@@ -150,7 +165,7 @@ define([ | |
*/ | ||
packedLength: { | ||
get: function() { | ||
return 1.0 + this._positions.length * 3 + 1.0 + 1.0 + Ellipsoid.packedLength + 1.0 + 1.0; | ||
return 1.0 + this._positions.length * 3 + 1.0 + 1.0 + 1.0 + Ellipsoid.packedLength + 1.0 + 1.0; | ||
} | ||
} | ||
}); | ||
|
@@ -195,12 +210,19 @@ define([ | |
var interpolatedBottomScratch = new Cartesian3(); | ||
var interpolatedTopScratch = new Cartesian3(); | ||
var interpolatedNormalScratch = new Cartesian3(); | ||
function interpolateSegment(start, end, minHeight, maxHeight, granularity, ellipsoid, normalsArray, bottomPositionsArray, topPositionsArray, cartographicsArray) { | ||
function interpolateSegment(start, end, minHeight, maxHeight, granularity, lineType, ellipsoid, normalsArray, bottomPositionsArray, topPositionsArray, cartographicsArray) { | ||
if (granularity === 0.0) { | ||
return; | ||
} | ||
var ellipsoidGeodesic = new EllipsoidGeodesic(start, end, ellipsoid); | ||
var surfaceDistance = ellipsoidGeodesic.surfaceDistance; | ||
|
||
var ellipsoidLine; | ||
if (lineType === LineType.GEODESIC) { | ||
ellipsoidLine = new EllipsoidGeodesic(start, end, ellipsoid); | ||
} else if (lineType === LineType.RHUMB) { | ||
shehzan10 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ellipsoidLine = new EllipsoidRhumbLine(start, end, ellipsoid); | ||
} | ||
|
||
var surfaceDistance = ellipsoidLine.surfaceDistance; | ||
if (surfaceDistance < granularity) { | ||
return; | ||
} | ||
|
@@ -214,7 +236,7 @@ define([ | |
var pointsToAdd = segments - 1; | ||
var packIndex = normalsArray.length; | ||
for (var i = 0; i < pointsToAdd; i++) { | ||
var interpolatedCartographic = ellipsoidGeodesic.interpolateUsingSurfaceDistance(distanceFromStart, interpolatedCartographicScratch); | ||
var interpolatedCartographic = ellipsoidLine.interpolateUsingSurfaceDistance(distanceFromStart, interpolatedCartographicScratch); | ||
var interpolatedBottom = getPosition(ellipsoid, interpolatedCartographic, minHeight, interpolatedBottomScratch); | ||
var interpolatedTop = getPosition(ellipsoid, interpolatedCartographic, maxHeight, interpolatedTopScratch); | ||
|
||
|
@@ -266,6 +288,7 @@ define([ | |
|
||
array[index++] = value.granularity; | ||
array[index++] = value.loop ? 1.0 : 0.0; | ||
array[index++] = value.lineType; | ||
|
||
Ellipsoid.pack(value._ellipsoid, array, index); | ||
index += Ellipsoid.packedLength; | ||
|
@@ -299,6 +322,7 @@ define([ | |
|
||
var granularity = array[index++]; | ||
var loop = array[index++] === 1.0; | ||
var lineType = array[index++]; | ||
|
||
var ellipsoid = Ellipsoid.unpack(array, index); | ||
index += Ellipsoid.packedLength; | ||
|
@@ -311,6 +335,7 @@ define([ | |
positions : positions, | ||
granularity : granularity, | ||
loop : loop, | ||
lineType : lineType, | ||
ellipsoid : ellipsoid | ||
}); | ||
geometry._projectionIndex = projectionIndex; | ||
|
@@ -321,6 +346,7 @@ define([ | |
result._positions = positions; | ||
result.granularity = granularity; | ||
result.loop = loop; | ||
result.lineType = lineType; | ||
result._ellipsoid = ellipsoid; | ||
result._projectionIndex = projectionIndex; | ||
result._scene3DOnly = scene3DOnly; | ||
|
@@ -384,6 +410,9 @@ define([ | |
var nextBottomScratch = new Cartesian3(); | ||
var vertexNormalScratch = new Cartesian3(); | ||
var intersectionScratch = new Cartesian3(); | ||
var cartographicScratch0 = new Cartographic(); | ||
var cartographicScratch1 = new Cartographic(); | ||
var cartographicIntersectionScratch = new Cartographic(); | ||
/** | ||
* Computes shadow volumes for the ground polyline, consisting of its vertices, indices, and a bounding sphere. | ||
* Vertices are "fat," packing all the data needed in each volume to describe a line on terrain or 3D Tiles. | ||
|
@@ -397,6 +426,7 @@ define([ | |
var loop = groundPolylineGeometry.loop; | ||
var ellipsoid = groundPolylineGeometry._ellipsoid; | ||
var granularity = groundPolylineGeometry.granularity; | ||
var lineType = groundPolylineGeometry.lineType; | ||
var projection = new PROJECTIONS[groundPolylineGeometry._projectionIndex](ellipsoid); | ||
|
||
var minHeight = WALL_INITIAL_MIN_HEIGHT; | ||
|
@@ -417,7 +447,12 @@ define([ | |
// may get split by the plane of IDL + Prime Meridian. | ||
var p0; | ||
var p1; | ||
var c0; | ||
var c1; | ||
var rhumbLine = new EllipsoidRhumbLine(undefined, undefined, ellipsoid); | ||
var intersection; | ||
var intersectionCartographic; | ||
var intersectionLongitude; | ||
var splitPositions = [positions[0]]; | ||
for (i = 0; i < positionsLength - 1; i++) { | ||
p0 = positions[i]; | ||
|
@@ -426,7 +461,21 @@ define([ | |
if (defined(intersection) && | ||
!Cartesian3.equalsEpsilon(intersection, p0, CesiumMath.EPSILON7) && | ||
!Cartesian3.equalsEpsilon(intersection, p1, CesiumMath.EPSILON7)) { | ||
splitPositions.push(Cartesian3.clone(intersection)); | ||
if (groundPolylineGeometry.lineType === LineType.GEODESIC) { | ||
splitPositions.push(Cartesian3.clone(intersection)); | ||
} else if (groundPolylineGeometry.lineType === LineType.RHUMB) { | ||
intersectionLongitude = ellipsoid.cartesianToCartographic(intersection, cartographicScratch0).longitude; | ||
c0 = ellipsoid.cartesianToCartographic(p0, cartographicScratch0); | ||
c1 = ellipsoid.cartesianToCartographic(p1, cartographicScratch1); | ||
rhumbLine.setEndPoints(c0, c1); | ||
intersectionCartographic = rhumbLine.findIntersectionWithLongitude(intersectionLongitude, cartographicIntersectionScratch); | ||
intersection = ellipsoid.cartographicToCartesian(intersectionCartographic, intersectionScratch); | ||
if (defined(intersection) && | ||
!Cartesian3.equalsEpsilon(intersection, p0, CesiumMath.EPSILON7) && | ||
!Cartesian3.equalsEpsilon(intersection, p1, CesiumMath.EPSILON7)) { | ||
splitPositions.push(Cartesian3.clone(intersection)); | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know if this will have an impact on javascript optimization, but this could be easier to read as two loops within There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this one is better the way it is. If we do something like
we'll need to duplicate the initial intersection test code. Since the test will always pass or always fail inside the for-loop, branch prediction should work well and minimize any performance drops. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're probably right that it won't make a difference for performance, but it still just feels kind of "off" to me reading that there's a boolean check with the same result repeated so many times. I'd still prefer separate loops, but it's not a huge deal. |
||
} | ||
splitPositions.push(p1); | ||
} | ||
|
@@ -438,7 +487,21 @@ define([ | |
if (defined(intersection) && | ||
!Cartesian3.equalsEpsilon(intersection, p0, CesiumMath.EPSILON7) && | ||
!Cartesian3.equalsEpsilon(intersection, p1, CesiumMath.EPSILON7)) { | ||
splitPositions.push(Cartesian3.clone(intersection)); | ||
if (groundPolylineGeometry.lineType === LineType.GEODESIC) { | ||
splitPositions.push(Cartesian3.clone(intersection)); | ||
} else if (groundPolylineGeometry.lineType === LineType.RHUMB) { | ||
intersectionLongitude = ellipsoid.cartesianToCartographic(intersection, cartographicScratch0).longitude; | ||
c0 = ellipsoid.cartesianToCartographic(p0, cartographicScratch0); | ||
c1 = ellipsoid.cartesianToCartographic(p1, cartographicScratch1); | ||
rhumbLine.setEndPoints(c0, c1); | ||
intersectionCartographic = rhumbLine.findIntersectionWithLongitude(intersectionLongitude, cartographicIntersectionScratch); | ||
intersection = ellipsoid.cartographicToCartesian(intersectionCartographic, intersectionScratch); | ||
if (defined(intersection) && | ||
!Cartesian3.equalsEpsilon(intersection, p0, CesiumMath.EPSILON7) && | ||
!Cartesian3.equalsEpsilon(intersection, p1, CesiumMath.EPSILON7)) { | ||
splitPositions.push(Cartesian3.clone(intersection)); | ||
} | ||
} | ||
} | ||
} | ||
var cartographicsLength = splitPositions.length; | ||
|
@@ -495,7 +558,7 @@ define([ | |
cartographicsArray.push(startCartographic.latitude); | ||
cartographicsArray.push(startCartographic.longitude); | ||
|
||
interpolateSegment(startCartographic, nextCartographic, minHeight, maxHeight, granularity, ellipsoid, normalsArray, bottomPositionsArray, topPositionsArray, cartographicsArray); | ||
interpolateSegment(startCartographic, nextCartographic, minHeight, maxHeight, granularity, lineType, ellipsoid, normalsArray, bottomPositionsArray, topPositionsArray, cartographicsArray); | ||
|
||
// All inbetween points | ||
for (i = 1; i < cartographicsLength - 1; ++i) { | ||
|
@@ -514,7 +577,7 @@ define([ | |
cartographicsArray.push(vertexCartographic.latitude); | ||
cartographicsArray.push(vertexCartographic.longitude); | ||
|
||
interpolateSegment(cartographics[i], cartographics[i + 1], minHeight, maxHeight, granularity, ellipsoid, normalsArray, bottomPositionsArray, topPositionsArray, cartographicsArray); | ||
interpolateSegment(cartographics[i], cartographics[i + 1], minHeight, maxHeight, granularity, lineType, ellipsoid, normalsArray, bottomPositionsArray, topPositionsArray, cartographicsArray); | ||
} | ||
|
||
// Last point - either loop or attach a normal "perpendicular" to the wall. | ||
|
@@ -542,7 +605,7 @@ define([ | |
cartographicsArray.push(endCartographic.longitude); | ||
|
||
if (loop) { | ||
interpolateSegment(endCartographic, startCartographic, minHeight, maxHeight, granularity, ellipsoid, normalsArray, bottomPositionsArray, topPositionsArray, cartographicsArray); | ||
interpolateSegment(endCartographic, startCartographic, minHeight, maxHeight, granularity, lineType, ellipsoid, normalsArray, bottomPositionsArray, topPositionsArray, cartographicsArray); | ||
index = normalsArray.length; | ||
for (i = 0; i < 3; ++i) { | ||
normalsArray[index + i] = normalsArray[i]; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At demo day, didn't we talk about coming up with a better name than
STRAIGHT
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can think of
DIRECT
. Do you have any suggestions?I do think
STRAIGHT
is a fair option since the enum isLineType
, so we get Geodesic line, Rhumb line and Straight line.Anyone else have any suggestions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer
STRAIGHT
, why do we think it would be confusing?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the enum name itself,
LineType
is open to change. As I wrote in the PR description, options I can think of areCurveType
PathType
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My only idea is something super pedantic like
NOT_CURVED_TO_ELLIPSOID_SURFACE
lolThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel super strongly, but I thought it was something Patrick pointed out during demo day
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about
ArcType
GEODESIC
RHUMB
orNONE
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rhumb lines are straight only in a Mercator projection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^ ^ ^
Proof that "STRAIGHT" is confusing. @GatorScott in this case we were using 'STRAIGHT' for a line with neither rhumb or geodesic subdivision that connects two points in space. ie a line from the ground to a satellite.
I'm in favor of switching to my suggestion above:
ArcType
GEODESIC
RHUMB
orNONE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not against the proposed ArcType enum and values, I tried to google around and didn't see anything anyone else was already doing that we could reference.