Fix KHR_mesh_visibility issue with animation pointers

packages/dev/core/src/Meshes/abstractMesh.ts

@@ -238,12 +238,6 @@ class _InternalAbstractMeshDataInfo {
      * We use that as a clue to force the material to sideOrientation = null
      */
     public _sideOrientationHint = false;
-
-    /**
-     * @internal
-     * if this is set to true, the mesh will be visible only if its parent(s) are also visible
-     */
-    public _inheritVisibility = false;
     /**
      * Used in frame graph mode only, to know which meshes to update when in frozen mode
      */
@@ -558,42 +552,6 @@ export abstract class AbstractMesh extends TransformNode implements IDisposable,
      */
     public alphaIndex = Number.MAX_VALUE;
 
-    /**
-     * If set to true, a mesh will only be visible only if its parent(s) are also visible (default is false)
-     */
-    public get inheritVisibility(): boolean {
-        return this._internalAbstractMeshDataInfo._inheritVisibility;
-    }
-
-    public set inheritVisibility(value: boolean) {
-        this._internalAbstractMeshDataInfo._inheritVisibility = value;
-    }
-
-    private _isVisible = true;
-    /**
-     * Gets or sets a boolean indicating if the mesh is visible (renderable). Default is true
-     */
-    public get isVisible(): boolean {
-        if (!this._isVisible || !this.inheritVisibility || !this._parentNode) {
-            return this._isVisible;
-        }
-        if (this._isVisible) {
-            let parent: Nullable<Node> = this._parentNode;
-            while (parent) {
-                const parentVisible = (parent as AbstractMesh).isVisible;
-                if (typeof parentVisible !== "undefined") {
-                    return parentVisible;
-                }
-                parent = parent.parent;
-            }
-        }
-        return this._isVisible;
-    }
-
-    public set isVisible(value: boolean) {
-        this._isVisible = value;
-    }
-
     /**
      * Gets or sets a boolean indicating if the mesh can be picked (by scene.pick for instance or through actions). Default is true
      */

packages/dev/core/src/node.ts

@@ -35,6 +35,8 @@ class _InternalNodeDataInfo {
     public _isReady = true;
     public _onEnabledStateChangedObservable = new Observable<boolean>();
     public _onClonedObservable = new Observable<Node>();
+    public _inheritVisibility = false;
+    public _isVisible = true;
 }
 
 /**
@@ -261,6 +263,34 @@ export class Node implements IBehaviorAware<Node> {
         return this._parentNode;
     }
 
+    /**
+     * If set to true, this node, when renderable, will only be visible if its parent(s) are also visible.
+     * @default false
+     */
+    public get inheritVisibility(): boolean {
+        return this._nodeDataStorage._inheritVisibility;
+    }
+
+    public set inheritVisibility(value: boolean) {
+        this._nodeDataStorage._inheritVisibility = value;
+    }
+
+    /**
+     * Gets or sets a boolean indicating whether this node is visible, either this node itself when it is renderable or its renderable child nodes when `inheritVisibility` is true.
+     * @default true
+     */
+    public get isVisible(): boolean {
+        if (this.inheritVisibility && this._parentNode && !this._parentNode.isVisible) {
+            return false;
+        }
+
+        return this._nodeDataStorage._isVisible;
+    }
+
+    public set isVisible(value: boolean) {
+        this._nodeDataStorage._isVisible = value;
+    }
+
     /**
      * @internal
      */

packages/dev/inspector-v2/src/components/properties/nodes/abstractMeshProperties.tsx

@@ -48,7 +48,6 @@ export const AbstractMeshGeneralProperties: FunctionComponent<{ mesh: AbstractMe
 
     return (
         <>
-            <BoundProperty component={SwitchPropertyLine} label="Is Visible" target={mesh} propertyKey="isVisible" />
             <StringifiedPropertyLine label="Vertices" value={mesh.getTotalVertices()} />
             <StringifiedPropertyLine label="Faces" value={mesh.getTotalIndices() / 3} />
             <StringifiedPropertyLine label="Sub-Meshes" value={subMeshes.length} />

packages/dev/inspector-v2/src/components/properties/nodes/nodeProperties.tsx

@@ -7,6 +7,7 @@ import { useObservableState } from "../../../hooks/observableHooks";
 import { SwitchPropertyLine } from "shared-ui-components/fluent/hoc/propertyLines/switchPropertyLine";
 import type { ISelectionService } from "../../../services/selectionService";
 import { LinkToEntityPropertyLine } from "../linkToEntityPropertyLine";
+import { BoundProperty } from "../boundProperty";
 
 export const NodeGeneralProperties: FunctionComponent<{ node: Node; selectionService: ISelectionService }> = (props) => {
     const { node, selectionService } = props;
@@ -18,6 +19,14 @@ export const NodeGeneralProperties: FunctionComponent<{ node: Node; selectionSer
         <>
             <LinkToEntityPropertyLine label="Parent" description="The parent of this node" entity={parent} selectionService={selectionService} />
             <SwitchPropertyLine label="Is Enabled" description="Whether the node is enabled or not." value={isEnabled} onChange={(checked) => node.setEnabled(checked)} />
+            <BoundProperty
+                component={SwitchPropertyLine}
+                label="Inherit Visibility"
+                description="Whether the node inherits visibility from its parent."
+                target={node}
+                propertyKey="inheritVisibility"
+            />
+            <BoundProperty component={SwitchPropertyLine} label="Is Visible" description="Whether the node is visible or not." target={node} propertyKey="isVisible" />
         </>
     );
 };

packages/dev/loaders/src/glTF/2.0/Extensions/KHR_animation_pointer.data.ts

@@ -1,7 +1,7 @@
 /* eslint-disable @typescript-eslint/naming-convention */
 
 import { Animation } from "core/Animations/animation";
-import type { ICamera, IKHRLightsPunctual_Light, IMaterial } from "../glTFLoaderInterfaces";
+import type { ICamera, IKHRLightsPunctual_Light, IMaterial, INode } from "../glTFLoaderInterfaces";
 import type { IAnimatable } from "core/Animations/animatable.interface";
 import { AnimationPropertyInfo } from "../glTFLoaderAnimation";
 import { Color3 } from "core/Maths/math.color";
@@ -73,6 +73,15 @@ class LightAnimationPropertyInfo extends AnimationPropertyInfo {
     }
 }
 
+class MeshAnimationPropertyInfo extends AnimationPropertyInfo {
+    /** @internal */
+    public buildAnimations(target: INode, name: string, fps: number, keys: any[]) {
+        return target._primitiveBabylonMeshes
+            ? target._primitiveBabylonMeshes.map((mesh) => ({ babylonAnimatable: mesh, babylonAnimation: this._buildAnimation(name, fps, keys) }))
+            : [];
+    }
+}
+
 SetInterpolationForKey("/cameras/{}/orthographic/xmag", [
     new CameraAnimationPropertyInfo(Animation.ANIMATIONTYPE_FLOAT, "orthoLeft", getMinusFloat, () => 1),
     new CameraAnimationPropertyInfo(Animation.ANIMATIONTYPE_FLOAT, "orthoRight", getNextFloat, () => 1),
@@ -326,3 +335,5 @@ SetInterpolationForKey("/extensions/KHR_lights_punctual/lights/{}/spot/outerCone
 
 SetInterpolationForKey("/nodes/{}/extensions/EXT_lights_ies/color", [new LightAnimationPropertyInfo(Animation.ANIMATIONTYPE_COLOR3, "diffuse", getColor3, () => 3)]);
 SetInterpolationForKey("/nodes/{}/extensions/EXT_lights_ies/multiplier", [new LightAnimationPropertyInfo(Animation.ANIMATIONTYPE_FLOAT, "intensity", getFloat, () => 1)]);
+
+SetInterpolationForKey("/nodes/{}/extensions/KHR_node_visibility/visible", [new MeshAnimationPropertyInfo(Animation.ANIMATIONTYPE_FLOAT, "isVisible", getFloat, () => 1)]);

packages/dev/loaders/src/glTF/2.0/Extensions/KHR_node_visibility.ts

@@ -52,12 +52,13 @@ export class KHR_node_visibility implements IGLTFLoaderExtension {
      * The name of this extension.
      */
     public readonly name = NAME;
+
     /**
      * Defines whether this extension is enabled.
      */
     public enabled: boolean;
 
-    private _loader: GLTFLoader;
+    private _loader?: GLTFLoader;
 
     /**
      * @internal
@@ -67,28 +68,27 @@ export class KHR_node_visibility implements IGLTFLoaderExtension {
         this.enabled = loader.isExtensionUsed(NAME);
     }
 
-    // eslint-disable-next-line @typescript-eslint/naming-convention, @typescript-eslint/no-misused-promises
-    public async onReady(): Promise<void> {
-        this._loader.gltf.nodes?.forEach((node) => {
-            node._primitiveBabylonMeshes?.forEach((mesh) => {
-                mesh.inheritVisibility = true;
-            });
-            // When the JSON Pointer is used we need to change both the transform node and the primitive meshes to the new value.
-            if (node.extensions?.KHR_node_visibility) {
-                if (node.extensions?.KHR_node_visibility.visible === false) {
-                    if (node._babylonTransformNode) {
-                        (node._babylonTransformNode as AbstractMesh).isVisible = false;
+    public onReady(): void {
+        if (!this._loader) {
+            return;
+        }
+
+        const nodes = this._loader.gltf.nodes;
+        if (nodes) {
+            for (const node of nodes) {
+                const babylonTransformNode = node._babylonTransformNode;
+                if (babylonTransformNode) {
+                    babylonTransformNode.inheritVisibility = true;
+                    if (node.extensions && node.extensions.KHR_node_visibility && node.extensions.KHR_node_visibility.visible === false) {
+                        babylonTransformNode.isVisible = false;
                     }
-                    node._primitiveBabylonMeshes?.forEach((mesh) => {
-                        mesh.isVisible = false;
-                    });
                 }
             }
-        });
+        }
     }
 
     public dispose() {
-        (this._loader as any) = null;
+        delete this._loader;
     }
 }
 

packages/dev/loaders/src/glTF/2.0/Extensions/objectModelMapping.ts

@@ -51,6 +51,9 @@ export interface IGLTFObjectModelTreeNodesObject<GLTFTargetType = INode, Babylon
                 multiplier: IObjectAccessor<INode, Light, number>;
                 color: IObjectAccessor<INode, Light, Color3>;
             };
+            KHR_node_visibility?: {
+                visible: IObjectAccessor<INode, Mesh, boolean>;
+            };
         };
     };
 }
@@ -386,6 +389,20 @@ const nodesTree: IGLTFObjectModelTreeNodesObject = {
                     },
                 },
             },
+            KHR_node_visibility: {
+                visible: {
+                    type: "boolean",
+                    get: (node: INode) => {
+                        return node._primitiveBabylonMeshes ? node._primitiveBabylonMeshes[0].isVisible : false;
+                    },
+                    getTarget: () => undefined, // TODO: what should this return?
+                    set: (value: boolean, node: INode) => {
+                        if (node._primitiveBabylonMeshes) {
+                            node._primitiveBabylonMeshes.forEach((mesh) => (mesh.isVisible = value));
+                        }
+                    },
+                },
+            },
         },
     },
 };

packages/tools/tests/test/visualization/config.json

@@ -1197,7 +1197,7 @@
         },
         {
             "title": "GLTF Node visibility test",
-            "playgroundId": "#80DN99#3",
+            "playgroundId": "#80DN99#6",
             "referenceImage": "gltfNodeVisibility.png"
         },
         {