alecjacobson / computer-graphics-mass-spring-systems

Computer Graphics Assignment – Mass Spring Systems
49 stars 20 forks source link

The Update is Wrong #9

Closed sey649980 closed 4 years ago

sey649980 commented 4 years ago

For Pinned vertices formula, you recently added C after C^T in commit 42be408af89b5cd505d57687342f78f0472cf052.

That is wrong. It's impossible for me to show the math to you by typing here. However, for three of us, after adding C to our code, the animation became wrong.

I believe that the original version of pinned vertices formula is correct.

Bests,

Sey

alecjacobson commented 4 years ago

I claim the current (updated) version is correct. Perhaps during the change, there's been some confusion. I think we can sort this out by being clear about dimensions and re-deriving

p^rest in n by 3
p is n by 3
C is |pinned| by n

then

w/2 trace( (C p - C p^rest)' (C p - C p^rest) )
w/2 trace( (C (p - p^rest))' (C(p - p^rest) )
w/2 trace(  (p - p^rest)' C'C (p - p^rest) )
w/2 trace( p' C'C p ) - w trace( p' C'C p^rest ) + constant

which matches the current readme.

Given the dimensions above p C' p^rest is non-sense because C' is n by |pinned| and p^rest is n by 3 so C' p^rest does not compute.

Perhaps in your code, you're already creating p^rest to be |pinned| by 3 ??

sey649980 commented 4 years ago

My deepest apologies! You are right. Sorry for taking your extra time to make the detailed math.

Should I close the case?