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

Feature Metadata extension (EXT_3dtiles_feature_metadata) #1

Closed
wants to merge 36 commits into from

Conversation

lilleyse
Copy link

@lilleyse lilleyse commented Feb 26, 2020

See #3 for the more up-to-date schema

For CesiumGS/3d-tiles#269
For CesiumGS/3d-tiles#385

Direct Link Version Changes
EXT_3dtiles_feature_metadata 0.0.0 Initial draft
EXT_feature_metadata 0.0.0 Second draft with numerous revisions and a new name. Currently just in schema form.

b3dm, pnts, i3dm, and cmpt are moving to glTF! This gives us the opportunity to rethink how feature metadata works and lean on glTF wherever possible.

Here's the gist of it:

  • The "Feature Table" concept in 3D Tiles 1.0 is being removed. The feature table stored position and appearance properties for point clouds and instanced models; instead we can use glTF attributes like POSITION, COLOR, and NORMAL. More on that in the 3DTILES_content_gltf extension.
  • The "Batch Table" concept in 3D Tiles 1.0 is staying, but it's being split into "Feature Types" and "Feature Collections". The concept of "batching" models still applies but it should be treated as a verb only in specific cases.
  • There's now support for multiple feature collections. You can define per-point properties (like intensity) and per-group properties (like architectural component) in the same tile.
  • CesiumJS implementation for the original EXT_3dtiles_feature_metadata: feature-metadata branch

To do:

  • How can this extension be written so that someone can import the model into a modelling program, export it, and not lose important information like the feature id attributes? Feature types and feature collections can now be stored separately (inspired by gmdf)
  • Provide complete examples
  • How can we improve styling capabilities?
    • Property min/max for color ramp. Require this in the accessor? Add min/max property for JSON arrays when type is number?
    • Enum values and labels, e.g. for LAS classification, and a per-enum count.
    • The property type (string, number, boolean, etc)
  • Do all vertices in a triangle need to have the same feature id? Otherwise, how is the feature id attribute interpolated? Barycentric or provoking vertex?
  • Per-triangle feature IDs?
  • Feature id ranges? Run-length encoded feature IDs 3d-tiles#366
  • Polyline counts to dynamically generate indices on the client and reduce payload size
  • Add back the texture metadata image
  • How can you group multiple instances to represent the same feature? E.g. a group of trees in a city? In general - should the EXT_mesh_gpu_instancing and EXT_feature_metadata interop be simplified?
  • Should this extension be split into multiple extensions? See conversation starting at Feature Metadata extension (EXT_3dtiles_feature_metadata) #1 (comment).
  • Add back batching explanation
  • Add table of contents
  • Multiple material ids per texel
  • Built-in longitude, latitude, and height semantics.
  • Per-feature bounding volume semantics: Per feature bounding volume information 3d-tiles#127
  • Allow for styling language to style based on semantics
  • Linkages between different feature tables (e.g. a property in one table is an index to a feature in another table)
  • Need a Draco point cloud extension like 3DTILES_draco_point_compression - Draco extension for point clouds KhronosGroup/glTF#1809 (OBE with Meshopt)
  • No clear alternative yet for RGB565 compressed colors. Draco and Meshopt offer better solutions so we should consider scrapping RGB565.
  • No clear alternative yet for oct-encoded point cloud normals. Draco or Meshopt offer better solutions.
  • The old point cloud feature table defines some custom attribute compression schemes like quantized positions but there's already a lot of overlap with KHR_mesh_quantiziation.
  • Feature table binary properties are now stored in glTF accessors. 3D Tiles supports double precision properties but glTF does not. glTF doesn't have a double componentType and padding for glTF chunks is 4 bytes which makes it more difficult to align double properties to 8-byte boundaries for usage with JavaScript's Float64Array. If this would be a breaking change to the glTF spec what can we do to get around it? Enforce that double properties are aligned to an 8-byte boundary relative to the start of the glb / glTF buffer? (OBE: now using Apache Arrow)
  • Feature table properties do not have as strict alignment requirements as glTF accessors. glTF requires that accessors are aligned to 4-byte boundaries even if the component type is less than 4 bytes (common for feature ids to be uint8 or uint6). See https://github.com/KhronosGroup/glTF/tree/master/specification/2.0#data-alignment. (OBE: now using Apache Arrow)
  • Still need to finalize the schema for external feature tables. See Feature Metadata extension (EXT_3dtiles_feature_metadata) #1 (comment) for the latest.
  • Write schema
  • Well known semantics for feature table properties? E.g. the property object contains a semantic field like "NAME" or "ID", and the client knows that it can get the name from that property.
  • How are per-vertex properties (not IDs) interpolated across a triangle? (OBE: all per-vertex properties are stored in feature collections via implicit feature IDs)
  • How should per-texel feature properties be sampled? NEAREST sampling makes sense most of the time but maybe it's best left up to the discretion of the tileset author to chose their own samplers.
  • Is it possible to incorporate global feature ids? Global feature IDs 3d-tiles#265. Does this affect the feature hierarchy extension? Is this what the "ID" semantic is?
  • What are valid property names? All utf-8 strings? See Variable identifier clarifications 3d-tiles#412
  • Make sure normalized is applicable to signed and unsigned types
  • Is EXT_3dtiles_feature_metadata the right name? Should the 3dtiles part be dropped?Feature Metadata extension (EXT_3dtiles_feature_metadata) #1 (comment)
  • Point size attribute? Line width? (Out of scope for this extension)
  • Remove feature metadata dependency on accessor and buffer view so it can be used by non-gltf payloads (vector tilers TBD) and 3D Tiles metadata
  • Do metadata textures belong in primitive or material?
  • Metadata at the node/mesh level. Can node metadata be stored in a feature table?
  • No clear alternative yet for CONSTANT_RGBA. (Topic moved to 3DTILES_content_gltf)

@lilleyse lilleyse changed the title 3D Tiles Batch Table extension (CESIUM_3dtiles_batch_table) 3D Tiles Batch Table extension Feb 26, 2020
@pjcozzi
Copy link

pjcozzi commented Feb 26, 2020

@lilleyse I am happy to be the "last" review on this and on all upcoming 3D Tiles / glTF updates. Is this ready for me to review? Or should others look at it first?

@lilleyse
Copy link
Author

@pjcozzi I'll bump you when it's ready for the "last" review, which isn't now

@Samulus
Copy link

Samulus commented Feb 26, 2020

  • Maybe call it CESIUM_3dtiles_batched_metadata?
  • I would be consistent with references to model vs feature to avoid confusing readers (even if they're synonymous)
  • "Multiple batch ids and batch tables are allowed to support feature layers." // We should have a specific example of why an end user might want to use multiple batch tables so they can facilitate the grouping of identical data using multiple classifications.
    • E.g in the point cloud example, you could use smoke from a fire spreading throughout a house (kind of morbid but first thing I thought of). One BatchTable could be used to group information about the intensity of smoke at each level, another BatchTable could be used to identify different regions of the house (these N points are the bathroom etc.)

@kring
Copy link
Member

kring commented Feb 27, 2020

Still need to figure out how to implement external batch tables.

Not sure if it helps, but I wrote a draft of an external batch table extension for the OGC pilot:
https://github.com/kring/3d-tiles/blob/external-batch-table/extensions/3DTILES_external_batch_table/README.md

Basic principle is that external batch table(s) are always referenced from the JSON portion. The JSON describes not only where to find the external batch table (the URI) but also which properties are available there, so that clients can choose to load only the external batch tables they care about. The external batch data itself can be either JSON or binary.

@KeyboardSounds
Copy link

Feel free to ignore all of this if you don't think it's useful, but I often find that when I'm explaining batch tables, the name "batch table" throws people a bit.

This is pure semantics and probably not a big deal, but I think it's generally better to name something after what it is rather than what it does-- although the distinction between these two things is often small. I assume the term "batch" comes from their use in batching models to optimise rendering, and I guess it is a table of batches, but I generally think of them in terms of storing feature information. I would say they should be called feature tables, but I know feature table has a different specific meaning in the context of 3d tiles. Entity tables, maybe? Some other synonym of feature?

On the other hand, keeping the name "Batch Table" might make it easier for people already familiar with 3d tiles to understand the new extension...

@lilleyse
Copy link
Author

lilleyse commented Mar 9, 2020

Updated to support the composite tile use case where heterogeneous data sources are combined into the same glTF. Required a mapping from the batch id accessor to the batch table index.

See composite example.

@lilleyse lilleyse changed the title 3D Tiles Batch Table extension 3D Tiles Feature Metadata extension Mar 10, 2020
@lilleyse
Copy link
Author

@KeyboardSounds agreed on all points.

The new working title for this extension is CESIUM_3dtiles_feature_metadata and "batch" has been replaced with "feature" in most cases. I updated the PR description to reflect this as well.

@lilleyse
Copy link
Author

Proof of concept for texture metadata:

texture-metadata

@Samulus
Copy link

Samulus commented Mar 20, 2020

We also need to find a home for EAST_NORTH_UP from the i3dm specficiation. It doesn't fall into the KHR_mesh_instancing or CESIUM_3dtiles_feature_metadata extension directly.

@lilleyse
Copy link
Author

Option 3

Here's a new approach that lets you pack properties. Note that featureTables changes from an array to a dictionary so that properties with the same names in different feature tables can be distinguished in the external json.

glTF

{
  "extensions": {
    "EXT_3dtiles_feature_metadata": {
      "featureTables": {
        "LandCover": {
          "featureCount": 256,
          "featureProperties": {
            "Names": {
              "array": {
                "type": "string",
                "values": "external_1.json"
              }
            },
            "Colors": {
              "array": {
                "type": "any",
                "values":  "external_1.json"
              }
            }
          }
        },
        "Trees": {
          "featureCount": 2492,
          "featureProperties": {
            "Heights": {
              "accessor": 0
            },
            "Colors": {
              "array": {
                "type": "any",
                "values":  "external_1.json"
              }
            }
          }
        },
        "Buildings": {
          "featureCount": 10,
          "featureProperties": {
            "Heights": [9, 9, 8, 3, 5, 4, 3, 9, 8, 3],
            "Colors": {
              "array": {
                "type": "any",
                "values":  "external_2.json"
              }
            }
          }
        }
      }
    }
  }
}

external_1.json

{
  "LandCover": {
    "Names": [...],
    "Colors": [...]
  },
  "Trees": {
    "Colors": [...]
  }
}

external_2.json

{
  "Buildings": {
    "Colors": [...]
  }
}

CC @kring @KeyboardSounds

@lilleyse
Copy link
Author

lilleyse commented Apr 23, 2020

Side note: I wonder if HTTP/2 multiplexing makes it less burdensome to do multiple requests in option 2. Though there's still some overhead and more complexity in the server.

@kring
Copy link
Member

kring commented Apr 24, 2020

I wonder if HTTP/2 multiplexing makes it less burdensome to do multiple requests in option 2.

Yeah, it certainly should! I don't think that eliminates the need for this capability, but it should probably at least make us unwilling to make too many compromises to enable it. I don't think it's too bad, though.

Note that featureTables changes from an array to a dictionary so that properties with the same names in different feature tables can be distinguished in the external json.

Not a huge fan of this, because it makes featureTables different from pretty much everything else in glTF. Also I think it's not great to overload values to be either an array of values or a string which is the name of an external file. That's ok-ish in JavaScript, but that sort of thing gives statically typed languages fits.

So maybe we can solve both problems by doing something like this:

{
  "extensions": {
    "EXT_3dtiles_feature_metadata": {
      "featureTables": [
        {
          "name": "Land Cover",
          "featureCount": 256,
          "featureProperties": {
            "Names": {
              "external": {
                "uri": "external_1.json",
                "key": "landCoverNames",
                "type": "string"
              }
            },
            "Colors": {
              "external": {
                "uri": "external_1.json",
                "key": "landCoverColors",
                "type": "any"
              }
            }
          }
        },
        {
          "name": "Trees",
          "featureCount": 2492,
          "featureProperties": {
            "Heights": {
              "accessor": 0
            },
            "Colors": {
              "external": {
                "uri":  "external_1.json",
                "key": "treeColors",
                "type": "any"
              }
            }
          }
        },
        {
          "name": "Buildings",
          "featureCount": 10,
          "featureProperties": {
            "Heights": [9, 9, 8, 3, 5, 4, 3, 9, 8, 3],
            "Colors": {
              "external": {
                "uri": "external_2.json",
                "key": "buildingColors",
                "type": "any"
              }
            }
          }
        }
      ]
    }
  }
}

The external files would have to be flattened, too:

external_1.json

{
  "landCoverNames": [...],
  "landCoverColors": [...],
  "treeColors": [...]
}

If there's no key property, it assumes the JSON is a single array. We could do fancy things like allowing an arbitrary JSON Path to select the array from the external JSON file, rather than just a key. But it's probably not worth it.

Originally I thought it might be useful to have an "externalBinary" with a URI, byte offset, and component type. But that use-case should already be covered pretty well by pointing the feature table at an accessor with an external buffer.

Copy link
Member

@kring kring left a comment

Choose a reason for hiding this comment

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

This looks great! The design choices all seem very sensible to me.

I started to suggest some edits to the writing, but I think I was getting carried away and Sarah is probably much more qualified there anyway. But if anyone thinks it'd be useful I'm happy to do a more thorough edit for clarity before or after Sarah.

Bikeshedding a bit, but I think a table of contents at the top would add a lot.


| Property | Description | Caveats |
|---------------------|-------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `featureTable` | Index of the feature table that this feature layer is using. | Multiple feature layers in a single primitive cannot use the same feature table. Feature tables used by a single primitive cannot have any of the same property names. |
Copy link
Member

Choose a reason for hiding this comment

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

Multiple feature layers in a single primitive cannot use the same feature table.

Probably rare that this would be useful, but does it need to be forbidden?

Feature tables used by a single primitive cannot have any of the same property names.

This seems like a really big limitation. Why is it necessary?

Copy link
Author

@lilleyse lilleyse Apr 24, 2020

Choose a reason for hiding this comment

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

Probably rare that this would be useful, but does it need to be forbidden?

I can't remember why anymore? But I could see a use case where a per-vertex ID layer and a per-texel ID layer want to use the same feature table. So maybe we should lift this restriction.

This seems like a really big limitation. Why is it necessary?

It was added for styling reasons to not overload variable names. Like ${Height} could mean two different things if multiple feature tables have a Height property. Maybe this could be solved by prefixing variables with the feature table name like ${Buidings:Height}, or something else. Styling is still a mostly unknown topic here. Personally I'd like to move closer to something like GLSL where conflicts like this can be worked around easier, and just gives more control in general. Styling really deserves its own issue.

Copy link
Member

Choose a reason for hiding this comment

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

Despite the "3dtiles" in the name (maybe it shouldn't even be there?), I think of this extension as being pretty general. glTF currently lacks a way to attach metadata to a parts of a model, and that's useful in a whole lot of scenarios. This extension should work well for most of them. So we shouldn't let (current) limitations in the styling language impose limitations on metadata expression in this extension. Why not just define in the styling spec what happens when there's a conflict? Last one wins, or whatever.

Copy link
Author

Choose a reason for hiding this comment

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

I removed both restrictions for now.

Despite the "3dtiles" in the name (maybe it shouldn't even be there?)

I've seen some confusion about the naming recently. I added it to the TODO list. I'm curious to see what @pjcozzi thinks when he reviews.

@lilleyse
Copy link
Author

@kring I tweaked your example so that external is an object within array. values and external are mutually exclusive (kind of like image.buffer and image.uri in glTF)

Before

"featureProperties": {
  "Names": {
    "external": {
      "uri": "external_1.json",
      "key": "landCoverNames",
      "type": "string"
    }
  }
}

After

"featureProperties": {
  "Names": {
    "array": {
      "type": "string",
      "external": {
        "uri": "external_1.json",
        "key": "landCoverNames"
      }
    }
  }
}

Full example:

{
  "extensions": {
    "EXT_3dtiles_feature_metadata": {
      "featureTables": [
        {
          "name": "Land Cover",
          "featureCount": 256,
          "featureProperties": {
            "Names": {
              "array": {
                "type": "string",
                "external": {
                  "uri": "external_1.json",
                  "key": "landCoverNames"
                }
              }
            },
            "Colors": {
              "array": {
                "type": "any",
                "external": {
                  "uri": "external_1.json",
                  "key": "landCoverColors"
                }
              }
            }
          }
        },
        {
          "name": "Trees",
          "featureCount": 2492,
          "featureProperties": {
            "Heights": {
              "accessor": 0
            },
            "Colors": {
              "array": {
                "type": "any",
                "external": {
                  "uri": "external_1.json",
                  "key": "treeColors"
                }
              }
            }
          }
        },
        {
          "name": "Buildings",
          "featureCount": 10,
          "featureProperties": {
            "Heights": {
              "array": {
                "type": "number",
                "values": [9, 9, 8, 3, 5, 4, 3, 9, 8, 3]
              }
            },
            "Colors": {
              "array": {
                "type": "any",
                "external": {
                  "uri": "external_2.json",
                  "key": "buildingColors"
                }
              }
            }
          }
        }
      ]
    }
  }
}

external_1.json

{
  "landCoverNames": [...],
  "landCoverColors": [...],
  "treeColors": [...]
}

external_2.json

{
  "buildingColors": [...]
}

@lilleyse
Copy link
Author

Offline @pjcozzi brought up the idea of storing all properties in binary, including strings, to simplify the schema. One drawback is less visibility of these properties, but that can be solved by better tooling, like support for this extension in the glTF Tools VSCode extension.

@emackey
Copy link

emackey commented May 15, 2020

Woah woah. Lemme pause right there. No strings in binary please, all strings in JSON for glTF. Binary can do the normal accessor type and componentType things, which do not include strings.

Among other things, the JSON schema helps validate the document in VSCode even without help from the official glTF Validator. So having the schema be more detailed, and more tightly controlling of the contents of the document, is an easy win for validation.

@emackey
Copy link

emackey commented May 15, 2020

Also, if you wish, you may use VSCode to essentially "debug" the schema, by testing it against documents that are known to be valid and others that are invalid. Basically you follow the instructions in the gltf-vscode repo to get a debug environment going, and then these instructions to add your schema into the set of schemas that are validated.

Probably we would get a branch of gltf-vscode going where the schema had been imported, so it could be tested against documents.

@KeyboardSounds
Copy link

Visual Studio Code also tends to crash when you try to load large files in it, particularly large glTFs. Relying on VSCode tools to be able to read feature metadata easily would be bad in those (albeit less common) cases.

As a side note, it's much, much easier to deal with JSON than binary in JavaScript code. A lot of things that support glTFs are written in JS. This is maybe not super important but I thought it was worth mentioning.

@lilleyse
Copy link
Author

lilleyse commented Dec 8, 2020

Closing but keeping the branch around for now. The new schema/spec for EXT_feature_metadata is in #2 #3 (thanks for the correction below @ptrgags)

@ptrgags could you check if there's anything in the TODO list above that's not account for in #2 #3 or internal issues?

@lilleyse lilleyse closed this Dec 8, 2020
@lilleyse lilleyse changed the title Feature Metadata extension Feature Metadata extension (EXT_3dtiles_feature_metadata) Dec 8, 2020
@ptrgags
Copy link

ptrgags commented Dec 8, 2020

#3 is the latest spec, #2 is the old feature hierarchy spec

@lilleyse I believe most everything is covered, except for the following:

  • polyline counts -- is this still relevant? I don't remember discussing this before.
  • per-face features -- I'm suddenly realizing that I must have overlooked this when writing the EXT_feature_metadata spec. If I remember correctly, we want to accomplish this by requiring that each vertex in a triangle has the same feature ID, correct? if so, I can add that feedback to the PR.

@lilleyse
Copy link
Author

lilleyse commented Dec 8, 2020

polyline counts -- is this still relevant? I don't remember discussing this before

Possibly relevant for vector data in glTF since GL_LINES is not the most efficient use of indices to describe polylines. Maybe not relevant right now.

per-face features -- I'm suddenly realizing that I must have overlooked this when writing the EXT_feature_metadata spec. If I remember correctly, we want to accomplish this by requiring that each vertex in a triangle has the same feature ID, correct? if so, I can add that feedback to the PR.

Yup, let's add that requirement there

@ptrgags
Copy link

ptrgags commented Dec 8, 2020

Opened https://github.com/CesiumGS/3d-tiles-next/issues/110 so we don't forget about it, and added the requirement to the EXT_feature_metadata PR

kring pushed a commit that referenced this pull request Jan 11, 2021
…ancing

Rename KHR_mesh_instancing -> EXT_mesh_gpu_instancing
kring pushed a commit that referenced this pull request Jan 11, 2021
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.

8 participants