-
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
Polyline volume #1125
Polyline volume #1125
Conversation
VertexFormat, | ||
WindingOrder) { | ||
"use strict"; | ||
|
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.
Remove whitespace.
Did you use the debugging aids to visualize the computed bounding sphere, texture coordinates, and normal/binormal/tangents? |
Don't forget to update the tutorial. Even before we merge is fine; the tutorial is still a draft and I expect we'll merge this quickly. |
var tube = new Cesium.GeometryInstance({ | ||
geometry: new Cesium.PolylineVolumeGeometry({ | ||
polylinePositions : positions, | ||
vertexFormat: Cesium.VertexFormat.ALL, |
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.
Whitespace before the :
throughout. Sorry. Probably should run the formatter assuming it works on HTML files.
The Sandcastle example is pretty awesome. |
I made a small change to the Sandcastle example:
In general, I would expect that the 2D origin in the plane of the polygon would be centered along the polyline, and then the |
We should take into account the height of each position in |
The missing triangles is related to Issue #1093. If you switch the order of the geometries in the |
Some of the shading can be jarring because we are using averaged vertex normals. For the wall and other primitives (including the corridor), we duplicate the vertex so we can have better normals. We'll need to do that here for the corridor primitive to use it. We should just be able to post-process the polygon (make a copy!) before extruding it. I would do it based on a heuristic of the angle between adjacent edges so we only duplicate vertices that need to be. This can be a generic function in |
@pjcozzi I am duplicating the points: |
OK. |
var indicesCount = (length - 1) * (shapeLength) * 6 + firstEndIndices.length * 2; | ||
var indices = IndexDatatype.createTypedArray(vertexCount, indicesCount); | ||
var i, j; | ||
var LL, UL, UR, LR; |
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.
We generally only use upper case for constants.
var pos = positions[i]; | ||
cartographic = ellipsoid.cartesianToCartographic(pos, cartographic); | ||
heights[i] = cartographic.height; | ||
positions[i] = ellipsoid.scaleToGeodeticSurface(pos); |
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.
Can you use a result parameter here?
@bagnell I fixed most of the things you mentioned, but I think I'll need help fixing the texture coordinates. After that, hopefully the tangents/binormals will better. Currently I'm using |
|
||
function computeRotationAngle(start, end, position, ellipsoid) { | ||
var tangentPlane = new EllipsoidTangentPlane(position, ellipsoid); | ||
var origin = tangentPlane.projectPointOntoPlane(position, originScratch); |
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 will always be Cartesian2.ZERO
. You can remove this and the subtractions below.
I'm still seeing an issue similar to what @pjcozzi was seeing: var blueStar = new Cesium.GeometryInstance({
geometry : new Cesium.PolylineVolumeGeometry({
polylinePositions : ellipsoid.cartographicArrayToCartesianArray([
Cesium.Cartographic.fromDegrees(-95.0, 32.0, 0.0),
Cesium.Cartographic.fromDegrees(-95.0, 36.0, 100000.0),
Cesium.Cartographic.fromDegrees(-95.0, 40.0, 100000.0)
]),
vertexFormat : Cesium.PerInstanceColorAppearance.VERTEX_FORMAT,
shapePositions : starPositions(7, 70000, 50000),
cornerType : Cesium.CornerType.ROUNDED
}),
attributes : {
color : Cesium.ColorGeometryInstanceAttribute.fromColor(Cesium.Color.BLUE)
}
}); |
@bagnell alright, this is ready for review |
Looks good. Merging. |
Geometry that takes a line and a polygon as parameters, then extrudes the polygon over the line. Has the option to round, miter or bevel corners.