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

3DTILES_implicit_tiling and 3DTILES_multiple_contents revisions #549

Closed
wants to merge 4 commits into from

Conversation

lilleyse
Copy link
Contributor

@lilleyse lilleyse commented Nov 10, 2021

I did a pass over 3DTILES_implicit_tiling mainly to tighten up the introduction and remove redundancy in a few areas (including removing the glossary). I also added an example of implicit tiling's interaction with multiple contents.

Since we already have sections in other extensions showcasing 3DTILES_metadata + 3DTILES_multiple_contents and 3DTILES_implicit_tiling + 3DTILES_multiple_contents I pared down 3DTILES_multiple_contents to the essentials. I may take another pass over it tomorrow to add back more details.

Previews

Copy link
Contributor

@donmccurdy donmccurdy left a comment

Choose a reason for hiding this comment

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

I agree with slimming down the 3DTILES_multiple_contents spec, with one exception — it defines the schema for subtree.3DTILES_multiple_contents.schema.json but says nothing about it otherwise, which may be confusing. We could move that schema to implicit tiling? E.g.

  "tileAvailability": {"bufferView": 0},
  "childSubtreeAvailability": {"bufferView": 1},
  "contentAvailability": [
    {"bufferView": 2},
    {"bufferView": 3},
  ]

extensions/3DTILES_implicit_tiling/README.md Outdated Show resolved Hide resolved
extensions/3DTILES_implicit_tiling/README.md Outdated Show resolved Hide resolved
@lilleyse
Copy link
Contributor Author

We could move that schema to implicit tiling? E.g.

  "tileAvailability": {"bufferView": 0},
  "childSubtreeAvailability": {"bufferView": 1},
  "contentAvailability": [
    {"bufferView": 2},
    {"bufferView": 3},
  ]

Yeah that could work. If a tileset doesn't use 3DTILES_multiple_contents would contentAvailability be an array with a single element? That would be a breaking change but I think it's where we want to go. Once 3DTILES_multiple_contents goes into baseline 3D Tiles content would likely become an array as well.

lilleyse and others added 2 commits November 11, 2021 19:18
Co-authored-by: Don McCurdy <dm@donmccurdy.com>
@donmccurdy
Copy link
Contributor

If a tileset doesn't use 3DTILES_multiple_contents would contentAvailability be an array with a single element?

I think that's the way to go, yes. 👍

@wallabyway
Copy link

wallabyway commented Nov 15, 2021

I generate data-sets that use every layer and tile, so every tile is visible. So I am not providing a 'tileAvailability' parameter.

Question: Is 'tileAvailability' a requirement ?

@javagl
Copy link
Contributor

javagl commented Nov 15, 2021

Question: Is 'tileAvailability' a requirement ?

@wallabyway I'm not entirely sure whether this is what you referred to, but

  • The tileAvailability property is required as of the schema
  • But if it is constant (i.e. when all tiles are available), then you can just say
    "tileAvailability": { "constant": 1 }
    as of the availability spec (so you don't have to store a (huge) buffer that is filled with 1's...)

Copy link
Contributor

@ptrgags ptrgags left a comment

Choose a reason for hiding this comment

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

@lilleyse looks good, though if we want to talk about the double-headed quadtrees (and I think we should, it's pretty common), we should be more descriptive there.

A key use for implicit tiling is enabling and/or accelerating tree traversal algorithms. Accessing a tile by coordinates is faster than traversing the entire tree. Likewise, raycasting algorithms and GIS algorithms can benefit from the abbreviated tree traversals. Tiles can be loaded immediately instead of going from top to bottom of a tree.

Accessing tiles by coordinates also helps accelerate spatial queries. For example, the highest resolution tile that contains a given point can be quickly located by computing the coordinates of the tile directly.
The `3DTILES_implicit_tiling` extension may be added to any tile in the tileset. This extension defines how the tile is subdivided and where to locate content resources. The extension may be added to multiple tiles to create more complex subdivision schemes like double-headed quadtrees.
Copy link
Contributor

Choose a reason for hiding this comment

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

"double-headed quadtree" is jargony, if you're going to mention it, explain that it's a quadtree for each hemisphere (e.g. eastern and western), with another root tile that covers the whole world. A diagram would also help. Something like this: https://sandcastle.cesium.com/?src=Imagery%20Layers%20Manipulation.html

image
image

Except fix the numbering for the eastern hemisphere

Note that I have an example for this: #512 (comment) albeit this overview section might be too early on to show it.

@@ -173,7 +150,7 @@ Implicit tiling only requires defining the subdivision scheme, refinement strate
| --- | --- |
Copy link
Contributor

Choose a reason for hiding this comment

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

should we add a row in this table to say that extensions is constant for all descendant tiles? or should we not define this except in specific cases like for multiple contents?

@lilleyse
Copy link
Contributor Author

If we introduce a breaking change for content availability we might also want to consider a breaking change for tile metadata where instead of using a 3DTILES_metadata extension we add a tileMetadata property. This should help to further reduce schema inter-dependencies. Thoughts @donmccurdy?

Current                                                  Proposed                                  
{
  "buffers": [
    {
      "uri": "metadata.bin",
      "byteLength": 336
    }
  ],
  "bufferViews": [
    {
      "buffer": 0,
      "byteOffset": 0,
      "byteLength": 168
    },
    {
      "buffer": 0,
      "byteOffset": 168,
      "byteLength": 168
    }
  ],
  "tileAvailability": {
    "constant": 1
  },
  "contentAvailability": {
    "constant": 0
  },
  "childSubtreeAvailability": {
    "constant": 0
  },
  "extensions": {
    "3DTILES_metadata": {
      "class": "tile",
      "properties": {
        "minimumHeight": {
          "bufferView": 0
        },
        "maximumHeight": {
          "bufferView": 1
        }
      }
    }
  }
}
{
  "buffers": [
    {
      "uri": "metadata.bin",
      "byteLength": 336
    }
  ],
  "bufferViews": [
    {
      "buffer": 0,
      "byteOffset": 0,
      "byteLength": 168
    },
    {
      "buffer": 0,
      "byteOffset": 168,
      "byteLength": 168
    }
  ],
  "tileAvailability": {
    "constant": 1
  },
  "contentAvailability": [{
    "constant": 0
  }],
  "childSubtreeAvailability": {
    "constant": 0
  },
  "tileMetadata": {
    "class": "tile",
    "properties": {
      "minimumHeight": {
        "bufferView": 0
      },
      "maximumHeight": {
        "bufferView": 1
      }
    }
  }
}

@donmccurdy
Copy link
Contributor

donmccurdy commented Nov 29, 2021

A couple points that seem to follow, if we go that direction:

  1. 3DTILES_metadata extension must still be present on the tileset when using tileMetadata, to provide the schema.
  2. 3DTILES_multiple_contents could arguably go the same way, since "group" is not otherwise a metadata concept:
{
  "extensions": {
    "3DTILES_metadata": {
      "schema": {
        "classes": {
          "layer": {
            "properties": {
              "name": {"componentType": "STRING", "semantic": "NAME", "required": true},
              "color": {"type": "VEC3", "componentType": "UINT8"},
              "priority": {"componentType": "UINT32"}
            }
          }
        }
      }
    },
    "3DTILES_multiple_contents": { // ⚠️ Previously '3DTILES_metadata'
      "groupMetadata": { // ⚠️ Previously 'groups'
        "buildings": {
          "class": "layer",
          "properties": {
            "name": "Buildings Layer",
            "color": [128, 128, 128],
            "priority": 0
          }
        },
        "trees": {
          "class": "layer",
          "properties": {
            "name": "Trees Layer",
            "color": [10, 240, 30],
            "priority": 1
          }
        }
      }
    }
  },
  "root": {
    "extensions": {
      "3DTILES_multiple_contents": {
        "content": [
          // ⚠️ Previously '3DTILES_metadata' contained 'group'
          {"uri": "buildings.glb", "group": "buildings"},
          {"uri": "trees.glb", "group": "trees"}
        ]
      }
    },
    ...
  }
}

This is a modified version of the example here.

@lilleyse
Copy link
Contributor Author

  1. 3DTILES_multiple_contents could arguably go the same way, since "group" is not otherwise a metadata concept:

While group metadata is most commonly used with 3DTILES_multiple_contents it can also be used with single content tiles, and once multiple contents is core spec there would be no multiple contents extension to own the concept of group metadata. So I think group metadata still belongs in 3DTILES_metadata.

But I also think there's too much nestedness with accessing group metadata - e.g. root.extensions.3DTILES_multiple_contents.content[0].extensions.3DTILES_metadata.group - and would love to simplify.

@lilleyse
Copy link
Contributor Author

Most of the changes here have been incorporated in the latest revisions: https://github.com/CesiumGS/3d-tiles/tree/extension-revisions

@lilleyse lilleyse closed this Feb 18, 2022
@lilleyse lilleyse deleted the implicit-revisions branch March 7, 2022 13:23
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.

5 participants