Refactor web ifc viewer (extract functions) #2
Conversation
| const geometry = getBufferGeometry(modelID, placedGeometry); | ||
| const material = getMeshMaterial(placedGeometry.color); | ||
| const mesh = new THREE.Mesh(geometry, material); | ||
| // mesh.frustumCulled = false; why? |
There was a problem hiding this comment.
Good question. For PC, I was getting more overhead from calculating the culling than I was gaining by not drawing something outside the frustum. For IPad, its inversed.
I have no strong feelings either way.
| function getBufferGeometry(modelID: number, placedGeometry: any) { | ||
| //@ts-ignore | ||
| const m: any = Module; | ||
| const geometry = m.GetGeometry(modelID, placedGeometry.geometryExpressID); |
There was a problem hiding this comment.
This module stuff should not be in the viewer, it should be part of web-ifc-api.ts but I never moved it over.
Also, we should look into renaming the Module to something more descriptive, or loading it differently in the browser...
There was a problem hiding this comment.
Yes, I agree. Probably the API design could be more abstract. In the current state, all users using the library will have to reproduces the code of web-ifc-viewer.ts or do something similar (including the low-level logic to get the geometry). Maybe it would be interesting to make a wrapper around web-ifc.js with a function like:
async function loadIfc(ifcData : string, module : any = Module) : THREE.Object3DWhere the function awaits the WASM to be loaded and the Object3D would have all the generated geometry as children. I think this would make the use of the library easier / plug-and-play.
In addition (but not necessarily) this may be an opportunity to decouple WASM from THREE, so that if tomorrow another API for Babylon (or a custom viewer in WebGL) needs to be made, it can be done easily without changing the parser.
There was a problem hiding this comment.
Indeed, but then it needs to be sufficiently abstracted, perhaps we can make a package called web-ifc-three or similar
| function getSubArray(heap, startPtr, sizeBytes) { | ||
| return heap.subarray(startPtr / 4, startPtr / 4 + sizeBytes).slice(0); |
There was a problem hiding this comment.
This slice(0) thing is needed because we can't point at the emscripten heap if it reallocates anything. The reallocation breaks the typed subarray, so we copy it by using slice.
Anybody working with the API would have to deal with this, so maybe we should move this to the web-ifc-api.ts
Decomposition of large functions into atomic functions. Extracting the scene creation code from Three.js into a separate file.