Copy link Copy Markdown

Contributor

The previous glomap logic only used SetElements, which does not handle the cleanup of observations. Switching to the use of ObservationManager is smooth and naturally solves the issue. The track_filter.h/cc in glomap is removed after the switching. The logic except for the observation handling issue stays unchanged.

Copy link Copy Markdown

Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Ideally, we cover the new filtering functionality with minimal tests in the observation manager.

Copy link Copy Markdown

Contributor Author

Realizing that this is not the only place where glomap does not respect observation handling. For example, SetElements without setting Point3D for 2d points is also the case in TrackEstablishment. This is probably due to the previous usage of scattered variables, and let s fix it in the follow-ups.

		view_graph,
		reconstruction,
		scaling * options_.inlier_thresholds.max_reprojection_error);
		filtered_num += obs_manager.FilterPoints3DWithLargeReprojectionError(

Copy link Copy Markdown

Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a behavior change here. In glomap the function returns number of filtered points, while in colmap we return number of filtered observations. This only makes the stopping criteria tighter and should not an issue in practice.

Copy link Copy Markdown

Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, you can probably avoid sqrt'ing and the acos by only computing it when not filtering. Otherwise LGTM.

…f glomap.

Closed