Skip to content

Commit

Permalink
Workaround for misalignment in heterogeneous builds
Browse files Browse the repository at this point in the history
	- Added a workaround for misaligned SubmapSlice
	  structure between cartographer and cartographer_ros
	  built using different compilers or compile contexts
	- Rearranged SubmapSlice field order after static
	  analysis performed with clangd
	- Added commented non-portable workaround with
	  \#pragma pack
	- Closes cartographer-project#1909
  • Loading branch information
twdragon committed Nov 11, 2022
1 parent ef00de2 commit fbf2a4b
Showing 1 changed file with 45 additions and 10 deletions.
55 changes: 45 additions & 10 deletions cartographer/io/submap_painter.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
#include "cartographer/mapping/value_conversion_tables.h"
#include "cartographer/transform/rigid_transform.h"

// Workaround to prevent SubmapSlice structure alignment issue with cartographer_ros
// Github issue #1909
// #pragma pack(16)

namespace cartographer {
namespace io {

Expand All @@ -39,23 +43,54 @@ struct PaintSubmapSlicesResult {
Eigen::Array2f origin;
};

struct SubmapSlice {
/* Structure rearranged according to the static analyzer recommendation
* SA Message:
* Excessive padding in 'struct cartographer::io::SubmapSlice' (32 padding bytes,
* where 0 is optimal). Optimal fields order:
* - slice_pose,
* - pose,
* - resolution,
* - surface,
* - cairo_data,
* - width,
* - height,
* - version,
* - metadata_version,
* consider reordering the fields or adding explicit padding members
* [clang-analyzer-optin.performance.Padding]
*
* Old version:
* // Texture data.
* int width;
* int height;
* int version;
* double resolution;
* ::cartographer::transform::Rigid3d slice_pose;
* ::cartographer::io::UniqueCairoSurfacePtr surface;
* // Pixel data used by 'surface'. Must outlive 'surface'.
* std::vector<uint32_t> cairo_data;
* // Metadata.
* ::cartographer::transform::Rigid3d pose;
* int metadata_version = -1;
*/

// Another C++11-compliant workaround to prevent SubmapSlice structure
// alignment issue with cartographer_ros
// Github issue #1909
struct alignas(32) SubmapSlice {
// Pixel data used by 'surface'. Must outlive 'surface'.
std::vector<uint32_t> cairo_data;
::cartographer::io::UniqueCairoSurfacePtr surface;
SubmapSlice()
: surface(::cartographer::io::MakeUniqueCairoSurfacePtr(nullptr)) {}

// Texture data.
double resolution;
// Metadata.
int width;
int height;
int version;
double resolution;
int metadata_version = -1;
::cartographer::transform::Rigid3d slice_pose;
::cartographer::io::UniqueCairoSurfacePtr surface;
// Pixel data used by 'surface'. Must outlive 'surface'.
std::vector<uint32_t> cairo_data;

// Metadata.
::cartographer::transform::Rigid3d pose;
int metadata_version = -1;
};

struct SubmapTexture {
Expand Down

0 comments on commit fbf2a4b

Please sign in to comment.