Examples: Refactor webgl_loader_ifc to external demo.
#25440
Conversation
examples/webgl_loader_ifc.html
Outdated
| "three/addons/": "./jsm/" | ||
| "three/addons/": "./jsm/", | ||
| "three/examples/jsm/utils/BufferGeometryUtils": "./jsm/utils/BufferGeometryUtils.js", | ||
| "three-mesh-bvh": "https://unpkg.com/three-mesh-bvh@^0.5.21/build/index.module.js", |
There was a problem hiding this comment.
In the other examples we used fixed releases to avoid inadvertant breakage from package updates.
There was a problem hiding this comment.
Also, is a import of three-mesh-bvh needed here?
There was a problem hiding this comment.
Hey @gkjohnson are you suggesting to import a fixed version of three?
@LeviPesin yes, the IFCLoader needs it!
There was a problem hiding this comment.
Hey @gkjohnson are you suggesting to import a fixed version of three?
I should have been more specific - the ^ version specifier means the version of three-mesh-bvh is not fixed. It would be
best to just lock the version at the current latest without the caret. See #23907 (comment).
| @@ -25,7 +25,11 @@ | |||
| { | |||
| "imports": { | |||
| "three": "../build/three.module.js", | |||
| "three/addons/": "./jsm/" | |||
| "three/addons/": "./jsm/", | |||
| "three/examples/jsm/utils/BufferGeometryUtils": "./jsm/utils/BufferGeometryUtils.js", | |||
There was a problem hiding this comment.
Why you are separately importing BufferGeometryUtils? Both three/examples/jsm/utils/BufferGeometryUtils.js and three/addons/utils/BufferGeometryUtils.js (preferred) imports should work -- not sure about extensionless import.
There was a problem hiding this comment.
Because the IFCLoader needs it to work without bundling 🤔 Should I change this?
There was a problem hiding this comment.
If you're specifying the three/addons path in the import map you can import from three/addons/utils/BufferGeometryUtils.js instead of adding another import map entry.
There was a problem hiding this comment.
I see. The current bundle of web-ifc-three imports from three/examples/jsm/utils/, so to get rid of this entry in the import map, I would need to update all the imports in the library. Is the three/examples/ route deprecated? If not, maybe I can change the imports in the web-ifc-three library and get rid of this entry of the importmaps in another PR?
There was a problem hiding this comment.
Is the three/examples/ route deprecated?
No, it isn't. We just prefer to use the "addons" term since it's more appropriate than "examples".
There was a problem hiding this comment.
If not, maybe I can change the imports in the web-ifc-three library and get rid of this entry of the importmaps in another PR?
👍
There was a problem hiding this comment.
I use the /examples/jsm folder in three-mesh-bvh, as well, so the import maps in the path tracer example list both three/addons/ and three/examples/, too. At the moment I'm not planning to change the bvh project imports:
three.js/examples/webgl_renderer_pathtracer.html
Lines 45 to 55 in 4503ef1
|
Seems like the screenshot requires updating. |
|
Is the screenshot the only thing missing here? |
|
Sorry, should I just upload an image? |
|
Just use |
|
I run the command
|
Because of a (fixed) bug there -- you can merge the
The E2E test sees that it is not the same. |
|
Hum, strange. I merged the changes from dev and did it, and the new screenshot looks black 🤔 But if I execute the viewer, it shows correctly.
|
|
I went ahead and updated the screenshot in the branch for you so this should be ready to merge. It's hard to say why the screenshot would render black on your machine 🤔. |
@agviegas What OS you are using? Does |
|
I'm on windows. I've no idea why, but now the screenshot is correct. I just re-run the command again 🤔 Thanks a lot for the help! |
Mugen87
changed the title
webgl_loader_ifc to external demo.
Description
As asked here, remove the code of the IFCLoader from the library to keep it here as the single source of truth. Leave the example import it using source maps, like it does with three-mesh-bvh.