Re: [Gnash-commit] gnash ChangeLog server/asobj/flash/geom/Matrix_...

[zou lunkai]

>                * server/asobj/flash/geom/Matrix_as.cpp: using boost ublas for
>                  inverting was massively over-complicated; do a simpler 
> implementation
>                  (as in server/matrix.h, but without the oddities needed for 
> the SWF
>                  matrix).

Glad you see this point:)  Also, a self-designed 3*3 matrix would
probably faster than the generic boost template, and can be easily
maintained.

> -           try
> -           {
> -           // We can probably invert, but the validity check above fails on
> -           // some machines. We can get an exception from boost::ublas
> -           ublas::lu_substitute<ublas::c_matrix<double, 3, 3> 
> >(currentMatrix, pm, inverseMatrix);
> -       }

The above code actually doesn't compile on my Fedora 5 system, GCC4.1.
 Fixing it by changing the source code instead of asking me to drop my
"old" system or upgrading my "old" compiler is a more pleasant way. So
I confirm dropping it is a right choice:)

--zou






On 6/10/08, Benjamin Wolsey <address@hidden> wrote:
> CVSROOT:        /sources/gnash
> Module name:    gnash
> Changes by:     Benjamin Wolsey <bwy>   08/06/10 11:28:02
>
> Modified files:
>        .              : ChangeLog
>        server/asobj/flash/geom: Matrix_as.cpp
>
> Log message:
>                * server/asobj/flash/geom/Matrix_as.cpp: using boost ublas for
>                  inverting was massively over-complicated; do a simpler 
> implementation
>                  (as in server/matrix.h, but without the oddities needed for 
> the SWF
>                  matrix). Add notes and a typedef.
>
> CVSWeb URLs:
> http://cvs.savannah.gnu.org/viewcvs/gnash/ChangeLog?cvsroot=gnash&r1=1.6887&r2=1.6888
> http://cvs.savannah.gnu.org/viewcvs/gnash/server/asobj/flash/geom/Matrix_as.cpp?cvsroot=gnash&r1=1.11&r2=1.12
>
> Patches:
> Index: ChangeLog
> ===================================================================
> RCS file: /sources/gnash/gnash/ChangeLog,v
> retrieving revision 1.6887
> retrieving revision 1.6888
> diff -u -b -r1.6887 -r1.6888
> --- ChangeLog   10 Jun 2008 11:22:53 -0000      1.6887
> +++ ChangeLog   10 Jun 2008 11:28:01 -0000      1.6888
> @@ -1,6 +1,10 @@
>  2008-06-10 Benjamin Wolsey <address@hidden>
>
>        * server/asobj/gen-asclass.pl: stop generating bad header guard names.
> +       * server/asobj/flash/geom/Matrix_as.cpp: using boost ublas for
> +         inverting was massively over-complicated; do a simpler 
> implementation
> +         (as in server/matrix.h, but without the oddities needed for the SWF
> +         matrix). Add notes and a typedef.
>
>  2008-06-10 Sandro Santilli <address@hidden>
>
>
> Index: server/asobj/flash/geom/Matrix_as.cpp
> ===================================================================
> RCS file: /sources/gnash/gnash/server/asobj/flash/geom/Matrix_as.cpp,v
> retrieving revision 1.11
> retrieving revision 1.12
> diff -u -b -r1.11 -r1.12
> --- server/asobj/flash/geom/Matrix_as.cpp       10 Jun 2008 05:25:58 -0000    
>   1.11
> +++ server/asobj/flash/geom/Matrix_as.cpp       10 Jun 2008 11:28:02 -0000    
>   1.12
> @@ -31,14 +31,10 @@
>  #include <cmath>
>  #include <boost/numeric/ublas/matrix.hpp> // boost matrix
>  #include <boost/numeric/ublas/io.hpp>
> -#include <boost/numeric/ublas/lu.hpp> // permutation matrix etc.
>  #include <sstream>
>  #include <memory> // std::auto_ptr
>
>  /// According to senocular, Flash docs get this wrong (b and c swapped).
> -/// However, most of the transformations only apply to a
> -/// subset of the elements, so it's unnecessary to use a full 3x3 matrix for
> -/// every operation.
>  ///
>  // A transformation matrix for affine transformations:
>  //    a  c  tx
> @@ -46,13 +42,19 @@
>  //    u  v  w
>  // The matrix can only operate in 2d space. The bottom row is immutable
>  // as 0  0  1.
> +/// Most transformations only apply to a subset of the elements
> +/// (partly because the bottom line is immutable), so it's unnecessary to
> +/// use a full 3x3 matrix for every operation: particularly for invert(),
> +/// where boost::ublas is overcomplicated and can easily fail its own
> +/// consistency checks. For simpler multiplication, boost::ublas is very
> +/// helpful for keep the code clear and tidy.
>
>  // Define this to get verbose debugging messages for matrix calculations
>  //#define GNASH_DEBUG_GEOM_MATRIX 1
>
>  namespace gnash {
>
> -class Matrix_as;
> +typedef boost::numeric::ublas::c_matrix<double, 3, 3> MatrixType;
>
>  // Forward declarations
>  static as_value Matrix_clone(const fn_call& fn);
> @@ -67,7 +69,7 @@
>  static as_value Matrix_toString(const fn_call& fn);
>  static as_value Matrix_transformPoint(const fn_call& fn);
>  static as_value Matrix_translate(const fn_call& fn);
> -static void fillMatrix(boost::numeric::ublas::c_matrix<double, 3, 3>& matrix,
> +static void fillMatrix(MatrixType& matrix,
>                          as_object* const matrixObject);
>
>  as_value Matrix_ctor(const fn_call& fn);
> @@ -215,12 +217,11 @@
>         return as_value();
>     }
>
> -    boost::numeric::ublas::c_matrix<double, 3, 3> concatMatrix;
> +    MatrixType concatMatrix;
>     fillMatrix(concatMatrix, obj);
>
> -
>     // Current ('this') Matrix
> -    boost::numeric::ublas::c_matrix<double, 3, 3> currentMatrix;
> +    MatrixType currentMatrix;
>     fillMatrix(currentMatrix, ptr.get());
>
>  #ifdef GNASH_DEBUG_GEOM_MATRIX
> @@ -373,80 +374,60 @@
>     return as_value();
>  }
>
> +
> +static inline double
> +getMinorDeterminant(const MatrixType& m)
> +{
> +    return m(0, 0) * m(1, 1) - m(0, 1) * m(1, 0);
> +}
> +
>  static as_value
>  Matrix_invert(const fn_call& fn)
>  {
> -    boost::intrusive_ptr<Matrix_as> ptr = ensureType<Matrix_as>(fn.this_ptr);
> -
> -    using namespace boost::numeric;
>
> -    const double u = 0.0;
> -    const double v = 0.0;
> -    const double w = 1.0;
> +    boost::intrusive_ptr<Matrix_as> ptr = ensureType<Matrix_as>(fn.this_ptr);
>
> -    // This starts off as the identity matrix. If it's impossible to invert 
> the
> -    // matrix, this is returned.
> -    ublas::c_matrix<double, 3, 3> inverseMatrix;
> -    inverseMatrix(0, 0) = 1.0;
> -    inverseMatrix(0, 1) = 0.0;
> -    inverseMatrix(0, 2) = 0.0;
> -    inverseMatrix(1, 0) = 0.0;
> -    inverseMatrix(1, 1) = 1.0;
> -    inverseMatrix(1, 2) = 0.0;
> -    inverseMatrix(2, 0) = u;
> -    inverseMatrix(2, 1) = v;
> -    inverseMatrix(2, 2) = w;
> +    MatrixType currentMatrix;
>
> -    // Get current matrix
> -    ublas::c_matrix<double, 3, 3> currentMatrix;
> +    // This just saves repeating code to get doubles for each
> +    // value.
>     fillMatrix(currentMatrix, ptr.get());
>
> -#ifdef GNASH_DEBUG_GEOM_MATRIX
> -    log_debug("(Matrix.invert) This matrix (pre-transform): %s", 
> currentMatrix);
> -#endif
> -
> -       ublas::permutation_matrix<double> pm(currentMatrix.size1());
> -
> -#ifdef GNASH_DEBUG_GEOM_MATRIX
> -    log_debug("(Matrix.invert) Permutation matrix: %s", pm);
> -#endif
> -
> -       int valid = ublas::lu_factorize<ublas::c_matrix<double, 3, 3> 
> >(currentMatrix, pm);
> +    const double determinant = getMinorDeterminant(currentMatrix);
>
> -#ifdef GNASH_DEBUG_GEOM_MATRIX
> -    log_debug("(Matrix.invert) Factorized matrix (return %f): %s", valid, 
> pm);
> -#endif
> -
> -       if( valid == 0 )
> +    // This is a double, so it could be worth checking for the epsilon.
> +    if (determinant == 0)
>        {
> -           try
> -           {
> -           // We can probably invert, but the validity check above fails on
> -           // some machines. We can get an exception from boost::ublas
> -           ublas::lu_substitute<ublas::c_matrix<double, 3, 3> 
> >(currentMatrix, pm, inverseMatrix);
> -       }
> -       catch (ublas::internal_logic &e)
> -       {
> -
> +        // Return the identity matrix
> +        ptr->set_member(NSV::PROP_A, as_value(1.0));
> +        ptr->set_member(NSV::PROP_B, as_value(0.0));
> +        ptr->set_member(NSV::PROP_C, as_value(0.0));
> +        ptr->set_member(NSV::PROP_D, as_value(1.0));
> +        ptr->set_member(NSV::PROP_TX, as_value(0.0));
> +        ptr->set_member(NSV::PROP_TY, as_value(0.0));
> +        return as_value();
>        }
>
> -       }
> +    const double a = currentMatrix(1, 1) / determinant;
> +    const double c = - currentMatrix(0, 1) / determinant;
> +    const double b = - currentMatrix(1, 0) / determinant;
> +    const double d = currentMatrix(0, 0) / determinant;
>
> -#ifdef GNASH_DEBUG_GEOM_MATRIX
> -        log_debug("(Matrix.invert) Inverse matrix: %s", inverseMatrix);
> -#endif
> +    const double tx = - (a * currentMatrix(0, 2) + c * currentMatrix(1, 2));
> +    const double ty = - (b * currentMatrix(0, 2) + d * currentMatrix(1, 2));
>
>     // Returns the identity matrix if unsuccessful.
> -    ptr->set_member(NSV::PROP_A, as_value(inverseMatrix(0, 0)));
> -    ptr->set_member(NSV::PROP_B, as_value(inverseMatrix(1, 0)));
> -    ptr->set_member(NSV::PROP_C, as_value(inverseMatrix(0, 1)));
> -    ptr->set_member(NSV::PROP_D, as_value(inverseMatrix(1, 1)));
> -    ptr->set_member(NSV::PROP_TX, as_value(inverseMatrix(0, 2)));
> -    ptr->set_member(NSV::PROP_TY, as_value(inverseMatrix(1, 2)));
> +    ptr->set_member(NSV::PROP_A, as_value(as_value(a)));
> +    ptr->set_member(NSV::PROP_B, as_value(as_value(b)));
> +    ptr->set_member(NSV::PROP_C, as_value(as_value(c)));
> +    ptr->set_member(NSV::PROP_D, as_value(as_value(d)));
> +    ptr->set_member(NSV::PROP_TX, as_value(as_value(tx)));
> +    ptr->set_member(NSV::PROP_TY, as_value(as_value(ty)));
>
>     return as_value();
>  }
>
> +
>  static as_value
>  Matrix_rotate(const fn_call& fn)
>  {
> @@ -629,7 +610,7 @@
>
>
>  // A helper function to create a boost matrix from a Matrix object
> -static void fillMatrix(boost::numeric::ublas::c_matrix<double, 3, 3>& matrix,
> +static void fillMatrix(MatrixType& matrix,
>                          as_object* const matrixObject)
>  {
>
>
>
> _______________________________________________
> Gnash-commit mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/gnash-commit
>