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

Fixed issue where we constantly fail to load the same image. #6710

Merged
merged 3 commits into from
Jun 21, 2018

Conversation

tfili
Copy link
Contributor

@tfili tfili commented Jun 20, 2018

Fixes #6567

Can easily be tested with the following. Before this fix it tries to load etna.jpg hundreds of times.

var viewer = new Cesium.Viewer('cesiumContainer');
var options = {
    camera : viewer.scene.camera,
    canvas : viewer.scene.canvas
};

viewer.dataSources.add(Cesium.KmlDataSource.load('https://gist.githubusercontent.com/shunter/03d24977213616ae9992f443d3c051ea/raw/6d2aeffc8e667b9100a2875388d19e9c64b945fd/KML_Samples.kml', options));

@cesium-concierge
Copy link

Signed CLA is on file.

@tfili, thanks for the pull request! Maintainers, we have a signed CLA from @tfili, so you can review this at any time.

⚠️ I noticed that CHANGES.md has not been updated. If this change updates the public API in any way, fixes a bug, or makes any non-trivial update, please add a bullet point to CHANGES.md and comment on this pull request so we know it was updated. For more info, see the Pull Request Guidelines.


I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

@tfili tfili requested a review from hpinkos June 20, 2018 16:23
@hpinkos
Copy link
Contributor

hpinkos commented Jun 20, 2018

@tfili is there a good way write a unit test to make sure the material only tries to load the resource once?

Other than that this change looks fine to me

@tfili
Copy link
Contributor Author

tfili commented Jun 20, 2018

@tfili is there a good way write a unit test to make sure the material only tries to load the resource once?

It doesn't look like it. It'll be pretty difficult and a ton of mocking to test this. I don't think it's worth the time it'll take.

@hpinkos
Copy link
Contributor

hpinkos commented Jun 21, 2018

Thanks @tfili

@hpinkos hpinkos merged commit 1ccd9e6 into master Jun 21, 2018
@hpinkos hpinkos deleted the kml-bad-image-fix branch June 21, 2018 15:10
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KML Image Overlays crash Cesium and Browser
3 participants