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

Adds 3DTILES_implicit_tiling extension #414

Closed
wants to merge 70 commits into from

Conversation

sanjeetsuhag
Copy link
Contributor

@sanjeetsuhag sanjeetsuhag commented Jun 4, 2020

This extension enables 3D Tiles to better support implicit tiling schemes. Specification is available here.

@sanjeetsuhag sanjeetsuhag marked this pull request as draft June 4, 2020 14:38
@pjcozzi
Copy link
Contributor

pjcozzi commented Jun 4, 2020

@sanjeetsuhag congrats on this! Please bump to me when it is ready for me to review. 😄

CC @lilleyse

@lilleyse lilleyse force-pushed the 3DTILES_implicit_tiling branch 3 times, most recently from 166c0ba to 47372fe Compare June 4, 2020 23:03
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.

I'll do a second pass in a little bit but I wanted to get this feedback out early. This is a great first draft. Structurally the concepts are presented well.


![Levels](figures/levels.png)

Every level of the tree can be thought of as a fixed grid of equally sized tiles, where the level occupies the same space as the previous level but with double the amount of tiles along each axis that gets split (2 in case of [quadtree](https://en.wikipedia.org/wiki/Quadtree) and 3 in case of [octree](https://en.wikipedia.org/wiki/Octree)).
Copy link
Contributor

Choose a reason for hiding this comment

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

equally sized tiles - in Cartesian coordinates yes, but not necessarily in lat/long. I think people will get the point but can it be phrased differently?

extensions/3DTILES_implicit_tiling/README.md Outdated Show resolved Hide resolved

#### External Tileset at Implicit Location

An external tileset may exist outside the file structure of its parent tileset, with the root of the external tileset being present in the `tile.json` at the implicit location of the tile with subdivision state `10`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I still think the list of external tilesets uris could go in the implicit tiling extension JSON to avoid the indirection and the need to store physical tile.json files. It would look similar to how string properties are stored in 3DTILES_tile_metadata but the array length would be the number of tiles with the 10 code. I'm not too worried about the file size; as long as it's a binary file the client is free to use HTTP range requests to get the external tileset uris it needs.

extensions/3DTILES_implicit_tiling/README.md Outdated Show resolved Hide resolved
extensions/3DTILES_implicit_tiling/README.md Outdated Show resolved Hide resolved
extensions/3DTILES_implicit_tiling/README.md Outdated Show resolved Hide resolved
extensions/3DTILES_implicit_tiling/README.md Show resolved Hide resolved
extensions/3DTILES_implicit_tiling/README.md Show resolved Hide resolved
extensions/3DTILES_implicit_tiling/README.md Outdated Show resolved Hide resolved
@@ -0,0 +1,631 @@
# 3DTILES_implicit_tiling Extension
Copy link
Contributor

Choose a reason for hiding this comment

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

Like CesiumGS/glTF#1, all sample tilesets that we send from here on out should have an extras.draftVersion that follows semver and distinguishes it from future revisions.

"extensions": {
  "3DTILES_implicit_tiling": {
    "extras": {
      "draftVersion": "0.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.

Just to clarify, no need to edit the examples in the spec, just make sure to include in sample tilesets we send out.

@@ -0,0 +1,631 @@
# 3DTILES_implicit_tiling Extension
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there plans to add contentBoundingVolume for this draft?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, that'll be added to the next version, primarily because it seems like we'll be changing the way bounding volumes get divided.

extensions/3DTILES_implicit_tiling/README.md Show resolved Hide resolved
extensions/3DTILES_implicit_tiling/README.md Show resolved Hide resolved
When a tile divides externally, the content and metadata for the root tile are obtained from the external tileset.
##### External Tileset at Implicit Location

An external tileset may exist within the file structure of its parent tileset, with the root of the external tileset being present at the implicit location of the tile with subdivision state `01`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This hints at it, but it needs to be clearer that tiles in implicit external tilesets exist within the global context of the parent tileset.

Though what happens if the external tileset is a different tiling scheme or bounding volume type? It doesn't make as much sense.

@@ -0,0 +1,631 @@
# 3DTILES_implicit_tiling Extension
Copy link
Contributor

Choose a reason for hiding this comment

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

Meta question, how does implicit tiling handle the case where a building is situated between two tiles? Does it get cut in half? Can looser bounding volumes be stored in tile metadata so that it extends itself to contain the building. See the Canary Wharf example in https://github.com/CesiumGS/3d-tiles/tree/master/specification#quadtrees.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implicit tiling has strict bounding volume subdivision, the building would get cut in half.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, that's how I read it right now too. I think we should consider this case some more for the next draft so I'm going to mark it back to unresolved.

extensions/3DTILES_implicit_tiling/README.md Show resolved Hide resolved
extensions/3DTILES_implicit_tiling/README.md Outdated Show resolved Hide resolved
extensions/3DTILES_implicit_tiling/README.md Outdated Show resolved Hide resolved
extensions/3DTILES_implicit_tiling/README.md Outdated Show resolved Hide resolved
extensions/3DTILES_implicit_tiling/README.md Outdated Show resolved Hide resolved

| |Type|Description|Required|
|---|----|-----------|--------|
|**boundingVolume**|`object`|A bounding volume that encloses the tileset.|☑️ Yes|
Copy link
Contributor

Choose a reason for hiding this comment

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

transform should also be included in this list

@sanjeetsuhag
Copy link
Contributor Author

sanjeetsuhag commented Jun 18, 2020

Sandcastle for bounding volume diagrams:

@lilleyse lilleyse changed the base branch from master to 3d-tiles-next October 9, 2020 19:05
@lilleyse
Copy link
Contributor

Superseded by #444

@lilleyse lilleyse closed this Jan 20, 2021
@lilleyse lilleyse deleted the 3DTILES_implicit_tiling branch October 19, 2021 12:25
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.

3 participants