feat(kpm): report every matched page, not only the best one

[kalwalt]

Summary

KPM's FREAK path could report one matched page per frame: VisualDatabase::query() kept a running maximum, and kpmMatching wrote a single result. This change makes it report every page that passes the matcher's tests, with one pose per page. That is the native half of multi-NFT-marker tracking in jsartoolkitNFT.

Companion PR: webarkit/jsartoolkitNFT#658. It bumps this submodule to this branch and makes the bindings track every reported page.

What changed (lib/SRC/KPM/)

• matchers/matcher_types.h: new image_match_t { id, inliers, geometry[9] } and image_matches_t.
• matchers/visual_database{.h,-inline.h}:
  • query() now collects every reference image that clears the existing inlier floor into mMatches, exposed as matches().
  • matchedId(), inliers() and matchedGeometry() are unchanged: they still return the single best match, so existing callers are unaffected.
• facade/visual_database_facade.{h,cpp}: forwards matches() across the pimpl boundary.
• kpmMatching.cpp:
  • Loops the matches and writes one pose per matched page.
  • Each page is registered as several keyframes (one per image scale; see kpmSetRefDataSet), so it keeps the best-supported keyframe per page.
  • The dead, commented-out per-page loop is removed. It called matchedId() inside the loop, so it gave every page the same id.

No thresholds changed. The inlier floor (kMinNumInliers = 8) is as before.

Behaviour change for single-result consumers

Consumers that looped over every camPoseF == 0 result and kept the last one used to be safe, because there was only ever one. Now they would pick the highest page number, not the best match. jsartoolkitNFT#658 updates its legacy bindings to pick the result with the most inliers (ties to the lowest error). Other consumers of kpmGetResult should do the same.

A note on lib/SRC/**

CONTRIBUTING.md asks contributors not to modify the vendored ARToolKit5 sources under lib/SRC/**. This change does, because the KPM matcher lives there and jsartoolkitNFT consumes it. It follows the precedent of #67 (fix/kpm-matched-id-sentinel), which changed lib/SRC/KPM/kpmMatching.cpp for the same consumer. The parked WebARKit/NFT relocation would move this code out of lib/SRC/; until then, KPM fixes land here.

Testing

Verified end-to-end through jsartoolkitNFT, since this repo's gtest suite does not cover KPM:

• One KPM pass over a photo containing both the pinball and kuva targets now reports Page[0] and Page[1] (previously only the best).
• jsartoolkitNFT's full suite passes against this commit: Vitest 112 passed, all 7 Karma targets green, and the Node example detects.

Refs webarkit/jsartoolkitNFT#635

🤖 Generated with Claude Code

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Report every matched KPM page with an independent pose

✨ Enhancement 🕐 20-40 Minutes

AI Description

• Retains every geometrically verified reference-image match while preserving legacy best-match
 accessors.
• Groups scaled keyframes by page and estimates one pose for each matched page.
• Enables multi-marker KPM output without changing the existing eight-inlier threshold.

Diagram

graph TD
  Q["Query frame"] --> V["Visual database"] --> T{"Inlier threshold"} -->|Pass| M["Image matches"] --> G["Best per page"] --> P["Pose estimation"] --> R["KPM results"]

Loading

High-Level Assessment

The chosen approach is appropriate: retaining image-level matches keeps VisualDatabase generic, while page-level deduplication remains in KPM where keyframe-to-page mappings are known. Preserving matchedId(), inliers(), and matchedGeometry() also minimizes compatibility risk compared with replacing the existing single-best-match contract.

Files changed (6) +78 / -52

Enhancement (6) +78 / -52

visual_database_facade.cppForward all verified matches through the facade +5/-1

Forward all verified matches through the facade

• Implements the facade accessor that exposes the visual database's complete collection of geometrically verified image matches across the pimpl boundary.

lib/SRC/KPM/FreakMatcher/facade/visual_database_facade.cpp

visual_database_facade.hExpose multi-match results in the facade API +3/-1

Expose multi-match results in the facade API

• Declares the matches() accessor so KPM consumers can retrieve every accepted reference-image match while retaining existing best-match methods.

lib/SRC/KPM/FreakMatcher/facade/visual_database_facade.h

matcher_types.hDefine reference-image match result types +12/-1

Define reference-image match result types

• Adds image_match_t to associate a reference image ID with its inlier correspondences and homography. Adds image_matches_t as the collection returned for each query.

lib/SRC/KPM/FreakMatcher/matchers/matcher_types.h

visual_database-inline.hCollect every geometrically verified image match +16/-5

Collect every geometrically verified image match

• Clears the multi-match collection at the start of each query and records every reference image meeting the existing inlier threshold. The strongest candidate continues to populate the legacy single-best-match fields.

lib/SRC/KPM/FreakMatcher/matchers/visual_database-inline.h

visual_database.hStore and expose all accepted database matches +10/-2

Store and expose all accepted database matches

• Adds query-scoped storage and a read-only accessor for all verified reference-image matches. Documents that legacy accessors continue to describe only the strongest match.

lib/SRC/KPM/FreakMatcher/matchers/visual_database.h

kpmMatching.cppProduce one pose for every matched page +32/-42

Produce one pose for every matched page

• Groups accepted image matches by page, retains the keyframe with the most inliers for each page, and estimates a separate pose for every unskipped page. Removes the obsolete commented single-match page loop and adds aggregate match logging.

lib/SRC/KPM/kpmMatching.cpp

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Sparse page numbers corrupt results ✓ Resolved 🐞 Bug ≡ Correctness

Description

The new multi-page loop uses the external pageNo from pageIDs directly as an index into the
densely allocated result array. When page numbers are nonzero, sparse, negative, or otherwise
differ from their positions in pageInfo, matching those pages accesses the wrong slot or memory
outside the allocation, and the new all-page behavior exposes this for every secondary match too.

Code

lib/SRC/KPM/kpmMatching.cpp[R659-660]

+    // result[] is indexed by page number, as it was before this change.
+    if (kpmHandle->result[pageNo].skipF) continue;

Evidence

result has exactly pageNum dense entries, while each matcher image is mapped to the independent
external pageInfo[k].pageNo; skip flags also demonstrate that result indexing is by k, not by
that external value. The changed loop nevertheless uses the external value for every result[]
access.

lib/SRC/KPM/kpmMatching.cpp[239-250]
lib/SRC/KPM/kpmMatching.cpp[270-298]
lib/SRC/KPM/kpmMatching.cpp[344-353]
lib/SRC/KPM/kpmMatching.cpp[646-672]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The multi-page matching path indexes `result` by the dataset's external page number, although `result` is allocated and managed by dense page position. Arbitrary page numbers therefore select the wrong result or access outside the allocation.
## Fix Focus Areas
- lib/SRC/KPM/kpmMatching.cpp[239-250]
- lib/SRC/KPM/kpmMatching.cpp[270-298]
- lib/SRC/KPM/kpmMatching.cpp[646-672]
- lib/SRC/KPM/kpmMatching.cpp[344-353]
## Recommended Fix
Map each database image ID to its dense `pageInfo` index, group and index `result` with that dense index, and write the corresponding external `pageInfo[index].pageNo` into `KpmResult::pageNo`. Keep skip handling keyed to the same dense result slot.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Valid page poses can be discarded ✓ Resolved 🐞 Bug ≡ Correctness

Description

The bestPerPage reduction discards every same-page keyframe except the one with the largest inlier
count before calling kpmUtilGetPose_binary. If initialization or pose fitting fails for that
candidate, another geometrically verified keyframe that could produce a valid pose is never
attempted, leaving that page unreported.

Code

lib/SRC/KPM/kpmMatching.cpp[R649-651]

+    auto best = bestPerPage.find(pageNo);
+    if (best == bestPerPage.end() || imageMatch.inliers.size() > best->second->inliers.size()) {
+        bestPerPage[pageNo] = &imageMatch;

Evidence

The matcher now retains all geometrically verified image matches, but the changed KPM loop selects
only one per page by inlier count and invokes pose estimation only for that selection.
kpmUtilGetPose_binary can still return failure during planar initialization, handle creation, or
pose fitting, and the caller has no fallback path to the discarded candidates.

lib/SRC/KPM/FreakMatcher/matchers/visual_database-inline.h[341-353]
lib/SRC/KPM/kpmMatching.cpp[646-671]
lib/SRC/KPM/kpmMatching.cpp[698-780]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Same-page matches are reduced by inlier count before pose estimation, so failure of the selected candidate suppresses all alternatives. Pose validity and error must be considered before choosing the page's result.
## Fix Focus Areas
- lib/SRC/KPM/kpmMatching.cpp[646-671]
- lib/SRC/KPM/kpmMatching.cpp[698-790]
## Recommended Fix
Group all image matches by page, run pose estimation for each candidate into temporary pose and error storage, discard failed estimates, and commit the best successful candidate for each page. Rank successful candidates by the intended quality policy, such as inlier count followed by lower pose error.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗