Fix DSPSIFT color pick

[originlake]

Hi, this PR tries to fix the issue for picking color of detected features.

The previous update #33 seems to make it easier to detect features at the edge. For example, assuming an image sized 1000x1000, vlfeat can produce feature point at [500, 999.65], rounding this coordinate will lead to out of bounds error when picking color with it.

I think this is related to pixel center, vlfeat might use (0.5, 0.5) as the center, maybe a better fix would be minus 0.5 before rounding? Not sure how other feature detectors deal with coordinates related pixel center, no such issue observed from other feature detectors.

[pierotofy]

Hey @originlake thanks for looking into this; have you found a test case (test images) where this error happens? Can you share it/them? I'd be curious to look into it as well.

[originlake]

The datasets I have are under NDA, but let me see if I can pull some reproducible image data or find a public dataset has such issue.

[originlake]

I read the source code of VLFeat. There is no pixel center issue related.
In the algorithm, it will check the boundary per octave level,
https://github.com/vlfeat/vlfeat/blob/1b9075fc42fe54b42f0e937f8b9a230d8e2c7701/vl/covdet.c#L1312-L1314
and then rescale the points with respect to original image size (no boundary check after this step)
https://github.com/vlfeat/vlfeat/blob/1b9075fc42fe54b42f0e937f8b9a230d8e2c7701/vl/covdet.c#L2044-L2045
For example, assuming an image with size w,h

1. octave level -1, raw range [0, 2*w-1], [0,2*h-1], step=0.5, scaled range [0, w-0.5], [0,h-0.5]
2. octave level 0, raw range [0, w-1], [0,h-1], step=1, scaled range [0, w-1], [0,h-1]
3. octave level 1, raw range [0, w/2-1], [0,h/2-1], step=2, scaled range [0, w-2], [0,h-2]
4. ......

I can tell the issue will only happen at octave level -1 and reaches the max width(w-0.5) or height(h-0.5), which will return w or h after rounding and hence causing error. So I think the clipping solution makes sense

OpenSfM/opensfm/features.py

Lines 636 to 638 in 097ef20

	xs = points[:, 0].round().astype(int)
	ys = points[:, 1].round().astype(int)
	colors = image[ys, xs]

Let me know your thoughts.

[pierotofy]

I think the clipping solution is probably OK, but I don't fully understand why other detectors don't suffer from this issue.

[originlake]

why other detectors don't suffer from this issue.

From what I can tell from SIFT implementation in OpenCV, it will ignore the keypoints on the border, the width of border is defined to 5 here
https://github.com/opencv/opencv/blob/93b607dc72e1d7953b17a58d8fb5f130b05c3d7a/modules/features2d/src/sift.simd.hpp#L92-L93
Orb also calls a function to remove keypoints on the border
https://github.com/opencv/opencv/blob/93b607dc72e1d7953b17a58d8fb5f130b05c3d7a/modules/features2d/src/keypoint.cpp#L105-L117

[originlake]

We could implement the same border filter, but I don't know if it has good or bad effect.

[pierotofy]

Ah, interesting! If other detectors ignore keypoints near the border, we should probably do something similar with DSP SIFT.

[originlake]

I implemented the filter to remove border keypoints, also added one additional argument to control the border size, default is 5 as given in OpenCV's SIFT implementation.

I think the most optimized and identical to OpenCV's logic way is to modify the VLfeat source code, but that's kind of out of my scope. Let me know if you plan to go that way, I'll close this PR.

[pierotofy]

Awesome, thank you @originlake ! Modifying VLfeat is probably not needed for the scope of this.