search

rock-core / base-types

C/C++ and Ruby definition of the base types in Rock
6 stars 39 forks source link

Fix of Eigen alignment exceptions with Depth Map #103

Closed mmunoz-gmv closed 7 years ago

mmunoz-gmv commented 7 years ago

Fix for several instances of an Eigen alignment exception:

test-range: /usr/include/eigen3/Eigen/src/Core/DenseStorage.h:86: Eigen::internal::plain_array<T, Size, MatrixOrArrayOptions, 16>::plain_array() [with T = double; int Size = 4; int MatrixOrArrayOptions = 0]: Assertion `(reinterpret_cast(eigen_unaligned_array_assert_workaround_gcc47(array)) & 0xf) == 0 && "this assertion is explained here: " "http://eigen.tuxfamily.org/dox-devel/group__TopicUnalignedArrayAssert.html" " READ THIS WEB PAGE !!! "' failed.

In DepthMap.hpp: using Eigen::aligned_allocator in the declarations of std::vector containing Eigen vectors or matrices.

In DepthMapVisualization.hpp: adding the Eigen macro for the declaration of classes with Eigen vector/matrix members.

saarnold commented 7 years ago

Hi @mmunoz-gmv thanks for the fix. The vectors in line 312 and 342 should be changed as well.

mmunoz-gmv commented 7 years ago

I've changed those vectors, and some others in DepthMap, LaserScan and their associated viz clases and test.

saarnold commented 7 years ago

As far as I know it is not necessary for Eigen types that are not a multiple of 16 bytes, since they do not fit into common SIMD register sizes. Therefor it shouldn't be necessary for a vector of Eigen::Vector3d (192 bit) or Eigen::Vector3f (96 bit). This is also true for the templated vectors in DepthMap, they need to be three dimensional.

mmunoz-gmv commented 7 years ago

Another try...

saarnold commented 7 years ago

Looks fine to me :+1: But I'm sure before this is merged someone will tell you to squash your commits. It is good practice to have as much commits as necessary to describe what you changed.

jhidalgocarrio commented 7 years ago

Just squash the commits :+1: for me too.

mmunoz-gmv commented 7 years ago

OK, reduced to 2 commits.

jhidalgocarrio commented 7 years ago

@doudou or @jmachowinski when possible. Could you merge it?