-
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
Wrap Longitude #502
Wrap Longitude #502
Conversation
…d IntersectionTests.lineSegmentPlane method.
@@ -20,10 +20,12 @@ Beta Releases | |||
* Added `Transforms.computeIcrfToFixedMatrix` and `computeFixedToIcrfMatrix`. | |||
* Added `EarthOrientationParameters`, `EarthOrientationParametersSample`, `Iau2006XysData`, and `Iau2006XysDataSample` classes to `Core`. | |||
* CZML now supports the ability to specify positions in the International Celestial Reference Frame (ICRF), and inertial reference frame. | |||
* Added a line segment-ray intersection test to `IntersectionTests`. |
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.
Add these to b14
@@ -361,8 +361,8 @@ define([ | |||
return positions; | |||
}; | |||
|
|||
Polyline.prototype._createSegments = function(ellipsoid) { | |||
return PolylinePipeline.wrapLongitude(ellipsoid, this.getPositions()); | |||
Polyline.prototype._createSegments = function(modelMatrix) { |
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.
This function is only one line now. Do we still need it? Actually, the whole segment management in PolylineCollection.js
could use some work. However, we can hold off if we think there are bigger changes to come with the material improvements.
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 we should wait until we start the material improvements.
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.
Yeap, OK with me.
@pjcozzi This is ready for another review. |
@mramato the code is good with me. Do you want to test and merge? |
Looks good, merging. |
This fixes #499 and #474.
PolylinePipeline.wrapLongitude
now takes a model matrix into account when computing line segments of a polyline that intersects the IDL.IntersectionTest.lineSegmentPlane
method.