On Wednesday, April 10, 2019, Matteo Bruni matteo.mystral@gmail.com wrote:
On Wed, Apr 10, 2019 at 12:10 PM Vijay Kiran Kamuju infyquest@gmail.com wrote:
On Tue, Apr 9, 2019 at 7:06 PM Matteo Bruni matteo.mystral@gmail.com
wrote:
On Sat, Apr 6, 2019 at 10:50 AM Vijay Kiran Kamuju <
infyquest@gmail.com> wrote:
This patch fixes last problem of bug 32572. https://bugs.winehq.org/show_bug.cgi?id=32572
From: Christian Costa titan.costa@gmail.com Signed-off-by: Vijay Kiran Kamuju infyquest@gmail.com
There are many stylistic and practical issues with this patch which I'll not get into the details of. I'll point out just one:
- /* Update positions that are influenced by bones */
- for (i = 0; i < skin->num_bones; i++) {
D3DXMATRIX bone_inverse, matrix;D3DXMatrixInverse(&bone_inverse, NULL,&skin->bones[i].transform);
D3DXMatrixMultiply(&matrix, &bone_transforms[i],&bone_inverse);
D3DXMatrixMultiply(&matrix, &matrix,&skin->bones[i].transform);
I will change the above code to like this for (i = 0; i < skin->num_bones; i++) { /* need to copy the contents of bone_transforms using another loop may
be */
D3DXMATRIX matrix = bone_transforms[i]; float det_bone; det_bone = D3DXMatrixDeterminant(&skin->bones[i].transform); D3DXMatrixScaling(&matrix, det_bone, det_bone, det_bone);This looks to me like a very expensive way of doing nothing.
I hope this is not expensive as far as I remember my Math, the above
turns to
det(skin->bones[i].transform) * bone_transforms[i]
My point is that the piece of code I referenced really does nothing useful with skin->bones[i].transform, which suggests deeper issues with the patch. The tests pass even with just "matrix = bone_transforms[i];" in place of that, FWIW.
It seems I have to do a deeper analysis of the code. I checked only whether the tests are successful or not.