Skip to content

orb.describe silently produces contaminated descriptors for keypoints near the image edge #110

Description

@kalwalt

Summary

orb.describe() writes one descriptor per keypoint regardless of where that keypoint sits. When a keypoint is close enough to the image edge, part of its sampling pattern falls outside the image and reads the constant fill value 128 instead of real pixels. The resulting descriptor is silently degraded — there is no return value, status flag, or warning telling the caller which descriptors are affected, and a contaminated descriptor still looks structurally valid and will still be matched against others.

Found while writing the ORB invariant tests for #87 (PR #109).

Mechanism

rectify_patch() warps the keypoint neighbourhood with a constant fill:

imgProcessor.warp_affine(src, dst, H, 128);

Patch pixels sampled outside src therefore become exactly 128 whatever the image brightness is. Every comparison in the descriptor that touches such a pixel is decided by that constant rather than by image content.

The clearest symptom is a broken invariance: ORB compares pixel pairs (p(a) < p(b)) on a bilinearly warped patch, so adding a constant to the whole image must not change a single bit — interpolation is linear and the offset is an integer, so every sampled value shifts identically. That holds exactly when the patch is inside the image, and breaks when it is not.

Measured on a synthetic scene, comparing descriptors of the same keypoints on an image and on the same image lifted by +20 grey levels (no saturation):

detection border Hamming distance
8 2 bits of 8960
16 0
≥ 20 0

The margin that actually matters

It is not the 32×32 patch size, which is the intuitive but wrong number. Only the 256 sampled pairs are ever read, and the largest coordinate component in bit_pattern_31 is 13, so the furthest sample lies

13 * sqrt(2) = 18.39 px

from the keypoint centre. Rotation preserves that radius, so adding the bilinear neighbour gives a guaranteed-safe margin of 20 px.

Empirically, sweeping a single keypoint over 2000 angles at varying distances from the edge: contamination appears at distance ≤ 16 and disappears from 17 upward — so 20 has slack rather than being tight.

For reference, OpenCV avoids the situation entirely by rejecting keypoints too close to the border (edgeThreshold, default 31) rather than describing them badly.

Proposed fix

(a) Document it — preferred first step. State the ≥ 20px margin requirement in describe()'s TSDoc, and explain that keypoints closer than that produce descriptors partly determined by the 128 fill. No API change, no behaviour change, no parity risk. This alone removes the "silent" part of the problem.

(b) Optional guard — follow-up. Give callers a way to know which keypoints were fully describable. This needs an API addition rather than a change: describe() returns void and writes exactly one row per corner, so it cannot skip keypoints without breaking the row↔corner mapping every caller relies on. Something opt-in (a status array, or a helper that filters keypoints by margin) would fit. Worth considering alongside #83, where descriptor quality starts to matter for matching and pose.

(c) Border replication instead of a constant fill — not recommended. It changes descriptor bytes, so it is a parity break requiring an entry in tests/divergences.test.ts, and it does not make an edge patch correct — only differently wrong.

Impact

Low severity, but real. Most pipelines detect corners with a generous border anyway, and the affected bit count is small. The problem is that it is undiagnosable from the outside: nothing in the API surface hints that a descriptor was built partly from a constant. Users doing their own detection (or feeding keypoints from another source) have no way to know.

Test coverage

PR #109 already pins both sides of this in tests/properties/detectors.test.ts:

  • exact brightness invariance at border 20,
  • and a characterization test asserting that invariance is lost at border 8, so the fill-value behaviour cannot change unnoticed.

Whatever fix lands should keep both green (or update the second deliberately).

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions