Skip to content

Commit

Permalink
[Core/Convex] Fix memory leak for double description. Using std::vector
Browse files Browse the repository at this point in the history
for `normals` and `offsets`.
  • Loading branch information
lmontaut committed Sep 5, 2023
1 parent 05040f1 commit d5509ee
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 35 deletions.
32 changes: 23 additions & 9 deletions include/hpp/fcl/serialization/convex.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,38 +31,52 @@ void serialize(Archive &ar, hpp::fcl::ConvexBase &convex_base,
ar &make_nvp("base", boost::serialization::base_object<hpp::fcl::ShapeBase>(
convex_base));
const unsigned int num_points_previous = convex_base.num_points;
const unsigned int num_normals_and_offsets_previous =
convex_base.num_normals_and_offsets;
ar &make_nvp("num_points", convex_base.num_points);
ar &make_nvp("num_normals_and_offsets", convex_base.num_normals_and_offsets);

if (Archive::is_loading::value) {
if (num_points_previous != convex_base.num_points) {
convex_base.points.reset(new std::vector<Vec3f>(convex_base.num_points));
convex_base.normals.reset(new Vec3f[convex_base.num_normals_and_offsets]);
convex_base.offsets.reset(
new double[convex_base.num_normals_and_offsets]);
convex_base.points.reset();
if (convex_base.num_points > 0)
convex_base.points.reset(
new std::vector<Vec3f>(convex_base.num_points));
}

if (num_normals_and_offsets_previous !=
convex_base.num_normals_and_offsets) {
convex_base.normals.reset();
convex_base.offsets.reset();
if (convex_base.num_normals_and_offsets > 0) {
convex_base.normals.reset(
new std::vector<Vec3f>(convex_base.num_normals_and_offsets));
convex_base.offsets.reset(
new std::vector<double>(convex_base.num_normals_and_offsets));
}
}
}

{
if (convex_base.num_points > 0) {
typedef Eigen::Matrix<FCL_REAL, 3, Eigen::Dynamic> MatrixPoints;
Eigen::Map<MatrixPoints> points_map(
reinterpret_cast<double *>(convex_base.points->data()), 3,
convex_base.num_points);
ar &make_nvp("points", points_map);
}

{
if (convex_base.num_normals_and_offsets > 0) {
typedef Eigen::Matrix<FCL_REAL, 3, Eigen::Dynamic> MatrixPoints;
Eigen::Map<MatrixPoints> normals_map(
reinterpret_cast<double *>(convex_base.normals.get()), 3,
reinterpret_cast<double *>(convex_base.normals->data()), 3,
convex_base.num_normals_and_offsets);
ar &make_nvp("normals", normals_map);
}

{
if (convex_base.num_normals_and_offsets > 0) {
typedef Eigen::Matrix<FCL_REAL, 1, Eigen::Dynamic> VecOfDoubles;
Eigen::Map<VecOfDoubles> offsets_map(
reinterpret_cast<double *>(convex_base.offsets.get()), 1,
reinterpret_cast<double *>(convex_base.offsets->data()), 1,
convex_base.num_normals_and_offsets);
ar &make_nvp("offsets", offsets_map);
}
Expand Down
30 changes: 20 additions & 10 deletions include/hpp/fcl/shape/geometric_shapes.h
Original file line number Diff line number Diff line change
Expand Up @@ -626,10 +626,10 @@ class HPP_FCL_DLLAPI ConvexBase : public ShapeBase {
unsigned int num_points;

/// @brief An array of the normals of the polygon.
std::shared_ptr<Vec3f> normals;
std::shared_ptr<std::vector<Vec3f>> normals;
/// @brief An array of the offsets to the normals of the polygon.
/// Note: there are as many offsets as normals.
std::shared_ptr<double> offsets;
std::shared_ptr<std::vector<double>> offsets;
unsigned int num_normals_and_offsets;

#ifdef HPP_FCL_HAS_QHULL
Expand Down Expand Up @@ -738,16 +738,26 @@ class HPP_FCL_DLLAPI ConvexBase : public ShapeBase {
}
}

const Vec3f* normals_ = normals.get();
const Vec3f* other_normals_ = other.normals.get();
for (unsigned int i = 0; i < num_normals_and_offsets; ++i) {
if (normals_[i] != other_normals_[i]) return false;
if ((!(normals.get()) && other.normals.get()) ||
(normals.get() && !(other.normals.get())))
return false;
if (normals.get() && other.normals.get()) {
const std::vector<Vec3f>& normals_ = *normals;
const std::vector<Vec3f>& other_normals_ = *(other.normals);
for (unsigned int i = 0; i < num_normals_and_offsets; ++i) {
if (normals_[i] != other_normals_[i]) return false;
}
}

const double* offsets_ = offsets.get();
const double* other_offsets_ = other.offsets.get();
for (unsigned int i = 0; i < num_normals_and_offsets; ++i) {
if (offsets_[i] != other_offsets_[i]) return false;
if ((!(offsets.get()) && other.offsets.get()) ||
(offsets.get() && !(other.offsets.get())))
return false;
if (offsets.get() && other.offsets.get()) {
const std::vector<double>& offsets_ = *offsets;
const std::vector<double>& other_offsets_ = *(other.offsets);
for (unsigned int i = 0; i < num_normals_and_offsets; ++i) {
if (offsets_[i] != other_offsets_[i]) return false;
}
}

return center == other.center;
Expand Down
8 changes: 4 additions & 4 deletions python/collision-geometries.cc
Original file line number Diff line number Diff line change
Expand Up @@ -174,22 +174,22 @@ struct ConvexBaseWrapper {
static Vec3f& normal(const ConvexBase& convex, unsigned int i) {
if (i >= convex.num_normals_and_offsets)
throw std::out_of_range("index is out of range");
return (convex.normals.get())[i];
return (*(convex.normals))[i];
}

static RefRowMatrixX3 normals(const ConvexBase& convex) {
return MapRowMatrixX3((convex.normals.get())[0].data(),
return MapRowMatrixX3((*(convex.normals))[0].data(),
convex.num_normals_and_offsets, 3);
}

static double offset(const ConvexBase& convex, unsigned int i) {
if (i >= convex.num_normals_and_offsets)
throw std::out_of_range("index is out of range");
return (convex.offsets.get())[i];
return (*(convex.offsets))[i];
}

static RefVecOfDoubles offsets(const ConvexBase& convex) {
return MapVecOfDoubles(convex.offsets.get(), convex.num_normals_and_offsets,
return MapVecOfDoubles(convex.offsets->data(), convex.num_normals_and_offsets,
1);
}

Expand Down
8 changes: 4 additions & 4 deletions src/shape/convex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,10 +220,10 @@ void ConvexBase::buildDoubleDescription() {

void ConvexBase::buildDoubleDescriptionFromQHullResult(const Qhull& qh) {
num_normals_and_offsets = static_cast<unsigned int>(qh.facetCount());
normals.reset(new Vec3f[num_normals_and_offsets]);
Vec3f* normals_ = normals.get();
offsets.reset(new double[num_normals_and_offsets]);
double* offsets_ = offsets.get();
normals.reset(new std::vector<Vec3f>(num_normals_and_offsets));
std::vector<Vec3f>& normals_ = *normals;
offsets.reset(new std::vector<double>(num_normals_and_offsets));
std::vector<double>& offsets_ = *offsets;
unsigned int i_normal = 0;
for (QhullFacet facet = qh.beginFacet(); facet != qh.endFacet();
facet = facet.next()) {
Expand Down
12 changes: 4 additions & 8 deletions src/shape/geometric_shapes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,17 +77,13 @@ ConvexBase::ConvexBase(const ConvexBase& other)
} else
nneighbors_.reset();

if (other.normals.get()) {
normals.reset(new Vec3f[num_normals_and_offsets]);
std::copy(other.normals.get(),
other.normals.get() + num_normals_and_offsets, normals.get());
if (other.normals.get() && other.normals->size() > 0) {
normals.reset(new std::vector<Vec3f>(*(other.normals)));
} else
normals.reset();

if (other.offsets.get()) {
offsets.reset(new double[num_normals_and_offsets]);
std::copy(other.offsets.get(),
other.offsets.get() + num_normals_and_offsets, offsets.get());
if (other.offsets.get() && other.offsets->size() > 0) {
offsets.reset(new std::vector<double>(*(other.offsets)));
} else
offsets.reset();

Expand Down

0 comments on commit d5509ee

Please sign in to comment.