fix: AssImp orientation, imported material color fallback, and set default animation preview model

[Doprez]

PR Details

I found a new issue in the 3d Importer PR where the models seems to be rotated incorrectly. I have attempted to port over a fix from the main AssImp repo that should fix the scaling and rotation issues I had.

Current issue in master:

the old importer:

Broken FBX example
Working FBX example

Types of changes

• Docs change / refactoring / dependency upgrade
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• My change requires a change to the documentation.
• I have added tests to cover my changes.
• All new and existing tests passed.
• I have built and run the editor to try this change out.

[Doprez]

Unfortunately this doesnt seem to fix the rotation issue? I am assuming this is related to the axis of the model being different than what AssImp expects but maybe I am not understanding something.

Do you have any ideas @Eideren or @Noah7071 ?

[Doprez]

ah ok, I should have looked closer at Noahs original PR. I see there was already a method for checking the upaxis specifically in this commit. 6c0d92a#diff-9bea816328a0761b726b259bbe8cfd7c4dde5b31dcb1a2e98e8d27ce1f39792cR128

[Doprez]

Seems like those methods were removed at some point in the PR and the method I have seems to change nothing. I think its because the metadata is the same so I think Im missing something.

Adding the files to the description above if someone else would like to test.

[Noah7071]

Unfortunately this doesnt seem to fix the rotation issue? I am assuming this is related to the axis of the model being different than what AssImp expects but maybe I am not understanding something.

Do you have any ideas @Eideren or @Noah7071 ?

Hi Doprez, sorry for late response, it should automatically take into account source axis system to LHS. which is Stride default! @Doprez

[Doprez]

Hi Doprez, sorry for late response, it should automatically take into account source axis system to LHS. which is Stride default! @Doprez

@Noah7071 No problem at all. so that would be what fixed this right?

Because I think the problem is different than the LHS that Stride uses. in this issue people were having similar problems due to FBX metadata which is where I got the CorrectRootTransform in this PR from. However, it seems like no matter where I try to debug this method it doesnt actually affect the model view in Stride but I can see the matrix values being changed in VS.

Thank you for confirming btw. I think you are one of the very few people who have delved deep into this importer 😄

[Basewq]

@Doprez I'm out of time to help look further into this at the moment, but here's what I can tell from what you've provided.

1. scene->MMetaData->MKeys[i].AsString call once because allocation.
2. Don't assume your data type calling GetMetaDataValue is correct. the assimp actually reports "UnitScaleFactor" as a float with metaData->MDataType, so it got corrupted data when read as double.
3. If there's a problem with the system axis, neither of your fbx files show this (they're both y-up)

I downloaded FBX 2013.3 Converter for Windows:
https://aps.autodesk.com/developer/overview/fbx-converter-archives

In the FBX explorer, both files have the same Up/Front/Coord Axis so no root correction applies to these meshes.
In the broken FBX, Objects -> Model 0, there's Lcl Scalng & Lcl Rotation of type 'A', but Model 1 and onwards (the child meshes) have Lcl Rotation & Lcl Scaling as 'A+'
Google says 'A' means absolute & 'A+' is relative to parent. It's possible the assimp reader is not accounting for this properly?
Side note: the FBX explorer actually reports UnitScaleFactor as a double, so the assimp in Stride is wrong in determining it as float.

The working FBX only has one Model (and it's 'A' type for scaling/rotation)

I don't understand the importer enough, it seems either I'm placing the breakpoints in the wrong code, or don't know how the Editor actually does the FBX -> Stride model data transfer (is it actually in a separate process, ie. can't breakpoint?).
If you know, do tell so we can determine what's being actually being read.

[Doprez]

I don't understand the importer enough, it seems either I'm placing the breakpoints in the wrong code, or don't know how the Editor actually does the FBX -> Stride model data transfer (is it actually in a separate process, ie. can't breakpoint?). If you know, do tell so we can determine what's being actually being read.

This has been a frustrating part of debugging this. I have breakpoints at the following places:
private unsafe Model ConvertAssimpScene(Scene* scene) line 198
private unsafe Rendering.Skeleton ProcessSkeleton(Scene* scene) line 258
private unsafe AnimationInfo ProcessAnimations(Scene* scene, int animationIndex) line 273

The breakpoints are hit or miss when I use hot reloading, only fully cleaning and rebuilding the project seems to consistently work for me. And then I have a base project that I constantly reimport the file into to try and trigger these.

Thank you btw, I was likely too focused on that one issue with the metadata. If its the same then Ill do some more looking around.

[Eideren]

Not sure A+ is relevant, here's the same model, with animations removed and re-exported through blender:
BrokenAfterExport.txt rename the extension to fbx
It shows the same issue but reports both Lcl ... as A for all models.

What I think is suspicious is Model::Armature.001 and Model::Character.001 both with {-90,0,0} rotation and both of them are parented to nothing from what I can gather from the connections:

[Basewq]

I'm not on my main machine, so just throwing this out there as points of interest, will look later but if anyone else wants to investigate feel free.
Looking at MeshConverter.cs
This is new code not from the original implementation in RegisterNodes method

stride/sources/tools/Stride.Importer.3D/MeshConverter.cs

Lines 580 to 589 in 0e08dfd

	if (filterInNodes!=null
	&& filterInNodes.Count>0)
	{
	if(!filterInNodes.Contains(fromNode->MName.AsString))
	{
	modelNodeDefinition.Transform.Rotation = Quaternion.Identity;
	modelNodeDefinition.Transform.Scale = Vector3.One;
	modelNodeDefinition.Transform.Position = Vector3.Zero;
	}
	}

This gets called like RegisterNodes(scene->MRootNode, -1, nodeNames, meshIndexToNodeIndex, GetBoneList(scene));
filterInNodes is GetBoneList(scene)
The root mesh might not have a bone, but actually did have rotation? So it's incorrectly 'unsetting' the rotation?

This line

stride/sources/tools/Stride.Importer.3D/MeshConverter.cs

Line 576 in 0e08dfd

var transform = fromNode->MTransformation.ToStrideMatrix();

used to be
auto transform = Matrix::Invert(rootTransform) * aiMatrixToMatrix(fromNode->mTransformation) * rootTransform;

Probably paired with this

stride/sources/tools/Stride.Importer.3D/MeshConverter.cs

Lines 662 to 663 in 0e08dfd

	LinkToMeshMatrix = bone->MOffsetMatrix.ToStrideMatrix()
	// LinkToMeshMatrix = rootTransformInverse * bone->MOffsetMatrix.ToStrideMatrix() * rootTransform

The commented out code is how the original worked, ie.
boneDef.LinkToMeshMatrix = rootTransformInverse * aiMatrixToMatrix(bone->mOffsetMatrix) * rootTransform;

[Eideren]

Yep, I was actually testing that while you were writing this, just commenting out the filterInNodes stuff does fix the issue, doesn't break animations nor breaks the other working model. Not sure we should leave this as is, best to look into the rest of what @Basewq wrote. I'm off to bed though, will look at this tomorrow night again if no one took over by then.
Took some time to cleanup the file as well, if you could cherrypick those two last commits:
https://github.com/Eideren/xenko/commits/fix-incorrect-axis-on-import/
No clue how I am supposed to push onto editable pull requests or if that's even a possibility.

[Basewq]

@Eideren I tried the cherry picked commits, and the imported models look correct.
I've tried to play the animations but it's not playing anything. Not sure if I've just forgotten how to do it properly or something else (no idea if this is importer related or just a bad export).
The animation doesn't appear in the Asset preview either (my understanding is Stride probably failed to load it but it remains silent on that matter).

[Eideren]

@Basewq works fine on my end:

Make sure that you set the preview model for that animation to the model you imported:

[Basewq]

Ha, I'm silly. I somehow forgot to set the Skeleton on the Model itself. The animation is indeed working.

I think the only last thing that needs is confirmation of the rootTransformInverse * [MATRIX] * rootTransform code that was removed from the original code.
Problem is that there is one code that still does this type of calculation, ie.

stride/sources/tools/Stride.Importer.3D/MeshConverter.cs

Lines 378 to 390 in 0e08dfd

	if (isTranslation)
	{
	// Change of basis: key.Value = (rootTransformInverse * Matrix::Translation(key.Value) * rootTransform).TranslationVector;
	Vector3.TransformCoordinate(ref key.Value, ref rootTransform, out key.Value);
	}
	else
	{
	// Change of basis: key.Value = (rootTransformInverse * Matrix::Scaling(key.Value) * rootTransform).ScaleVector;
	var scale = Vector3.One;
	Vector3.TransformNormal(ref scale, ref rootTransformInverse, out scale);
	scale *= key.Value;
	Vector3.TransformNormal(ref scale, ref rootTransform, out key.Value);
	}

Was there ever a problem with the original code, or was this a case of 'didn't see an affect with the code on or off'?
I think the main problem is finding a specific fbx that has a non-identity root transformation to find out what will happen, I think?
This would also confirm whether the CorrectRootTransform code is necessary or not.

[Eideren]

Checked with previous and current LinkToMeshMatrix, with and without CorrectRootTransform, with and without the branch with Vector3.Transform..., all of them removed and both state of LinkToMeshMatrix. No difference whatsoever.
My tests included:

• Both models referenced above
• A random sketchfab model
• The same random sketchfab model with its root rotated sideways
• The same random sketchfab model with up on +X and front on -Z
• he00_normal_idle (sources/data/tests/knight/scenes/he00_normal_idle.fbx)

Now if we comment out

stride/sources/tools/Stride.Importer.3D/MeshConverter.cs

Line 181 in 0e08dfd

assimp.SetImportPropertyInteger(propStore, "IMPORT_FBX_PRESERVE_PIVOTS", 0);

Differences do start showing up, but there's a bunch of other issues that are introduced as well.

@Jklawreszuk do you remember where this comes from ?

stride/sources/tools/Stride.Importer.3D/MeshConverter.cs

Lines 378 to 390 in 0e08dfd

	if (isTranslation)
	{
	// Change of basis: key.Value = (rootTransformInverse * Matrix::Translation(key.Value) * rootTransform).TranslationVector;
	Vector3.TransformCoordinate(ref key.Value, ref rootTransform, out key.Value);
	}
	else
	{
	// Change of basis: key.Value = (rootTransformInverse * Matrix::Scaling(key.Value) * rootTransform).ScaleVector;
	var scale = Vector3.One;
	Vector3.TransformNormal(ref scale, ref rootTransformInverse, out scale);
	scale *= key.Value;
	Vector3.TransformNormal(ref scale, ref rootTransform, out key.Value);
	}

@Doprez opened a PR, let me know if that's easier for you than cherrypicking Doprez#16

[Doprez]

Pr works great for me, should be merged now. I should have some time either tomorrow or Friday to test out the changes, just been busy with work the last couple of days.

[Basewq]

One more sanity check on the fbx, please.
I've dumped the "broken" mesh onto a scene, but the purple box (I believe is the bounding box) is much larger than actual mesh.
Can someone confirm this is the case, or I did something silly again?

[Doprez]

For some reason I cant seem to get my models to do what you guys have. I still have the same result as the original description even with a git clean and dotnet clean

I've dumped the "broken" mesh onto a scene, but the purple box (I believe is the bounding box) is much larger than actual mesh.
Can someone confirm this is the case, or I did something silly again?

You could try the below methods to see if the bounding box is the same as before the AssImp update. I also saw this even with the samll model but I thought it may just be the model being weird and might not be related to the new importer changes.
https://github.com/stride3d/stride-community-toolkit/blob/main/src/Stride.CommunityToolkit/Engine/ModelComponentExtensions.cs

[Basewq]

@Doprez Whenever you use a nuget package (ie. your game Stride projects with <PackageReference Include="Stride.Engine" Version="4.2.0.2122" />, etc) it gets extracted to C:\Users\USERNAME\.nuget\packages\stride.* folders.
Even if you rebuild the nuget packages, Visual Studio knows which ones have already extracted and does not reextract again, so you need to delete the subfolders of whatever you named the new version (eg. I use 4.2.0.9999-dev so I delete those subfolders). Also make sure the game project you open is referencing the dev version.
Also check your task manager to end processes that lock the dll like the Stride router app.

The purple box issue might not be related to the importer change - if you've still got the version with the old importer, can you check if the purple box is also wrong there? If so then we should make a separate issue.

[Doprez]

Ill try fully deleting the nugets where they get exported, everything should be correct otherwise.

The purple box issue might not be related to the importer change - if you've still got the version with the old importer, can you check if the purple box is also wrong there? If so then we should make a separate issue.

The problem with the old importer is that it didnt have the model bounding box feature at the time. This was added after the new importer was merged. #2294

I will have time tonight to do some testing, what Ill do is just run the new and old version and get the mesh bounding boxes through code. If the bounding boxes are the same this may be a separate issue.

[Doprez]

I still cant get this model fix to work for me. Am I on the right commit? 02af13e

Edit: Im an idiot, nevermind...

[Doprez]

Even though I cant get the fix to work I did use the below script to check the bounding box of the model while running.

public class Test : SyncScript
{

	public ModelComponent ModelComponent { get; set; }

	public override void Start()
	{
		ModelComponent = Entity.Get<ModelComponent>();
	}

	public override void Update()
	{
		var test = ModelComponent.Model.BoundingBox;
	}
}

And placing a breakpoint after the test var gives me the below result:

I dont think those values match up with the image of the bounding box @Basewq so there may be one more thing to look into.

[Doprez]

ayyyy ok, it seems fine at runtime so this looks like an error just in gamestudio.

Tested with the same script on version 4.2.0.1:

I think we can either mark this issue as a seperate issue or look into it a bit further but it does look good to me so far. let me know if you have any concerns with this @Basewq I can do some more testing today.

@Eideren Ill run some of the previous tests with this branch to make sure none if the previous issues have reappeared and we may be good to go if nothing comes up.

[Doprez]

Everything looks good from me so far (apart from the bound box but it seems fine outside of the new gizmo) I opened it for review.

[Basewq]

@Doprez I think the model bounding box issue can be treated separate, especially since I didn't realize it was actually a new feature.

I noticed in MeshConverter.cs you restored line 622 to use rootInverse * xx * root, however you didn't restore it for line 576 (as mentioned in #2291 (comment)).
I think for consistency it should all be restored. Whether it actually has an affect or not seems to be determined if we can find an fbx that has a non-identity root matrix, which hasn't been found. I'm leaning on the opinion of restoring it is the safer option, (ie. if no one had an issue before, most likely might have solved someone's weird edge case), however I haven't messed with enough fbx models to have an issue either so it doesn't affect me.

[Doprez]

I couldnt find where the line used to be boneDef.LinkToMeshMatrix = rootTransformInverse * aiMatrixToMatrix(bone->mOffsetMatrix) * rootTransform; in the old CPP MEsh converter but since this is within the else statement for non-parent nodes it should be the local transform right?

[Basewq]

Sorry, major mistake on my part, I had been comparing it with Stride.Importer.Assimp.cpp
https://github.com/stride3d/stride/blob/f33a0eeda74379a407eea4390b0742ae11ad50a8/sources/tools/Stride.Importer.Assimp/Stride.Importer.Assimp.cpp
instead of MeshConverter.cpp
https://github.com/stride3d/stride/blob/f33a0eeda74379a407eea4390b0742ae11ad50a8/sources/tools/Stride.Importer.FBX/MeshConverter.cpp
The code was extremely similar and didn't notice until you just pointed this out!

EDIT: I'm noticing the new MeshConverter seems to be a mix between the two files, which is why I got confused.

[Eideren]

@Basewq anything else we should look into before merging ?

[Basewq]

@Basewq anything else we should look into before merging ?

Not from me. I think if any new issues come up, it'll be from someone who has a problematic fbx.

[Eideren]

Thanks !