chore(tile-converter): i3s-converter - mitigate "tiles" module dependency #2517
Conversation
belom88
commented
Jun 16, 2023
•
belom88
commented
- removed Tileset3D & Tile3D dependency
- it gives a memory usage improvement (about 10% for Frankfurt dataset)
belom88
changed the title
There was a problem hiding this comment.
I am confused about the goals of this PR and I worry that we may have a misunderstanding.
The goal is not to hide the generic Tile3D from the tile-converter and replace it with a tiles3d specific class. In fact, this approach seems to significantly increase the amount of format specific knowledge that the converter has to deal with, and taking less advantage of the generic tiles module.
Regardless there some good typing here - I will approve but wish we had discussed more first.
| @@ -163,3 +163,16 @@ export type TypedArrayConstructor = | |||
| | Uint32ArrayConstructor | |||
| | Float32ArrayConstructor | |||
| | Float64ArrayConstructor; | |||
|
|
|||
| export type Tiles3DLoadOptions = { | |||
There was a problem hiding this comment.
Why are these options defined in the tile-converter?
There was a problem hiding this comment.
This is the i3s-converter specific options
There was a problem hiding this comment.
I've found that we can use Tiles3DLoaderOptions here. Fixed in #2520
| @@ -7,9 +7,10 @@ import convertB3dmToI3sGeometry, { | |||
| getPropertyTable | |||
| } from '../../../src/i3s-converter/helpers/geometry-converter'; | |||
| import {PGMLoader} from '../../../src/pgm-loader'; | |||
| import {calculateTransformProps} from '../../../../tiles/src/tileset/helpers/transform-utils'; | |||
There was a problem hiding this comment.
Are these importing across module boundaries? Preferably we should use scoped imports.
There was a problem hiding this comment.
In tests we use other modules internals to make fake data.
We have different classes:
So it is OK that Initially we decided to get rid of |
belom88
changed the title
|
TODO: add tests and TSDoc comments for new code in another PR |
dsavinov-actionengine
added
the
ActionEngine
