Skip to content
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.

Sign up for GitHub

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

Altered matrix inversion code to behave correctly with single precision zero scaling #7037

Merged
merged 3 commits into from
Sep 17, 2018
Merged

Altered matrix inversion code to behave correctly with single precision zero scaling #7037

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b85a382
fix for issue. added appropriate tests to the matrix4 spec
Sep 12, 2018
7c361c1
Merge branch 'master' into modelViewMatrixInversionBug
Sep 12, 2018
93cb38e
Minor tweaks.
bagnell Sep 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Change Log

##### Fixes :wrench:
* Fixed an issue in the 3D Tiles traversal where empty tiles would be selected instead of their nearest loaded ancestors. [#7011](https://github.com/AnalyticalGraphicsInc/cesium/pull/7011)
* Fixed an issue where scaling near zero could cause rendering to stop. [#6954](https://github.com/AnalyticalGraphicsInc/cesium/pull/6954)

### 1.49 - 2018-09-04

Expand Down
51 changes: 25 additions & 26 deletions Source/Core/Matrix4.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2211,31 +2211,6 @@ define([
Check.typeOf.object('matrix', matrix);
Check.typeOf.object('result', result);
//>>includeEnd('debug');

// 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) &&
Cartesian4.equals(Matrix4.getRow(matrix, 3, scratchBottomRow), scratchExpectedBottomRow)) {

result[0] = 0.0;
result[1] = 0.0;
result[2] = 0.0;
result[3] = 0.0;
result[4] = 0.0;
result[5] = 0.0;
result[6] = 0.0;
result[7] = 0.0;
result[8] = 0.0;
result[9] = 0.0;
result[10] = 0.0;
result[11] = 0.0;
result[12] = -matrix[12];
result[13] = -matrix[13];
result[14] = -matrix[14];
result[15] = 1.0;
return result;
}

//
// Ported from:
// ftp://download.intel.com/design/PentiumIII/sml/24504301.pdf
Expand Down Expand Up @@ -2308,7 +2283,31 @@ define([
// calculate determinant
var det = src0 * dst0 + src1 * dst1 + src2 * dst2 + src3 * dst3;

if (Math.abs(det) < CesiumMath.EPSILON20) {
if (Math.abs(det) < 1e-21) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use CesiumMath.EPSILON20. Or if it needs to be 21 add CesiumMath.EPSILON21

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(to Math.js)

// 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) &&
Cartesian4.equals(Matrix4.getRow(matrix, 3, scratchBottomRow), scratchExpectedBottomRow)) {

result[0] = 0.0;
result[1] = 0.0;
result[2] = 0.0;
result[3] = 0.0;
result[4] = 0.0;
result[5] = 0.0;
result[6] = 0.0;
result[7] = 0.0;
result[8] = 0.0;
result[9] = 0.0;
result[10] = 0.0;
result[11] = 0.0;
result[12] = -matrix[12];
result[13] = -matrix[13];
result[14] = -matrix[14];
result[15] = 1.0;
return result;
}

throw new RuntimeError('matrix is not invertible because its determinate is zero.');
}

Expand Down
32 changes: 32 additions & 0 deletions Specs/Core/Matrix4Spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -993,6 +993,38 @@ defineSuite([
expect(expected).toEqualEpsilon(result, CesiumMath.EPSILON20);
});

it('inverse behaves acceptably with near single precision zero scale matrix', function() {
var trs = new TranslationRotationScale(new Cartesian3(0.0, 0.0, 0.0),
Quaternion.fromAxisAngle(Cartesian3.UNIT_X, 0.0),
new Cartesian3(1.0e-7, 1.0e-7, 1.1e-7));

var matrix = Matrix4.fromTranslationRotationScale(trs);

var expected = new Matrix4(1e7, 0, 0, 0,
0, 1e7, 0, 0,
0, 0, (1.0/1.1)*1e7, 0,
0, 0, 0, 1);

var result = Matrix4.inverse(matrix, new Matrix4());
expect(expected).toEqualEpsilon(result, CesiumMath.EPSILON15);
});

it('inverse behaves acceptably with single precision zero scale matrix', function() {
var trs = new TranslationRotationScale(new Cartesian3(0.0, 0.0, 0.0),
Quaternion.fromAxisAngle(Cartesian3.UNIT_X, 0.0),
new Cartesian3(1.8e-8, 1.2e-8, 1.2e-8));

var matrix = Matrix4.fromTranslationRotationScale(trs);

var expected = new Matrix4(0, 0, 0, -matrix[12],
0, 0, 0, -matrix[13],
0, 0, 0, -matrix[14],
0, 0, 0, 1);

var result = Matrix4.inverse(matrix, new Matrix4());
expect(expected).toEqualEpsilon(result, CesiumMath.EPSILON20);
});

it('inverseTransformation works', function() {
var matrix = new Matrix4(1, 0, 0, 10,
0, 0, 1, 20,
Expand Down