Skip to content

Commit 5b6fbee

Browse files
kalwaltclaude
andauthored
refactor(kpm): factor VisualDatabase::query matching loop + add query_from_keyframe (#147) (#217)
Recover part of the C++ query onion. Step 1 (no public API change): extract the per-keyframe matching loop into `match_against_database` and the per-query reset into `reset_query_state`. Step 2: expose `query_from_keyframe(Keyframe)` — runs only the matching loop against the database (skips pyramid build + feature extraction), for deterministic testing and callers that already hold a `Keyframe`. Mirrors C++ `query(const keyframe_t*)`. `query(&Matrix<u8>)` is now a thin wrapper (build keyframe → match → stash), behaviorally identical (existing query tests unchanged + green). Skipped per YAGNI: `query_from_pyramid` — KPM uses GaussianScaleSpacePyramid while AR2 uses BoxFilterPyramid8u, so there is no possible cross-pipeline caller (documented in #147). Test: query_from_keyframe self-matches an identical keyframe. Closes #147. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 03fd9d7 commit 5b6fbee

1 file changed

Lines changed: 72 additions & 10 deletions

File tree

crates/core/src/kpm/freak/visual_database.rs

Lines changed: 72 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -354,10 +354,7 @@ impl VisualDatabase {
354354
///
355355
/// C equivalent: `query(const Image&)` (`visual_database-inline.h:155`).
356356
pub fn query(&mut self, image: &Matrix<u8>) -> Result<bool, KpmError> {
357-
// Reset per-query state (C++ `visual_database-inline.h:194-195`).
358-
self.inliers.clear();
359-
self.matched_db_id = -1;
360-
self.matched_geometry = [0.0; 9];
357+
self.reset_query_state();
361358

362359
// Build the pyramid and the query keyframe.
363360
self.ensure_pyramid(image)?;
@@ -368,21 +365,52 @@ impl VisualDatabase {
368365
query_kf.store.num_features()
369366
);
370367

371-
// Iterate the database; the keyframe with the most inliers wins.
372-
// Collect ids upfront because we need a `&mut self` for the matcher
373-
// build inside the loop body.
368+
let matched = self.match_against_database(&query_kf)?;
369+
self.query_keyframe = Some(query_kf);
370+
Ok(matched)
371+
}
372+
373+
/// Query with a pre-extracted [`Keyframe`] instead of a raw image.
374+
///
375+
/// Skips pyramid construction and feature extraction, running only the
376+
/// matching loop against the database. Useful for deterministic testing
377+
/// (feed a hand-built keyframe) and for callers that already hold a
378+
/// `Keyframe`. The supplied keyframe becomes the stashed `query_keyframe`.
379+
///
380+
/// C equivalent: `query(const keyframe_t*)` (`visual_database.h:193`).
381+
pub fn query_from_keyframe(&mut self, query_kf: Keyframe) -> Result<bool, KpmError> {
382+
self.reset_query_state();
383+
let matched = self.match_against_database(&query_kf)?;
384+
self.query_keyframe = Some(query_kf);
385+
Ok(matched)
386+
}
387+
388+
/// Reset per-query result state (C++ `visual_database-inline.h:194-195`).
389+
fn reset_query_state(&mut self) {
390+
self.inliers.clear();
391+
self.matched_db_id = -1;
392+
self.matched_geometry = [0.0; 9];
393+
}
394+
395+
/// Run the matching loop against every stored keyframe; the keyframe with
396+
/// the most inliers (above `min_num_inliers`) wins. Assumes per-query state
397+
/// was already reset; does not touch `query_keyframe`.
398+
///
399+
/// Shared inner loop of [`query`](Self::query) and
400+
/// [`query_from_keyframe`](Self::query_from_keyframe)
401+
/// (C++ `visual_database-inline.h:200-241`).
402+
fn match_against_database(&mut self, query_kf: &Keyframe) -> Result<bool, KpmError> {
403+
// Collect ids upfront because we need `&mut self` for try_match_one.
374404
let ids: Vec<usize> = self.keyframes.keys().copied().collect();
375405
for id in ids {
376-
if let Some((inliers, h)) = self.try_match_one(&query_kf, id)? {
406+
if let Some((inliers, h)) = self.try_match_one(query_kf, id)? {
377407
if inliers.len() >= self.min_num_inliers && inliers.len() > self.inliers.len() {
378408
self.matched_geometry = h;
379409
self.inliers = inliers;
380410
self.matched_db_id = id as i32;
381411
}
382412
}
383413
}
384-
385-
self.query_keyframe = Some(query_kf);
386414
Ok(self.matched_db_id >= 0)
387415
}
388416

@@ -1067,6 +1095,40 @@ mod tests {
10671095
assert!(db.keyframe(0).unwrap().index().is_some());
10681096
}
10691097

1098+
#[test]
1099+
#[cfg_attr(miri, ignore)] // real-image pyramid + DoG + BHC pipeline — too slow under Miri
1100+
fn test_query_from_keyframe_self_matches() {
1101+
// #147: query_from_keyframe runs the matching loop on a pre-built
1102+
// keyframe. An identical keyframe must self-match the database entry.
1103+
let img = load_grayscale("../../benchmarks/data/found.jpg");
1104+
let build_kf = |img: &Matrix<u8>| -> Keyframe {
1105+
let mut pyr = crate::kpm::freak::gaussian_pyramid::GaussianScaleSpacePyramid::new(3);
1106+
pyr.build(img).unwrap();
1107+
let det =
1108+
crate::kpm::freak::detector::DoGScaleInvariantDetector::new(3.0, 4.0, 500, true);
1109+
let mut kf = Keyframe::new(img.cols as i32, img.rows as i32).unwrap();
1110+
find_features(&mut kf, &pyr, &det).expect("find_features");
1111+
kf
1112+
};
1113+
1114+
let mut db = VisualDatabase::new().expect("new");
1115+
db.add_keyframe(build_kf(&img), 0).expect("add_keyframe");
1116+
1117+
let matched = db
1118+
.query_from_keyframe(build_kf(&img))
1119+
.expect("query_from_keyframe");
1120+
assert!(matched, "an identical keyframe must self-match");
1121+
assert_eq!(db.matched_db_id(), 0);
1122+
assert!(
1123+
db.inliers().len() >= 8,
1124+
"self-match should be a strong match"
1125+
);
1126+
assert!(
1127+
db.query_keyframe().is_some(),
1128+
"query_from_keyframe must stash the query keyframe"
1129+
);
1130+
}
1131+
10701132
#[test]
10711133
fn test_facade_parity_accessors() {
10721134
// Build a tiny keyframe directly (no image pipeline) so the getters are

0 commit comments

Comments
 (0)