From bcaa0d38bd1a8533cb489da497b4077e206efc2e Mon Sep 17 00:00:00 2001 From: enricoturri1966 Date: Fri, 21 Jan 2022 11:26:44 +0100 Subject: [PATCH] Fixed uninitialized variables reported by MemorySanitizer in Geometry::Transformation --- src/libslic3r/Geometry.cpp | 129 +++++++++++++++---------------------- src/libslic3r/Geometry.hpp | 24 ++++--- 2 files changed, 64 insertions(+), 89 deletions(-) diff --git a/src/libslic3r/Geometry.cpp b/src/libslic3r/Geometry.cpp index ecb29d4b8..00fac6c38 100644 --- a/src/libslic3r/Geometry.cpp +++ b/src/libslic3r/Geometry.cpp @@ -325,40 +325,36 @@ Vec3d extract_euler_angles(const Eigen::Matrix& // reference: http://www.gregslabaugh.net/publications/euler.pdf Vec3d angles1 = Vec3d::Zero(); Vec3d angles2 = Vec3d::Zero(); - if (std::abs(std::abs(rotation_matrix(2, 0)) - 1.0) < 1e-5) - { - angles1(2) = 0.0; - if (rotation_matrix(2, 0) < 0.0) // == -1.0 - { - angles1(1) = 0.5 * (double)PI; - angles1(0) = angles1(2) + ::atan2(rotation_matrix(0, 1), rotation_matrix(0, 2)); + if (std::abs(std::abs(rotation_matrix(2, 0)) - 1.0) < 1e-5) { + angles1.z() = 0.0; + if (rotation_matrix(2, 0) < 0.0) { // == -1.0 + angles1.y() = 0.5 * double(PI); + angles1.x() = angles1.z() + ::atan2(rotation_matrix(0, 1), rotation_matrix(0, 2)); } - else // == 1.0 - { - angles1(1) = - 0.5 * (double)PI; - angles1(0) = - angles1(2) + ::atan2(- rotation_matrix(0, 1), - rotation_matrix(0, 2)); + else { // == 1.0 + angles1.y() = - 0.5 * double(PI); + angles1.x() = - angles1.y() + ::atan2(- rotation_matrix(0, 1), - rotation_matrix(0, 2)); } angles2 = angles1; } - else - { - angles1(1) = -::asin(rotation_matrix(2, 0)); - double inv_cos1 = 1.0 / ::cos(angles1(1)); - angles1(0) = ::atan2(rotation_matrix(2, 1) * inv_cos1, rotation_matrix(2, 2) * inv_cos1); - angles1(2) = ::atan2(rotation_matrix(1, 0) * inv_cos1, rotation_matrix(0, 0) * inv_cos1); + else { + angles1.y() = -::asin(rotation_matrix(2, 0)); + const double inv_cos1 = 1.0 / ::cos(angles1.y()); + angles1.x() = ::atan2(rotation_matrix(2, 1) * inv_cos1, rotation_matrix(2, 2) * inv_cos1); + angles1.z() = ::atan2(rotation_matrix(1, 0) * inv_cos1, rotation_matrix(0, 0) * inv_cos1); - angles2(1) = (double)PI - angles1(1); - double inv_cos2 = 1.0 / ::cos(angles2(1)); - angles2(0) = ::atan2(rotation_matrix(2, 1) * inv_cos2, rotation_matrix(2, 2) * inv_cos2); - angles2(2) = ::atan2(rotation_matrix(1, 0) * inv_cos2, rotation_matrix(0, 0) * inv_cos2); + angles2.y() = double(PI) - angles1.y(); + const double inv_cos2 = 1.0 / ::cos(angles2.y()); + angles2.x() = ::atan2(rotation_matrix(2, 1) * inv_cos2, rotation_matrix(2, 2) * inv_cos2); + angles2.z() = ::atan2(rotation_matrix(1, 0) * inv_cos2, rotation_matrix(0, 0) * inv_cos2); } // The following euristic is the best found up to now (in the sense that it works fine with the greatest number of edge use-cases) // but there are other use-cases were it does not // We need to improve it - double min_1 = angles1.cwiseAbs().minCoeff(); - double min_2 = angles2.cwiseAbs().minCoeff(); - bool use_1 = (min_1 < min_2) || (is_approx(min_1, min_2) && (angles1.norm() <= angles2.norm())); + const double min_1 = angles1.cwiseAbs().minCoeff(); + const double min_2 = angles2.cwiseAbs().minCoeff(); + const bool use_1 = (min_1 < min_2) || (is_approx(min_1, min_2) && (angles1.norm() <= angles2.norm())); return use_1 ? angles1 : angles2; } @@ -374,14 +370,6 @@ Vec3d extract_euler_angles(const Transform3d& transform) return extract_euler_angles(m); } -Transformation::Flags::Flags() - : dont_translate(true) - , dont_rotate(true) - , dont_scale(true) - , dont_mirror(true) -{ -} - bool Transformation::Flags::needs_update(bool dont_translate, bool dont_rotate, bool dont_scale, bool dont_mirror) const { return (this->dont_translate != dont_translate) || (this->dont_rotate != dont_rotate) || (this->dont_scale != dont_scale) || (this->dont_mirror != dont_mirror); @@ -407,15 +395,14 @@ Transformation::Transformation(const Transform3d& transform) void Transformation::set_offset(const Vec3d& offset) { - set_offset(X, offset(0)); - set_offset(Y, offset(1)); - set_offset(Z, offset(2)); + set_offset(X, offset.x()); + set_offset(Y, offset.y()); + set_offset(Z, offset.z()); } void Transformation::set_offset(Axis axis, double offset) { - if (m_offset(axis) != offset) - { + if (m_offset(axis) != offset) { m_offset(axis) = offset; m_dirty = true; } @@ -423,19 +410,18 @@ void Transformation::set_offset(Axis axis, double offset) void Transformation::set_rotation(const Vec3d& rotation) { - set_rotation(X, rotation(0)); - set_rotation(Y, rotation(1)); - set_rotation(Z, rotation(2)); + set_rotation(X, rotation.x()); + set_rotation(Y, rotation.y()); + set_rotation(Z, rotation.z()); } void Transformation::set_rotation(Axis axis, double rotation) { rotation = angle_to_0_2PI(rotation); - if (is_approx(std::abs(rotation), 2.0 * (double)PI)) + if (is_approx(std::abs(rotation), 2.0 * double(PI))) rotation = 0.0; - if (m_rotation(axis) != rotation) - { + if (m_rotation(axis) != rotation) { m_rotation(axis) = rotation; m_dirty = true; } @@ -443,15 +429,14 @@ void Transformation::set_rotation(Axis axis, double rotation) void Transformation::set_scaling_factor(const Vec3d& scaling_factor) { - set_scaling_factor(X, scaling_factor(0)); - set_scaling_factor(Y, scaling_factor(1)); - set_scaling_factor(Z, scaling_factor(2)); + set_scaling_factor(X, scaling_factor.x()); + set_scaling_factor(Y, scaling_factor.y()); + set_scaling_factor(Z, scaling_factor.z()); } void Transformation::set_scaling_factor(Axis axis, double scaling_factor) { - if (m_scaling_factor(axis) != std::abs(scaling_factor)) - { + if (m_scaling_factor(axis) != std::abs(scaling_factor)) { m_scaling_factor(axis) = std::abs(scaling_factor); m_dirty = true; } @@ -459,9 +444,9 @@ void Transformation::set_scaling_factor(Axis axis, double scaling_factor) void Transformation::set_mirror(const Vec3d& mirror) { - set_mirror(X, mirror(0)); - set_mirror(Y, mirror(1)); - set_mirror(Z, mirror(2)); + set_mirror(X, mirror.x()); + set_mirror(Y, mirror.y()); + set_mirror(Z, mirror.z()); } void Transformation::set_mirror(Axis axis, double mirror) @@ -472,8 +457,7 @@ void Transformation::set_mirror(Axis axis, double mirror) else if (abs_mirror != 1.0) mirror /= abs_mirror; - if (m_mirror(axis) != mirror) - { + if (m_mirror(axis) != mirror) { m_mirror(axis) = mirror; m_dirty = true; } @@ -491,9 +475,8 @@ void Transformation::set_from_transform(const Transform3d& transform) // we can only detect if the matrix contains a left handed reference system // in which case we reorient it back to right handed by mirroring the x axis Vec3d mirror = Vec3d::Ones(); - if (m3x3.col(0).dot(m3x3.col(1).cross(m3x3.col(2))) < 0.0) - { - mirror(0) = -1.0; + if (m3x3.col(0).dot(m3x3.col(1).cross(m3x3.col(2))) < 0.0) { + mirror.x() = -1.0; // remove mirror m3x3.col(0) *= -1.0; } @@ -530,8 +513,7 @@ void Transformation::reset() const Transform3d& Transformation::get_matrix(bool dont_translate, bool dont_rotate, bool dont_scale, bool dont_mirror) const { - if (m_dirty || m_flags.needs_update(dont_translate, dont_rotate, dont_scale, dont_mirror)) - { + if (m_dirty || m_flags.needs_update(dont_translate, dont_rotate, dont_scale, dont_mirror)) { m_matrix = Geometry::assemble_transform( dont_translate ? Vec3d::Zero() : m_offset, dont_rotate ? Vec3d::Zero() : m_rotation, @@ -560,8 +542,7 @@ Transformation Transformation::volume_to_bed_transformation(const Transformation // Just set the inverse. out.set_from_transform(instance_transformation.get_matrix(true).inverse()); } - else if (is_rotation_ninety_degrees(instance_transformation.get_rotation())) - { + else if (is_rotation_ninety_degrees(instance_transformation.get_rotation())) { // Anisotropic scaling, rotation by multiples of ninety degrees. Eigen::Matrix3d instance_rotation_trafo = (Eigen::AngleAxisd(instance_transformation.get_rotation().z(), Vec3d::UnitZ()) * @@ -594,8 +575,8 @@ Transformation Transformation::volume_to_bed_transformation(const Transformation scale(i) = pts.col(i).dot(qs.col(i)) / pts.col(i).dot(pts.col(i)); out.set_rotation(Geometry::extract_euler_angles(volume_rotation_trafo)); - out.set_scaling_factor(Vec3d(std::abs(scale(0)), std::abs(scale(1)), std::abs(scale(2)))); - out.set_mirror(Vec3d(scale(0) > 0 ? 1. : -1, scale(1) > 0 ? 1. : -1, scale(2) > 0 ? 1. : -1)); + out.set_scaling_factor(Vec3d(std::abs(scale.x()), std::abs(scale.y()), std::abs(scale.z()))); + out.set_mirror(Vec3d(scale.x() > 0 ? 1. : -1, scale.y() > 0 ? 1. : -1, scale.z() > 0 ? 1. : -1)); } else { @@ -614,19 +595,15 @@ Transform3d transform3d_from_string(const std::string& transform_str) assert(is_decimal_separator_point()); // for atof Transform3d transform = Transform3d::Identity(); - if (!transform_str.empty()) - { + if (!transform_str.empty()) { std::vector mat_elements_str; boost::split(mat_elements_str, transform_str, boost::is_any_of(" "), boost::token_compress_on); - unsigned int size = (unsigned int)mat_elements_str.size(); - if (size == 16) - { + const unsigned int size = (unsigned int)mat_elements_str.size(); + if (size == 16) { unsigned int i = 0; - for (unsigned int r = 0; r < 4; ++r) - { - for (unsigned int c = 0; c < 4; ++c) - { + for (unsigned int r = 0; r < 4; ++r) { + for (unsigned int c = 0; c < 4; ++c) { transform(r, c) = ::atof(mat_elements_str[i++].c_str()); } } @@ -640,17 +617,17 @@ Eigen::Quaterniond rotation_xyz_diff(const Vec3d &rot_xyz_from, const Vec3d &rot { return // From the current coordinate system to world. - Eigen::AngleAxisd(rot_xyz_to(2), Vec3d::UnitZ()) * Eigen::AngleAxisd(rot_xyz_to(1), Vec3d::UnitY()) * Eigen::AngleAxisd(rot_xyz_to(0), Vec3d::UnitX()) * + Eigen::AngleAxisd(rot_xyz_to.z(), Vec3d::UnitZ()) * Eigen::AngleAxisd(rot_xyz_to.y(), Vec3d::UnitY()) * Eigen::AngleAxisd(rot_xyz_to.x(), Vec3d::UnitX()) * // From world to the initial coordinate system. - Eigen::AngleAxisd(-rot_xyz_from(0), Vec3d::UnitX()) * Eigen::AngleAxisd(-rot_xyz_from(1), Vec3d::UnitY()) * Eigen::AngleAxisd(-rot_xyz_from(2), Vec3d::UnitZ()); + Eigen::AngleAxisd(-rot_xyz_from.x(), Vec3d::UnitX()) * Eigen::AngleAxisd(-rot_xyz_from.y(), Vec3d::UnitY()) * Eigen::AngleAxisd(-rot_xyz_from.z(), Vec3d::UnitZ()); } // This should only be called if it is known, that the two rotations only differ in rotation around the Z axis. double rotation_diff_z(const Vec3d &rot_xyz_from, const Vec3d &rot_xyz_to) { - Eigen::AngleAxisd angle_axis(rotation_xyz_diff(rot_xyz_from, rot_xyz_to)); - Vec3d axis = angle_axis.axis(); - double angle = angle_axis.angle(); + const Eigen::AngleAxisd angle_axis(rotation_xyz_diff(rot_xyz_from, rot_xyz_to)); + const Vec3d axis = angle_axis.axis(); + const double angle = angle_axis.angle(); #ifndef NDEBUG if (std::abs(angle) > 1e-8) { assert(std::abs(axis.x()) < 1e-8); diff --git a/src/libslic3r/Geometry.hpp b/src/libslic3r/Geometry.hpp index e84a7aace..c825f8254 100644 --- a/src/libslic3r/Geometry.hpp +++ b/src/libslic3r/Geometry.hpp @@ -347,25 +347,23 @@ class Transformation { struct Flags { - bool dont_translate; - bool dont_rotate; - bool dont_scale; - bool dont_mirror; - - Flags(); + bool dont_translate{ true }; + bool dont_rotate{ true }; + bool dont_scale{ true }; + bool dont_mirror{ true }; bool needs_update(bool dont_translate, bool dont_rotate, bool dont_scale, bool dont_mirror) const; void set(bool dont_translate, bool dont_rotate, bool dont_scale, bool dont_mirror); }; - Vec3d m_offset; // In unscaled coordinates - Vec3d m_rotation; // Rotation around the three axes, in radians around mesh center point - Vec3d m_scaling_factor; // Scaling factors along the three axes - Vec3d m_mirror; // Mirroring along the three axes + Vec3d m_offset{ Vec3d::Zero() }; // In unscaled coordinates + Vec3d m_rotation{ Vec3d::Zero() }; // Rotation around the three axes, in radians around mesh center point + Vec3d m_scaling_factor{ Vec3d::Ones() }; // Scaling factors along the three axes + Vec3d m_mirror{ Vec3d::Ones() }; // Mirroring along the three axes - mutable Transform3d m_matrix; + mutable Transform3d m_matrix{ Transform3d::Identity() }; mutable Flags m_flags; - mutable bool m_dirty; + mutable bool m_dirty{ false }; public: Transformation(); @@ -435,7 +433,7 @@ extern double rotation_diff_z(const Vec3d &rot_xyz_from, const Vec3d &rot_xyz_to // Is the angle close to a multiple of 90 degrees? inline bool is_rotation_ninety_degrees(double a) { - a = fmod(std::abs(a), 0.5 * M_PI); + a = fmod(std::abs(a), 0.5 * PI); if (a > 0.25 * PI) a = 0.5 * PI - a; return a < 0.001;