IFC categories with hierarchical relation

onderilkesever commented 4 months ago

Describe the bug 📝

I have an IFC model, I am loading to my scene using IfcLoader, as I used to do before. Then using classifier, i populate its entities. I am only interested in entities, so i only use 'byEntity' method and using classifier, i have entities and their FragmentIdMap. By using that maps, i have couple features such as hide/show, color.

I do not know after which exact version it started to behave differently but, after certain version, the classifier started to give a new category(seems to be parent of some) whose FragmentIdMap values not working correctly. Meaning, if i use its FragmentIdMap to pass to ifcFragmentHider to hide, it errors out that fragments could not be found.

I uploaded same model to BIMCollab tool to take a look at the hierarchical structure, and realised the newly populated entity is actually parent of some other entities. It hints me that the classifier now has a problem with ifc entities(categories) that has hierarchical relations.

Reproduction ▶️

No response

Steps to reproduce 🔢

1) load provided IFC file (https://drive.google.com/file/d/12neL8g3Sbl9NmgT1tj8g3f1FXIZycGUA/view?usp=sharing) 2) classify it using IFC classifier specifically byEntity method 3) observe classifier, see that IFCSTAIR exist, as well ass IFCMEMBER, IFCRAILING (children of IFCSTAIR) 4) try to hide IFCSTAIR by using FragmentHider 5) see that it errors out, and IFCSTAIR still visible

System Info 💻

System:
    OS: macOS 14.4.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 125.58 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.19.0 - ~/.nvm/versions/node/v18.19.0/bin/node
    npm: 10.2.3 - ~/.nvm/versions/node/v18.19.0/bin/npm
  Browsers:
    Chrome: 125.0.6422.112
    Edge: 125.0.2535.67
    Firefox: 123.0.1
    Safari: 17.4.1

Used Package Manager 📦

npm

Error Trace/Logs 📃

No response

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.

arunmuthiyarkath commented 4 months ago

I am also facing same issue. Do we have any update on this ?

agviegas commented 3 months ago

Hey! I checked it with the latest version of the library in the fragment hider tutorial and it works fine. See:

https://github.com/ThatOpen/engine_components/assets/56475338/20b32684-e9f0-4d62-99ed-43233f4dec7e

Indeed, when classifying per entity the library doesn't take children into account, and I think this is the correct behavior because it's less opinionated and avoids problems such as:

If you have an IFC with an IFCSITE that has a visual representation (a landscape, trees, etc), it wouldn't be possible to just hide that if the entity filter also took the children into account.
In order to take spatial hierarchy into account, the model needs to be indexed using the indexer. Indexing a model can be a heavy operation, especially in bigger IFC files. If someone really wants to just use categories, I don't think they should be force to index the model.

That said, you can easily take children into account in your app. You can use the indexer to compute the indirect relationships of your model (which also include the children), and when you hide certain elements, check their indices to also hide their children recursively.

The library also comes with a premade spatial tree you can use. Here you have the file I show in the video (I just took the hider tutorial and loaded your model instead).

If you have any questions about any of this, let us know and we'll help you out!

onderilkesever commented 3 months ago

Thanks for the detailed answer @agviegas ! I have tried your provided solution and indeed it works when you load ifc file directly. However, in our workflow, we convert IFC files to fragment file and store like that as suggested by you to have faster load and smaller file space. And I realised when using fragment file, classifier gives new 'extra' ifc category 'IFCSTAIR' which is not working as described in the issue itself. here is the fragment file for you to access -> https://drive.google.com/file/d/1TVkwCpJuBc0NORFYmg6E9lG4tsSW8cXj/view?usp=drive_link I thought maybe it might be related to properties and relations, and provided them to load function but still no luck. Can this be pointing to a bug on extract/import fragment workflow? or maybe i am missing something to configure. Thanks a lot for your help here!

agviegas commented 3 months ago

Hey @onderilkesever got it! Let me take a look and let you know when I solve it.

agviegas commented 2 months ago

It was indeed a but in the fragments importer. Fixed from @thatopen/fragments@2.1.2. Now both models (ifc and fragments) have the same behavior (the one that had the IFC, which is the correct one). Let me know otherwise. Cheers!

onderilkesever commented 1 month ago

Thanks @agviegas, i just validated fix on my project too. Thanks again for the support and the fix!

ThatOpen / engine_components