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

Check projection instead of tiling scheme #6524

Merged
merged 3 commits into from
May 2, 2018

Conversation

wallw-teal
Copy link
Contributor

This allows custom tiling scheme classes by actually checking the projection in the tiling scheme rather than the tiling scheme class itself.

The use case was outlined in the forum here. However, I went ahead and changed all the places that similar checks were being done, rather than just the one I mentioned in the forum.

This allows custom tiling scheme classes by actually checking
the projection in the tiling scheme rather than the tiling
scheme class itself.
@cesium-concierge
Copy link

Signed CLA is on file.

@wallw-bits, thanks for the pull request! Maintainers, we have a signed CLA from @wallw-bits, 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.

🌍 🌎 🌏

@wallw-teal
Copy link
Contributor Author

This should be a no-change update for Cesium itself. It only affects those attempting to use custom TilingScheme classes. Hence no update to CHANGES.md.

@hpinkos
Copy link
Contributor

hpinkos commented Apr 30, 2018

Thanks @wallw-bits! If this fixes a bug when you were trying to use a custom tiling scheme, you should make a note in CHANGES.md so other doing the same thing know it's fixed.

Who can review this? @kring do you have time to take a look?

@wallw-teal
Copy link
Contributor Author

CHANGES.md updated per your request.

@kring
Copy link
Member

kring commented Apr 30, 2018

Yep I'll take a look @hpinkos.

@kring
Copy link
Member

kring commented May 2, 2018

Looks good, thanks @wallw-bits!

@kring kring merged commit e4b3c75 into CesiumGS:master May 2, 2018
@cesium-concierge
Copy link

Congratulations on closing the issue! I found these Cesium forum links in the comments above:

https://groups.google.com/forum/#!topic/cesium-dev/a8hNaDBnAEw

If this issue affects any of these threads, please post a comment like the following:

The issue at #6524 has just been closed and may resolve your issue. Look for the change in the next stable release of Cesium or get it now in the master branch on GitHub https://github.com/AnalyticalGraphicsInc/cesium.


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

🌍 🌎 🌏

@wallw-teal wallw-teal deleted the fix-projection-checks branch May 2, 2018 14:37
@wallw-teal
Copy link
Contributor Author

I think this was merged after the release was cut, which makes the CHANGES.md file incorrect on master.

Should I make another PR to fix that?

mramato added a commit that referenced this pull request May 2, 2018
Move update from #6524 to next release (since it was merged after the release happened).
@mramato
Copy link
Contributor

mramato commented May 2, 2018

Good catch @wallw-bits, I fixed this directly in master so you don't have to worry about it 76d64a1

@NaderCHASER
Copy link
Contributor

I believe that this fixed using NASA GIBS' Geographic Tiling Scheme (https://github.com/nasa-gibs/gibs-web-examples/blob/master/lib/gibs/gibs.js) which previously produced a blurry image at lower zoom levels. Pulling master into my fork this morning fixed this issue.

I'm now able to use a custom GibsGeographicTilingScheme.js (based on gibs.js link above) to display Blue Marble and other NASA tiles without issue, whereas before I had to use their reprojected tiles for them to display correctly in Cesium.

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.

6 participants