fix: image move constructor passes wrong stride unit to base constructor

[miwechner]

Closes #118

This debug assertion in Image could be hit:

nvblox/nvblox/include/nvblox/sensors/image.h

Lines 186 to 191 in 48aa18a

	__host__ __device__ static int strideFromBytesToElements(
	const int stride_bytes) {
	NVBLOX_DCHECK(stride_bytes % (sizeof(ElementType)) == 0,
	"Stride must be a multiple of the element size in bytes");
	return stride_bytes / (sizeof(ElementType));
	}

, because for the constructor ImageBase::ImageBase(int rows, int cols, int stride_bytes, int num_elements_per_pixel, OtherElementType* data) the Image move constructor passed the stride in number of elements instead of number bytes.

The fix is straightforward: pass the strides in numbers of bytes instead of numbers of elements on that call.

[david-tingdahl-nvidia]

Thanks for finding and fixing this bug. Could easily lead to some pretty confusing/weird errors. It hasn’t affected us much since we rarely use std::move with DepthImages, and when we do the image is (luckily) immediately resized, which recomputes the strides.

Could you please:

• Update the commit message to briefly describe the fix (so readers don’t need to open the link), and
• Fix the CI formatting error

Following this bug, we are also looking into extending the test suite for Image

[miwechner]

Hi,

Thanks for the quick response. Good to hear that this was actually an important problem and I guess we were mostly lucky that moving images did not happen much. Without the debug assert, I would have never noticed.

Hope the updates on the commit message and the PR description are ok now, as well as the formatting.

[david-tingdahl-nvidia]

Thanks for the fix!