Batch table #4328
Batch table #4328
Conversation
… usage changed code.
…n attribute which brings polylines down to 13 vertex attributes. Fix failing test.
Submit an issue or update the appropriate roadmap. |
| * @example | ||
| * // create the batch table | ||
| * var attributes = [{ | ||
| * functionName : 'getShow()', |
There was a problem hiding this comment.
() here, but not with getPickColor. Which is it?
| * // ... | ||
| * } | ||
| */ | ||
| function BatchTable(attributes, numberOfInstances) { |
There was a problem hiding this comment.
This can be @private, right?
There was a problem hiding this comment.
Although a good bit of this code is clearly from the 3D Tiles batch table, it doesn't look realistic that 3D Tiles and Cesium primitives could share a common class. Do you agree?
If so, is there significant helper functions that are worth sharing?
There was a problem hiding this comment.
Yes, it is @private.
I don't see why 3D Tiles wouldn't use this as a helper class. Unless you want to keep the option of not having to require VTF, then we could separate this out into helper functions.
| BatchTable.prototype.update = function(frameState) { | ||
| var context = frameState.context; | ||
| if (this._pixelDatatype === PixelDatatype.FLOAT && !context.floatingPointTexture) { | ||
| // TODO: We could probably pack the floats to RGBA unsigned bytes but that would add a lot CPU and memory overhead. |
There was a problem hiding this comment.
Leave the comment, but drop the TODO; I don't expect that we would implement this fallback given how rare it should be.
| * @exception {DeveloperError} instanceIndex is out of range. | ||
| * @exception {DeveloperError} attributeIndex is out of range. | ||
| */ | ||
| BatchTable.prototype.getEntry = function(instanceIndex, attributeIndex, result) { |
There was a problem hiding this comment.
Throughout is Entry the best term? Seems like we just introduced it. What is this called in Cesium primitives, an attribute, right? Perhaps getBatchedAttribute and friends?
| } | ||
| } | ||
|
|
||
| // TODO: add support for color from the batch table? |
|
@pjcozzi This is ready for another look. |
|
Even though this doesn't change the Cesium API, can you update CHANGES.md to say that the extension is now required? |
|
The following test passes in master and fails in this branch: |
| var length = attributes.length; | ||
| for (var i = 0; i < length; ++i) { | ||
| if (attributes[i].componentDatatype !== ComponentDatatype.UNSIGNED_BYTE) { | ||
| foundFloatDatatype = true; |
|
Just those comments. |
|
@pjcozzi This is ready. |
Adds a batch table similar to what is used by 3D tiles, but can also be used for batched polylines and geometry. Currently only hooked up to the
PolylineCollection. It can replace the implementation of per-instance attributes forPrimitive, but that won't happen for this PR.This needs to be merged before #4309.