Weird Toolkit incosistencies and code dublication #218
Comments
Does this makes sense? |
"SharpDX.Toolkit.Compiler uses ModelData.Bone to write data to a compiled model file, and SharpDX.Toolkit.Graphics is using it to read model data from the file at runtime." No, it isn't. Runtime Model loading hold no connection to ModelData. ModelCompiler compiles ModelData from file using assimp, then saves it as implemented in ModelData.Serialize, then, at runtime, the model is loaded NOT using Model data serialization implementation, but duplication of serialization method implemented into ModelReader.ReadModel(ref Model), called by Model.Load(...). From ModelData.Serialize(..): Well.. we could do it somehow more elegant but I guess this is fine. |
Ok, if you can propose any improvements - feel free to open a pull request. |
The main reason for code duplication is to avoid double allocation at runtime, as a model can have serveral bones/materials...etc and I wanted to avoid a bit to pressure the GC. That's why the serializer are somewhat duplicated. One for offline compiling (ModelData) with simple layout, no runtime dependencies (Texture2D...etc.) and the pure runtime version (Model) that has runtime dependencies. The work to maintain it is relatively low; It was a couple of hours to develop and a version handling skinning should not generate lost of additional work for the serialization part. |
I'm working on implementing skinned mesh and node animation for Toolkit and digging deeper into Model come across really confusing pattern, where parts of code are duplicated across SharpDX.Toolkit and SharpDX.Toolkit.Graphics projects, while all belonging to the same namespace (SharpDX.Toolkit.Graphics).
For example:
SharpDX.Toolkit\Graphics\ModelData.Bone
vs
SharpDX.Toolkit.Graphics\ModelBone
Both hold the same data, reside in the same namespace, yet they are of separate class and inherit different things. All related 'stuff' implemented in SharpDX.Toolkit project implement IDataSerializable, while stuff in SharpDX.Toolkit.Graphics derive from ComponentBase.
It is very confusing and I don't see any reason why would you need to duplicate this? Is there something I'm missing? What should I do to correctly implement Model class update?
The text was updated successfully, but these errors were encountered: