[Scummvm-git-logs] scummvm master -> 66024fdd2b9519560718cbc8a4ace3959279243a

dreammaster dreammaster at scummvm.org
Wed Aug 16 03:28:50 CEST 2017


This automated email contains information about 4 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
0f600dc21a TITANIC: daffine refactoring, non-functional changes
8eb7bf3807 TITANIC: daffine refactor, call clear before setting rot matrix
781c679c7f TITANIC: daffine refactor, changed Yaxis rotation convention
66024fdd2b Merge pull request #991 from dafioram/daffine_refactor


Commit: 0f600dc21ac0469f581856c34876a63794023b06
    https://github.com/scummvm/scummvm/commit/0f600dc21ac0469f581856c34876a63794023b06
Author: David Fioramonti (dafioram at gmail.com)
Date: 2017-08-15T04:11:59-07:00

Commit Message:
TITANIC: daffine refactoring, non-functional changes

Made default constructor col4 construction explicit.
Change amount argument to be angle_deg. Added constant
from dvector that does conversion from degrees to radians".

Also moved conversion constants for angles in dvector to
header file so daffine could use that.

Changed paths:
    engines/titanic/star_control/daffine.cpp
    engines/titanic/star_control/daffine.h
    engines/titanic/star_control/dvector.cpp
    engines/titanic/star_control/dvector.h


diff --git a/engines/titanic/star_control/daffine.cpp b/engines/titanic/star_control/daffine.cpp
index a7bed77..56c705b 100644
--- a/engines/titanic/star_control/daffine.cpp
+++ b/engines/titanic/star_control/daffine.cpp
@@ -29,7 +29,7 @@ namespace Titanic {
 DAffine *DAffine::_static;
 
 DAffine::DAffine() :
-	_col1(0.0, 0.0, 0.0), _col2(0.0, 0.0, 0.0), _col3(0.0, 0.0, 0.0) {
+	_col1(0.0, 0.0, 0.0), _col2(0.0, 0.0, 0.0), _col3(0.0, 0.0, 0.0), _col4(0.0, 0.0, 0.0) {
 }
 
 DAffine::DAffine(int mode, const DVector &src) {
@@ -55,8 +55,8 @@ DAffine::DAffine(int mode, const DVector &src) {
 	}
 }
 
-DAffine::DAffine(Axis axis, double amount) {
-	setRotationMatrix(axis, amount);
+DAffine::DAffine(Axis axis, double angleDeg) {
+	setRotationMatrix(axis, angleDeg);
 }
 
 DAffine::DAffine(const FMatrix &src) {
@@ -74,10 +74,9 @@ void DAffine::deinit() {
 	_static = nullptr;
 }
 
-void DAffine::setRotationMatrix(Axis axis, double amount) {
-	const double FACTOR = 0.0174532925199433;
-	double sinVal = sin(amount * FACTOR);
-	double cosVal = cos(amount * FACTOR);
+void DAffine::setRotationMatrix(Axis axis, double angleDeg) {
+	double sinVal = sin(angleDeg * Deg2Rad);
+	double cosVal = cos(angleDeg * Deg2Rad);
 
 	switch (axis) {
 	case X_AXIS:
diff --git a/engines/titanic/star_control/daffine.h b/engines/titanic/star_control/daffine.h
index 50f8b95..50450b9 100644
--- a/engines/titanic/star_control/daffine.h
+++ b/engines/titanic/star_control/daffine.h
@@ -51,18 +51,30 @@ public:
 public:
 	DAffine();
 	DAffine(int mode, const DVector &src);
-	DAffine(Axis axis, double amount);
+	DAffine(Axis axis, double angleDeg);
 	DAffine(const FMatrix &src);
 
 	/**
-	 * Sets up a matrix for rotating on a given axis by a given amount
+	 * Sets up an affine matrix for rotating on a given axis by an amount in degrees
 	 */
-	void setRotationMatrix(Axis axis, double amount);
+	void setRotationMatrix(Axis axis, double angleDeg);
 
+	/**
+	 * Return the Inverse of this Daffine
+	 */
 	DAffine inverseTransform() const;
 
+	/**
+	 * Change this Daffine to have its first three columns be the src matrix
+         * and the 4rth column to be (three) zeros
+	 */
 	void loadTransform(const CMatrixTransform &src);
 
+	/**
+	 * Do the affine product between this Daffine on the left
+         * and the m Daffine matrix on the right. This is product is NOT the same
+         * as multiplying two matrices of dimensions 4x4.
+	 */
 	DAffine compose(const DAffine &m);
 };
 
diff --git a/engines/titanic/star_control/dvector.cpp b/engines/titanic/star_control/dvector.cpp
index 1f873b5..9f35ff6 100644
--- a/engines/titanic/star_control/dvector.cpp
+++ b/engines/titanic/star_control/dvector.cpp
@@ -26,9 +26,6 @@
 
 namespace Titanic {
 
-const double Rad2Deg = 180.0 / M_PI;
-const double Deg2Rad = 1.0 / Rad2Deg;
-
 double DVector::normalize() {
 	double hyp = sqrt(_x * _x + _y * _y + _z * _z);
 	assert(hyp);
diff --git a/engines/titanic/star_control/dvector.h b/engines/titanic/star_control/dvector.h
index eda69f9..bff271d 100644
--- a/engines/titanic/star_control/dvector.h
+++ b/engines/titanic/star_control/dvector.h
@@ -27,6 +27,9 @@
 
 namespace Titanic {
 
+const double Rad2Deg = 180.0 / M_PI;
+const double Deg2Rad = 1.0 / Rad2Deg;
+
 class DAffine;
 
 /**


Commit: 8eb7bf3807198f2ff200550050e32fe852d6054e
    https://github.com/scummvm/scummvm/commit/8eb7bf3807198f2ff200550050e32fe852d6054e
Author: David Fioramonti (dafioram at gmail.com)
Date: 2017-08-15T04:46:43-07:00

Commit Message:
TITANIC: daffine refactor, call clear before setting rot matrix

The previous code wasn't reseting all the other elements to zero
when setting up a rotation matrix. This would of left other values
in the not set elements leading to a matrix not quite what the caller
wanted. This should lead to the function getFrameTransform returning
a different Daffine matrix.

Also added lots of todos

Changed paths:
    engines/titanic/star_control/daffine.cpp
    engines/titanic/star_control/daffine.h


diff --git a/engines/titanic/star_control/daffine.cpp b/engines/titanic/star_control/daffine.cpp
index 56c705b..8c956da 100644
--- a/engines/titanic/star_control/daffine.cpp
+++ b/engines/titanic/star_control/daffine.cpp
@@ -65,6 +65,7 @@ DAffine::DAffine(const FMatrix &src) {
 	_col3 = src._row3;
 }
 
+//TODO: What is _static_ for?
 void DAffine::init() {
 	_static = nullptr;
 }
@@ -74,7 +75,24 @@ void DAffine::deinit() {
 	_static = nullptr;
 }
 
+void DAffine::clear() {
+	_col1._x = 0.0;
+	_col1._y = 0.0;
+	_col1._z = 0.0;
+	_col2._x = 0.0;
+	_col2._y = 0.0;
+	_col2._z = 0.0;
+	_col3._x = 0.0;
+	_col3._y = 0.0;
+	_col3._z = 0.0;
+	_col4._x = 0.0;
+	_col4._y = 0.0;
+	_col4._z = 0.0;
+}
+
 void DAffine::setRotationMatrix(Axis axis, double angleDeg) {
+        clear();
+
 	double sinVal = sin(angleDeg * Deg2Rad);
 	double cosVal = cos(angleDeg * Deg2Rad);
 
@@ -108,6 +126,7 @@ void DAffine::setRotationMatrix(Axis axis, double angleDeg) {
 	}
 }
 
+//TODO: Check math and provide source
 DAffine DAffine::inverseTransform() const {
 	double val1 = _col1._x * _col3._z * _col2._y;
 	double val2 = 0.0;
@@ -173,6 +192,7 @@ DAffine DAffine::inverseTransform() const {
 	return m;
 }
 
+//TODO: Check math and provide source
 void DAffine::loadTransform(const CMatrixTransform &src) {
 	double total = src.fn1();
 	double factor = (total <= 0.0) ? 0.0 : 2.0 / total;
@@ -200,6 +220,7 @@ void DAffine::loadTransform(const CMatrixTransform &src) {
 	_col4._z = 0;
 }
 
+//TODO: Check math and provide source
 DAffine DAffine::compose(const DAffine &m) {
 	DAffine dm;
 	dm._col1._x = m._col3._x * _col1._z + m._col2._x * _col1._y
diff --git a/engines/titanic/star_control/daffine.h b/engines/titanic/star_control/daffine.h
index 50450b9..dda8732 100644
--- a/engines/titanic/star_control/daffine.h
+++ b/engines/titanic/star_control/daffine.h
@@ -55,6 +55,11 @@ public:
 	DAffine(const FMatrix &src);
 
 	/**
+	 * Sets all elements to zero
+	 */
+	void clear();
+
+	/**
 	 * Sets up an affine matrix for rotating on a given axis by an amount in degrees
 	 */
 	void setRotationMatrix(Axis axis, double angleDeg);
@@ -65,8 +70,9 @@ public:
 	DAffine inverseTransform() const;
 
 	/**
-	 * Change this Daffine to have its first three columns be the src matrix
-         * and the 4rth column to be (three) zeros
+	 * Change this Daffine to have its first three columns be some mapping from src matrix
+         * and the 4rth column to be (three) zeros. The mapping is not as simple as replacing
+         * matching row/colmn indices
 	 */
 	void loadTransform(const CMatrixTransform &src);
 


Commit: 781c679c7f8e761e35c3d72ac27e16350c55964a
    https://github.com/scummvm/scummvm/commit/781c679c7f8e761e35c3d72ac27e16350c55964a
Author: David Fioramonti (dafioram at gmail.com)
Date: 2017-08-15T15:02:02-07:00

Commit Message:
TITANIC: daffine refactor, changed Yaxis rotation convention

The X and Z rotation already follow the convention given in wikipedia,
but the Y axis rotation doesn't (its the negative angle) so I switched
that and updated where that was used.

This allowed stray negatives for angle calls to this function (for Y
rotations) to be removed from other parts of the code (dvector).

In theory this was a non-functional change. In dvector the code was taking
the negative of the angle so it was essentially doing the negative of the
negative, but when it was used once in star_camera it was not
(when it should of been). So That was changed. That part of the code
was used for locking onto the third star after the 2nd was already locked.

I can't tell if the star control puzzle has improved after this change.
It can still have issues locking onto the 2nd star and also not.

Also added lots of todos for things to check.

Changed paths:
    engines/titanic/star_control/daffine.cpp
    engines/titanic/star_control/daffine.h
    engines/titanic/star_control/dvector.cpp


diff --git a/engines/titanic/star_control/daffine.cpp b/engines/titanic/star_control/daffine.cpp
index 8c956da..6dfee29 100644
--- a/engines/titanic/star_control/daffine.cpp
+++ b/engines/titanic/star_control/daffine.cpp
@@ -65,7 +65,7 @@ DAffine::DAffine(const FMatrix &src) {
 	_col3 = src._row3;
 }
 
-//TODO: What is _static_ for?
+//TODO: What is _static for?
 void DAffine::init() {
 	_static = nullptr;
 }
@@ -90,6 +90,7 @@ void DAffine::clear() {
 	_col4._z = 0.0;
 }
 
+// Source: https://en.wikipedia.org/wiki/Rotation_matrix
 void DAffine::setRotationMatrix(Axis axis, double angleDeg) {
         clear();
 
@@ -107,9 +108,9 @@ void DAffine::setRotationMatrix(Axis axis, double angleDeg) {
 
 	case Y_AXIS:
 		_col1._x = cosVal;
-		_col1._z = sinVal;
+		_col1._z = -sinVal;
 		_col2._y = 1.0;
-		_col3._x = -sinVal;
+		_col3._x = sinVal;
 		_col3._z = cosVal;
 		break;
 
diff --git a/engines/titanic/star_control/daffine.h b/engines/titanic/star_control/daffine.h
index dda8732..631b600 100644
--- a/engines/titanic/star_control/daffine.h
+++ b/engines/titanic/star_control/daffine.h
@@ -50,6 +50,7 @@ public:
 	static void deinit();
 public:
 	DAffine();
+        //TODO: consider making mode an enum since that is more helpful when it is used in code
 	DAffine(int mode, const DVector &src);
 	DAffine(Axis axis, double angleDeg);
 	DAffine(const FMatrix &src);
@@ -69,18 +70,18 @@ public:
 	 */
 	DAffine inverseTransform() const;
 
-	/**
-	 * Change this Daffine to have its first three columns be some mapping from src matrix
-         * and the 4rth column to be (three) zeros. The mapping is not as simple as replacing
-         * matching row/colmn indices
-	 */
+       /**
+        * Change this Daffine to have its first three columns be some mapping from src matrix
+        * and the 4rth column to be (three) zeros. The mapping is not as simple as replacing
+        * matching row/colmn indices
+        */
 	void loadTransform(const CMatrixTransform &src);
 
-	/**
-	 * Do the affine product between this Daffine on the left
-         * and the m Daffine matrix on the right. This is product is NOT the same
-         * as multiplying two matrices of dimensions 4x4.
-	 */
+       /**
+        * Do the affine product between this Daffine on the right
+        * and the m Daffine matrix on the left. This product is NOT the same
+        * as multiplying two matrices of dimensions 3x4.
+        */
 	DAffine compose(const DAffine &m);
 };
 
diff --git a/engines/titanic/star_control/dvector.cpp b/engines/titanic/star_control/dvector.cpp
index 9f35ff6..c7481a2 100644
--- a/engines/titanic/star_control/dvector.cpp
+++ b/engines/titanic/star_control/dvector.cpp
@@ -86,13 +86,13 @@ DAffine DVector::getFrameTransform(const DVector &v) {
 
 	DVector vector1 = getAnglesAsVect();
 	matrix1.setRotationMatrix(X_AXIS, vector1._y * Rad2Deg);
-	matrix2.setRotationMatrix(Y_AXIS, -(vector1._z * Rad2Deg));
+	matrix2.setRotationMatrix(Y_AXIS, vector1._z * Rad2Deg);
 	matrix3 = matrix1.compose(matrix2);
 	matrix4 = matrix3.inverseTransform();
 
 	vector1 = v.getAnglesAsVect();
 	matrix1.setRotationMatrix(X_AXIS, vector1._y * Rad2Deg);
-	matrix2.setRotationMatrix(Y_AXIS, -(vector1._z * Rad2Deg));
+	matrix2.setRotationMatrix(Y_AXIS, vector1._z * Rad2Deg);
 	matrix3 = matrix1.compose(matrix2);
 
 	return matrix4.compose(matrix3);
@@ -102,7 +102,7 @@ DAffine DVector::rotXY() const {
 	DVector v1 = getAnglesAsVect();
 	DAffine m1, m2;
 	m1.setRotationMatrix(X_AXIS, v1._y * Rad2Deg);
-	m2.setRotationMatrix(Y_AXIS, -(v1._z * Rad2Deg));
+	m2.setRotationMatrix(Y_AXIS, v1._z * Rad2Deg);
 	return m1.compose(m2);
 }
 


Commit: 66024fdd2b9519560718cbc8a4ace3959279243a
    https://github.com/scummvm/scummvm/commit/66024fdd2b9519560718cbc8a4ace3959279243a
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2017-08-15T21:28:46-04:00

Commit Message:
Merge pull request #991 from dafioram/daffine_refactor

TITANIC: DAffine refactor

Changed paths:
    engines/titanic/star_control/daffine.cpp
    engines/titanic/star_control/daffine.h
    engines/titanic/star_control/dvector.cpp
    engines/titanic/star_control/dvector.h







More information about the Scummvm-git-logs mailing list