Export BatchTable, complementing some types #1434
Conversation
gkjohnson
left a comment
gkjohnson
left a comment
There was a problem hiding this comment.
Thanks! I added a few comments - I think it's best to keep the changes limited to BatchedMesh here if that's what you need.
I offload some computation in a web worker, where I need to re-instantiate the BatchTable, which I can only do if it is properly exported.
I'm curious to hear more about this. Are you seeing meaningful performance benefits? Which file formats are you using? Which bottlenecks is this helping with?
| fallbackPlane: Plane; | ||
| up: Vector3; | ||
| pivotPoint: Vector3; | ||
| clock: Clock; |
There was a problem hiding this comment.
Generally I'm only adding types for features I intend to be accessed by a user - basically the "public" API. So I'd prefer to keep the clock variable excluded here.
There was a problem hiding this comment.
Okay. I was only experimenting with it. I was wondering why the parameter in update() defaults to deltaTime = Math.min( this.clock.getDelta(), 64 / 1000 )? I was looking if it makes a difference when it always just getDelta controls.update(controls.clock.getDelta());. I couldn't directly see something (rendering on change, so some deltas should be much bigger I guess). So, what's the point here?
There was a problem hiding this comment.
The "delta" argument is included in "update" so users can pass a delta time value from their application and ensure a consistent delta time is used for animations across their application. A default delta time is included, however, to enable smooth animation even when a delta time is not passed to the function. And that delta is clamped to prevent large deltas due to frame spikes by default which can cause issues and large positional jumps with calculations that expect a small frame delta (eg a decaying inertia value).
There was a problem hiding this comment.
Thanks. I was just wondering why the delta is clamped.
src/core/renderer/index.d.ts
Outdated
| export * as TraversalUtils from './utilities/TraversalUtils.js'; | ||
| export * as LoaderUtils from './utilities/LoaderUtils.js'; |
There was a problem hiding this comment.
These are also not documented nor intended to be user-accessible. They're exported so they can be used in plugins, etc. I'm hesitant to make them public because I'm not prepared to consider these part of the API and document things like breaking changes.
There was a problem hiding this comment.
I am not using them. It was more like wondering that they are exported, but not the types. So I added them.
When you say they are supposed to be used in plugins, shouldn't it be possilbe to also use them in custom plugins not defined in this library? On the other side, it could be exported without officially documented and users who may know about it may use them at their own risk. Now, I can already import them as well, but only with extra // ts-ignore comments.
As I said I am not using them. So, I actually don't mind in this case. But I can imagine that at least TraversalUtils might be of interest.
There was a problem hiding this comment.
I would prefer not to expose functions publicly or give the impression they are reliable without a deliberate purpose so they can be changed without having to break user code. If someone has a need for them they can make an issue and we discuss adding these to the public API.
|
I am working with the Cesium OSM tiles. But I also have some custom 3D models of buildings. So, at those places where these 3D models are positioned, I want to hide the OSM buildings. But each OSM tile is one big geometry with batch ids. My first approach was to separate the big mesh geometry into individual geometries based on the OSM id and the corresponding batch ids. This quite expensive task I offloaded into a web worker, having the one geometry and batch table as input and multiple geometries as output. However, having these several hundred extra meshes in the scene decreased performance a lot. So, I ended up customizing a shader to just hide respective parts of the geometry (FragColor alpha = 0.0). I am still using a web worker to generate some edges for visual enhancement, and to map any batch id to respective OSM ids to quicker update the shader with the set of batch ids it's supposed to hide. This mapping isn't really a heavy task, but since I am using the web worker anyway I am also doing this in the web worker (saving maybe a few milliseconds on the main thread) as part of processing the OSM tile and enhancing information. |
|
I have undone some of the types. So, it's only BatchTable now. |
|
Thanks! |
2 participants
BatchTableis exported in the types.d.ts, however not in the.jsfiles. I offload some computation in a web worker, where I need to re-instantiate the BatchTable, which I can only do if it is properly exported. I also added some other types to the BatchTable/FeatureTable that are necessary for instantiation.I also saw that
TraversalUtilsandLoaderUtilsare exported in the.jsfiles, but not as types. So I added some types here as well.Also adding
clockinEnvironmentControls.d.ts.