fix(bfmatcher,index): reject mismatched descriptor widths and export the new types

AGENTS.md

@@ -20,7 +20,8 @@
 ## Architecture — read this before editing
 
 - **One real module per algorithm** under `src/<module>/<module>.ts`, each extending the base class in **`src/core/core.ts`** (constants, data-type helpers, the shared cache). `src/jsfeatNext.ts` is a thin aggregator that only attaches modules to the namespace; `src/index.ts` default-exports the namespace directly.
-- **Calling convention (since 0.9.0, issue #41):** the 14 algorithm modules (`imgproc`, `math`, `matmath`, `linalg`, `transform`, `fast_corners`, `yape`, `yape06`, `orb`, `optical_flow_lk`, `motion_estimator`, `affine2d`, `homography2d`, plus the `cache` pool) are **singleton instances** on the namespace — `jsfeatNext.imgproc.grayscale(...)`, no `new` — matching original jsfeat. The data-structure classes (`matrix_t`, `keypoint_t`, `pyramid_t`, `ransac_params_t`) remain constructors.
+- **Calling convention (since 0.9.0, issue #41):** the 14 algorithm modules (`imgproc`, `math`, `matmath`, `linalg`, `transform`, `fast_corners`, `yape`, `yape06`, `orb`, `optical_flow_lk`, `motion_estimator`, `affine2d`, `homography2d`, plus the `cache` pool) are **singleton instances** on the namespace — `jsfeatNext.imgproc.grayscale(...)`, no `new` — matching original jsfeat. The data-structure classes (`matrix_t`, `keypoint_t`, `pyramid_t`, `ransac_params_t`, `match_t`, `pose_t`) remain constructors, and so does `pose_estimator`.
+  - **`pose_estimator` is deliberately NOT a singleton and deliberately does not extend `core`.** It is *stateful* — it holds the inverted intrinsics `K⁻¹` — and the public API constructs it with a `K` (`new jsfeatNext.pose_estimator(K)`), which also makes its `intrinsics()` factory reachable as a `static` on the namespace class. It therefore belongs with the constructors above, not with the stateless algorithm singletons. Review tooling has flagged this twice as a rule violation; it is not one, and the reasoning is repeated in the class's own JSDoc.
 - **One shared buffer pool:** all modules borrow scratch buffers from the single `shared_cache` exported by `src/core/core.ts` (public as `jsfeatNext.cache`), exactly like jsfeat's global `jsfeat.cache`. Balance every `get_buffer` with a `put_buffer`.
 - Full background: `docs/jsfeat-parity-and-refactor-audit.md` (the plan) and `docs/migration-0.9.md` (the 0.9.0 API break and its motivation).
 

src/bfmatcher/bfmatcher.ts

@@ -44,6 +44,7 @@ import jsfeatNext from "../core/core";
 import { matrix_t } from "../matrix_t/matrix_t";
 import { match_t } from "./match_t";
 import { JSFEAT_CONSTANTS } from "../constants/constants";
+import { shared_data_type } from "../data_type/data_type";
 
 /**
  * Brute-force Hamming matcher for binary descriptors (ORB today; TEBLID/FREAK
@@ -82,19 +83,69 @@ export class bfmatcher extends jsfeatNext {
         return (((n + (n >> 4)) & 0x0f0f0f0f) * 0x01010101) >> 24;
     }
 
+    /**
+     * Physical width of one descriptor row, in bytes.
+     *
+     * NOT `cols`. `matrix_t.allocate` sizes its buffer as
+     * `cols * sizeof(type) * channel * rows`, so a row's storage depends on all
+     * three. Two matrices can share a `cols` and still have different row
+     * widths — a U8/C1 and a U8/C2 with `cols = 32` occupy 32 and 64 bytes —
+     * which is why the stride check below compares this rather than `cols`.
+     */
+    private static rowBytes(m: matrix_t): number {
+        return m.cols * m.channel * shared_data_type._get_data_type_size(m.type);
+    }
+
     /**
      * Int32-word view over a descriptor matrix's full backing buffer, matching
      * `matrix_t.buffer.i32` — the same access the original sample uses.
      *
-     * @throws {Error} if `descriptors.cols` is not a multiple of 4 bytes.
+     * @throws {Error} if the descriptors are not `U8`, or if a row is not a
+     *         whole number of 4-byte words.
      */
     private static words(descriptors: matrix_t): Int32Array {
-        if ((descriptors.cols & 3) !== 0) {
+        // Hamming over an i32 view only means anything for packed bytes. A F32
+        // matrix would have its float bit patterns XORed and popcounted, which
+        // produces numbers with no relation to descriptor similarity.
+        if (!(descriptors.type & JSFEAT_CONSTANTS.U8_t)) {
+            throw new Error("jsfeatNext.bfmatcher: descriptors must be U8");
+        }
+        const bytes = bfmatcher.rowBytes(descriptors);
+        if ((bytes & 3) !== 0) {
+            throw new Error(`jsfeatNext.bfmatcher: descriptor width must be a multiple of 4 bytes, got ${bytes}`);
+        }
+        return descriptors.buffer.i32;
+    }
+
+    /**
+     * Int32 views over a query/train PAIR, plus the row stride in words.
+     *
+     * Both matrices are addressed with a single stride, so a width mismatch is
+     * not a mild inconsistency: `train` rows would be read at `ti * word_len`,
+     * an offset computed from the QUERY width. The reads walk across train row
+     * boundaries and, past the end, an out-of-range `Int32Array` index yields
+     * `undefined`, which XOR coerces to 0. The result is a full set of
+     * confident, silently wrong Hamming distances rather than any error — the
+     * matcher would report its best guess over garbage.
+     *
+     * Compares {@link rowBytes}, not `cols`, so a channel or element-type
+     * difference cannot slip past a matching column count.
+     *
+     * @throws {Error} if the row widths differ, or either matrix is rejected by
+     *         {@link words}.
+     */
+    private static pairWords(query: matrix_t, train: matrix_t): { qw: Int32Array; tw: Int32Array; word_len: number } {
+        const qw = bfmatcher.words(query);
+        const tw = bfmatcher.words(train);
+        const qb = bfmatcher.rowBytes(query);
+        const tb = bfmatcher.rowBytes(train);
+        if (qb !== tb) {
             throw new Error(
-                `jsfeatNext.bfmatcher: descriptor width must be a multiple of 4 bytes, got ${descriptors.cols}`
+                `jsfeatNext.bfmatcher: query and train descriptors must have the same row width, ` +
+                    `got ${qb} and ${tb} bytes`
             );
         }
-        return descriptors.buffer.i32;
+        return { qw, tw, word_len: qb >> 2 };
     }
 
     private static hamming(qw: Int32Array, qoff: number, tw: Int32Array, toff: number, word_len: number): number {
@@ -119,9 +170,7 @@ export class bfmatcher extends jsfeatNext {
     match(query: matrix_t, train: matrix_t, max_distance = 256): match_t[] {
         const q_cnt = query.rows;
         const t_cnt = train.rows;
-        const word_len = query.cols >> 2;
-        const qw = bfmatcher.words(query);
-        const tw = bfmatcher.words(train);
+        const { qw, tw, word_len } = bfmatcher.pairWords(query, train);
         const out: match_t[] = [];
 
         if (!this.cross_check) {
@@ -192,9 +241,7 @@ export class bfmatcher extends jsfeatNext {
     knnMatch(query: matrix_t, train: matrix_t, k = 2): match_t[][] {
         const q_cnt = query.rows;
         const t_cnt = train.rows;
-        const word_len = query.cols >> 2;
-        const qw = bfmatcher.words(query);
-        const tw = bfmatcher.words(train);
+        const { qw, tw, word_len } = bfmatcher.pairWords(query, train);
 
         const result: match_t[][] = [];
         for (let qi = 0; qi < q_cnt; ++qi) {

src/index.ts

@@ -71,5 +71,7 @@ export type { keypoint_t } from "./keypoint_t/keypoint_t";
 export type { pyramid_t } from "./pyramid_t/pyramid_t";
 export type { point_t, IPoint_t } from "./point_t/point_t";
 export type { ransac_params_t } from "./motion_estimator/ransac_params_t";
+export type { match_t, IMatch_T } from "./bfmatcher/match_t";
+export type { pose_t, IPose_T } from "./pose_estimator/pose_estimator";
 export type { ICache } from "./cache/cache";
 export type { TypedArray, NumericArray, MotionKernel } from "./types";

src/pose_estimator/pose_estimator.ts

@@ -112,9 +112,23 @@ export class pose_t implements IPose_T {
 export class pose_estimator {
     /** Inverse intrinsics `K⁻¹`, row-major, assuming zero skew. */
     private Kinv: Float64Array;
+    /**
+     * Scratch for `B = K⁻¹·H` in {@link estimate}.
+     *
+     * An instance field rather than a per-call allocation: an estimator is
+     * built once and reused across frames, so at 30-60 fps a local would be a
+     * fresh array every frame for no benefit. It is not borrowed from the
+     * shared cache either — that pool exists for image-sized buffers, and
+     * balancing a get/put across the degenerate early return to save 72 bytes
+     * would cost more in bookkeeping than it saves.
+     *
+     * Overwritten in full on every call, so no state carries between frames.
+     */
+    private readonly B: Float64Array;
 
     constructor(K: matrix_t) {
         this.Kinv = pose_estimator.invertIntrinsics(K);
+        this.B = new Float64Array(9);
     }
 
     /** Replace the intrinsics (e.g. after a resolution change). */
@@ -180,8 +194,9 @@ export class pose_estimator {
         const h = H.data;
         const ki = this.Kinv;
 
-        // B = K⁻¹ · H, row-major.
-        const B = new Float64Array(9);
+        // B = K⁻¹ · H, row-major. Reuses the instance scratch; every entry is
+        // assigned below, so nothing survives from the previous call.
+        const B = this.B;
         for (let r = 0; r < 3; ++r) {
             for (let c = 0; c < 3; ++c) {
                 B[r * 3 + c] = ki[r * 3] * h[c] + ki[r * 3 + 1] * h[3 + c] + ki[r * 3 + 2] * h[6 + c];

tests/properties/bfmatcher.test.ts

@@ -114,6 +114,72 @@ describe("bfmatcher.match", () => {
         const bad = new jsfeatNext.matrix_t(30, 4, OU8C1);
         expect(() => jsfeatNext.bfmatcher.match(bad, bad)).toThrow(/multiple of 4/);
     });
+
+    it("throws when query and train row widths differ", () => {
+        // Both matrices are addressed with ONE stride, taken from the query.
+        // A mismatch therefore reads train rows at the wrong offsets rather
+        // than failing, so this has to be rejected up front.
+        const q = randomDescriptors(4, 32, 9101);
+        const t = randomDescriptors(6, 16, 9102);
+        expect(() => jsfeatNext.bfmatcher.match(q, t)).toThrow(/same row width/);
+        expect(() => jsfeatNext.bfmatcher.knnMatch(q, t)).toThrow(/same row width/);
+    });
+
+    it("rejects equal cols with different channel counts", () => {
+        // The subtle case: cols alone does not determine a row's storage.
+        // matrix_t sizes its buffer as cols * sizeof(type) * channel * rows, so
+        // a U8/C1 and a U8/C2 both at cols=32 occupy 32 and 64 bytes per row.
+        // Comparing cols would wave this through into the same stride
+        // corruption the guard exists to stop.
+        const c1 = new jsfeatNext.matrix_t(32, 4, jsfeatNext.U8_t | jsfeatNext.C1_t);
+        const c2 = new jsfeatNext.matrix_t(32, 4, jsfeatNext.U8_t | jsfeatNext.C2_t);
+        expect(c1.cols).toBe(c2.cols); // indistinguishable by the old check
+        expect(() => jsfeatNext.bfmatcher.match(c1, c2)).toThrow(/same row width/);
+        expect(() => jsfeatNext.bfmatcher.knnMatch(c1, c2)).toThrow(/same row width/);
+    });
+
+    it("rejects non-U8 descriptors", () => {
+        // Hamming over an i32 view is only meaningful for packed bytes; F32
+        // bit patterns would be XORed and popcounted into nonsense.
+        const f32 = new jsfeatNext.matrix_t(32, 4, jsfeatNext.F32_t | jsfeatNext.C1_t);
+        const u8 = randomDescriptors(4, 32, 9107);
+        expect(() => jsfeatNext.bfmatcher.match(f32, f32)).toThrow(/must be U8/);
+        expect(() => jsfeatNext.bfmatcher.match(u8, f32)).toThrow(/must be U8/);
+    });
+
+    it("the width-mismatch message names both widths", () => {
+        const q = randomDescriptors(2, 32, 9103);
+        const t = randomDescriptors(2, 64, 9104);
+        expect(() => jsfeatNext.bfmatcher.match(q, t)).toThrow(/32 and 64/);
+    });
+
+    it("without the guard, a narrower train silently yields wrong distances", () => {
+        // Pins WHY the guard earns its place. Reading a 16-byte-wide train set
+        // with the query's 32-byte stride walks two train rows per step and,
+        // past the end, indexes out of range -- undefined, which XOR coerces to
+        // 0. Nothing throws; the distances are simply wrong. Reproduced here
+        // against the honest per-row computation.
+        const q = randomDescriptors(3, 32, 9105);
+        const narrow = randomDescriptors(8, 16, 9106);
+
+        const popcnt = (n: number) => {
+            n -= (n >> 1) & 0x55555555;
+            n = (n & 0x33333333) + ((n >> 2) & 0x33333333);
+            return (((n + (n >> 4)) & 0x0f0f0f0f) * 0x01010101) >> 24;
+        };
+        // what the old code did: stride 8 words for both
+        const qw = q.buffer.i32;
+        const tw = narrow.buffer.i32;
+        let bogus = 0;
+        for (let k = 0; k < 8; k++) bogus += popcnt(qw[k] ^ (tw[k] | 0));
+        // what an honest 16-byte comparison would be: 4 words
+        let honest = 0;
+        for (let k = 0; k < 4; k++) honest += popcnt(qw[k] ^ tw[k]);
+
+        expect(bogus).not.toBe(honest);
+        // and the guard means neither number is ever produced
+        expect(() => jsfeatNext.bfmatcher.match(q, narrow)).toThrow();
+    });
 });
 
 describe("bfmatcher.knnMatch / ratio_test", () => {