Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] type <--> name swapped on metaScene.metaObjects #1185

Closed
cvillagrasa opened this issue Oct 19, 2023 · 8 comments
Closed

[FIX] type <--> name swapped on metaScene.metaObjects #1185

cvillagrasa opened this issue Oct 19, 2023 · 8 comments

Comments

@cvillagrasa
Copy link
Contributor

Describe the bug
Hi! I'm a new user of xeokit, and I'm trying to include it on Jupyter Notebooks cells so as to provide visualizations for IfcOpenShell-generated models in Python. Reference: IfcOpenShell/ifcopenshell-jupyter#1 (comment)

I'm using a custom data source (the IFC models in memory themselves), and a WebIFCLoaderPlugin. The issue I'm having appears when querying metaObjects. The type key refers to the Name, and the name key sometimes refers to the IFC class, and sometimes to a seemingly random value, depending on the lib version being used.

To Reproduce
console.log(viewer.metaScene.metaObjects);

I've tried with [email protected], as well as with 2.4.0-alpha-90. In the Colab from the link above there's the full JavaScript, but it's mainly what I explained: custom data source from a string + WebIFCLoaderPlugin, and there's something strange triggering this swap. When using web-ifc from WebIFCLoaderPlugin._ifcAPI, GetLine returns the properties as expected.

Expected behavior
type <--> name the other way around

Screenshots
wall_name_type

Desktop (please complete the following information):

  • OS: Ubuntu 22.04.3 as local, and Ubuntu 22.04.2 as cloud (Google Colab)
  • Browser: Chrome
  • Version: 118.0.5993.70

Thanks in advance!

@cvillagrasa
Copy link
Contributor Author

Mmm so the little bug fix makes the name appear on the name, but the type is still behaving weirdly with the minimized version of the lib from a CDN:

wall_type_bug

The screenshot corresponds to using WebIFCLoaderPlugin()._ifcAPI.GetLine on 2.4.0-alpha-99/dist/xeokit-sdk.es.min.js.

The non-minimized version xeokit-sdk.es.js works ok and returns the expected "IfcWall" for constructor.name. The last stable 2.3.9 version also works ok in its minimized form, so it should be something between web-ifc 0.0.34 and 0.0.41 making rollup unhappy.

The web-ifc built-in GetNameFromTypeCode does work on both minimized and non-minimized versions, but as shown, returns the IFC class uppercase, which is not ideal.

In any case, I'll bundle it and avoid the problem, so feel free to close the issue if deemed not super important.

@xeolabs
Copy link
Member

xeolabs commented Oct 22, 2023 via email

@cvillagrasa
Copy link
Contributor Author

Type is still "Hr" on alpha-100 minified.

@xeolabs
Copy link
Member

xeolabs commented Oct 24, 2023

I think to track this bug/fix properly, would you be able to provide a standalone HTML page that loads the file and shows the problem?

@cvillagrasa
Copy link
Contributor Author

So in the notebook from the following link, you just need to press Play on the highlighted four cells:

https://colab.research.google.com/drive/1wVjX4H3w-NFqFa_FVnefP14bDqzHTbjX?usp=sharing

image

And will see the following output on the console:

image

Then, if you unfold the "Setup of xeokit" tab, you'll see some boilerplate on the first two cells, the JS on the third, the CSS on the fourth, and the HTML on the fifth.

On that third cell with the JS, this function near the end is what's outputting that intriguing "Hr" to the console:

image

At the top, you may easily comment / uncomment the minified / non-minified versions:

image

After changing the JS, just run that third cell, then the fifth (HTML) and lastly the "Model viz with xeokit" at the end (so press play in those three cells). That allows to live edit the code, and will output the new results to the console.

You'll see that with the non-minified, everything works as expected:

image

Intriguing!

@paireks
Copy link
Member

paireks commented Jan 17, 2024

Hello Carlos!

First of all - great notebook :)

It looks like minimizing JS files with default settings changes class names to make them shorter, so the class name were renamed to Hr, therefore later after use of constructor.name it probably returns minimized version. Generally constructor.name should't be used client-side because of that, however one of the solutions on a Xeokit side might be probably changing terser options to keep original class names (keep_classnames: true). But it's up to @xeolabs to decide if such change in configuration is necessary and justified.

@cvillagrasa
Copy link
Contributor Author

Hey @paireks ! nice to see you around here ;) I need to find some time to work on this repo and finally have a nice ifcopenshell + xeokit pip-installable package for notebooks.

Good research! Maybe the best global solution is that GetNameFromTypeCode from web-ifc API returned the IFC class in PascalCase, instead of UPPERCASE, which is ultimately destructive. But they've had it like this for so long, and it's a breaking change...

@xeolabs
Copy link
Member

xeolabs commented Apr 21, 2024

AFAIK this is fixed with the PR mentioned above - will re-open if not.

@xeolabs xeolabs closed this as completed Apr 21, 2024
@xeolabs xeolabs changed the title type <--> name swapped on metaScene.metaObjects [FIX] type <--> name swapped on metaScene.metaObjects Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants