Reworked memory management mechanism, resolving position and history restoration issues

[ergoxiv]

This pull request proposes a bunch of changes to Anamnesis:

• A reworked memory management mechanism to improve application stability, reduce memory desyncs, prevent memory corruption, and eliminate the need for flaky thread delays randomly placed after synchronization with game memory.
• Updates to the History and HistoryService classes to fix the broken undo procedure, leading to skeleton deformations.
• Changes to PosePage, SkeletonVisual3d and BoneVisual3d to resolve position "smearing", leading to skeleton deformations over time.

---

How is the new memory management mechanism different from the old one?

The new mechanism uses a hierarchical thread locking approach. In other words, if an object is claimed by a thread for synchronization, either reading or writing, the object will not only claim its own thread lock but also all of its descendants' as well. In comparison, the old mechanism used per-object thread locking, allowing for reads from/writes to a memory object, while its ascendant is possibly being updated.

Hierarchical locking ensures that for an overlapping memory-bound property, either reading or writing is being performed at a given time but not both. For example, it is important that whenever a skeleton's memory is changed (e.g. when you change race to one with more bones, forcing the skeleton object in memory to move addresses). you do not access skeleton's member properties until it is synchronized. The old system was attempting to read from/write to transform/bone memories while the skeleton's address was changing, which resulted in corrupted memory and several failures. This will not occur in the new implementation, as the write procedure will wait for the thread locks to be released first. Ultimately, memory and thread safety are achieved at the cost of performance.

Furthermore, game memory changes caused by the game and the user now have independent paths of execution to prevent the synchronization process from calling the internal property changed event handler, which previously resulted in memory de-synchronizations due to Bind.IsReading:

---

Detailed changes:

• Reworked memory management classes. Please refer to the dedicated section for more information.
• History changes are now commited after mouse left button release, in addition to the existing "time after last change" condition.
  • Reason: Previously, if a user was holding down a slider or the gizmo, the history would create entries even though they are not done editing the skeleton. With this additional check, this unintended behaviour will no longer occur.
• Recorded history actions are now filtered, saving only the latest entry for a given bind. The old value is transferred over to make sure the oldest and newest values are represented by a single change.
• The application no longer reads skeleton transforms if motion is disabled or world positions are frozen. Even though BoneVisual3d's ReadTransform() and WriteTransform() now have transform locks, we want to avoid redundant calls whenever possible.
• Skeleton transform updates are now performed with the use of a "snapshot": All transform memories are collected beforehand to decrease the likelihood of desyncs as calculatations take significantly longer than a bunch of property retrievals.
• Disabled offset caching for all memory-bound properties where the offset might change (e.g. skeleton, transforms, bones array, etc.)
• Use of debounce timers to allow for a synchronization to finish (i.e., for all properties to be updated) before we reflect changes in the application (e.g. CustomizeEditor).
• Updated the constant size of Utf8String to 0x40 (64 bits).
• Converted all memory size and length values to hexadecimal values wherever applicable to increase consistency with the rest of the code.
• Changed SkeletonMemory array length type from short to ushort.
• Corrected HkaPoseMemory transforms array memory offsets. Previously, the count was incorrectly offset due to the weird offset of the entire array.
• Updated AnimationSlotCount from 13 to 14.
• Removed gender converter for hrothgar in CustomizeEditor.xaml.cs. There was a converter forcing the gender of Hrothgar to male. As there is now a female counterpart that users can select, we do not need it.
• Rewrote CustomizeEditor.xaml.cs to act nicely with the new memory mechanism.
• Added proper disposal for all of our NOP hooks. Now all NOP-ed instructions are reverted to their original values on application close.
  • Note: Keep in mind that this does not apply to "Stop Debugging" in Visual Studio. If you stop the application using that action, the memories will still end up corrupted.
• Positions are now enabled by default whenever pose mode is toggled on.
• Increased Brio API requests timeout to 10 seconds.
  • Reason: It seems that the first redraw takes significantly longer than the others. We need to get in touch with the maintainers of Brio to find out why this happens. Furthermore, sometimes the redrawn character is missing their primary weapon, making them invisible.
• Added in-line documentation to modified files.

And possibly more. I tried to include everything I could think of.

---

Investigated issues:

• Some bones stretch when posing is enabled (most noticeable with viera ears).
  • Reason: This is caused by AddressService.SkeletonFreezePhysics and AddressService.SkeletonFreezePhysics2. The currently NOP-ed SIMD instructions in the game's memory result in this funky behaviour. No steps were taken to resolve this issue. It should be handled in a separate pull request.
• 3D Pose panel skeleton flickering
  • Reason: The flickering is caused by the game. Some of the bones' transform memories change, thus causing the flickering. This is not a critical issue as the flickering stops as soon as we freeze positions. Furthermore, the application no longer reads skeleton transforms if motion is disabled or world positions are frozen.

---

Good luck with the review o7
If you have any questions, don't hesitate to ask.

[CaptainSticky]

I've done some quick tests today and have some observations so far :>

First Observation

The first observation relates to importing when posing mode is off. Nothing happens when I import the first time, but when I import a second time, the pose will load. This behavior happens in the following scenarios:

1. Import -> Body Pose
2. Import -> Expression

When manually toggling on posing mode before attempting to import, only the following scenario is affected:

1. Import -> Expression

I've attached the portion of output for the above. (Taken by clearing before performing an action.)
Import - Body Pose.txt

Second Observation

I noticed in your notes that when posing mode is enabled, freeze positions is now enabled by default. Just curious, what is the reason for this? When doing a general import or importing by body pose, the body becomes mangled. When toggling freeze positions off, and then importing, the result is a good import: (ignoring the known caveat with expressions)

Other results

I am not noticing any smearing! I did a couple of quick, rough tests with some smearing that I'm used to, particularly with the fingies and face, and wasn't able to get smearing. Basic validation with some memorized values for all the fingies.

Despite the first two items, I was able to run through some of my normal workflows and complete a few simple poses.

I will keep testing as I have time :> Overall its looking good!

[ergoxiv]

Oopsies, that's on me. I completely forgot to test pose importing.

I noticed in your notes that when posing mode is enabled, freeze positions is now enabled by default. Just curious, what is the reason for this?

Now that posing with positions enabled no longer seems to be buggy in Anamnesis, I think it makes sense to have it toggled on as part of the default user experience. It is a necessity for creating anything beyond basic facial expressions. Regardless of whether they are enabled or disabled, I think that pose importing should work in both scenarios and behaviour should be visually indistinguishable. So, in my opinion, this should be considered a drawback of the current pose import implementation.

That's why I think it makes most sense to now focus our attention on updating pose importing. I think our next changes should be:

• What is currently "full transform" importing should be the default behaviour when people click on "Import" while positions are enabled. If positions are disabled (i.e., if the user has manually turned it off), then the code behind the current "Import" should trigger instead. This way, we consolidate two buttons into one, simplifying use for the end user.
• The functionality of the body-only pose import should be extended to work for both scenarios.
• Since facial-expression-only pose imports require positions, this can stay the same for enabled positions. The button should either be grayed out (i.e., disabled) if you manually disable positions, or we can keep it interactable but with a prompt that positions will be re-enabled as a result of the import. From a user standpoint, I think keeping it interactable would be better.

There are two ways we can approach the implementation:

1. I can revert commit 2113a7e and create a separate pull request that updates the pose importing functionality separately and then we can re-enable positions.
2. I keep the commit and instead I update pose importing and introduce it as a commit in this pull request with the drawback of scope creep.

Still, that's just my opinion so I am interested to hear what you think.

[StoiaCode]

Havent had the time to look at it in practice yet, only reviewd the premise.

I do agree that, if posing works reliable, it should be turned on by default and in turn "Full Transform" (aka. importing with Position, not necessarily Scale) becomming the norm. I have some worries about the long standing and often revised .pose files and if they have always been correctly exporting position, but as long as we keep a way to load without position it should be fine.

Additionally we need to consider some things:

• Remove the warning of "you are trying to import an expression with Position turned on"
• How do people import a Pose without position AFTER position is turned on? The usecase may be rare, but it is there.
• The toggles cannot be edited before entering posing mode, so importing before turning it on will dictate a certain set of enabled/disabled. Can we find a way that allows people to change this?
• Importing with positions on expressions pre-DT now is entirely unfixable. While before it often was as well, if we default to position import they become eldritch abominations that HAVE to be fixed via import. We may entirely exclude the head on import on pre-DT poses, using the same code we do right now for the warning on impression import?

thats it for now, I may add more things later.

[CaptainSticky]

Both of the observations I noted above have been resolved, and I haven't noticed any other issues during my (brief) testing. Thanks!