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

Speedup anim import & various other fixes #506

Merged
merged 55 commits into from
Jun 2, 2022

Conversation

HENDRIX-ZT2
Copy link
Contributor

@HENDRIX-ZT2 HENDRIX-ZT2 commented Mar 28, 2022

@niftools/blender-niftools-addon-reviewer

Overview

speed up anim keyframe import, the more keys, the more speedup. about 10x on some sample nifs

Detailed Description

  • anim keyframes are created in bulk rather than being added sequentially to the keyframe_points
  • restructured calls to key imports, might be refactored more
  • unbreaks UV anim import
  • fixes a decoding issue of strings
  • improves embedded textures (speed, naming, support)

Fixes Known Issues

closes #180
closes #495
closes #500
closes #510
closes #517

Documentation

none

Testing

import nif / kf with animated controllers

Manual

[Set of steps to manually verify updates are working correctly]

Automated

[List of tests run, updated or added to avoid future regressions]

Additional Information

[Anything else you deem relevant]

@HENDRIX-ZT2 HENDRIX-ZT2 changed the base branch from master to develop March 29, 2022 15:25
Copy link
Member

@neomonkeus neomonkeus left a comment

Choose a reason for hiding this comment

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

Only some minor observations.
Functionally look good, much cleaner and users will be excited for perf boost. 👍

@niftools niftools deleted a comment from HENDRIX-ZT2 Mar 30, 2022
@HENDRIX-ZT2
Copy link
Contributor Author

Ah damn, UV anim import is broken, still coded for old tex slots system. Needs upgrade for nodes api. All the other anim imports have been upgraded to the new faster api and tested successfully.

@HENDRIX-ZT2 HENDRIX-ZT2 changed the title Speedup anim import Speedup anim import & various other fixes Apr 2, 2022
Copy link
Member

@neomonkeus neomonkeus left a comment

Choose a reason for hiding this comment

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

Not sure about moving BSInvMarker from object > scene level.
Rest look good.

@@ -94,6 +93,15 @@ class Scene(PropertyGroup):
default='NONE',
update=update_version_from_game)

rootnode: EnumProperty(
Copy link
Member

Choose a reason for hiding this comment

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

Should this be at scene level?

Copy link
Member

Choose a reason for hiding this comment

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

Should we have a reference to an object which would used as the root node?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we have a reference to an object which would used as the root node?

I think then we just delegate the problem. It's better to tag root objects (eg. those without a parent are automatically root objects)

Copy link
Member

Choose a reason for hiding this comment

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

I was more thinking in the case that we have multiple objects tags as root object or do we allow that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate speeding up keyframe insertion
4 participants