Support batch table binary section for pnts #4112
Conversation
| colors = new Uint8Array(length); | ||
| for (var i = 0; i < length; ++i) { | ||
| colors[i] = 64; | ||
| } |
There was a problem hiding this comment.
Hm I think I'm going to fix this so the attribute is just a single value.
There was a problem hiding this comment.
Actually this won't work easily with GeometryAttribute.
There was a problem hiding this comment.
Can't we provide a uniform using an appearance?
Otherwise put a TODO here (not PERFORMANCE_IDEA) so we optimize this before merging into master.
It doesn't need to be part of this PR and it might be a new class in Cesium, but we will want per-point properties that can be used for dynamic styling and vertex shader generation, see CesiumGS/3d-tiles#22 (comment) |
OK...it should include alpha and be the same range as the per-point color, e.g., |
|
That reminds me I should also include a TILES3D_RGBA semantic. |
+1 |
|
Just those comments. |
|
Updated to support RGBA. What is the best way to render |
|
Ah I messed up the merge. Fixing that now. |
lilleyse
force-pushed
the
batch-table-binary
branch
from
1393820 to
45e6a94
Compare
Never mind, I have an idea. It will require modifying the |
| // Get the point colors | ||
| var tiles3DRGB = batchTableJSON.TILES3D_RGB; | ||
| var tiles3DRGBA = batchTableJSON.TILES3D_RGBA; | ||
| if (defined(tiles3DRGB)) { |
There was a problem hiding this comment.
What do you think about CesiumGS/3d-tiles#22 (comment)
Should the tile be invalid (in practice if someone wants multiple per-point colors, they would use their own semantics)? Or give should RGBA win over RGB?
|
Updated. I'm building the shaders based on whether it's RGB, RGBA, or constant color, but still using |
|
Very nice, thanks @lilleyse! |
| var batchTableBinaryByteLength = view.getUint32(byteOffset, true); | ||
| byteOffset += sizeOfUint32; | ||
|
|
||
| var positions = new Float32Array(arrayBuffer, byteOffset, pointsLength * 3); |
There was a problem hiding this comment.
Actually, in b3dm and i3dm, the batch table comes before glTF. We should be consistent here for now and move the positions payload after the batch table; however, we might want to switch them all later if there is a benefit, e.g., to avoid parsing it in some cases?
There was a problem hiding this comment.
Ah that's true, I'll make the change in a new PR.
There was a problem hiding this comment.
I'm surprised we haven't run into this before, but TypedArray constructors expect their byte offset to be aligned to the type. For two of the points tilesets, the byte length of the batch table is odd and this causes problems when creating a Float32Array of the positions. Do we want to require byte alignment in the spec?
There was a problem hiding this comment.
Yes, definitely need to say this in the spec. @tfili and I have ran into this before and I know some of the work we did included the padding
There was a problem hiding this comment.
glTF spec guarantees this btw. https://github.com/KhronosGroup/glTF/tree/master/specification#bufferview-and-accessor-byte-alignment
For CesiumGS/3d-tiles#32 (comment)
The pnts format now loads colors through the batch table JSON/binary, or uses a default color if there is no batch table or no
TILES3D_RGBproperty. I'm not creating abatchTableResourcesforPoints3DTileContentsince that will probably require some more thought it we even want per-point properties.One question: do we want to allow the pnts format to specify a default color? If it's in the batch table it could look like: