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

Tileset content.url to content.uri #6744

Merged
merged 6 commits into from
Jul 5, 2018
Merged

Tileset content.url to content.uri #6744

merged 6 commits into from
Jul 5, 2018

Conversation

ggetz
Copy link
Contributor

@ggetz ggetz commented Jun 27, 2018

Part of #6697

To reflect updates to the 3D Tiles spec, deprecated support for 3D Tiles content.url in favor of content.uri.

Also added a test to make sure tilesets using data uri's for their content load and render.

TODO:

@cesium-concierge
Copy link

Signed CLA is on file.

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

🌍 🌎 🌏

@ggetz ggetz requested a review from lilleyse June 27, 2018 21:05
CHANGES.md Outdated
@@ -8,6 +8,8 @@ Change Log
* Dropped support for directory URLs when loading tilesets to match the updated [3D Tiles spec](https://github.com/AnalyticalGraphicsInc/3d-tiles/issues/272). [#6502](https://github.com/AnalyticalGraphicsInc/cesium/issues/6502)
* KML and GeoJSON now use `PolylineGraphics` instead of `CorridorGraphics` for polylines on terrain. [#6706](https://github.com/AnalyticalGraphicsInc/cesium/pull/6706)

* Support for 3D Tiles `content.url` is deprecated to reflect updates to the [3D Tiles spec](https://github.com/AnalyticalGraphicsInc/3d-tiles/pull/301) and will be removed in Cesium 1.50. Use `content.uri instead`. [#6744](https://github.com/AnalyticalGraphicsInc/cesium/pull/6744)
Copy link
Contributor

Choose a reason for hiding this comment

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

whitespace above this

Copy link
Contributor

@lilleyse lilleyse left a comment

Choose a reason for hiding this comment

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

Looks good @ggetz!

@@ -0,0 +1,41 @@
{
"asset": {
"version": "0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Set to version 1.0.

CHANGES.md Outdated
@@ -8,6 +8,9 @@ Change Log
* Dropped support for directory URLs when loading tilesets to match the updated [3D Tiles spec](https://github.com/AnalyticalGraphicsInc/3d-tiles/issues/272). [#6502](https://github.com/AnalyticalGraphicsInc/cesium/issues/6502)
* KML and GeoJSON now use `PolylineGraphics` instead of `CorridorGraphics` for polylines on terrain. [#6706](https://github.com/AnalyticalGraphicsInc/cesium/pull/6706)

##### Deprecated :hourglass_flowing_sand:
* Support for 3D Tiles `content.url` tileset JSON property is deprecated to reflect updates to the [3D Tiles spec](https://github.com/AnalyticalGraphicsInc/3d-tiles/pull/301) and will be removed in Cesium 1.50. Use `content.uri instead`. [#6744](https://github.com/AnalyticalGraphicsInc/cesium/pull/6744)

Copy link
Contributor

Choose a reason for hiding this comment

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

Given the discussion in #5363, remove the note about being removed in Cesium 1.50 and mention that it is deprecated but will be kept around for backwards compatibility purposes.

CHANGES.md Outdated
@@ -8,6 +8,9 @@ Change Log
* Dropped support for directory URLs when loading tilesets to match the updated [3D Tiles spec](https://github.com/AnalyticalGraphicsInc/3d-tiles/issues/272). [#6502](https://github.com/AnalyticalGraphicsInc/cesium/issues/6502)
* KML and GeoJSON now use `PolylineGraphics` instead of `CorridorGraphics` for polylines on terrain. [#6706](https://github.com/AnalyticalGraphicsInc/cesium/pull/6706)

##### Deprecated :hourglass_flowing_sand:
* Support for 3D Tiles `content.url` tileset JSON property is deprecated to reflect updates to the [3D Tiles spec](https://github.com/AnalyticalGraphicsInc/3d-tiles/pull/301) and will be removed in Cesium 1.50. Use `content.uri instead`. [#6744](https://github.com/AnalyticalGraphicsInc/cesium/pull/6744)

Copy link
Contributor

Choose a reason for hiding this comment

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

"Use content.uri instead" -> "Use content.uri instead"

CHANGES.md Outdated
@@ -8,6 +8,9 @@ Change Log
* Dropped support for directory URLs when loading tilesets to match the updated [3D Tiles spec](https://github.com/AnalyticalGraphicsInc/3d-tiles/issues/272). [#6502](https://github.com/AnalyticalGraphicsInc/cesium/issues/6502)
* KML and GeoJSON now use `PolylineGraphics` instead of `CorridorGraphics` for polylines on terrain. [#6706](https://github.com/AnalyticalGraphicsInc/cesium/pull/6706)

##### Deprecated :hourglass_flowing_sand:
* Support for 3D Tiles `content.url` tileset JSON property is deprecated to reflect updates to the [3D Tiles spec](https://github.com/AnalyticalGraphicsInc/3d-tiles/pull/301) and will be removed in Cesium 1.50. Use `content.uri instead`. [#6744](https://github.com/AnalyticalGraphicsInc/cesium/pull/6744)

Copy link
Contributor

Choose a reason for hiding this comment

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

I was late for 1.47, so just move up to the 1.48 section.

@@ -117,6 +117,8 @@ defineSuite([
var tilesetReplacementWithViewerRequestVolumeUrl = 'Data/Cesium3DTiles/Tilesets/TilesetReplacementWithViewerRequestVolume/tileset.json';

var tilesetWithExternalResourcesUrl = 'Data/Cesium3DTiles/Tilesets/TilesetWithExternalResources/tileset.json';
var tilesetUrlWithContentUri = 'Data/Cesium3DTiles/Tilesets/TilesetWithContentUri/tileset.json';
Copy link
Contributor

Choose a reason for hiding this comment

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

Will there be a pull request into https://github.com/AnalyticalGraphicsInc/3d-tiles-tools for this new tileset?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created PR CesiumGS/3d-tiles-validator#149 for running changes. I would think review that last after the Cesium changes.

@ggetz
Copy link
Contributor Author

ggetz commented Jul 5, 2018

Thanks @lilleyse , updated!

@lilleyse
Copy link
Contributor

lilleyse commented Jul 5, 2018

Thanks!

@lilleyse lilleyse merged commit b387130 into master Jul 5, 2018
@lilleyse lilleyse deleted the tileset-url-to-uri branch July 5, 2018 15:28
# 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.

4 participants