Skip to content

Fix small-marker detection under downsampling (AKAZE) — #53 - #64

Open
kalwalt with Copilot wants to merge 18 commits into
devfrom
copilot/investigate-small-marker-detection-issue
Open

Fix small-marker detection under downsampling (AKAZE) — #53#64
kalwalt with Copilot wants to merge 18 commits into
devfrom
copilot/investigate-small-marker-detection-issue

Conversation

Copilot AI commented Aug 23, 2026

Copy link
Copy Markdown

Summary

Investigates and fixes #53: the small-marker scene
pinball-demo.jpg (2000×1500, detection pyrLevel=1) was not acquired by the
C++/WASM tracker even though the same reference + scene track in WebARKitLib-rs
and jsartoolkitNFT. Root cause analysis + a minimal fix on the AKAZE path,
validated end-to-end via the webarkit-testing harness.

Features accomplished

Issues resolved

What's next

kalwalt and others added 15 commits January 5, 2024 16:52
- emcc version > 3.1.40 cause issue with isnan
…lly, keyframes, and BHC clusters

The matcher had three `std::unordered_map` typedefs whose iteration
order depended on the STL implementation (libstdc++ on Linux, MSVC
STL on Windows, libc++ on macOS / Emscripten). Code paths that
iterate these maps and pick a winner-on-tie produced different
results on different platforms, causing the matcher to be
non-deterministic across builds. Concretely:

1. `HoughSimilarityVoting::hash_t` (vote tally) is consumed by
   `getMaximumNumberOfVotes`, which iterates and picks the bin with
   the highest count. Ties between Hough bins are common at
   borderline matches and were broken inconsistently per platform.

2. `VisualDatabase::keyframe_map_t` is iterated by `query()`. Ties
   on inlier count between keyframes are broken first-wins, so the
   winning keyframe at borderline ties depended on which iteration
   order the platform's STL chose.

3. `BinaryHierarchicalClustering::cluster_map_t` is iterated during
   BHC tree construction; ordering affects the resulting topology
   and therefore which features cluster together, which propagates
   into the eventual inlier set.

All three typedefs become `std::map<...>`. `std::map`'s ascending-
key iteration is consistent across STL implementations (and matches
the BTreeMap fix on the pure-Rust port, webarkit/WebARKitLib-rs
issue #170).

API surface change: none. `std::map` and `std::unordered_map` share
the operations used here (`operator[]`, `find`, `insert`, `erase`,
`clear`, `iterator`). Performance: `O(log N)` lookup instead of
`O(1) amortized`, but N is small for all three maps (number of
keyframes ~1-10, number of Hough bins voted for in a query ~10s,
number of BHC clusters per level ~1-100), so the difference is
negligible.

`VisualDatabaseImpl::point3d_map_t` in
`facade/visual_database_facade.cpp` is left as `std::unordered_map`
because it is used lookup-only (`map[image_id] = ...`,
`return map[image_id]`); changing it has no functional benefit and
would be cosmetic only.

Motivation + measurements live in webarkit/WebARKitLib-rs issue
#170, which has the cross-platform repro from CI.
Integrate the WebARKit OCVT tracker line: dev → master
Copilot AI and others added 2 commits September 1, 2026 18:57
- Add full-resolution retry in processFrame when pyrDown'd detection
  yields <= minRequiredDetectedFeatures (small markers fell below the
  detector threshold after downsampling).
- Pass the actual detection scale factor into MatchFeatures so matched
  keypoints are rescaled correctly on both the downsampled and the
  full-res-retry paths.
- Align AKAZE path with artoolkitX OCVT: threshold 3e-4 -> 1e-3,
  nn_match_ratio 0.7 -> 0.8 (new AKAZE_NN_MATCH_RATIO), minNumMatches
  40 -> 15.
- Add per-level keypoint-count logging for diagnosability.

Co-authored-by: kalwalt <1275858+kalwalt@users.noreply.github.com>
…shold

- Drop misleading `f` suffix on double constants (DEFAULT/TEBLID/AKAZE
  NN_MATCH_RATIO); use EXPECT_DOUBLE_EQ in the config test.
- Retry condition uses `<` (not `<=`) to match the `>` matching gate, so a
  frame with exactly minRequiredDetectedFeatures keypoints is not needlessly
  retried.

Co-authored-by: kalwalt <1275858+kalwalt@users.noreply.github.com>
@kalwalt
kalwalt marked this pull request as ready for review September 2, 2026 08:45
@kalwalt kalwalt added bug Something isn't working enhancement New feature or request C/C++ code concerning the C/C++ code design and improvements Emscripten labels Sep 2, 2026
@kalwalt kalwalt changed the title [WIP] Investigate small-marker pinball-demo.jpg detection under downsampling Fix small-marker detection under downsampling (AKAZE) — #53 Sep 2, 2026
@kalwalt
kalwalt changed the base branch from master to dev September 2, 2026 08:45
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix AKAZE small-marker detection after frame downsampling

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Restores AKAZE small-marker acquisition with artoolkitX-compatible detection and matching
 thresholds.
• Retries feature extraction at full resolution when downsampling yields insufficient keypoints.
• Preserves coordinate accuracy using the scale of the actual detection frame.
Diagram

graph TD
  A["Input frame"] --> B{"Use pyramid?"}
  B -->|Yes| C["Downsample frame"] --> D["Extract features"] --> E{"Enough features?"}
  B -->|No| D
  E -->|No| F["Retry full resolution"] --> G["Match features"] --> H["Fit homography"]
  E -->|Yes| G
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Retry after matching failure
  • ➕ Handles frames containing many unrelated keypoints where downsampled marker matching still fails.
  • ➕ Covers ratio-test or homography failures that keypoint-count gating cannot detect.
  • ➖ Can trigger expensive full-resolution detection on every unsuccessful frame.
  • ➖ Requires matching to return a structured failure reason and rerun more of the pipeline.
2. Always detect at full resolution
  • ➕ Provides the simplest behavior and maximizes retention of small-marker detail.
  • ➕ Avoids detection-scale switching and fallback boundary conditions.
  • ➖ Regresses the large-frame performance optimization introduced by the pyramid fast path.
  • ➖ Increases CPU and WASM processing costs even when downsampled detection succeeds.

Recommendation: Keep the PR's conditional full-resolution retry and artoolkitX-aligned AKAZE parameters for this focused fix because they preserve the existing downsampled fast path. Consider failure-driven retry only if future scenes demonstrate sufficient total keypoints but repeated matching or homography failures; a dedicated image-based regression test would further protect this behavior.

Files changed (4) +51 / -16

Bug fix (1) +44 / -12
WebARKitTracker.cppRestore AKAZE acquisition for small downsampled markers +44/-12

Restore AKAZE acquisition for small downsampled markers

• Aligns AKAZE threshold, match ratio, and minimum-match settings with artoolkitX OCVT. Adds a full-resolution extraction retry for insufficient downsampled keypoints, passes the active scale into feature matching, and logs per-level keypoint counts.

WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitTracker.cpp

Tests (1) +3 / -2
webarkit_test.ccVerify double-valued tracker matching constants +3/-2

Verify double-valued tracker matching constants

• Uses double-aware assertions for nearest-neighbor ratios and covers the new AKAZE-specific value.

tests/webarkit_test.cc

Other (2) +4 / -2
WebARKitConfig.cppAdd an AKAZE-specific nearest-neighbor match ratio +3/-2

Add an AKAZE-specific nearest-neighbor match ratio

• Defines the artoolkitX-compatible AKAZE ratio-test value of 0.8. Existing ratio constants now use double literals consistent with their declared types.

WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitConfig.cpp

WebARKitConfig.hExpose the AKAZE match-ratio constant +1/-0

Expose the AKAZE match-ratio constant

• Declares the new AKAZE-specific nearest-neighbor matching ratio for tracker configuration and tests.

WebARKit/WebARKitTrackers/WebARKitOpticalTracking/include/WebARKitTrackers/WebARKitOpticalTracking/WebARKitConfig.h

@qodo-code-review

qodo-code-review Bot commented Sep 2, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. AKAZE floor remains unenforced ✗ Dismissed 🐞 Bug ≡ Correctness
Description
Assigning minNumMatches = 15 does not lower an effective AKAZE match floor because MatchFeatures
never reads that member. The tracker instead sends any positive ratio-test result to homography
validation, which can accept only five inliers, so the intended 15-match requirement is not
implemented.
Code

WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitTracker.cpp[67]

+            minNumMatches = 15;
Evidence
The AKAZE branch assigns 15, but the member has no reads in the tracker. Matching proceeds on any
positive number of ratio-test matches, and homography validity is set with more than four inliers
rather than the configured floor.

WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitTracker.cpp[62-72]
WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitTracker.cpp[511-560]
WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitTracker.cpp[798-800]
WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitHomographyInfo.cpp[8-15]

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 changed AKAZE `minNumMatches` value is dead configuration: matching and homography acceptance do not consult it, so the advertised 15-match floor is not enforced.

## Issue Context
`MatchFeatures` uses `minRequiredDetectedFeatures` only for the number of KNN candidates, then proceeds whenever at least one ratio-test match exists. Homography validity requires only more than four RANSAC inliers.

## Fix Focus Areas
- WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitTracker.cpp[62-67]
- WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitTracker.cpp[521-560]
- WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitHomographyInfo.cpp[8-15]

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


2. Retry skips boundary count ✓ Resolved 🐞 Bug ≡ Correctness
Description
The full-resolution retry runs only below 50 keypoints, while matching requires more than 50. A
downsampled frame with exactly 50 keypoints therefore receives neither a retry nor a matching
attempt, even when full-resolution detection could acquire the marker.
Code

WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitTracker.cpp[401]

+            if (static_cast<int>(frameKeyPts.size()) < minRequiredDetectedFeatures && _featureDetectPyrLevel > 0) {
Evidence
The configured threshold is 50. The added retry condition requires a count below that threshold, but
the following matching gate requires a count above it, leaving equality unhandled.

WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitConfig.cpp[10-10]
WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitTracker.cpp[395-416]

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 retry condition excludes exactly `minRequiredDetectedFeatures` keypoints, but the subsequent matching condition also excludes that count. Such a frame silently skips both paths.

## Issue Context
`minRequiredDetectedFeatures` is 50. Retry currently uses `< 50`, while matching uses `> 50`.

## Fix Focus Areas
- WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitTracker.cpp[401-416]
- WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitConfig.cpp[10-10]

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



Informational

3. AKAZE rationale is inverted ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The new comment says raising the AKAZE threshold keeps more keypoints, but OpenCV defines it as the
detector response threshold required to accept a point. Raising it from 3e-4 to 1e-3 therefore
rejects more weak responses, making the rationale misleading even if the OCVT-parity value is
intentional.
Code

WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitTracker.cpp[R846-849]

+            // WebARKitLib#53: use the artoolkitX OCVT default threshold (0.001) instead of
+            // the more aggressive 3e-4. The higher threshold keeps more keypoints, which is
+            // important for small markers whose features are sparse after any downsampling.
+            const double akaze_thresh = 1e-3; // AKAZE detection threshold (artoolkitX OCVT default)
Evidence
The changed comment claims the higher threshold keeps more keypoints, while OpenCV's API
documentation identifies this value as the response threshold a point must satisfy to be accepted.

WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitTracker.cpp[843-852]
🌐 OpenCV documents AKAZE threshold as the detector response threshold to accept a point.

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 threshold comment describes behavior opposite to OpenCV AKAZE semantics. Preserve the parity value if intended, but document that the higher response cutoff filters more weak keypoints rather than retaining more keypoints.

## Issue Context
OpenCV documents AKAZE's threshold as the detector response threshold used to accept a point.

## Fix Focus Areas
- WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitTracker.cpp[846-849]

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


Grey Divider

Context sources
✅ Web pages:
  +2 more
Review mode: ⚖️ Balanced: This changes core tracker detection, feature matching coordinates, retry behavior, and AKAZE thresholds, creating meaningful algorithmic and regression risk across several interacting paths.

Grey Divider

Tip of the day
💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread WebARKit/WebARKitTrackers/WebARKitOpticalTracking/WebARKitTracker.cpp Outdated
Addresses Qodo review findings on #64:
- The full-res retry fired only when keypoints < minRequiredDetectedFeatures,
  while matching required > minRequiredDetectedFeatures. A frame with exactly
  minRequiredDetectedFeatures keypoints hit neither path. Widen the retry
  condition to <= so that boundary case is covered.
- Correct the AKAZE threshold comment: raising the detector response
  threshold from 3e-4 to 1e-3 is stricter, not looser, even though the
  OCVT-aligned value was validated to fix small-marker detection.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working C/C++ code concerning the C/C++ code design and improvements Emscripten enhancement New feature or request

Projects

Status: To do

Development

Successfully merging this pull request may close these issues.

Investigate: small-marker pinball-demo.jpg not detected under downsampling (works in WebARKitLib-rs & jsartoolkitNFT)

2 participants