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

GeoJsonDataSource should have a process method similar to CzmlDataSource #9275

Open
thw0rted opened this issue Dec 10, 2020 · 1 comment
Open

Comments

@thw0rted
Copy link
Contributor

thw0rted commented Dec 10, 2020

The current implementation of GeoJsonDataSource does not give any way to dynamically load features in multiple batches, because it always clears the entity collection when load is called. CzmlDataSource offers a process method which simply calls the underlying (private) load method without clearing existing entities. This method should be added to GeoJsonDataSource for consistency.

(I previously had a comment here about doing the same thing for KmlDataSource, but on reflection that doesn't make sense. CZML and GeoJson are based on packets/features and it makes sense to support stream-wise loading; KML is a document format that usually includes e.g. style references and it makes sense to only support loading when the whole thing is available.)

@thw0rted
Copy link
Contributor Author

I put together a simple PR adding this to GeoJsonDataSource, because it was trivial. I'm reasonably confident that the current implementation will handle processing various kinds of input without inducing any weird behavior. I added one test, to call load with some features, and then process with additional features, and that works. More tests could be added to poke at edge cases but I suspect it's fine. (The team could of course have insight to the contrary!)

@thw0rted thw0rted changed the title All file-based data sources should have a process method similar to CzmlDataSource GeoJsonDataSource should have a process method similar to CzmlDataSource Dec 10, 2020
thw0rted added a commit to thw0rted/cesium that referenced this issue Apr 12, 2022
thw0rted added a commit to thw0rted/cesium that referenced this issue Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants