feat(orb): expose ic_angle, the keypoint orientation step (#96) - #182
Merged
Conversation
The intensity-centroid orientation ORB needs to be rotation-invariant lived only in the example pages, duplicated verbatim in sample_orb.html and sample_orb_pinball.html. Nothing in src/ ever set keypoint_t.angle, so the library could not run detect -> describe on its own: describe() rotates the sampling patch by whatever angle it is handed, and the keypoint_t default of -1 rotates by -1 radian rather than leaving the patch upright. This is the same gap #133 closed for match_pattern, and it blocks the #96 CvBackend adapter: an out-of-repo adapter cannot implement detect + describe without reimplementing the orientation itself, which would leave PureCV's Rust port with no TS oracle to cross-validate against. Moves the routine into orb as a public method, with the u_max table at module scope so the hot loop allocates nothing per call. Both examples now call it instead of carrying their own copy. Also corrects orb_test.html, which claimed angle = -1 lets ORB work out the orientation itself. rectify_patch feeds the value straight into cos/sin, so that was never true; the page now fixes the angle at 0 and says why. Tests: parity against the examples' inline implementation (the only oracle - original jsfeat kept ic_angle in its sample too), plus invariants: gradient direction, brightness invariance, atan2(0,0) not NaN, range, and exact 90 degree rotation equivariance on a fixpoint-aligned odd-sized image. A closing test pins the point of the method: 11 bits apart with orientation versus 87 without, out of 256.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
kalwalt
added a commit
that referenced
this pull request
Sep 2, 2026
…ange Adding the module-scope u_max table in #182 inserted it between the orb class's JSDoc block and the class declaration, which orphaned the comment: TSDoc binds a block to the declaration immediately following it, and that was now u_max, which carries its own. The effect reached the published artifacts. The ORB description was absent from types/src/orb/orb.d.ts entirely, so editors showed nothing on hover and TypeDoc would have rendered the class undocumented - and 0.15.0 was about to ship that. Moving u_max above the class doc restores the binding. Swept every other exported class in types/ for the same pattern; orb was the only one affected. Also drops a {@link u_max} from the ic_angle docs. u_max is module-private and not exported, so the link had no resolvable target. Reported by the Qodo review on #183.
kalwalt
added a commit
that referenced
this pull request
Sep 3, 2026
…ange Adding the module-scope u_max table in #182 inserted it between the orb class's JSDoc block and the class declaration, which orphaned the comment: TSDoc binds a block to the declaration immediately following it, and that was now u_max, which carries its own. The effect reached the published artifacts. The ORB description was absent from types/src/orb/orb.d.ts entirely, so editors showed nothing on hover and TypeDoc would have rendered the class undocumented - and 0.15.0 was about to ship that. Moving u_max above the class doc restores the binding. Swept every other exported class in types/ for the same pattern; orb was the only one affected. Also drops a {@link u_max} from the ic_angle docs. u_max is module-private and not exported, so the link had no resolvable target. Reported by the Qodo review on #183.
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #96 — the jsfeatNext-side work item ("stream A": keep the modules adapter-ready). The
CvBackendcontract and the adapter itself live inwebarkit/webarkitand are not part of this PR.The audit
Four of the five primitives the adapter delegates to are already adapter-ready — public, neutral-mappable I/O, no hidden state:
matchbfmatcher.match/knnMatch/ratio_testmatrix_tin,match_t[]out)estimateHomographymotion_estimator.ransac+homography2dmatrix_t→Uint8Array)poseFromHomographypose_estimator.estimatematrix_t+Float64Array)detectfast_corners/yape/yape06describeorb.describeUseful incidental finding for whoever writes the adapter:
point_tandkeypoint_tare structurally identical (x, y, score, level, angle), so a single pre-allocated pool can be passed to bothfast_corners.detect(typedpoint_t[]) andorb.describe(typedkeypoint_t[]).The gap this PR closes
Keypoint orientation was missing from the library.
ic_angle— the intensity-centroid measure that makes ORB descriptors rotation-invariant — existed only in the example pages, duplicated verbatim insample_orb.htmlandsample_orb_pinball.html. Nothing undersrc/ever assignedkeypoint_t.angle.This is the same shape of gap #133 closed for
match_pattern.Why it matters for #96: the contract's
Keypointtype has ananglefield anddetect()is expected to return it populated. jsfeatNext's detectors never set it, so an out-of-repo adapter would have to reimplement the orientation itself — leaving PureCV's Rust port with no TypeScript oracle to cross-validate against, which is half the reason the jsfeatNext backend exists.To be precise about severity: this is not a hard blocker. An adapter could pass
angle = 0and the code would run — it would just produce descriptors that are not rotation-invariant.ic_anglenow lives inorbas a public method, with theu_maxtable at module scope so the hot loop allocates nothing per call. Placement follows the ORB paper (the "o" in oFAST is that paper's own contribution) and OpenCV, which keepsIC_Angleinorb.cpp;bit_pattern_31is the existing precedent for ORB-specific data living here.Both examples now call the library method instead of carrying their own copy.
Documentation fix
examples/orb_test.htmlclaimedangle = -1lets ORB work out the patch orientation itself. That was never true:rectify_patchfeeds the value straight intocos/sin, so-1rotates the patch by −1 radian (≈ −57°) rather than leaving it upright. The page now fixes the angle at 0 — it only compares a patch against itself under a brightness change — and says why.describe's own docs now state that orientation is a required prior step, not an optional one.Tests
10 new (295 → 305, all green).
ic_anglein its sample rather than the library. Same arrangement astests/parity/bfmatcher.test.ts.atan2(0,0)returning 0 rather than NaN on a flat patch, and range.One note on that last test, since the numbers look suspiciously convenient: the first version used a pixel-noise image and failed at 131 vs 130. That was not a defect in
ic_angle— ORB rectifies the patch by bilinear resampling, which does not preserve content at the Nyquist limit, so two views of a noise field give uncorrelated descriptors however well the orientation is recovered. The test now uses smooth structure, and the comment explains the constraint.Verification
npm test— 305 passednpm run typecheck— cleannpm run format-check— cleannode scripts/check-license-headers.mjs— 100 files OKdist/andtypes/are deliberately not committed — they are rebuilt at release time.Not in this PR
@webarkit/cv-backend-spec(that package is still at its pre-amendment version).cv-backend-jsfeatnextadapter package itself.Both are
webarkit/webarkitwork.