Merged

Conversation

While investigating the model_splitter issue for the current revision 1f95f3e described in #2096 I found a code regression introduced with commit 67029ad

This affects model_cropper and model_splitter tools.

As the previous implementation used the Image::ResetPoint3DForPoint2D method inside the loop, the num_points3D member was reset to 0 correctly.

	void Image::ResetPoint3DForPoint2D(const point2D_t point2D_idx) {
	struct Point2D& point2D = points2D_.at(point2D_idx);
	if (point2D.HasPoint3D()) {
	point2D.point3D_id = kInvalidPoint3DId;
	num_points3D_ -= 1;
	}
	}

When directly assigning the kInvalidPoint3DId value, new_image keeps the value of the source num_points3D representing an incorrect state of the newly created cropped_reconstruction and finally leads to a failing check at

THROW_CHECK_LE(image.NumPoints3D(), image.NumPoints2D());

Fixing this by adding the call to Image::ResetPoint3DForPoint2D inside the loop again as it was first implemented in 99c7904 .

Debugger screenshots:

Current Revision 1f95f3e :

Old implementation:

…truction

		new_image.SetRegistered(false);
		for (auto& point2D : new_image.Points2D()) {
		point2D.point3D_id = kInvalidPoint3DId;
		for (point2D_t pid = 0; pid < new_image.NumPoints2D(); ++pid) {

		for (auto& point2D : new_image.Points2D()) {
		point2D.point3D_id = kInvalidPoint3DId;
		for (point2D_t pid = 0; pid < new_image.NumPoints2D(); ++pid) {
		new_image.ResetPoint3DForPoint2D(pid);

Merged

Labels

None yet