vpenades / SharpGLTF

glTF reader and writer for .NET Standard
MIT License
467 stars 75 forks source link

Small optimizations to Animation Channel creation #72

Closed Crauzer closed 3 years ago

Crauzer commented 3 years ago

This PR is an attempt at fixing #71.

Changes:

Result

I was able to achieve around a 50% increase in performance while creating animation channels. The time it takes to do this same exact task with the test files provided in the issue in the current version of SharpGLTF takes around 3.5 minutes on my machine. This PR reduces that time to 2 minutes. devenv_sRhY7p46Qm

Generated file: GeneratedFile.zip

vpenades commented 3 years ago

Hmm... Is changing the IReadOnlyList to IEnumerable required?

Keep in mind that the IEnumerable.Count(); and IEnumerable.ElementAt() methods traverse the enumerable collection every time , so in your particular use case you might have improved the performance, but for other use cases the performance might have worsened.

Crauzer commented 3 years ago

Yes, I agree with you.

I'm currently trying to create a more efficient method that doesn't do as much memory copying. If it does end up being good I will commit that. If it ends up being bad I will change the IEnumerable back to IReadOnlyList.

Do you have any other ideas that might improve the performance of these methods ?

vpenades commented 3 years ago

Do you have any other ideas that might improve the performance of these methods ?

well, I wanted to do a full review of the case first, but this week is being particularly busy...

Also, I wanted to try creating a scene with a lot of animations using the SceneBuilder instead of attacking Schema2 namespace directly.

Keep in mind that SceneBuilder can do a lot of tricks under the hood that Schema2 cannot do by itself

Crauzer commented 3 years ago

I actually took a look at SceneBuilder as you suggested in the issue, but I can't seem to find a way to add animations with it.

vpenades commented 3 years ago

SceneBuilder is quite different to what you're used to with the Schema2.ModelRoot namespace. (At some point I should write tutorials, help!). try looking at some examples using it...

Btw, I created a discord channel.

vpenades commented 3 years ago

I'm closing this pull request since the latest changes already fix the issue.