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