Extrude polygon #855
Extrude polygon #855
Conversation
Conflicts: Apps/CesiumViewer/CesiumViewer.js Source/Core/PolygonGeometry.js
Conflicts: Apps/CesiumViewer/CesiumViewer.js
| geometry.geometry = computeTopBottomAttributes(vertexFormat, geometry.geometry, outerPositions, ellipsoid, stRotation, true, false); | ||
| } | ||
|
|
||
| if (typeof geometry !== 'undefined') { |
There was a problem hiding this comment.
You can get rid of this check because it was already made.
| var length = positions.length / 2; | ||
|
|
||
| for ( var i = 0; i < length; i += 3) { | ||
| p.x = positions[i]; |
There was a problem hiding this comment.
Use Cartesian3.fromArray here.
|
@hpinkos Remember to update |
Conflicts: Apps/CesiumViewer/CesiumViewer.js
|
See 876718c for Also, the batching branch has some Polygon-related test failures. Ignore them for now. Most will be fixed when we finish Columbus view. |
The initial Polygon code also did not account for the |
|
@pjcozzi Alright, I think I corrected everything you mentioned above. |
| @@ -194,6 +773,19 @@ define([ | |||
| * }] | |||
| * } | |||
| * }); | |||
| * | |||
| * create extruded polygon | |||
There was a problem hiding this comment.
Make this a comment.
// create extruded polygon
|
@bagnell some texture coordinates are beyond |
|
Are we sure the normals are right? Given normals in WGS84 coordinates, shouldn't the green face be red? Also since normals range from [-1. 1]. The current |
|
Pull f2aea0a for slightly better normal visualization. Also verify the binormals and tangents. |
|
@hpinkos have you verified all the attributes? Is this ready for another review? |
Where do we stand with this? Is this code ready for another review? @hpinkos checkout 8d34314 to visualize your normals, binormals, and tangents.
Any update here? Anything need fixes beyond walls? |
|
@pjcozzi This is ready for another review. I'll let you know if I find any other primitives with incorrect normals/binormals/tangents. The one's I've looked at so far look correct. |
|
There are some test failures. |
|
@bagnell this is ready for review |
Conflicts: Apps/CesiumViewer/CesiumViewer.js Source/Core/PolygonGeometry.js
|
@hpinkos Looks good. Merge in master and open a pull request to that then I'll merge. |
No description provided.