[PATCH] d3dx9_36: Implement ID3DXSkinInfoImpl_UpdateSkinnedMesh.

[Matteo Bruni]

On Wed, Apr 10, 2019 at 12:10 PM Vijay Kiran Kamuju <infyquest at gmail.com> wrote:
>
> On Tue, Apr 9, 2019 at 7:06 PM Matteo Bruni <matteo.mystral at gmail.com> wrote:
> >
> > On Sat, Apr 6, 2019 at 10:50 AM Vijay Kiran Kamuju <infyquest at 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 at gmail.com>
> > > Signed-off-by: Vijay Kiran Kamuju <infyquest at 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.