Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the double __root__ issue for glb #7257

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
85 changes: 71 additions & 14 deletions loaders/src/glTF/2.0/glTFLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,8 @@ export class GLTFLoader implements IGLTFLoader {
}
}

const rootNode = this._createRootNode();
const rootNode = this._createRootNode(this._gltf.nodes);

for (const node of this._gltf.nodes) {
const parentIndex = nodeParents[node.index];
node.parent = parentIndex === undefined ? rootNode : this._gltf.nodes[parentIndex];
Expand Down Expand Up @@ -453,21 +454,76 @@ export class GLTFLoader implements IGLTFLoader {
this.log(GLTFLoaderState[this._state]);
}

private _createRootNode(): INode {
this._rootBabylonMesh = new Mesh("__root__", this._babylonScene);
this._rootBabylonMesh.setEnabled(false);
private _checkForRootNode(): Nullable<INode>{
if (this._gltf.scenes == null) {
return null;
}

const rootNode: INode = {
_babylonTransformNode: this._rootBabylonMesh,
index: -1
};
var indexScene = this._gltf.scene || 0;
var scene = this._gltf.scenes[indexScene];

// Root node must be unique
if (scene.nodes.length != 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (scene.nodes.length != 1) {
if (scene.nodes.length !== 1) {

return null;
}
else if(this._gltf.nodes != null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
else if(this._gltf.nodes != null) {
else if(this._gltf.nodes !== null) {

var indexRoot = scene.nodes[0];
var root = this._gltf.nodes[indexRoot];

// Check rotation and scale
if(root.rotation != undefined && root.scale != undefined){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok...stopping here but please fix all triple equals ;)

if (root.rotation.join(',') != "0,0,0,1" || root.scale.join(',') != "1,1,-1") {
return null;
}
}

// Check name
if (root.name != "__root__") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure we want to check this one. As long as we have the correct transforms we are fine

return null;
}
}else{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spaces

return null;
}

return root;
}

private _createRootNode(nodes : INode[]): INode {

var ExistingRootNode = this._checkForRootNode();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no capital first letter

var RootAlreadyImplemented = false;
var INodeToReturn;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need a type


//Check if the root node already exists
if (ExistingRootNode != null && nodes != null) {
RootAlreadyImplemented = true;

this._forEachPrimitive(ExistingRootNode, (babylonMesh) => {
this._rootBabylonMesh = babylonMesh as Mesh;
});

INodeToReturn = ExistingRootNode;
}
else
{
this._rootBabylonMesh = new Mesh("__root__", this._babylonScene);

this._rootBabylonMesh.setEnabled(false);

const rootNode: INode = {
_babylonTransformNode: this._rootBabylonMesh,
index: -1
};

INodeToReturn = rootNode;
}

switch (this._parent.coordinateSystemMode) {
case GLTFLoaderCoordinateSystemMode.AUTO: {
if (!this._babylonScene.useRightHandedSystem) {
rootNode.rotation = [0, 1, 0, 0];
rootNode.scale = [1, 1, -1];
GLTFLoader._LoadTransform(rootNode, this._rootBabylonMesh);
if (!this._babylonScene.useRightHandedSystem && !RootAlreadyImplemented) {
INodeToReturn.rotation = [0, 1, 0, 0];
INodeToReturn.scale = [1, 1, -1];
GLTFLoader._LoadTransform(INodeToReturn, this._rootBabylonMesh);
}
break;
}
Expand All @@ -481,9 +537,10 @@ export class GLTFLoader implements IGLTFLoader {
}

this._parent.onMeshLoadedObservable.notifyObservers(this._rootBabylonMesh);
return rootNode;
}

return INodeToReturn;
}

/**
* Loads a glTF scene.
* @param context The context when loading the asset
Expand Down