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

Fixed typo when creating GroundPrimitives. #4165

Merged
merged 4 commits into from
Aug 2, 2016

Conversation

tfili
Copy link
Contributor

@tfili tfili commented Aug 1, 2016

GroundPrimitives changed its interface to accept more than one GeometryInstance at some point and the dynamic geometries that used them weren't updated.

Not exactly sure how to test this, as I can't verify a primitive has any GeometryInstances once its created by the dynamic geometry updater.

CC #4160 @hpinkos

@@ -138,8 +138,10 @@
}
}];

var viewer = new Cesium.Viewer('cesiumContainer');
viewer.dataSources.add(Cesium.CzmlDataSource.load(czml));
Cesium.GroundPrimitive.initializeTerrainHeights().then(function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed here? The Entity visualization system should already be doing this automatically for the user, if it's not then we need to add code to make that happen; not force users to do it themselves.

Tom Fili added 2 commits August 2, 2016 10:32
… Added a test that makes sure update returns false till the terrain heights promise resolves.
@tfili
Copy link
Contributor Author

tfili commented Aug 2, 2016

@mramato Should be good to go.

@mramato
Copy link
Contributor

mramato commented Aug 2, 2016

There are some failing tests.

… where we need them and uninitialize when done, so we don't effect other tests.
@mramato
Copy link
Contributor

mramato commented Aug 2, 2016

Thanks @tfili!

@mramato mramato merged commit 2671660 into master Aug 2, 2016
@mramato mramato deleted the dynamic-geometries-on-terrain branch August 2, 2016 15:42
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.

2 participants