erich666 / GraphicsGems

Code for the "Graphics Gems" book series
Other
1.39k stars 265 forks source link

Rank 1 matrices are not polar-decomposed properly #14

Closed martinbis11 closed 7 years ago

martinbis11 commented 7 years ago

When using polar_decomp from GraphicsGems/gemsiv/polar_decomp/Decompose.c, matrices of rank 1 are not handled properly.

The problem is that polar_decomp calls do_rank2 with the same matrix Mk as the M and the Q parameter. This is not a problem (yet), because do_rank2 does not change Q before being done with reading from M, with the exception of the call to do_rank1.

In do_rank1, again the same matrix ends up being represented by both parameters M and Q. However, the function starts by assigning the identity matrix to Q (which at the same time overwrites M). Therefore, do_rank1 will always return the same matrix when M and Q are the same object.

The fix is trivial: we simple have to move the assignment of identity to matrix Q. It just has to move from the beginning of the function to after the computation of the s factor (last line reading from M). In order to also work with matrices of rank 0, it must also be put in the early return case if there is no maximum column.

erich666 commented 7 years ago

Thanks, and if you want to be a total hero, I'd appreciate you making a pull request (or even just emailing me the code snippet). I can imagine messing this up myself.

martinbis11 commented 7 years ago

It might have to wait until next week, but I can do that.

erich666 commented 7 years ago

Thanks, no rush.

martinbis11 commented 7 years ago

Done, see #15.

erich666 commented 7 years ago

Quick work - thanks, and merged.