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

Instanced 3D Model updates #33

Closed
3 tasks done
pjcozzi opened this issue Nov 6, 2015 · 28 comments
Closed
3 tasks done

Instanced 3D Model updates #33

pjcozzi opened this issue Nov 6, 2015 · 28 comments

Comments

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 6, 2015

  • We can significantly optimize the size of each instance field, e.g., positions can be quantized relative to the bounding volume (see EXT_quantized_attributes). Rotation could also be optional (part of the batchTable) and default to Y up (glTF default) in a local frame for the instance's position. Rotation could use fewer fields. RTC. Delta encoding... SOA vs. AOS... Look into point cloud compression...
  • Should instance.batchId be omitted when header.batchTableLength === 0. This will complicate the client, but save memory; however, the case it saves memory for is probably rare in practice.
  • Semantics for batch table properties such as scale.
@lilleyse
Copy link
Contributor

lilleyse commented Nov 6, 2015

Omitting instance.batchId when header.batchTableLength === 0 seems fine to me and should be easy to handle on the client side.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Nov 7, 2015

Sounds good, updated the spec in 17743a0.

Please update CesiumGS/cesium#3158 if you haven't already.

@lilleyse
Copy link
Contributor

lilleyse commented Nov 8, 2015

All updated there.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Nov 8, 2015

Thanks 👍

@lilleyse
Copy link
Contributor

We have a lot of fields that say "This must be greater than or equal to zero" that are of type uint32. Is that redundant?

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Nov 16, 2015

Good catch, #40.

@arneschilling
Copy link

Hi all,

we are now stating to implement I3DM for our vegetation datasets.
After reading the spec I have a couple of questions and suggestions for improvements.

  1. the instances field should be extended to include position, scale, rotation and the model's center point. This could be done by adding more numbers to each instance, e.g.
    Tx, Ty, Tz, Rx, Ry, Rz, Ra, Sx, Sy, Sz, Cx, Cy, Cz
    ( T: translation, R: rotation as rotation axis vector and angle in rad, S: scale, C: center point within the model used for positioning). See [http://www.web3d.org/documents/specifications/19775-1/V3.2/Part01/components/group.html#Transform] for X3D Transform spec.
    or simply by using a transformation matrix ,e,g
    1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0 (like in gltF Node)

    For visualizing tree models the rotation could be set randomly, giving the final model a more natural touch. Scaling in all 3 dimensions is also important for tree models.

  2. the in the instance field, the location is given as longitude, latitude. Is the instance always positioned on the terrain in Cesium? A height or elevation field should be included. Secondly, the height could be relative to ground or absolute.In our case we have always absolute coordinates, so relative to ground is not really necessary. Others might want to convert 2D shape files.

  3. Giving the location as longitude latitude means that coordinate transformation is necessary for each instance. Could this be also relative to a tile center in Cartesian coordinates? If so, a CESIUM_RTC point must be included somewhere.

  4. My plan is to process a vegetation dataset which contains many different tree species, each with its own prototype. I would like to include all of these in a single I3DM file. Is it possible to provide several models in the binary glTF block and make references from the instances field? In this case there should be a proto identifier along with the transformation. Maybe using batch IDs, which are stored in glTF, e.g.
    batchId, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0, instanceId
    (instanceId here is what is now called batchId in the current I3DM spec.).
    If batchId cannot be used for geometry instancing, then maybe we can have an array of glTFs?

best regards,
Arne

@lilleyse
Copy link
Contributor

Thanks for the feedback @arneschilling. The i3dm format is definitely a work in progress, storing only the longitude and latitude was a temporary solution for just getting something simple to work. We have considered a lot of the points you mention and the final version of the spec should look similar to that.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented May 19, 2016

@arneschilling thanks for this input. As @lilleyse noted, it is inline with the direction i3dm is heading and I expect the tile format will be flushed out this summer as we work towards a draft 1.0 spec. Keep an eye on this issue.

In the meantime, for (4), use a composite tile to combine multiple i3dm tiles.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Jun 20, 2016

@lasalvavida @lilleyse here's what I propose to finish up the i3dm tile format:

  • Instead of storing longitude and latitude as doubles, store x, y, z in WGS84 quantized in the same manner as the glTF extension. Store any metadata needed to decode in the tile's header.
  • Instead of assuming east/north/up, store each instance's rotation as an oct-encoded normal that transforms from the instance's local coordinate system to WGS84.
  • Introduce batchId semantics, TILES3D_SCALE and TILES3D_NON_UNIFORM_SCALE that are uniform and non-uniform (vec3) scale respectively.
    • @lilleyse if it is reasonable enough, we should expose these through the Cesium API and allow Declarative Styles to modify these.
  • Consider storing SOA instead of AOS for better compression?

Long-term, if we need to optimize the case for instances "on the ground" with or without a rotation, we can, but let's not introduce special cases like this until truly needed.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Jun 20, 2016

@lasalvavida also, I'll do the spec updates and you can focus on the implementation.

@lasalvavida
Copy link
Contributor

Instead of assuming east/north/up, store each instance's rotation as an oct-encoded normal that transforms from the instance's local coordinate system to WGS84.

@pjcozzi Do you think maybe we should use a quaternion here instead? It might be useful to be able to make instances face different directions.

@lasalvavida
Copy link
Contributor

For example, if I have an instanced streetlamp,

I want to be able to rotate the instances so that they overhang the street.

Talked with @lilleyse and you should be able to do this with two vectors, which may be preferable to a quaternion.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Jun 25, 2016

Instead of assuming east/north/up, store each instance's rotation as an oct-encoded normal that transforms from the instance's local coordinate system to WGS84.
@pjcozzi Do you think maybe we should use a quaternion here instead? It might be useful to be able to make instances face different directions.

Sorry, I mean to stay store two normals. The third normal is just z = cross(x, y). This is enough to define a transform to rotate from the local coordinate system to the global one (at load time, you can construct a 4x4 matrix with this as the top-left 3x3 rotation and the position as the top-right vec3 translation). Let me know if you need help with the math.

The normals are preferred over the quanterion for simplicity, which is important for making this a standard.

@arneschilling
Copy link

With only 2 axes/normals you need an additonal vector for scaling along x/y/z. Using 3 axes would provide enough information for rotation, scaling, shearing. I am sure we can find the math for translating this into a 4x4 matrix.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Jun 28, 2016

@arneschilling the third vector is just z = cross(x, y) as described above. Note that these are of unit length so that they can compress well. Scaling will be handled with semantics in the batch table as described above, #33 (comment). This approach should meet all your use cases with a concise representation.

@lasalvavida
Copy link
Contributor

lasalvavida commented Jul 5, 2016

@pjcozzi, @lilleyse What do you think about making the gltfFormat header field an options bitmask? Like this:

7-5 4-3 2-1 0
For future use 0->east/north/up, 1->2 normals, 2->2 normals oct-encoded 0->lat/long, 1->lat/long/height, 2->x/y/z, 3->x/y/z quantized 0->glTF uri, 1->glTF embedded

Doing this has the benefit of making sure that existing i3dm models don't break with these changes.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Jul 5, 2016

This is too complicated and backwards compatibility with existing i3dm is not important since it is not widely used. I would stick with what I proposed above; providing this many options makes the spec harder to implement, harder to understand, and harder to get standardized.

@lasalvavida
Copy link
Contributor

Alright, will do. My only real concern here is that forcing support for oct-encoding and quantization instead of making it optional may make it difficult for other engines to implement.

@lasalvavida
Copy link
Contributor

lasalvavida commented Jul 5, 2016

@pjcozzi For decoding the quantized coordinates, my suggestion would be to just store a vec3 float scale and have the origin implicitly at (0,0,0). Where in the header do we want to store the scale; should I just tack it on the end?

@lasalvavida
Copy link
Contributor

Also do we want to store that as float32 or float64?

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Jul 5, 2016

There should be a float64 vec3 "origin", then each instance should be quantized float32 vec3, which will be decoded and added to the origin.

Yes, the origin can be added to the end of the header along with any other decode data, mirroring the glTF extension.

@lasalvavida
Copy link
Contributor

Maybe I should have been clearer. Quantization is usually a scale and a translation. I'm saying to just store the scale and use the min long/lat/height from the tile json as the translation.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Jul 5, 2016

For precision, we'll want to store the translation as doubles and, to save memory, each instance as floats so I don't think we should use the min for translation. This is similar to CESIUM_RTC.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Jul 8, 2016

Cesium updates by @lasalvavida are in CesiumGS/cesium#4101.

I'll do spec updates after code review.

@lasalvavida
Copy link
Contributor

@pjcozzi Do you mind if I put in a pull request for some rough spec updates while it's still fresh in my mind?

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Jul 8, 2016

Please don't make them "rough." If you want to take a shot at doing a complete and careful writeup, then please do so, but doing "rough" edits will create more work for both of us.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Aug 19, 2016

The final touches are being added to #100, which I expect will be merged tomorrow.

@pjcozzi pjcozzi closed this as completed Aug 19, 2016
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

No branches or pull requests

4 participants