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
728 stars 287 forks source link

[FIX] SceneModelTransform: parentTransformId parameter doesn't work #1441

Closed lzoia-bc closed 6 months ago

lzoia-bc commented 6 months ago

Describe the bug

The parentTransformId parameter in the createTransform method of a SceneModel is ignored due to the following error in the code

if (this.parentTransformId) {
    parentTransform = this._transforms[cfg.parentTransformId];
    if (!parentTransform) {
        this.error("[createTransform] SceneModel.createTransform() config missing: id");
        return;
    }
}

Here the code should check cfg.parentTransformId instead of this.parentTransformId.

To Reproduce Steps to reproduce the behavior:

  1. Create a SceneModel
  2. Create the base SceneModelTransform
  3. Create a new SceneModelTransform specifying the previous one as parent
  4. Create a new SceneModelMesh using the latest transform id

Expected behavior The create mesh should be transformed according to the composition of ParentTransform and Transform

Desktop (please complete the following information):

Additional context This error should fixed in a new release of the 2.5 version

lzoia-bc commented 6 months ago

I tried the latest version 2.6.0 and the parentTransformId doesn't work still. I suppose the error now is inside SceneModel.createTransform when the transform is created. I see that the parent transform is sent to the SceneModelTransform constructor using the parameter parentTransform, while the SceneModelTransform constructor code is reading cfg.parent. I think this bug is still present since is not possible to use the parentTransformId feature yet

xeolabs commented 6 months ago

I haven't made a test yet, but seemed an obvious thing to patch - try: https://github.com/xeokit/xeokit-sdk/releases/tag/v2.6.1

lzoia-bc commented 6 months ago

I just tried and I'm getting the following error

TypeError: childTransform._setWorldMatrixDirty is not a function
    at SceneModelTransform._addChildTransform (xeokit-sdk.es.js:80390:1)
    at new SceneModelTransform (xeokit-sdk.es.js:80383:1)
    at SceneModel.createTransform (xeokit-sdk.es.js:83363:1)
xeolabs commented 6 months ago

I can't test right now, but _setWorldMatrixDirty() should probably should be _transformDirty(), if you'd like to try modifying that and testing against your code.

lzoia-bc commented 6 months ago

I just tried with 2.6.2 and I'm getting the following error

TypeError: childTransform._setAABBDirty is not a function
    at SceneModelTransform._addChildTransform (xeokit-sdk.es.js:80391:1)
    at new SceneModelTransform (xeokit-sdk.es.js:80383:1)
    at SceneModel.createTransform (xeokit-sdk.es.js:83363:1)
lzoia-bc commented 6 months ago

I modified locally childTransform._setAABBDirty(); with childTransform._setSubtreeAABBsDirty(this); and it seems to work fine against my code

lzoia-bc commented 6 months ago

I created this PR with the above change https://github.com/xeokit/xeokit-sdk/pull/1469

lzoia-bc commented 6 months ago

Re-opened PR https://github.com/xeokit/xeokit-sdk/pull/1471

lzoia-bc commented 6 months ago

I tried with 2.6.6 and it's working fine. Maybe it could be worth to update the docs and tell that the parentTransformId is available starting from 2.6.6