-
Notifications
You must be signed in to change notification settings - Fork 470
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
First Round of 3D Tiles 1.0 updates #301
Conversation
Add link to OneSky blog post
Please target a new 1.0 branch. |
For #8, add Can you also open a PR making any spec changes into the vector tiles branch? |
TileFormats/PointCloud/README.md
Outdated
@@ -121,7 +121,9 @@ Per-point normals are an optional property that can help improve the visual qual | |||
The normals will be transformed using the inverse transpose of the tileset transform. | |||
|
|||
#### Oct-encoded Normal Vectors | |||
Oct-encoding is described in [*A Survey of Efficient Representations of Independent Unit Vectors* by Cigolle et al.](http://jcgt.org/published/0003/02/01/). An implementation for encoding and decoding these unit vectors can be found in Cesium's | |||
Oct-encoding is described in [*A Survey of Efficient Representations of Independent Unit Vectors* by Cigolle et al.](http://jcgt.org/published/0003/02/01/). Oct-encoded values are stored in unsigned, unnormalized range ([0, 65535] or [0, 255]) and then converted to a signed normalized range ([-1.0, 1.0]) at runtime. | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few tweaks for this line
- pnts only uses oct16 encoding so only the [0, 255] range needs to be mentioned
- Minor wording
Oct-encoded values are stored in unsigned, unnormalized range [0, 255] and then mapped to a signed normalized range [-1.0, 1.0] at runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A similar explanation needs to be added i3dm
in the Oct-encoded Normal Vectors
section but using the range [0, 65535].
TileFormats/Batched3DModel/README.md
Outdated
@@ -64,18 +64,19 @@ These semantics define global properties for all features. | |||
| Semantic | Data Type | Description | Required | | |||
| --- | --- | --- | --- | | |||
| `BATCH_LENGTH` | `uint32` | The number of distinguishable models, also called features, in the batch. If the Binary glTF does not have a `batchId` attribute, this field _must_ be `0`. | :white_check_mark: Yes. | | |||
| `RTC_CENTER` | `float32[3]` | A 3-component array of numbers defining the center position when instance positions are defined relative-to-center. | :red_circle: No. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drop the word instance
since it is specific to i3dm.
|
||
`.i3dm` | ||
Instanced 3D modela tiles use the `.i3dm` extension and `application/octet-stream` MIME type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instanced 3D modela
-> Instanced 3D Model
schema/extension.schema.json
Outdated
"type": "object", | ||
"description": "Dictionary object with extension-specific objects.", | ||
"properties": { | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to "properties": {},
README.md
Outdated
`b3dm` and `i3dm` tiles embed glTF. According to the [glTF spec](https://github.com/KhronosGroup/glTF/tree/master/specification/1.0#coordinate-system-and-units) glTF uses a right-handed coordinate system and defines the y axis as up. By default embedded models are considered to be y-up, but in order to support a variety of source data, including models defined directly in WGS84 coordinates, embedded glTF models may be defined as x-up, y-up, or z-up with the `asset.gltfUpAxis` property of `tileset.json`. In general an implementation should transform glTF assets to z-up at runtime to be consistent with the z-up coordinate system of the bounding volume hierarchy. | ||
#### Tile content | ||
|
||
`b3dm` and `i3dm` tiles embed glTF. According to the [glTF spec](https://github.com/KhronosGroup/glTF/tree/master/specification/1.0#coordinate-system-and-units) glTF uses a right-handed coordinate system and defines the y axis as up. By default embedded models are considered to be y-up, but in order to support a variety of source data, including models defined directly in WGS84 coordinates, embedded glTF models may be defined as x-up, y-up, or z-up with the `asset.gltfUpAxis` property of the tileset JSON. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Styling/README.md
Outdated
## MIME Type | ||
|
||
_TBA, [#60](https://github.com/AnalyticalGraphicsInc/3d-tiles/issues/60)_ | ||
> Implementation Note: Point cloud styling engines may often use a shader (GLSL) implementation, however some features of the expression language are not possible in pure a GLSL implementation. Features that must account for these inconsistencies include evaluation of `isNan` and `isFinite` (GLSL 2.0+ supports `isnan` and `isinf` for these functions respectively); the types `null` and `undefined`; strings, including accessing object properties (`color()['r']`) and batch table values; regular expressions; arrays of lengths other than 2, 3, or 4; mismatched type comparisons (`float` to `bool`); and handling of "index out of bounds" errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to have a bulleted list within a quoted section? That might be easier to read.
Styling/README.md
Outdated
`application/json` | ||
## File Extensions and MIME Types | ||
|
||
* Tileset styles use the `.json` extension and the `application/json` mime type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same note as before where the extension is not required.
TileFormats/Batched3DModel/README.md
Outdated
@@ -119,16 +120,20 @@ The vertex shader can be modified at runtime to use `a_batchId` to access indivi | |||
|
|||
When a Batch Table is present or the `BATCH_LENGTH` property is greater than `0`, the `batchId` attribute (with the parameter semantic `_BATCHID`) is required; otherwise, it is not. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The gltf snipped above is still based on 1.0 techniques/shaders. For 2.0 all that's required is a primitive contains a _BATCHID
attribute.
TileFormats/Batched3DModel/README.md
Outdated
|
||
The file extension is optional. Valid implementations ignore it and identify a content's format by the `magic` field in its header. | ||
> Implementation Note: Clients may also use the glTF [CESIUM_RTC](https://github.com/KhronosGroup/glTF/tree/master/extensions/1.0/Vendor/CESIUM_RTC) extension and define points relative to center. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are phasing away from this extension, so I wouldn't include this note.
TileFormats/Batched3DModel/README.md
Outdated
|
||
`.b3dm` | ||
Vertex positions may be defined relative-to-center for high-precision rendering [1]. If defined, `RTC_CENTER` specifies the center position and all vertex positions are treated as relative to this value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here it should be noted that RTC_CENTER
is z-up whereas vertex positions are y-up.
In general I think our coordinate system messaging is still not quite there... still need to think about the clearest way possible to describe it.
|
I made some edits as I went along in 192a632. More to come later. |
#295 and #280 will need a bit of work before they are done, maybe we can discuss offline I think the rest of the issues look good or are covered by comments. |
Added glTF |
Updated figures. |
Awesome work! Ready to merge to |
First round of edits for 3D Tiles 1.0, tried to minimize organizational and formatting changes to be tackled later.
Fixes #298
Fixes #295
Fixes #280
Fixes #272
Fixes #263
Fixes #225
Fixes #211
Fixes #184
Fixes #140
Fixes #94
Fixes #60
Fixes #8