NASA-AMMOS / 3DTilesRendererJS

Renderer for 3D Tiles in Javascript using three.js
https://nasa-ammos.github.io/3DTilesRendererJS/example/bundle/mars.html
Apache License 2.0
1.46k stars 265 forks source link

[Batch table] implement a method to get data for a batch `ID` #595

Open mgermerie opened 1 week ago

mgermerie commented 1 week ago

Is your feature request related to a problem? Please describe.

Hi, thanks for this awesome library ! I'm currently working on using it to render 3d tiles data in iTowns which is a geographic data visualization framework. I am more specifically working on batch tables at the moment.

In our previous implementation, we had a method that read all data in a batch table for a given batch ID, whether those data are stored directly in the batch table or using 3DTILES_batch_table_hierarchy extension.

If you would be willing, I wish to implement such a method in your library.

Describe alternatives you've considered

The BatchTable you already implemented provides properties and methods which allow users to implement such a method on their side. However, I considered two elements on that idea.

Describe the solution you'd like

I propose to implement a getDataForBatchId method in BatchTable (the name is just a suggestion) which, for a given batchID parameter, would return an object as such :

{
    batchTableProperty1: value1,
    batchTableProperty2: value2,
    // ...All other properties in the batch table
    extensions: {
        3DTILES_batch_table_hierarchy: {
            className1: { ...valuesForClass1Properties },
            className2: { ...valuesForClass2Properties },
            // ...All other classes in the extension of the batch table
        },
    },
}

Would the idea of implementing such a method suit you ?

gkjohnson commented 6 days ago

Hello! I think adding such a function is a good addition. I'm imagining the API looking like so:

batchTable.getDataForId( id, target = {} );

The target object is the object to set all values on so garbage creation can be avoided.

extensions: {
       3DTILES_batch_table_hierarchy: {
           className1: { ...valuesForClass1Properties },
           className2: { ...valuesForClass2Properties },
           // ...All other classes in the extension of the batch table
       },
   },

Can you explain this? I'm not understanding why we would return the list of extensions on every result. These extensions should be retrieved from the batch table directly if needed, I think.

Moreover, reading all entries in a binary batch table for a given batch ID with your BatchTable.getData method requires parsing the whole binary batch table when we could just parse the buffer part corresponding to the batch ID

Just a note that the getData function is not doing anything complicated per call. The overhead should be very low beyond creating a new array buffer typed array wrapper instance. These could be cached if needed, though.