Skip to content
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

Corridor outline #1099

Merged
merged 12 commits into from
Sep 3, 2013
Merged

Corridor outline #1099

merged 12 commits into from
Sep 3, 2013

Conversation

hpinkos
Copy link
Contributor

@hpinkos hpinkos commented Aug 30, 2013

No description provided.

@@ -81,7 +81,7 @@ var geometry = BoxGeometry.createGeometry(box);
* Fixed triangulation for polygons that cross the international date line.
* Fixed `EllipsoidPrimitive` rendering for some oblate ellipsoids. [#1067](https://github.com/AnalyticalGraphicsInc/cesium/pull/1067).
* Upgraded Knockout from version 2.2.1 to 2.3.0.
* Added `CorridorGeometry`.
* Added `CorridorGeometry` and `CorridorGeometryOutline.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may not make b20.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's alright, I'm just being optimistic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The end tick after CorridorGeometryOutline is missing.

var originScratch = new Cartesian3();
var nextScratch = new Cartesian3();
var prevScratch = new Cartesian3();
CorridorGeometryLibrary.angleIsGreaterThanPi = function(forward, backward, position, ellipsoid) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should add @private doc to all functions that can be accessed from CorridorGeometryLibrary.

@bagnell
Copy link
Contributor

bagnell commented Aug 30, 2013

@hpinkos Those are the only comments I have. They are mostly formatting and doc changes so you can probably get this into b20. Let me know when its ready for another review.

@pjcozzi
Copy link
Contributor

pjcozzi commented Aug 31, 2013

@hpinkos don't try to fit this into b20. The timing will make it too stressful. b21 is perfectly fine for this.

@pjcozzi
Copy link
Contributor

pjcozzi commented Aug 31, 2013

Extrusions look wrong.

    var corridorOutline = new Cesium.GeometryInstance({
        geometry: new Cesium.CorridorOutlineGeometry( {
            positions: positions,
            width: width,
            height: 700000,
            extrudedHeight: 100000
        }),
        attributes: {
            color : solidWhite
        }
    });

image

@pjcozzi
Copy link
Contributor

pjcozzi commented Aug 31, 2013

It wasn't added in this pull request, but CornerType needs better doc.

@pjcozzi
Copy link
Contributor

pjcozzi commented Aug 31, 2013

@bagnell somewhat unrelated to this pull request, but we should think about how we can pipe outline geometries into your upcoming polyline geometry and appearances so the outlines can have all the styling of the current PolylineCollection. It should be pretty easy.

@hpinkos
Copy link
Contributor Author

hpinkos commented Sep 3, 2013

@bagnell this is ready for review again

bagnell added a commit that referenced this pull request Sep 3, 2013
@bagnell bagnell merged commit 6c86ae0 into CesiumGS:master Sep 3, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants