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

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

Open
mgermerie opened this issue Jun 28, 2024 · 1 comment
Open

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

mgermerie opened this issue Jun 28, 2024 · 1 comment
Labels
enhancement New feature or request
Milestone

Comments

@mgermerie
Copy link

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.

  • The first is that such a method could perhaps be useful to other users than us, though I admit I didn't investigate on this matter.
  • 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. I think this is an optimization issue, especially when dealing with a large number of batches.

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 ?

@mgermerie mgermerie added the enhancement New feature or request label Jun 28, 2024
@mgermerie mgermerie changed the title [Batch table] implement a method to get data for a batch `ID [Batch table] implement a method to get data for a batch ID Jun 28, 2024
@gkjohnson
Copy link
Contributor

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.

@gkjohnson gkjohnson added this to the v0.3.36 milestone Jun 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants