Feature Table Specification #118

lasalvavida · 2016-08-22T15:56:51Z

Feature Table specification document.

Add reference to this in Pnts when Pnts update #116 gets merged.

lilleyse · 2016-08-22T16:11:21Z

TileFormats/FeatureTable/README.md

+
+This may vary between implementations, but in javascript, a `TypedArray` cannot be created on data unless it is byte-aligned.
+This means that a `Float32Array` must be stored in memory such that its data begins on a byte multiple of four since each `float` contains four bytes.
+The data types used in 3D Tiles have a maximum length of four bytes, so padding to a multiple of four will work for all cases, since smaller types with lengths of one and two will also be byte-aligned.


We support doubles too.

At least, maybe not yet but potentially.

Okay, I'll make this less specific.

pjcozzi · 2016-08-22T16:20:04Z

@lasalvavida @lilleyse when you open spec PRs, please include a link like https://github.com/lasalvavida/3d-tiles/tree/feature-table/TileFormats/FeatureTable

pjcozzi · 2016-08-22T16:22:46Z

TileFormats/FeatureTable/README.md

+
+## Contributors
+
+* Sean Lilley, [@lilleyse](https://twitter.com/lilleyse)


Add

Dan Bagnell, [@bagnell](https://github.com/bagnell)

The order can be Sean, you, Dan, and me.

I'll make this change along with some other edits.

lilleyse · 2016-08-22T16:37:12Z

Check out my note here which may be relevant to this PR: #22 (comment)

pjcozzi · 2016-08-22T16:59:36Z

Made some tweaks in 877ffb9

pjcozzi · 2016-08-22T16:59:57Z

feature-table-layout.png

Change featureTableJSON to JSON header
Change featureTableBinary to binary body

feature-table-binary-index.png

Change featureTableJSON to JSON header
Change featureTableBinary to binary body

In the context of this spec, the featureTable prefix is not needed and not consistent with the spec's text.

Keep featureTableBinaryByteLength since I imagine that is from the tile's header (explicitly mention that in the text).

pjcozzi · 2016-08-22T17:00:07Z

JSON Header

Include an complete-ish example like the tile format specs have.

pjcozzi · 2016-08-22T17:03:00Z

Create a JSON schema for the JSON header. It should be pretty small, but we need to be precise, e.g., right now byteOffset isn't really defined well. What data type is it? Can it be negative? Is it required? To ensure consistent implementations, we need to be precise.

For JSON schema examples, see

Use JSON Schema 3. You will not be able to represent all requirements/relationships, but others can be put in the description and the spec's text.

Make the schema a subdirectory of this new directory and include a link to it from the spec.

pjcozzi · 2016-08-22T17:04:07Z

Add reference to this in Pnts when #116 gets merged.

Merge in master and you can do this.

pjcozzi · 2016-08-22T17:28:00Z

TileFormats/FeatureTable/README.md

+
+## Implementation Notes
+
+In JavaScript, a `TypedArray` cannot be created on data unless it is byte-aligned to the data type.


Do we have to pad the JSON string as opposed to the start of the binary body?

I would like this spec to include language similar to this from b3dm:

The body section immediately follows the header section, and is composed of two fields: Batch Table and Binary glTF.

I guess in either padding case, this is still true.

Padding at the start of the binary body is not easily possible. When you add padding to the binary the byteOffset values in the JSON need to change, which then may extend the JSON length and breaks the alignment of the binary.

I believe this is the same reason why binary gltf appends whitespace to the JSON rather than padding the binary.

…to feature-table

lasalvavida · 2016-08-22T20:16:57Z

@pjcozzi Is there a particular reason we are tied to v3 of the json-schema spec? I haven't found a way to define that POSITION is required unless POSITION_QUANTIZED is present and vice-versa in v3 of the spec, but it's pretty easy to do in v4 with the anyOf keyword. https://github.com/json-schema/json-schema/wiki/anyOf,-allOf,-oneOf,-not

featureTable.schema.json

{
    "$schema" : "http://json-schema.org/draft-04/schema",
    "title" : "featureTable",
    "type" : "object",
    "description" : "A set of tile-format-specific semantics defining behavior for each feature in a tile.",
    "patternProperties" : {
        ".*" : {
            "properties" : {
                "byteOffset" : {
                    "type" : "integer",
                    "minimum" : 0           
                }
            },
            "required" : ["byteOffset"]
        }
    }
}

i3dm.featureTable.schema.json

{
  "$schema": "http://json-schema.org/draft-04/schema",
  "title": "i3dm featureTable",
  "type": "object",
  "description": "A set of Instanced3DModel semantics defining behavior for each instance.",
  "extends": { "$ref" : "featureTable.schema.json" },
  "properties": {
    "POSITION": {},
    "POSITION_QUANTIZED": {},
    "NORMAL_UP": {},
    "NORMAL_RIGHT": {},
    "NORMAL_UP_OCT32P": {},
    "NORMAL_RIGHT_OCT32P": {},
    "SCALE": {},
    "SCALE_NON_UNIFORM": {},
    "BATCH_ID": {},
    "INSTANCES_LENGTH": {},
    "QUANTIZED_VOLUME_OFFSET": {},
    "QUANTIZED_VOLUME_SCALE": {}
  },
  "anyOf" : [
    {
      "required": ["POSITION"]
    },
    {
      "required": ["POSITION_QUANTIZED"]
    }
  ],
  "dependencies": {
    "POSITION_QUANTIZED": [
      "QUANTIZED_VOLUME_OFFSET",
      "QUANTIZED_VOLUME_SCALE"
    ],
    "NORMAL_UP": [
      "NORMAL_RIGHT"
    ],
    "NORMAL_RIGHT": [
      "NORMAL_UP"
    ],
    "NORMAL_UP_OCT32P": [
      "NORMAL_RIGHT_OCT32P"
    ],
    "NORMAL_RIGHT_OCT32P": [
      "NORMAL_UP_OCT32P"
    ]
  },
  "additionalProperties" : false
}

pjcozzi · 2016-08-22T20:36:19Z

@pjcozzi Is there a particular reason we are tied to v3 of the json-schema spec?

Tooling. There are more tools for v3 than v4, e.g., like this tool we wrote: https://github.com/AnalyticalGraphicsInc/wetzel

With that said, the v4 features look handy and we actually don't have that much JSON schema for 3D Tiles (we are definitely not updating glTF to v4 right now). If you want to update all of the 3D Tiles JSON schema to v4 right now, go for it, but please stay focused on the end goal of 3D Tiles and don't get too caught up in schema land. I have been there.

lasalvavida · 2016-08-22T20:40:19Z

Sounds good to me. Changes should be pretty minimal for the schema we already have.

lasalvavida · 2016-08-23T16:04:53Z

This should be ready for a look.

pjcozzi · 2016-08-23T16:41:58Z

Made some tweaks in d161ccb.

pjcozzi · 2016-08-23T16:49:56Z

TileFormats/FeatureTable/schema/featureTable.schema.json

+    "title" : "Feature Table",
+    "type" : "object",
+    "description" : "A set of semantics containing per-tile and per-feature values defining the position and appearance properties for features in a tile.",
+    "patternProperties" : {


If a property is an object, does it have to have a byteOffset property?

For example, if a tile format wants to define a Feature Table with a global semantic like

"KEY" : { "some-property" : 0.0, "another-property" : [1, 2, 3] }

Can it do so as the schema is written here? (It might, I just don't know v4).

I suggest that we let the schema be flexible (and note this in the Feature Table spec) and rely on each tile format to specifically scope what is valid. This will be forward-compatible as we and others add new tile formats.

Your example currently will fail since it doesn't have a byteOffset. Do you want to just make byteOffset optional?

To elaborate, a semantic value is currently defined as either an object with a byteOffset referencing the binary, an array, or a single value, and I think we currently forbid using semantics not defined for a tile format, so if it is an object, I think you should always have a byteOffset.

I read that in the spec, but this schema doesn't, for example, explicitly disallow strings.

I would make the schema for the generic Feature Table as generic as possible. Basically all we can say is that if a key is an object, and that object has a byteOffset property, then byteOffset should be:

"type" : "integer", "minimum" : 0

Then let each tile format specifically scope what is valid for its Feature Table. This is no more work for an implementation with our current tile formats, and allows future tile formats maximum flexibility to define their data structures.

pjcozzi · 2016-08-23T16:52:06Z

TileFormats/FeatureTable/schema/i3dm.featureTable.schema.json

+      "$ref" : "featureTable.schema.json"
+    },
+    {
+      "properties": {


Each of these need to define the value values, e.g., is it a scalar or an array, min/max, etc.

pjcozzi · 2016-08-24T20:32:41Z

If we're going to explicitly forbid it, should the Cesium implementation throw an exception if the values are in the JSON?

The validator, CesiumGS/3d-tiles-validator#5, would do full validation. The runtime could just throw an exception if the semantic is not an object with a byteOffset.

Each semantic should also be explicit that it needs to be binary.

lasalvavida · 2016-08-25T14:59:54Z

Updated

pjcozzi · 2016-08-25T16:15:21Z

schema/boundingVolume.schema.json

@@ -1,6 +1,7 @@
 {
-    "$schema" : "http://json-schema.org/draft-03/schema",
-    "title" : "boundingVolume",
+    "$schema" : "http://json-schema.org/draft-04/schema",


In JSON schema v4, is there a way to say that only one of the three properties below can be present? I changed this in #123.

pjcozzi · 2016-08-25T16:17:40Z

Very nice, @lasalvavida.

@lilleyse can you do a final read and merge? That will also help you prep for the Batch Table spec.

lasalvavida · 2016-08-25T16:22:57Z

In JSON schema v4, is there a way to say that only one of the three properties below can be present? I changed this in #123.

The schema has now been updated to reflect this.

pjcozzi · 2016-08-25T16:23:32Z

The schema has now been updated to reflect this.

Nice!

lilleyse · 2016-08-25T18:48:11Z

Everything looks good to me. This will be good to go after clarifying the byte alignment for the embedded glTF.

lasalvavida · 2016-08-25T18:57:32Z

Made the edit, let me know what you think.

lilleyse · 2016-08-25T19:01:33Z

Add a note that this is suggested to keep proper btye alignment for the embedded glTF for the b3dm and i3dm formats.

lasalvavida · 2016-08-25T19:11:19Z

Updated

lilleyse · 2016-08-25T19:22:13Z

Looks good to me, @pjcozzi do you want to do a quick read? Then I can merge.

pjcozzi · 2016-08-25T20:00:39Z

TileFormats/FeatureTable/README.md

@@ -70,4 +70,4 @@ In JavaScript, a `TypedArray` cannot be created on data unless it is byte-aligne
 For example, a `Float32Array` must be stored in memory such that its data begins on a byte multiple of four since each `float` contains four bytes.

 The string generated from the JSON header should be padded with space characters in order to ensure that the binary body is byte-aligned.
-The binary body should also be padded if necessary when there is data following the Feature Table.
+The binary body should also be padded to a multiple of eight bytes. This ensures byte-alignment for the embedded glTF in Batched 3D Models and Instanced 3D Models.


Are we sure about this? I thought the guarantees would be:

Typed arrays in the Feature Table's binary body are byte-aligned to the size of their component type, e.g., 4 bytes for
float.

Typed arrays in the Batch Table's binary body are byte-aligned to the size of their component type, e.g., 4 bytes for float.

In b3dm and i3dm (put the wording in their specs), the Binary glTF is 8-byte aligned, so glTF's byte-alignment guarantees can be meet with respect to the tile's entire arraybuffer.

lasalvavida · 2016-08-25T21:06:01Z

Updated. Is that better?

lasalvavida · 2016-08-25T21:08:27Z

Actually I'm not sure about this now. For the i3dm it works fine since the header is 32 bytes. For batched it's only 20 right now. If you don't have a batch table to pad, how are you supposed to start it on a multiple of 8?

pjcozzi · 2016-08-26T12:31:13Z

Actually I'm not sure about this now. For the i3dm it works fine since the header is 32 bytes. For batched it's only 20 right now. If you don't have a batch table to pad, how are you supposed to start it on a multiple of 8?

Good catch. I hate to say that the header (both b3dm and i3dm) need an explicit gltfOffset to allow "padding the header" (or really anything). @lilleyse do you have a better suggestion?

Also, on the implementation side, I would like to merge all breaking b3dm changes in one PR since b3dm is so widely used.

lasalvavida · 2016-08-26T13:39:28Z

We could make it implicit that if there's no batch table, the glTF starts at the next multiple of 8 (24)

lasalvavida · 2016-08-26T13:40:52Z

Or alternatively pad the beginning and read until you hit the glTF magic.

lilleyse · 2016-08-26T15:55:03Z

Well the simplest thing to do right now is nothing since the b3dm header is about to change to 24 bytes.

lasalvavida · 2016-08-26T15:56:43Z

Alrighty, then this is probably good to merge, unless anyone has anything else?

pjcozzi · 2016-08-26T16:17:44Z

Nice, big progress here!

lasalvavida added 4 commits August 22, 2016 11:48

Added feature table spec

567ddcf

Fixed relative paths

a3b0804

Edit

0dcb9a0

Added link to feature table specification

4cf46ee

lilleyse reviewed Aug 22, 2016
View reviewed changes

pjcozzi reviewed Aug 22, 2016
View reviewed changes

lilleyse mentioned this pull request Aug 22, 2016

Finish Point Cloud Tile spec #22

Closed

Tweaks

877ffb9

pjcozzi reviewed Aug 22, 2016
View reviewed changes

Merge branch 'master' of github.com:AnalyticalGraphicsInc/3d-tiles in…

32eeef4

…to feature-table

lasalvavida added 3 commits August 23, 2016 11:19

Initial commit of schema changes

3558daa

Finalize v4 schema changes

1d5040b

Fix a grammatical error in tile.content schema

d627dcb

Tweaks

d161ccb

Tweak description

53481cb

pjcozzi reviewed Aug 23, 2016
View reviewed changes

Tweak description

eca2eff

pjcozzi reviewed Aug 23, 2016
View reviewed changes

Per-feature properties for i3dm and pnts must be binary body references

702cbcc

pjcozzi reviewed Aug 25, 2016
View reviewed changes

Only one type of bounding volume is allowed

9e84298

lasalvavida added 2 commits August 25, 2016 14:56

Made byte-alignment restriction more verbose

8ade7ba

Missed a dash

05cc8bc

lasalvavida added 2 commits August 25, 2016 15:10

Tweak

c362daf

Specify embedded

d48c5b5

pjcozzi mentioned this pull request Aug 25, 2016

Byte alignment guarantees #105

Closed

pjcozzi reviewed Aug 25, 2016
View reviewed changes

Byte-alignment guarantee

856ec21

pjcozzi merged commit 77ec38a into CesiumGS:master Aug 26, 2016

pjcozzi deleted the feature-table branch August 26, 2016 16:17


		## Contributors

		* Sean Lilley, [@lilleyse](https://twitter.com/lilleyse)


		## Implementation Notes

		In JavaScript, a `TypedArray` cannot be created on data unless it is byte-aligned to the data type.

Feature Table Specification #118

Feature Table Specification #118

Conversation

lasalvavida commented Aug 22, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjcozzi commented Aug 22, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lilleyse commented Aug 22, 2016

pjcozzi commented Aug 22, 2016

pjcozzi commented Aug 22, 2016

pjcozzi commented Aug 22, 2016

pjcozzi commented Aug 22, 2016

pjcozzi commented Aug 22, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lasalvavida commented Aug 22, 2016 • edited Loading

pjcozzi commented Aug 22, 2016

lasalvavida commented Aug 22, 2016

lasalvavida commented Aug 23, 2016

pjcozzi commented Aug 23, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lasalvavida Aug 23, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjcozzi commented Aug 24, 2016

lasalvavida commented Aug 25, 2016

Choose a reason for hiding this comment

pjcozzi commented Aug 25, 2016

lasalvavida commented Aug 25, 2016

pjcozzi commented Aug 25, 2016

lilleyse commented Aug 25, 2016

lasalvavida commented Aug 25, 2016

lilleyse commented Aug 25, 2016

lasalvavida commented Aug 25, 2016

lilleyse commented Aug 25, 2016

Choose a reason for hiding this comment

lasalvavida commented Aug 25, 2016

lasalvavida commented Aug 25, 2016 • edited Loading

pjcozzi commented Aug 26, 2016

lasalvavida commented Aug 26, 2016

lasalvavida commented Aug 26, 2016

lilleyse commented Aug 26, 2016

lasalvavida commented Aug 26, 2016

pjcozzi commented Aug 26, 2016

lasalvavida commented Aug 22, 2016 •

edited

Loading

lasalvavida commented Aug 22, 2016 •

edited

Loading

lasalvavida Aug 23, 2016 •

edited

Loading

lasalvavida commented Aug 25, 2016 •

edited

Loading