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

[Architecture] inconsistent role of C3DTilesBatchTable #2261

Closed
mgermerie opened this issue Jan 16, 2024 · 1 comment
Closed

[Architecture] inconsistent role of C3DTilesBatchTable #2261

mgermerie opened this issue Jan 16, 2024 · 1 comment

Comments

@mgermerie
Copy link
Contributor

Your Environment

  • Version used: 2.41.0

Context

When parsing B3DM 3d-tiles with B3dmParser and if needed, we instantiate a C3DTilesBatchTable at those lines. The arguments given for C3DTilesBatchTable instantiation are an ArrayBuffer containing raw batch table data, and lengths of the table as well as its json and binary parts. Then, in C3DTilesBatchTable constructor, the ArrayBuffer is parsed : the json part and the eventual binary attribute values are decoded and the resulting JS Object is stored in a C3DTilesBatchTable property.

I believe this is an error in terms of architecture. A C3DTilesBatchTable should only store data from a given batch table and grant method to access, edit or eventually delete this data. It should not be also responsible for parsing raw data from a B3DM file.

Possible Cause/Fix/Solution

The parsing of the batch table from a B3DM file should be done directly in the B3dmParser, and C3DTilesBatchTable should take an already parsed batch table as constructor argument.

@jailln
Copy link
Contributor

jailln commented Sep 26, 2024

Obsolete since we moved to 3DTilesRendererJS

@jailln jailln closed this as completed Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants