Skip to content

Commit

Permalink
Deprecate functions in Gf that take and return GfVec3f values and can
Browse files Browse the repository at this point in the history
potentially result in loss of precision. We plan to remove these methods
altogether in the 24.11 release.

The type GfMatrix4d is commonly used in the Pixar pipeline to represent
local-to-world matrices. When local space points are represented using the type
GfVec3f, calls to functions such as GfMatrix4d::Transform lose precision, due to
overloads that take arguments of type GfVec3f and return values of the same
type. Production experience has shown us this precision loss can be critical,
e.g., due to large translations in local-to-world matrices that require points
in world space to be represented with higher precision than is necessary in
local space (which is presumably chosen so that the points are sufficiently
close to the local origin).

In order to avoid this loss of precision, it is currently necessary to
explicitly cast a GfVec3f argument to GfVec3d in order to select the overload
that computes and returns a double-precision result; it is easy to call the
lossy overload by accident. And when lost precision causes observable problems
downstream, it can be very difficult and time-consuming to track down the source
of the problem.

Therefore, we are deprecating functions that are known potential sources of
precision loss (marking them as deprecated in the documentation) in the 24.08
release, and we plan to remove these methods in 24.11. Note that the above
geometric interpretation doesn't apply to all of the operations we are planning
to deprecate, but for consistency, we plan to remove all methods that take and
return GfVec3f (or GfVec4f or GfVec2f) on Gf classes that internally store
doubles.

(Internal change: 2332487)
  • Loading branch information
adamrwoodbury authored and pixar-oss committed Jun 28, 2024
1 parent bbabd3c commit 947c971
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 0 deletions.
10 changes: 10 additions & 0 deletions pxr/base/gf/matrix.template.h
Original file line number Diff line number Diff line change
Expand Up @@ -331,11 +331,21 @@ class {{ MAT }}
{% if SCL == 'double' %}
/// Returns the product of a matrix \e m and a column vector \e vec.
/// Note that the return type is a \c GfVec{{ DIM }}f.
///
/// \deprecated
/// This function is deprecated, as it can result in unintentional loss of
/// precision. Call GfVec{{ SUFFIX }} operator *(const {{ MAT }}&, const GfVec{{ SUFFIX }} &)
/// instead and explicitly convert the result to GfVec3f, if necessary.
GF_API
friend GfVec{{ DIM }}f operator *(const {{ MAT }}& m, const GfVec{{ DIM }}f& vec);

/// Returns the product of row vector \e vec and a matrix \e m.
/// Note that the return type is a \c GfVec{{ DIM }}f.
///
/// \deprecated
/// This function is deprecated, as it can result in unintentional loss of
/// precision. Call GfVec{{ SUFFIX }} operator *(const GfVec{{ SUFFIX }} &, const {{ MAT }}&)
/// instead and explicitly convert the result to GfVec3f, if necessary.
GF_API
friend GfVec{{ DIM }}f operator *(const GfVec{{ DIM }}f &vec, const {{ MAT }}& m);

Expand Down
10 changes: 10 additions & 0 deletions pxr/base/gf/matrix2d.h
Original file line number Diff line number Diff line change
Expand Up @@ -331,11 +331,21 @@ class GfMatrix2d

/// Returns the product of a matrix \e m and a column vector \e vec.
/// Note that the return type is a \c GfVec2f.
///
/// \deprecated
/// This function is deprecated, as it can result in unintentional loss of
/// precision. Call GfVec2d operator *(const GfMatrix2d&, const GfVec2d &)
/// instead and explicitly convert the result to GfVec3f, if necessary.
GF_API
friend GfVec2f operator *(const GfMatrix2d& m, const GfVec2f& vec);

/// Returns the product of row vector \e vec and a matrix \e m.
/// Note that the return type is a \c GfVec2f.
///
/// \deprecated
/// This function is deprecated, as it can result in unintentional loss of
/// precision. Call GfVec2d operator *(const GfVec2d &, const GfMatrix2d&)
/// instead and explicitly convert the result to GfVec3f, if necessary.
GF_API
friend GfVec2f operator *(const GfVec2f &vec, const GfMatrix2d& m);

Expand Down
10 changes: 10 additions & 0 deletions pxr/base/gf/matrix3d.h
Original file line number Diff line number Diff line change
Expand Up @@ -409,11 +409,21 @@ class GfMatrix3d

/// Returns the product of a matrix \e m and a column vector \e vec.
/// Note that the return type is a \c GfVec3f.
///
/// \deprecated
/// This function is deprecated, as it can result in unintentional loss of
/// precision. Call GfVec3d operator *(const GfMatrix3d&, const GfVec3d &)
/// instead and explicitly convert the result to GfVec3f, if necessary.
GF_API
friend GfVec3f operator *(const GfMatrix3d& m, const GfVec3f& vec);

/// Returns the product of row vector \e vec and a matrix \e m.
/// Note that the return type is a \c GfVec3f.
///
/// \deprecated
/// This function is deprecated, as it can result in unintentional loss of
/// precision. Call GfVec3d operator *(const GfVec3d &, const GfMatrix3d&)
/// instead and explicitly convert the result to GfVec3f, if necessary.
GF_API
friend GfVec3f operator *(const GfVec3f &vec, const GfMatrix3d& m);

Expand Down
21 changes: 21 additions & 0 deletions pxr/base/gf/matrix4.template.h
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,13 @@ class GfMatrix3{{ SCL[0] }};
/// This treats the vector as a 4-component vector whose fourth component
/// is 1. This is an overloaded method; it differs from the other version
/// in that it returns a different value type.
{% if SCL == 'double' %}
///
/// \deprecated
/// This method is deprecated, as it can result in unintentional loss of
/// precision. Call GfVec3d Transform(const GfVec3d &) instead and
/// explicitly convert the result to GfVec3f, if necessary.
{% endif %}
GfVec3f Transform(const GfVec3f &vec) const {
{% if SCL == 'float' %}
return (GfProject(GfVec4f(
Expand Down Expand Up @@ -337,6 +344,13 @@ class GfMatrix3{{ SCL[0] }};
/// a 4-component vector whose fourth component is 0. This is an
/// overloaded method; it differs from the other version in that it
/// returns a different value type.
{% if SCL == 'double' %}
///
/// \deprecated
/// This method is deprecated, as it can result in unintentional loss of
/// precision. Call GfVec3d TransformDir(const GfVec3d &) instead and
/// explicitly convert the result to GfVec3f, if necessary.
{% endif %}
GfVec3f TransformDir(const GfVec3f &vec) const {
return GfVec3f(
{{ LIST("vec[%(i)s] * _mtx[%(i)s][0]", sep=" + ", num=3) }},
Expand All @@ -359,6 +373,13 @@ class GfMatrix3{{ SCL[0] }};
/// This treats the vector as a 4-component vector whose fourth component
/// is 1 and ignores the fourth column of the matrix (i.e. assumes it is
/// (0, 0, 0, 1)).
{% if SCL == 'double' %}
///
/// \deprecated
/// This method is deprecated, as it can result in unintentional loss of
/// precision. Call GfVec3d TransformAffine(const GfVec3d &) instead and
/// explicitly convert the result to GfVec3f, if necessary.
{% endif %}
GfVec3f TransformAffine(const GfVec3f &vec) const {
return GfVec3f(
{{ LIST("vec[%(i)s] * _mtx[%(i)s][0]", sep=" + ", num=3) }} + _mtx[3][0],
Expand Down
25 changes: 25 additions & 0 deletions pxr/base/gf/matrix4d.h
Original file line number Diff line number Diff line change
Expand Up @@ -490,11 +490,21 @@ class GfMatrix4d

/// Returns the product of a matrix \e m and a column vector \e vec.
/// Note that the return type is a \c GfVec4f.
///
/// \deprecated
/// This function is deprecated, as it can result in unintentional loss of
/// precision. Call GfVec4d operator *(const GfMatrix4d&, const GfVec4d &)
/// instead and explicitly convert the result to GfVec3f, if necessary.
GF_API
friend GfVec4f operator *(const GfMatrix4d& m, const GfVec4f& vec);

/// Returns the product of row vector \e vec and a matrix \e m.
/// Note that the return type is a \c GfVec4f.
///
/// \deprecated
/// This function is deprecated, as it can result in unintentional loss of
/// precision. Call GfVec4d operator *(const GfVec4d &, const GfMatrix4d&)
/// instead and explicitly convert the result to GfVec3f, if necessary.
GF_API
friend GfVec4f operator *(const GfVec4f &vec, const GfMatrix4d& m);

Expand Down Expand Up @@ -661,6 +671,11 @@ class GfMatrix4d
/// This treats the vector as a 4-component vector whose fourth component
/// is 1. This is an overloaded method; it differs from the other version
/// in that it returns a different value type.
///
/// \deprecated
/// This method is deprecated, as it can result in unintentional loss of
/// precision. Call GfVec3d Transform(const GfVec3d &) instead and
/// explicitly convert the result to GfVec3f, if necessary.
GfVec3f Transform(const GfVec3f &vec) const {
return GfVec3f(GfProject(GfVec4d(
vec[0] * _mtx[0][0] + vec[1] * _mtx[1][0] + vec[2] * _mtx[2][0] + _mtx[3][0],
Expand All @@ -686,6 +701,11 @@ class GfMatrix4d
/// a 4-component vector whose fourth component is 0. This is an
/// overloaded method; it differs from the other version in that it
/// returns a different value type.
///
/// \deprecated
/// This method is deprecated, as it can result in unintentional loss of
/// precision. Call GfVec3d TransformDir(const GfVec3d &) instead and
/// explicitly convert the result to GfVec3f, if necessary.
GfVec3f TransformDir(const GfVec3f &vec) const {
return GfVec3f(
vec[0] * _mtx[0][0] + vec[1] * _mtx[1][0] + vec[2] * _mtx[2][0],
Expand All @@ -708,6 +728,11 @@ class GfMatrix4d
/// This treats the vector as a 4-component vector whose fourth component
/// is 1 and ignores the fourth column of the matrix (i.e. assumes it is
/// (0, 0, 0, 1)).
///
/// \deprecated
/// This method is deprecated, as it can result in unintentional loss of
/// precision. Call GfVec3d TransformAffine(const GfVec3d &) instead and
/// explicitly convert the result to GfVec3f, if necessary.
GfVec3f TransformAffine(const GfVec3f &vec) const {
return GfVec3f(
vec[0] * _mtx[0][0] + vec[1] * _mtx[1][0] + vec[2] * _mtx[2][0] + _mtx[3][0],
Expand Down
5 changes: 5 additions & 0 deletions pxr/base/gf/rotation.h
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,11 @@ class GfRotation {
double *thetaTw, double *thetaFB, double *thetaLR, double *thetaSw);

/// Transforms row vector \p vec by the rotation, returning the result.
///
/// \deprecated
/// This method is deprecated, as it can result in unintentional loss of
/// precision. Call GfVec3d TransformDir(const GfVec3d &) instead and
/// explicitly convert the result to GfVec3f, if necessary.
GF_API
GfVec3f TransformDir( const GfVec3f &vec ) const;

Expand Down

0 comments on commit 947c971

Please sign in to comment.