Commit 5f77a5e9 authored by Jacob DeWitt's avatar Jacob DeWitt Committed by Commit Bot

Use DCHECK in XRRigidTransform::DecomposeMatrix

This is better than failing silently by setting position + orientation
to identity because an XRRigidTransform should never be created with a
matrix that can't be decomposed. The previous approach would result in
an invalid XRRigidTransform object since position + orientation values
wouldn't match the transformation matrix.

Bug: 969149
Change-Id: Id848a301d61e0fd9fdcab612effd0a3de27854fe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1717457
Commit-Queue: Jacob DeWitt <jacde@chromium.org>
Reviewed-by: default avatarKlaus Weidner <klausw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#681104}
parent 4f630888
...@@ -24,22 +24,18 @@ void XRRigidTransform::DecomposeMatrix() { ...@@ -24,22 +24,18 @@ void XRRigidTransform::DecomposeMatrix() {
// decompose matrix to position and orientation // decompose matrix to position and orientation
TransformationMatrix::DecomposedType decomposed; TransformationMatrix::DecomposedType decomposed;
bool succeeded = matrix_->Decompose(decomposed); bool succeeded = matrix_->Decompose(decomposed);
if (succeeded) { DCHECK(succeeded) << "Matrix decompose failed for " << matrix_->ToString();
position_ =
DOMPointReadOnly::Create(decomposed.translate_x, decomposed.translate_y, position_ =
decomposed.translate_z, 1.0); DOMPointReadOnly::Create(decomposed.translate_x, decomposed.translate_y,
decomposed.translate_z, 1.0);
// TODO(https://crbug.com/929841): Minuses are needed as a workaround for
// bug in TransformationMatrix so that callers can still pass non-inverted // TODO(https://crbug.com/929841): Minuses are needed as a workaround for
// quaternions. // bug in TransformationMatrix so that callers can still pass non-inverted
orientation_ = makeNormalizedQuaternion( // quaternions.
-decomposed.quaternion_x, -decomposed.quaternion_y, orientation_ = makeNormalizedQuaternion(
-decomposed.quaternion_z, decomposed.quaternion_w); -decomposed.quaternion_x, -decomposed.quaternion_y,
} else { -decomposed.quaternion_z, decomposed.quaternion_w);
// TODO(crbug.com/969149): Is this the correct way to handle a failure here?
position_ = DOMPointReadOnly::Create(0.0, 0.0, 0.0, 1.0);
orientation_ = DOMPointReadOnly::Create(0.0, 0.0, 0.0, 1.0);
}
} }
XRRigidTransform::XRRigidTransform(DOMPointInit* position, XRRigidTransform::XRRigidTransform(DOMPointInit* position,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment