ThatOpen / web-ifc-viewer

Graphics engine and toolkit for client applications.
MIT License
931 stars 231 forks source link

Subset is not hidden when removeFromParent is called #116

Closed m1900 closed 2 years ago

m1900 commented 2 years ago

I have followed the examples regarding subsets and hiding and have created my own code to create subsets per storey to hide and show storeys. The subsets are created correctly, as far as I can tell. If I set a material, the material is removed when I deselect a storey, and added when I select a storey. However, if I do not set a material for the subsets, the subsets are not hidden from view when I deselect a storey. That puzzles me, because I use nearly the same code as in the examples. The only difference is that I use the IfcViewerAPI, whereas in the examples IfcLoader is used.

These are the lines I have for adding and removing subsets:

if (checked) { viewer.IFC.context.scene.add(subset); } else { await subset.removeFromParent(); }

And I create the subsets in this way:

const subset = viewer.IFC.loader.ifcManager.createSubset({ modelID: modelID, scene: viewer.IFC.context.scene, ids: childIDs, // ids of the child elements of the storey removePrevious: false, customID: storeyName, });

An extract of my code can be found in commit 849284e9ed73798e446493e303c96f96b9b7ae3b on my repo ProtoIfcViewer. I wonder if I missed something.

axelra82 commented 2 years ago

I'm also having problems with the removeFromParent method.

Versions

Main package

"vue": "^2.6.11",

Node modules

"name": "web-ifc-three",
"version": "0.0.116",
"name": "web-ifc",
"version": "0.0.33",
"name": "three",
"version": "0.135.0",

Material

const material = new MeshLambertMaterial({
  transparent: true,
  opacity: 0.6,
  color: 0xff88ff,
  depthTest: false,
});

Subset toggle function

itemId is expressID of storey and ids is an array of expressIDs from recursive children of storey.

async ifcHideLayers(
  { commit, getters }: { commit: Commit; getters: any },
  payload: { itemId: number; ids: number[] }
): Promise<void> {
  const { itemId, ids } = payload;
  const ifcManager: IFCManager = getters["getIfcManager"];
  const modelId: number = getters["getIfcModelId"];
  const scene: Scene = getters["getScene"];
  const hidden: number[] = getters["getIfcHidden"];
  const isHidden: boolean = hidden.includes(itemId);
  const subset = ifcManager.createSubset({
    modelID: modelId,
    scene,
    ids,
    removePrevious: true,
    customID: itemId.toString(),
  });

  if (isHidden) {
    commit("removeIfcHidden", ids);
    scene.add(subset);
  } else {
    commit("addIfcHidden", ids);
    subset.removeFromParent();
  }
}

Previously I was using the IFCLoader from three/examples/jsm/loaders/IFCLoader and with the showItems and hideItems methods this was working just fine/as expected. Sort of wondering why those where removed? 🤔 Seems a lot easier to provide model ID and ids to show/hide than having to create a subset and then (unsuccessfully, in mine and apparently other cases) having to remove it 🤷‍♂️

if (isHidden) {
  commit("removeIfcHidden", ids);
  ifcManager.showItems(modelId, ids);
} else {
  commit("addIfcHidden", ids);
  ifcManager.hideItems(modelId, ids);
}
brifitz commented 2 years ago

You can delete a subset with subsetManager.removeSubset(modelID: number, material?: Material, customID?: string), or remove individual entities (i.e. hide them) from a subset using subsetManager.removeFromSubset(modelID: number, ids: number[], customID?: string, material?: Material).

axelra82 commented 2 years ago

You can delete a subset with subsetManager.removeSubset(modelID: number, material?: Material, customID?: string), or remove individual entities (i.e. hide them) from a subset using subsetManager.removeFromSubset(modelID: number, ids: number[], customID?: string, material?: Material).

@brifitz thank you for the reply 😊 However, I'm not sure I follow:

Just to clarify what I'm trying to achieve (which was easily done with the deprecated web-ifc from ThreeJS) is to hide/show parts of an existing model (I'm not looking to change colors for highlight, I already have a working function for that).

So far using the removeSubset and removeFromSubset hasn't worked. Granted I'm surely missing something in my logic, I just can't see what that might be.

Here are examples of what I've tried (see my initial post for details on the entire function):

const subset = ifcManager.createSubset({
  modelID: modelId,
  scene,
  ids,
  removePrevious: true,
  customID: itemId.toString(),
});
scene.add(subset);
ifcManager.removeSubset(modelId, undefined, itemId.toString());
ifcManager.removeFromSubset(modelId, ids, itemId.toString());

I've tried these, without the isHidden check. The expected result would be that the selected ids should not show, but they persist. Commenting out the "remove" functions and simply logging the ifcManager I can see that the subset is created correctly.

scene.add(subset);
console.log(ifcManager);

Then in the log IFCManager > subsets (SubsetManager) > subsets I have an object 0 - DEFAULT - 183 with:

bvh: false,
ids: Set(252), // 3 arrays
mesh: Mesh, // mesh object

Using removeSubset removes the subset as expected and with removeFromSubset the subset stays but the ids (set) is empty. Both of these seem to work as intended. The problem is that the subset has no effect on the model itself, compared to the showItems and hideItems methods, which hides and shows the items in ids (an array of express ID numbers).

Any thoughts on what could be missing/going wrong here? 🤓

brifitz commented 2 years ago

You should replace the model itself by a subset at the start of your code and from then on only work with subsets instead of the model. You can see an example of how this is done here here - https://github.com/IFCjs/hello-world/tree/main/examples/web-ifc-viewer/visibility

axelra82 commented 2 years ago

@brifitz ok, thank you again for your reply! 👍

I will try that then, but overall that makes it a lot more complicated to just show/hide than with the previous methods that were available. Do you know why they were removed?

This way I have to know in advance what subsets the user will want... that might not always be feasible 🤔

Take for example a large building model, with multiple storeys with walls, windows, floors etc. Before displaying the model I would have to go over the entire spatial tree and then build subsets for a bunch of different scenarios... as opposed to just doing it "on the fly" (with the older. ThreeJS web-ifc showItems and hideItems methods) with a set of ids.

Just so I understand correctly 🤓

brifitz commented 2 years ago

If you turn the model into a subset at the start, then passing a set of ids to of entities to hide to removeFromSubset is effectively the same as hideItems, and it can be used to hide items on the fly. For example, hiding a set of entities that the user has selected by passing the ids of the selected entities to removeFromSubset. To undo the hiding, just recreate the subset with the full set of ids.

I'm not aware of why showItems and hideItems were removed, maybe it was to standardize the way of doing things around subsets, rather than allowing operations with both the model and subsets.

axelra82 commented 2 years ago

@brifitz yes, I realized that this must be the way to use it just after writing my previous reply.

The only thing that's unclear at this point is how to "turn the (entire) model into a subset" 🤔

Below is an excerpt of my attempt to "turn the model into a subset" in my IFC loading method. It doesn't work, and I wasn't expecting it to either, I just wanted to show the initial test. The only issue I can see here is if I have to include all express IDs of all children, i.e. doing a recursive ids.push(child.expressID) type function. That ids array could get ridiculously big on real world building models. Then this doesn't seem very feasible anymore 😅

[...]
const modelId = model.modelID;
const ifcModel = await ifcManager.getSpatialStructure(modelId);
const ids = [ifcModel.expressID];

console.log(modelId); // 0 for now since there is currently only one (1) model in scene for dev
console.log(ids); // one id (120) in an array

ifcManager.createSubset({
  scene,
  modelID: modelId,
  ids,
  applyBVH: true,
  removePrevious: true,
  customID: `model-${modelId}`,
});

scene.add(model);
[...]

This would effectively produce a subset, with a custom id of model-0. So it can be properly identified. However, the subset only contains the IFCPROJECT id (120). In turn creating a new subset to "hide", again, does nothing here. Since, from what I'm starting to gather, I would have to temporarily remove the ids from that subset, which at the moment is just one number (the IFC Project express ID).

The only way I see this working now is if the model has a subset of all available express IDs in the entire model.

I haven't found any existing method for getting all recursive children express IDs with the web-ifc.

I've made my own recursive child express ID function which works for highlighting, but I don't think that would work on the entire model (if I'm looking ahead when there will be real world models... the array of IDs could get in the tens of thousands or more maybe).

Regarding removing the "show/hide" methods, if any developers get wind of this... please bring that back 😂 It was an extremely clean and clear way of dealing with this very scenario (showing/hiding parts of the model).

PS. @brifitz thank you for the feedback/dialogue 🙌

brifitz commented 2 years ago

@axelra82 No problem, I'm happy to help out :thumbsup:

See this for an example of how you turn the entire model into a subset. I believe that example does everything you need.

As you can see, the ids for the entire model are taken directly from the internal expressID array in getAllIds() so there is no need for recursive parsing of the tree.

By the way, there is no need to use the model id as part of the customID, as internally the model id you passed in during the call to createSubset() is already being used to identify subsets. You could use something like 'full-model' for the customID instead. Internally, subsets are identified using strings with the following format - {modelID} - {material.uuid} - {customID}

axelra82 commented 2 years ago

@brifitz that looks like a good approach 👍 However when I tested it and logged out the result of getAllIds I noticed that there were ids missing.

What I'm doing is showing the model as expandable groups (accordion style) in a sidebar when selecting the model in a list of items from the ThreeJS scene, on the side (this list is also in a sidebar and it switches to the model content when selecting the model).

It's divided in the following way:

[MODEL_NAME_FROM_SECENE_ITEMS_LIST]

I'm also filtering out all IFCSPACE types. All of these have express IDs but some don't have any geometry data. ifcModel.geometry.attributes.expressID.array seems to only get ids of items that have geometry data.

I did however find something in the ifcApi that allowed me to get the express ID of every single item in the model, ifcAPI.LoadAllGeometry(modelId). The reason for wanting all ids is so you can click one item in the list and from there recursively get all child ids, to hide/show/highlight etc 🤓

interface ModelGeometry {
  _data: Vector<FlatMesh>;
  _size: number;
}

const loadModelGeometry = await ifcManager.ifcAPI.LoadAllGeometry(modelId);
const modelGeometry = loadModelGeometry as unknown as ModelGeometry;
const geometryData = modelGeometry._data;
const ids = Array.from(Object.values(geometryData), (item: FlatMesh) => item.expressID);

// ids now has all ids in model, including those without any data in geometry.

ifcManager.createSubset({
  modelID: modelId,
  ids,
  applyBVH: true,
  scene,
  removePrevious: true,
  customID: "model-subset",
});

model.removeFromParent(); // taken from https://github.com/IFCjs/hello-world/blob/main/examples/web-ifc-viewer/visibility/app.js#L34
scene.add(model);

While this does create a subset with all the ids in the model, still, nothing happens when removing ids from the subset. My best guess is because the model in itself hasn't changed, so I'm still just removing ids from a subset, but the original is still there.

model.removeFromParent(); doesn't seem to have any effect 🤷‍♂️

The only difference for me with the example is that I'm using the web-ifc-three and my scene ("viewer" in example) is a ThreeJS scene, so I can't do the same as in the function replaceOriginalModelBySubset.

This is getting close though, I can almost taste it 😅 So far all thanks to the help of @brifitz 🙌

axelra82 commented 2 years ago

@brifitz It worked 🥳 the problem was with my recursive child selection function that was getting the ids to remove. It wasn't properly traversing every item after the non-geometric objects. So effectively I was only removing ids of groups that didn't contain any geometry 😂

I've been dealing with this for a couple of days now, but once I have everything working properly I'll try and find the time to put together some documentation on what I did, so that others that might be struggling with the same thing can take part of the result 👍

As a final question, do you know of any built in way of getting all the child ids of an IFC node? Say for example I have a group called "Basic Roof", which has express ID 180 + one (1) IFC node in children, which has express ID 26233 + one (1) IFC node in children, which has express ID 26273 with no nodes in children (an empty children array would be the break point).

Are there any tools in web-ifc to get all recursive child ids from that (i.e. resulting in an array of [180, 26233, 26273])?

Currently I have a custom recursive child function (though I'm dealing with some async issues) and it's not properly getting ids of the last items.

Just wanted to check before putting to much time on a custom function if there already is one 🤓

Big thanks again @brifitz 🙌 for all your help. I'm sure this would have taken me at least a week longer if I had to try and decipher everything, with documentation on IFCjs that doesn't really mention any of this.

axelra82 commented 2 years ago

Interestingly enough there doesn't seem to be any addToSubset method 🤔 If there's a removeFromSubset wouldn't it make sense to have an "add" method?

Getting all the ids of a model doesn't help in the previously described scenario.

Let's say the user hides a selection of ids (effectively removing them from the subset, which is now the entire model), "putting back" that selection would just mean running the getAllIds function again.

However, consider a (common) case, where the user hides 2 or more storeys, and then toggles back 1 of those to show. Then we would have to get the entire list of ids from the model again (getAllIds), filter out those that are left in the "hidden" state and do this every time. It just seems very messy 😅

Although I still think that the original showItems and hideItems methods made for a lot better DX, in regards to this whole process. Now that I (finally, and again, with the help of @brifitz) have it "working" with the removeFromSubset I'm again halted by the fact that there's no corresponding way of adding ids to a subset.

Overall it just feels like there are some methods missing on the IFC Manager for easily dealing with the model.

Sidenote

Even the loader could have an option to load the model as a subset, abstracting that away so developers don't have to do this 👇 every time

IFC loading logic

const ifcLoader = new IFCLoader();
const ifcManager = ifcLoader.ifcManager;

commit("setIfcManager", ifcManager);

await ifcManager.useWebWorkers(true, "/IFCjs/IFCWorker.js");
ifcManager.applyWebIfcConfig({
  COORDINATE_TO_ORIGIN: false,
  USE_FAST_BOOLS: false,
});

ifcManager.setupThreeMeshBVH(computeBoundsTree, disposeBoundsTree, acceleratedRaycast);

// THIS
// ************
const model: IFCModel = await ifcLoader.loadAsync(modelURL);

const modelId = model.modelID;
const ids = await dispatch("ifcGetAllIds", modelId);

ifcManager.createSubset({
  modelID: modelId,
  ids,
  applyBVH: true,
  scene,
  removePrevious: true,
  customID: "full-model",
});

scene.add(model);
model.removeFromParent();
// ************
[...]

Get all IDs function

A Vue store module in this case.

async ifcGetAllIds({ getters }, modelId): Promise<number[]> {
  const ifcManager: IFCManager = getters["getIfcManager"];
  const loadModelGeometry = await ifcManager.ifcAPI.LoadAllGeometry(modelId);
  const modelGeometry = loadModelGeometry as unknown as ModelGeometry;
  const geometryData = modelGeometry._data;
  const ids = Array.from(Object.values(geometryData), (item: FlatMesh) => item.expressID);
  return ids;
}

IFC loading suggestion

In this one all the "get all ids" and "create subset" etc is moved in to the loading logic. Developers still have granular control of the subset, by using an optional config object in the loader, e.g.

[...]
const convertToSubsetConfig = {
  applyBVH: true,
  scene,
  removePrevious: true,
  customID: "full-model",
}
const model: IFCModel = await ifcLoader.loadAsync(modelURL, convertToSubsetConfig);
scene.add(model);

All ids would be processed when loading the model and the model ID would be implicit from the model itself. This looks a lot cleaner to me 🤓

agviegas commented 2 years ago

Hey @m1900 I just uploaded a hiding example for web-ifc-viewer. You can check it out here, and the deployed version here. Regarding the rest, feel free to open new issues for other questions / problems. 🙂 Soon we'll focus our resources into web-ifc-three and web-ifc-viewer, so hopefully all the issues will go away. Cheers!