Adding mesh double description #450

lmontaut · 2023-07-27T15:23:57Z

A polytope can be seen as either a set of vertices or a set of planes (normals + offsets).
The double description is normals + offsets.
In GJK/EPA we only use the vertices of polytopes. However, for computing normals/witness points, having the double description may be usefull in the future.

jcarpent · 2023-07-27T15:31:17Z

src/shape/geometric_shapes.cpp

+  if (other.neighbors) {
+    neighbors = new Neighbors[num_points];
+    std::copy(other.neighbors, other.neighbors + num_points, neighbors);
+  } else
+    neighbors = nullptr;
+
+  if (other.nneighbors_) {
+    std::size_t c_nneighbors = 0;
+    for (std::size_t i = 0; i < num_points; ++i)
+      c_nneighbors += neighbors[i].count();
+    nneighbors_ = new unsigned int[c_nneighbors];
+    std::copy(other.nneighbors_, other.nneighbors_ + c_nneighbors, nneighbors_);
+  } else
+    nneighbors_ = nullptr;


What is the strategy for own_storage_?

For the moment the ConvexBase/Convex<PolygonT> always owns its normals and offsets.
However I can do the same strategy as for points.
For it to make sense though, this requires adding a constructor to Convex<PolygonT> or at least a function buildDoubleRepresentation.

@jcarpent I can put a function buildDoubleRepresentation in ConvexBase and remove the normals construction from ConvexBase::convexHull.
Then, either the user calls buildDoubleRepresentation or we force the call to buildDoubleRepresentation at the end of convexHull (in the future, we might systematically need the double representation information for witness points after calling GJK/EPA).

At first glance, the double description should not be computed when creating the objects to avoid any computational burden.

Yes I agree but since we call Qhull inside ConvexBase::convexHull, we get the double representation for free. Computing the double representation later would result in computationnal burden (need to call Qhull again).
I will restructure the code so that ConvexBase::convexHull still constructs the double representation but buildDoubleRepresentation can still be called later i.e for Convex objects which have not been constructed by the ConvexBase::convexHull function.
Does this sound ok for you?

Good to me.

Maybe, adding a method hasDoubleDescription() could help by checking the content of the pointers (if they are not NULL for instance)

jcarpent · 2023-07-27T15:31:50Z

src/shape/geometric_shapes.cpp

+  if (other.normals) {
+    normals = new Vec3f[num_normals_and_offsets];
+    std::copy(other.normals, other.normals + num_normals_and_offsets, normals);
+  } else
+    normals = nullptr;
+
+  if (other.offsets) {
+    offsets = new FCL_REAL[num_normals_and_offsets];
+    std::copy(other.offsets, other.offsets + num_normals_and_offsets, offsets);
+  } else
+    offsets = nullptr;


The same question applies here.

lmontaut · 2023-07-27T17:00:01Z

@jcarpent what do you think about changing points, normals and offsets in ConvexBase from a regular pointer to a shared_ptr? This would remove the concept of own_storage and simplify having to keep track whether a Convex owns its points representation and/or its normals + offsets representation.

jcarpent · 2023-07-28T09:22:53Z

@jcarpent what do you think about changing points, normals and offsets in ConvexBase from a regular pointer to a shared_ptr? This would remove the concept of own_storage and simplify having to keep track whether a Convex owns its points representation and/or its normals + offsets representation.

Looks good to me. I do think we need to transition from raw pointers to smart ones in all the code.
A huge task, but will simplify our life in the very near future.

`ConvexBase`

include/hpp/fcl/shape/geometric_shapes.h

src/shape/convex.cpp

jcarpent · 2023-08-10T14:04:09Z

python/collision-geometries.cc

+  typedef Eigen::Map<RowMatrixX3> MapVecOfDoubles;
+  typedef Eigen::Ref<RowMatrixX3> RefVecOfDoubles;


This is not totally correct. This is just a copy-pasting of previous lines.

jcarpent · 2023-08-10T14:05:03Z

python/collision-geometries.cc

+  typedef Eigen::Matrix<double, Eigen::Dynamic, 1, Eigen::RowMajor>
+      VecOfDoubles;


I would call it VectorX.

Row::Major is not needed. You can also use Eigen::VectorXd

jcarpent · 2023-08-10T14:05:39Z

python/collision-geometries.cc

+  typedef Eigen::Matrix<double, Eigen::Dynamic, 1, Eigen::RowMajor>
+      VecOfDoubles;


Row::Major is not needed. You can also use Eigen::VectorXd

The default in Eigen is column-major (https://eigen.tuxfamily.org/dox/group__TopicStorageOrders.html) but `Matrixx3f` and `Matrixx3i` are used to store coordinates of points and neighbors. Row major is more appropriate.

and `primitive_indices`.

for `normals` and `offsets`.

for more information, see https://pre-commit.ci

[pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci

lmontaut requested a review from jcarpent July 27, 2023 15:24

jcarpent reviewed Jul 27, 2023

View reviewed changes

lmontaut marked this pull request as draft July 28, 2023 17:02

lmontaut mentioned this pull request Aug 2, 2023

Convex and BVHModel: moving from raw to std::shared_ptr #451

Merged

lmontaut force-pushed the topic/hppfcl3x/mesh-double-description branch from 6ceb435 to 4a8a162 Compare August 9, 2023 17:32

lmontaut marked this pull request as ready for review August 9, 2023 17:33

lmontaut added 7 commits August 10, 2023 12:21

[misc] fix typo in comment

510921c

[core/Convex] Adding mesh double description.

0fbcea3

[core/Convex] Updating copy constructor and initialize

78a780f

[core/Convex] Updating isEqual method

4d1bb61

[serialization/Convex] Serializing normals and offsets of

8a7b26f

`ConvexBase`

[python/Convex] Python bindings of normals and offsets of

03a2397

`ConvexBase`

[core/Convex] Fix missing ifdef flag

08d2dd5

jcarpent force-pushed the topic/hppfcl3x/mesh-double-description branch from 651813c to 08d2dd5 Compare August 10, 2023 10:21

jcarpent reviewed Aug 10, 2023

View reviewed changes

include/hpp/fcl/shape/geometric_shapes.h Outdated Show resolved Hide resolved

jcarpent reviewed Aug 10, 2023

View reviewed changes

src/shape/convex.cpp Outdated Show resolved Hide resolved

[core/Convex] qhull: moving includes back to .cpp file

daa6d90

jcarpent reviewed Aug 10, 2023

View reviewed changes

jcarpent requested changes Aug 10, 2023

View reviewed changes

lmontaut added 7 commits August 10, 2023 16:31

[python/Convex] Fix eigen map and ref for offsets

caeda28

[core/data_types] Changing to Eigen::RowMajor.

77cfb6d

The default in Eigen is column-major (https://eigen.tuxfamily.org/dox/group__TopicStorageOrders.html) but `Matrixx3f` and `Matrixx3i` are used to store coordinates of points and neighbors. Row major is more appropriate.

[Core/Convex] Fixing memory leaks for points. Using std::vector.

172560f

[Core/BVHModel] Fixing memory leak for vertices. Using std::vector.

40f5886

[Core/Convex] Fixing memory leak for polygons. Using std::vector.

44a374e

[Core/BVHModel] Fixing memory leak for tri_indices Using std::vector.

cbb3b6d

[Core/Convex] Fixing memory leak for neighbors. Using std::vector.

6b84faa

lmontaut and others added 5 commits September 5, 2023 19:10

[Core/BVHModel] Fixing memory leak for bvs. Using std::vector for bvs

05040f1

and `primitive_indices`.

[Core/Convex] Fix memory leak for double description. Using std::vector

d5509ee

for `normals` and `offsets`.

[pre-commit.ci] auto fixes from pre-commit.com hooks

2da373a

for more information, see https://pre-commit.ci

[Misc/Convex] Adding missing include for windows

75e05ec

[BVHModel] adding safeguard in isEqual in case bvs does not exist.

5479466

lmontaut force-pushed the topic/hppfcl3x/mesh-double-description branch from f0161a4 to a89bd24 Compare September 7, 2023 13:43

[BVHModel] Update serialization

3ea3001

[pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci

lmontaut force-pushed the topic/hppfcl3x/mesh-double-description branch from 0926120 to 3ea3001 Compare September 7, 2023 14:05

jcarpent merged commit 3766c24 into coal-library:hppfcl3x Sep 22, 2023
18 of 20 checks passed

lmontaut deleted the topic/hppfcl3x/mesh-double-description branch November 13, 2023 16:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding mesh double description #450

Adding mesh double description #450

lmontaut commented Jul 27, 2023

jcarpent Jul 27, 2023

lmontaut Jul 27, 2023

lmontaut Jul 27, 2023

jcarpent Jul 28, 2023

lmontaut Jul 28, 2023 •

edited

Loading

jcarpent Jul 28, 2023

jcarpent Jul 28, 2023

jcarpent Jul 27, 2023

lmontaut commented Jul 27, 2023

jcarpent commented Jul 28, 2023

jcarpent Aug 10, 2023

jcarpent Aug 10, 2023

jcarpent Aug 10, 2023

jcarpent Aug 10, 2023

		typedef Eigen::Map<RowMatrixX3> MapVecOfDoubles;
		typedef Eigen::Ref<RowMatrixX3> RefVecOfDoubles;

		typedef Eigen::Matrix<double, Eigen::Dynamic, 1, Eigen::RowMajor>
		VecOfDoubles;

Adding mesh double description #450

Adding mesh double description #450

Conversation

lmontaut commented Jul 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmontaut Jul 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmontaut commented Jul 27, 2023

jcarpent commented Jul 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmontaut Jul 28, 2023 •

edited

Loading