Summary
Flagged by Qodo's review on #64. WebARKitTrackerImpl::minNumMatches is assigned per tracker type (e.g. 15 for AKAZE as of #64, previously 40) but is never read anywhere else in WebARKitTracker.cpp — it's dead configuration.
Details
MatchFeatures() gates on matches.size() > minRequiredDetectedFeatures (a fixed 50, unrelated to minNumMatches), then accepts any maxMatches > 0.- Homography validity in
WebARKitHomographyInfo.cpp only requires matches.size() > 4 inliers.
minNumMatches is logged (WEBARKIT_LOGd("Min Num Matches: %d\n", minNumMatches);) but otherwise unused.
So the per-tracker match-count floor implied by minNumMatches (15 for AKAZE, 15 for the default case, MIN_NUM_MATCHES = 8 elsewhere) has no actual effect on whether a marker is accepted as detected.
Why a separate issue
This is pre-existing dead code, not introduced by #64 (the AKAZE value there just changed from 40 to 15). Wiring it in would change runtime matching behavior — making acceptance stricter for at least the AKAZE path — which needs its own validation against existing test scenes rather than being folded into the #53 regression fix.
Suggested fix
Either:
- Actually enforce
minNumMatches (e.g. require totalGoodMatches >= minNumMatches before computing homography, and/or use it in place of the hardcoded 4 in WebARKitHomographyInfo), or
- Remove the field and the misleading per-tracker assignments if the low homography-inlier bar is intentional.
Validate against the existing tracker test scenes (including the #53 pinball-demo case) before merging, since tightening this could regress newly-fixed AKAZE detection.
Summary
Flagged by Qodo's review on #64.
WebARKitTrackerImpl::minNumMatchesis assigned per tracker type (e.g.15for AKAZE as of #64, previously40) but is never read anywhere else inWebARKitTracker.cpp— it's dead configuration.Details
MatchFeatures()gates onmatches.size() > minRequiredDetectedFeatures(a fixed 50, unrelated tominNumMatches), then accepts anymaxMatches > 0.WebARKitHomographyInfo.cpponly requiresmatches.size() > 4inliers.minNumMatchesis logged (WEBARKIT_LOGd("Min Num Matches: %d\n", minNumMatches);) but otherwise unused.So the per-tracker match-count floor implied by
minNumMatches(15 for AKAZE, 15 for the default case,MIN_NUM_MATCHES= 8 elsewhere) has no actual effect on whether a marker is accepted as detected.Why a separate issue
This is pre-existing dead code, not introduced by #64 (the AKAZE value there just changed from 40 to 15). Wiring it in would change runtime matching behavior — making acceptance stricter for at least the AKAZE path — which needs its own validation against existing test scenes rather than being folded into the #53 regression fix.
Suggested fix
Either:
minNumMatches(e.g. requiretotalGoodMatches >= minNumMatchesbefore computing homography, and/or use it in place of the hardcoded4inWebARKitHomographyInfo), orValidate against the existing tracker test scenes (including the #53 pinball-demo case) before merging, since tightening this could regress newly-fixed AKAZE detection.