Refactor: SkinningControl optimization + javadoc

[capdevon]

These changes aim to make the SkinningControl more understandable, maintainable, and robust.

Here’s a short description of the changes made between the original and new version of the file:

The SkinningControl class has undergone significant refactoring and modernization. Key changes include:

• Improved Naming and Documentation: Variable and method names have been clarified (wasMeshUpdated → meshUpdateRequired, switchToHardware → enableHardwareSkinning, etc.), and class/method Javadocs have been rewritten for clarity, providing more detailed explanations of software vs. hardware skinning.
• Refined Hardware Skinning Logic: The code now more clearly separates and documents the logic for enabling/disabling and testing hardware skinning support, including more robust logging and error handling.
• Target Collection Refactor: The process for collecting animated geometries from the scene graph is now recursive and consolidated into collectAnimatedGeometries, replacing the older, less flexible findTargets methods.
• Parameter Management: Handling of MatParamOverride parameters for bone data is streamlined, with initialization moved to field declarations and simplified serialization logic.
• Buffer Update Methods Renamed: Functions for updating mesh buffers (for software skinning) have clearer names and documentation, and redundant code has been removed.
• General Cleanup: Unused imports are removed, copyright years are updated, and the code structure is improved for maintainability.

Overall, the changes enhance readability, maintainability, and clarity while maintaining the original functionality.

[github-actions]

🖼️ Screenshot tests have failed.

The purpose of these tests is to ensure that changes introduced in this PR don't break visual features. They are visual unit tests.

📄 Where to find the report:

• Go to the (failed run) > Summary > Artifacts > screenshot-test-report
• Download the zip and open jme3-screenshot-tests/build/reports/ScreenshotDiffReport.html

⚠️ If you didn't expect to change anything visual:
Fix your changes so the screenshot tests pass.

✅ If you did mean to change things:
Review the replacement images in jme3-screenshot-tests/build/changed-images to make sure they really are improvements and then replace and commit the replacement images at jme3-screenshot-tests/src/test/resources.

✨ If you are creating entirely new tests:
Find the new images in jme3-screenshot-tests/build/changed-images and commit the new images at jme3-screenshot-tests/src/test/resources.

Note; it is very important that the committed reference images are created on the build pipeline, locally created images are not reliable. Similarly tests will fail locally but you can look at the report to check they are "visually similar".

See https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-screenshot-tests/README.md for more information

[capdevon]

I don't understand why the Screenshot Test fails ...

[yaRnMcDonuts]

Here's from the ScreenshotDiffReport.html file that was generated by the screenshot test:

The difference would be impossible to notice if it weren't for the image showing thed differences... So it could be a non-issue, or it could be indicative of a bigger bug that isn't showing itself fully in this testOgreConvert() example that uses a simple model.

I know that sometimes animation related bugs can appear minor or unnoticeable on simple models but are much more noticeable on rigs with more complex bone structures.

So the screenshot tests could point to an overlooked bug, but if there's no apparent difference with a wider variety of models and if testing doesn't reveal any serious issues, then it might end up being okay to ignore the failed test but we'd definitely want to do as much testing as possible in that case to be sure.

[yaRnMcDonuts]

I looked at the screenshots again, and one set of them does actually show some slightly more visibly noticeable differences:

It looks this one expects the model from testOgreConvert() to be stepping its right foot forward in the middle of a walking pose, but the new changes are generating a screenshot where the model is still in its bind pose.

[capdevon]

@yaRnMcDonuts
I tested several models with this version of SkinningControl, and they all worked correctly. I don't know if it's worth ignoring the screenshot test for this pull request (PR). This PR is very important to me because it improves the maintainability and readability of the class. It also eliminates the serialization of MatParamOverrides, including the jointMatricesParam parameter. For a human model with 64 bones, this parameter can contain 64 matrices, each with 16 float variables. I would like to see this change included in the next alpha test version.

Edit:
So, 64 matrices of 16 floats each will consume 4096 bytes (4 KB) if saved as raw float data (e.g., using a DataOutputStream or writing the raw bytes). If you use Java's built-in object serialization (e.g., ObjectOutputStream), the actual file size will be larger due to serialization overhead (metadata, headers, etc.)

If you want the minimal size, write the floats directly as binary data. If you serialize objects, expect additional overhead.

[yaRnMcDonuts]

I tested several models with this version of SkinningControl, and they all worked correctly. I don't know if it's worth ignoring the screenshot test for this pull request (PR).

It may be possible this is a false-positive and can be ignored, but the screenshot test are still new so I'm not sure how probable that is. Maybe @richardTingle has some insight he can offer on this since he created them.

Have you also run the TestOgreConvert example and confirmed that it still works all the same on your branch as it does on master? (I believe that example specifically uses an old model with an AnimControl that gets converted to the newer animation system, so you may not have experienced the error if your models weren't converted this way. I haven't looked too much into jme's animations system but I do know it handles joint transforms different for a model that's been converted from the old system compared to a fresh one converted from gltf, so its always important to test with both type of armatures)

[richardTingle]

Running TestOgreConvert locally (the one in jme3-examples, not the screenshot test one) I can see the model walking. If I check out the capdevon-SkinningControl branch and run TestOgreConvert I can see the model standing still (ignoring the walk animation).

Something is definitely really broken, it isn't a false positive

[capdevon]

Thanks for the feedback, guys! I'm looking into it.

Edit:
Fixed
@yaRnMcDonuts @richardTingle

[richardTingle]

I can confirm I can now see the walking animation correctly in TestOgreConvert (and the screenshot tests pass)