-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
scale geometric error with tileset #7411
Changes from all commits
3632059
4a8c92d
ca38e70
fd5f6a6
899f799
905d783
ebb058f
ed7bd07
698cc16
36e569a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2149,6 +2149,13 @@ define([ | |||||
return result; | ||||||
}; | ||||||
|
||||||
/** | ||||||
* @deprecated moved to Matrix4.getMatrix3 | ||||||
*/ | ||||||
Matrix4.getRotation = function(matrix, result) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's fine to keep the original description and then insert the /**
* Gets the upper left 3x3 rotation matrix of the provided matrix, assuming the matrix is a affine transformation matrix.
*
* @param {Matrix4} matrix The matrix to use.
* @param {Matrix3} result The object onto which to store the result.
* @returns {Matrix3} The modified result parameter.
*
* @deprecated This function has been deprecated. Use {@link Matrix4#getMatrix3} instead.
*
* @example
* // returns a Matrix3 instance from a Matrix4 instance
*
* // m = [10.0, 14.0, 18.0, 22.0]
* // [11.0, 15.0, 19.0, 23.0]
* // [12.0, 16.0, 20.0, 24.0]
* // [13.0, 17.0, 21.0, 25.0]
*
* var b = new Cesium.Matrix3();
* Cesium.Matrix4.getRotation(m,b);
*
* // b = [10.0, 14.0, 18.0]
* // [11.0, 15.0, 19.0]
* // [12.0, 16.0, 20.0]
*/
Matrix4.getRotation = function(matrix, result) {
deprecationWarning('Matrix4.getRotation', 'Matrix4.getRotation is deprecated and will be removed in Cesium 1.65. Use Matrix4.getMatrix3 instead.');
return Matrix4.getMatrix3(matrix, result);
}; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add the deprecation notice to
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are still a lot of occurrences of |
||||||
return Matrix4.getMatrix3(matrix, result); | ||||||
}; | ||||||
|
||||||
/** | ||||||
* Gets the upper left 3x3 rotation matrix of the provided matrix, assuming the matrix is a affine transformation matrix. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated typo I noticed.
Suggested change
|
||||||
* | ||||||
|
@@ -2165,13 +2172,13 @@ define([ | |||||
* // [13.0, 17.0, 21.0, 25.0] | ||||||
* | ||||||
* var b = new Cesium.Matrix3(); | ||||||
* Cesium.Matrix4.getRotation(m,b); | ||||||
* Cesium.Matrix4.getMatrix3(m,b); | ||||||
* | ||||||
* // b = [10.0, 14.0, 18.0] | ||||||
* // [11.0, 15.0, 19.0] | ||||||
* // [12.0, 16.0, 20.0] | ||||||
*/ | ||||||
Matrix4.getRotation = function(matrix, result) { | ||||||
Matrix4.getMatrix3 = function(matrix, result) { | ||||||
//>>includeStart('debug', pragmas.debug); | ||||||
Check.typeOf.object('matrix', matrix); | ||||||
Check.typeOf.object('result', result); | ||||||
|
@@ -2286,7 +2293,7 @@ define([ | |||||
if (Math.abs(det) < CesiumMath.EPSILON21) { | ||||||
// Special case for a zero scale matrix that can occur, for example, | ||||||
// when a model's node has a [0, 0, 0] scale. | ||||||
if (Matrix3.equalsEpsilon(Matrix4.getRotation(matrix, scratchInverseRotation), scratchMatrix3Zero, CesiumMath.EPSILON7) && | ||||||
if (Matrix3.equalsEpsilon(Matrix4.getMatrix3(matrix, scratchInverseRotation), scratchMatrix3Zero, CesiumMath.EPSILON7) && | ||||||
Cartesian4.equals(Matrix4.getRow(matrix, 3, scratchBottomRow), scratchExpectedBottomRow)) { | ||||||
|
||||||
result[0] = 0.0; | ||||||
|
@@ -2353,7 +2360,7 @@ define([ | |||||
//>>includeEnd('debug'); | ||||||
|
||||||
//This function is an optimized version of the below 4 lines. | ||||||
//var rT = Matrix3.transpose(Matrix4.getRotation(matrix)); | ||||||
//var rT = Matrix3.transpose(Matrix4.getMatrix3(matrix)); | ||||||
//var rTN = Matrix3.negate(rT); | ||||||
//var rTT = Matrix3.multiplyByVector(rTN, Matrix4.getTranslation(matrix)); | ||||||
//return Matrix4.fromRotationTranslation(rT, rTT, result); | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -639,6 +639,41 @@ defineSuite([ | |
expect(result).toEqual(expected); | ||
}); | ||
|
||
it('getRotation returns matrix without scale', function() { | ||
var matrix = new Matrix3(1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0); | ||
var result = new Matrix3(); | ||
var expected = Matrix3.fromArray([ | ||
0.12309149097933272, 0.4923659639173309, 0.8616404368553291, | ||
0.20739033894608505, 0.5184758473652127, 0.8295613557843402, | ||
0.26726124191242440, 0.5345224838248488, 0.8017837257372732 | ||
]); | ||
var scale = new Cartesian3(); | ||
var expectedScale = new Cartesian3(1.0, 1.0, 1.0); | ||
result = Matrix3.getRotation(matrix, result); | ||
console.log(result); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove |
||
var resultScale = Matrix3.getScale(result, scale); | ||
expect(resultScale).toEqualEpsilon(expectedScale, CesiumMath.EPSILON14); | ||
lilleyse marked this conversation as resolved.
Show resolved
Hide resolved
|
||
expect(result).toEqualEpsilon(expected, CesiumMath.EPSILON14); | ||
}); | ||
|
||
it('getRotation does not modify rotation matrix', function() { | ||
var tmp = new Matrix3(); | ||
var result = new Matrix3(); | ||
var rotation = Matrix3.clone(Matrix3.IDENTITY, new Matrix3()); | ||
Matrix3.multiply(rotation, Matrix3.fromRotationX(1.0, tmp), rotation); | ||
Matrix3.multiply(rotation, Matrix3.fromRotationY(2.0, tmp), rotation); | ||
Matrix3.multiply(rotation, Matrix3.fromRotationZ(3.0, tmp), rotation); | ||
result = Matrix3.getRotation(rotation, result); | ||
expect(rotation).toEqualEpsilon(result, CesiumMath.EPSILON14); | ||
expect(rotation).not.toBe(result); | ||
}); | ||
|
||
it('getRotation throws without a matrix', function() { | ||
expect(function() { | ||
return Matrix3.getRotation(); | ||
}).toThrowDeveloperError(); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also add a separate check for |
||
|
||
it('transpose works with a result parameter that is an input result parameter', function() { | ||
lilleyse marked this conversation as resolved.
Show resolved
Hide resolved
|
||
var matrix = new Matrix3(1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0); | ||
var expected = new Matrix3(1.0, 4.0, 7.0, 2.0, 5.0, 8.0, 3.0, 6.0, 9.0); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.