-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
createGroundPolylineGeometry dependencies, Scene cleanup #6946
Conversation
Thanks for the pull request @kring!
Reviewers, don't forget to make sure that:
I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome. 🌍 🌎 🌏 |
@kring is this generally helpful for anyone using webpack? If so, is it worth a mention in CHANGES.md? |
}); | ||
|
||
return GroundPolylinePrimitive._initPromise; | ||
return ApproximateTerrainHeights.initialize(); |
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.
Should we make this same change to GroundPrimitive.initializeTerrainHeights
?
That's my only comment, thanks @kring! |
I'm not sure. The fun thing about webpack is it has a billion configuration options, so other webpack configurations might not have the same problem. I'll add a note though, can't hurt. |
This is ready for another look. |
Thanks @kring! |
Hopefully fairly uncontroversial stuff here from our Cesium fork:
createGroundPolylineGeometry
onGroundPolylinePrimitive
. It was only needed for initializing terrain heights, which was delegated toApproximateTerrainHeights
anyway. This change should make that web worker smaller and in my webpack configuration, at least, it avoids a dependency cycle that makes the build hang.Scene
more consistent in how it destroys sub-objects. Random, inconsequential, but overall positive change I had kicking around.I don't these things need to be mentioned in CHANGES.md, but let me know if you disagree.