engine_fragment icon indicating copy to clipboard operation
engine_fragment copied to clipboard

FragmentMesh.clone fails with `Cannot read properties of undefined`

Open wlinna opened this issue 1 year ago • 7 comments

Describe the bug 📝

I'm trying to clone a FragmentsGroup. I assumed this would be possible, because FragmentsGroup extends Group. However, it fails with Cannot read properties of undefined.

The root of the problem seems to be that FragmentMesh (or maybe FragmentsGroup) doesn't defined a clone-method which results in the default clone being called. The default implementation is the following:

  clone(recursive) {
    return new this.constructor().copy(this, recursive);
  }

This calls the constructor of FragmentMesh. Unfortunately, FragmentMesh doesn't define default arguments which leads to problems, such as geometry being undefined.

To fix the issue, either a clone that calls the constructor with proper arguments should be implemented, or a constructor with defaults arguments should be implemented. I feel that the latter would be the Three.js way, but I haven't read enough fragment code to make a judgement.

Reproduction ▶️

No response

Steps to reproduce 🔢

  1. Create a FragmentsGroup anyway you want. In my case:
    const file = await selectFile();
    if (!file) {
        console.warn('File not loaded');
        return;
    }
    const data = await file.arrayBuffer();
    const buffer = new Uint8Array(data);
    const model = await fragmentIfcLoader.load(buffer, "example");
    components.scene.get().add(model);
  1. Try cloning a FragmentsGroup. For example, after loading the model call:
// fragments is a FragmentsManager
const group = fragments.groups[0];
group.clone(true);

Result:

Cannot read properties of undefined

System Info 💻

System:
    OS: Linux 6.6 Pop!_OS 22.04 LTS
    CPU: (20) x64 13th Gen Intel(R) Core(TM) i5-13600KF
    Memory: 13.53 GB / 31.19 GB
    Container: Yes
    Shell: 5.8.1 - /usr/bin/zsh
  Binaries:
    Node: 20.10.0 - /usr/bin/node
    npm: 10.2.3 - /usr/bin/npm
  Browsers:
    Chromium: 120.0.6099.71
  npmPackages:
    openbim-components: ^1.2.0 => 1.2.0

Used Package Manager 📦

npm

Error Trace/Logs 📃

Console logs!
openbim-components.js?v=4bd4a061:45990 Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'index')
    at _FragmentMesh.newFragmentGeometry (openbim-components.js?v=4bd4a061:45990:19)
    at new _FragmentMesh (openbim-components.js?v=4bd4a061:45942:26)
    at _FragmentMesh.clone (chunk-ADNH2OTW.js?v=4bd4a061:5238:12)
    at FragmentsGroup2.copy (chunk-ADNH2OTW.js?v=4bd4a061:5263:24)
    at FragmentsGroup2.clone (chunk-ADNH2OTW.js?v=4bd4a061:5238:35)
    at cloneAndUnravel (ifcmain.ts:107:23)
    at produceGlb (ifcmain.ts:102:17)
    at exportGlb (ifcmain.ts:137:21)
    at Object.exportGlb (ifcmain.ts:163:20)
    at HTMLButtonElement.<anonymous> (three_examples_jsm_libs_lil-gui__module__min.js?v=4bd4a061:145:44)

Validations ✅

  • [X] Read the docs.
  • [X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
  • [X] Make sure this is a repository issue and not a framework-specific issue. For example, if it's a THREE.js related bug, it should likely be reported to mrdoob/threejs instead.
  • [X] Check that this is a concrete bug. For Q&A join our Community.
  • [X] The provided reproduction is a minimal reproducible example of the bug.

wlinna avatar Dec 22 '23 15:12 wlinna

Monkey patching FragmentMesh.clone is complicated, because the fragment field is not easily cloneable. My workaround is to use Serializer on FragmentsGroup:

    const serializer = new Serializer();
    const exported = serializer.export(group);
    const clone = serializer.import(exported);

wlinna avatar Dec 22 '23 16:12 wlinna

Hey @wlinna - I'm getting the same issue. Is this still your workaround? Also where is Serializer defined?

mkarklins avatar Apr 01 '24 13:04 mkarklins

Hello I had forgotten this issue already. Yesterday I actually happened to try clone directly, and I think it worked, though I only tested with one model.

Are you using the newest version of engine_components and engine_fragment?

wlinna avatar Apr 02 '24 06:04 wlinna

Yes, using the latest code. Thanks for your response - this might be an indication that the problem is on my side.

mkarklins avatar Apr 02 '24 08:04 mkarklins

Sorry, I made a mistake. I didn't actually try the clone method, but instead tried exporting with GLTFExporter without cloning. I somehow confused the two. I might try clone later (for fun).

You can import Serializer from bim-fragment.

import { Serializer } from 'bim-fragment'

wlinna avatar Apr 02 '24 10:04 wlinna

If framents is... const fragments = new OBC.FragmentManager(components);

... and I import a model, then

fragments.groups[0].clone(false) (non-recursive clone) works, at least superficially (didn't verify), but fragments.groups[0].clone(true) (recursive clone) doesn't

Thus, for me the best option seems to export and import with Serializer. Do note, however, that the imported clone doesn't remember which items were hidden, and it might lack some other stateful data.

wlinna avatar Apr 06 '24 11:04 wlinna

I'm currently using your serialization approach. It did miss some "stateful data" as you mentioned, but that's easy to populate.

Anyhow - thanks for your guidance ✌️

mkarklins avatar Apr 06 '24 12:04 mkarklins

This is done! Closing this

agviegas avatar Jul 31 '24 11:07 agviegas