xeokit / xeokit-sdk

Open source JavaScript SDK for viewing high-detail, full-precision 3D BIM and AEC models in the Web browser.
https://xeokit.io
Other
715 stars 286 forks source link

Fix Scene#setObjectsPickable #434

Closed tothatt81 closed 3 years ago

tothatt81 commented 3 years ago

Hey @xeolabs ,

When loading a model into the viewer, it is possible to set several properties that will act upon the whole model. Some of these properties that we have used so far are: edges, visible, pickable, xrayed, etc. For instance, if we load a model as such:

const model = xktLoader.load({
  id: "myModel",
  src: "./models/xkt/duplex/duplex.xkt",
  metaModelSrc: "./metaModels/duplex/metaModel.json",
  xrayed: true,
});

all elements of the model will be xrayed as expected. This works fine. So far so good.

In our use case however, once the model has been loaded into the viewer, we may want to overwrite some of these properties of certain individual elements on demand (ie user action), like so:

model.on("loaded", () => {
  viewer.scene.setObjectsXrayed(["0pNy6pOyf7JPmXRLgxs3sW"], false);
  // alternatively this should work too:
  // model._nodes["0pNy6pOyf7JPmXRLgxs3sW"].xrayed = false;
});

Now, what we found is that it works for certain properties but not for others. It seems to work fine with the example (xrayed property) above. It also seems to work with the properties visible, selected, highlighted. However, it does not work with pickable and culled. For our present use case, I have to stress that pickable would be the most important but in our testing session we've seen that overwriting culled doesn't seem to work either.

This seems like a bug to us, or at the very least unexpected, inconsistent behaviour, given that overwriting visible, xrayed, selected and highlighted works as expected. As such, we'd like to ask you to please look into the issue and let us know of your findings.

For your convenience, you'll find the full example with pickable here: https://gist.github.com/tothatt81/46e00f78395a83c243e934b932b08f44

Thank you!

Best, Attila

xeolabs commented 3 years ago

Hi Attila,

That does look like a bug with Scene#setPickable() - I'll fix in the next release this week.

Not being able to batch-set "culled" is intentional though, where it's expected that visibility culling logic would be responsible to iterating over batches of objects to set that property.

eriadam commented 3 years ago

Thanks @xeolabs. Would it be also possible to port it back to 1.3.x?

xeolabs commented 3 years ago

@eriadam not sure yet, but if not, I can always provide a patch that you could apply locally to your 1.3.x code base.

I'm thinking that it seems like a good idea going forwards to have a branch for each release, then releases can be patched like this.

eriadam commented 3 years ago

Thanks @xeolabs , do you maybe have a date for the release of 1.4.2?

xeolabs commented 3 years ago

@eriadam no probs, I've just now published 1.4.2.