-
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
Frustum geometry #5649
Frustum geometry #5649
Conversation
Submitted #5651 |
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.
Just those comments.
|
||
var frustumGeometry = new Cesium.FrustumGeometry({ | ||
frustum : frustum, | ||
position : positionOnEllipsoid, |
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 guest position
is good since it is consistent with the rest of the Cesium API; I wonder if there is something better in the context of frustum like viewer
or origin
(e.g., we use origin
for a ray, not position
).
Can you look around and come up with the best name?
Cesium.Matrix3.multiply(rotation, Cesium.Matrix3.fromRotationX(-Cesium.Math.PI_OVER_TWO), rotation); | ||
var orientation = Cesium.Quaternion.fromRotationMatrix(rotation); | ||
|
||
var frustum = new Cesium.PerspectiveFrustum(); |
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.
Does this have an options
constructor we can use?
* @constructor | ||
* | ||
* @param {Object} options Object with the following properties: | ||
* @param {PerspectiveFrustum|OrthographicFrustum} options.frustum The frustum. |
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.
Is it easy to compute the input to a box geometry given an orthographic frustum? If so, perhaps this should only take a perspective frustum and be renamed.
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.
Sure, but I think it's simpler this way.
Source/Core/FrustumGeometry.js
Outdated
var orientation = options.orientation; | ||
var position = options.position; | ||
var vertexFormat = defaultValue(options.vertexFormat, VertexFormat.DEFAULT); | ||
var drawNearPlane = defaultValue(options._drawNearPlane, true); |
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 a comment about why this is private...or I wonder if we would want it to be public.
* @alias FrustumOutlineGeometry | ||
* @constructor | ||
* | ||
* @param {Object} options Object with the following properties: |
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.
Same comments, of course.
* @param {Cartesian3} options.position The position of the frustum. | ||
* @param {Quaternion} options.orientation The orientation of the frustum. | ||
*/ | ||
function FrustumOutlineGeometry(options) { |
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.
Is it possible for this to share any code with FrustumGeometry
since all the positions are the same?
Or is that more trouble than it is worth?
@@ -246,7 +246,7 @@ define([ | |||
array[startingIndex++] = value.st ? 1.0 : 0.0; | |||
array[startingIndex++] = value.tangent ? 1.0 : 0.0; | |||
array[startingIndex++] = value.bitangent ? 1.0 : 0.0; | |||
array[startingIndex++] = value.color ? 1.0 : 0.0; | |||
array[startingIndex] = value.color ? 1.0 : 0.0; |
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.
Good eye
Is there anywhere else that needs to be updated, e.g., is this used at all for 3D Tiles or terrain debugging? |
No, it's only |
@pjcozzi This is ready for another look. |
Very nice! |
Adds
FrustumGeometry
andFrustumOutlineGeometry