Skip to content

Commit 168786c

Browse files
kalwaltclaude
andcommitted
fix(simple,marker): honour the buff/pixel_format contract (#103)
Closes the symptom from issue #103 (cf ~= 0.0965 on hiro template). The library extraction was already correct (cf ~= 0.89 in the PR-A diagnostic on the same path); the bug was that simple.rs declared ar_pixel_format = MONO but fed an RGBA buffer into AR2VideoBufferT.buff, violating the contract documented at artoolkit5/video2.c:682: when pixel_format == MONO, buff IS the luma data. Fixes: - simple.rs now feeds the luma buffer into both buff and buff_luma (matching pixfmt = MONO), with a comment pointing at the upstream contract. - ar_detect_marker now validates that frame.buff.len() matches xsize * ysize * pixel_size (and buff_luma matches xsize * ysize), emitting arlog_e! and returning Err on mismatch — turns silent contract violations into loud, immediate failures. - ar_detect_marker doc comment grew a "Frame buffer contract" section documenting the rule and linking issue #103. After this change, the simple example reports cf=0.8925 and id=0 on benchmarks/data/img.jpg (was cf=0.0965, id=-1 before). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 5c36a3b commit 168786c

2 files changed

Lines changed: 57 additions & 4 deletions

File tree

crates/core/examples/simple.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -218,12 +218,15 @@ fn main() {
218218

219219
let luma_buffer_vec = luma_img.into_raw();
220220
let mut out_img = color_img.clone();
221-
let color_buffer_vec = color_img.into_raw();
222221

223-
// Construct the AR2VideoBufferT pointing to our image data
222+
// ARToolKit5's contract (cf. artoolkit5/video2.c:682): `buff` MUST hold
223+
// data in the format declared by ar_pixel_format. When pixel_format is
224+
// MONO, that means buff IS the luma data — buffLuma is just an alias.
225+
// Feeding RGBA into buff while declaring MONO produces silent low-cf
226+
// detections (see issue #103).
224227
let frame = AR2VideoBufferT {
225-
buff: Some(color_buffer_vec), // Color data
226-
buff_luma: Some(luma_buffer_vec), // Gray data used for binarization
228+
buff: Some(luma_buffer_vec.clone()), // pixfmt=MONO → buff IS luma
229+
buff_luma: Some(luma_buffer_vec.clone()),
227230
buf_planes: None,
228231
buf_plane_count: 0,
229232
fill_flag: true,

crates/core/src/ar/marker.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,19 @@ pub enum ImageProcMode {
8282
///
8383
/// Results are written into `ar_handle.marker_info` (up to `AR_SQUARE_MAX` markers).
8484
///
85+
/// # Frame buffer contract
86+
///
87+
/// `frame.buff` MUST hold data in the format declared by
88+
/// `ar_handle.ar_pixel_format`. ARToolKit5's `video2.c:682` documents this
89+
/// contract: when `ar_pixel_format == MONO`, `buff` IS the luma data
90+
/// (and `buff_luma` is just an alias to it); otherwise `buff` holds the
91+
/// declared format and `buff_luma` is a separately-converted greyscale view.
92+
///
93+
/// Mismatching the buffer layout against the declared format used to
94+
/// produce silent low-confidence matches (see
95+
/// [issue #103](https://github.com/webarkit/WebARKitLib-rs/issues/103));
96+
/// `ar_detect_marker` now rejects such mismatches up front.
97+
///
8598
/// Mirrors the C `arDetectMarker()` entry point from `arDetectMarker.c:80-322`.
8699
///
87100
/// # Example
@@ -131,6 +144,43 @@ pub fn ar_detect_marker(
131144
}
132145
};
133146

147+
// Frame-buffer contract (artoolkit5/video2.c:682, issue #103): when
148+
// ar_pixel_size > 0 (i.e. a known fixed bytes-per-pixel format), `buff`
149+
// length must equal xsize * ysize * pixel_size. Sub-sampled formats
150+
// (NV21, 420v, 420f) have pixel_size == 0 and are skipped.
151+
let pixel_size = ar_handle.ar_pixel_size as usize;
152+
if pixel_size > 0 {
153+
let expected = (ar_handle.xsize as usize) * (ar_handle.ysize as usize) * pixel_size;
154+
if color_buff.len() != expected {
155+
arlog_e!(
156+
"ar_detect_marker: AR2VideoBufferT.buff length {} does not match \
157+
expected {} for {}x{} {:?} (pixel_size={}). \
158+
buff must hold data in the format declared by ar_pixel_format \
159+
(cf. artoolkit5 video2.c:682).",
160+
color_buff.len(),
161+
expected,
162+
ar_handle.xsize,
163+
ar_handle.ysize,
164+
ar_handle.ar_pixel_format,
165+
pixel_size
166+
);
167+
return Err("AR2VideoBufferT.buff length mismatches ar_pixel_format");
168+
}
169+
}
170+
// Buff_luma is always 1 byte per pixel.
171+
let expected_luma = (ar_handle.xsize as usize) * (ar_handle.ysize as usize);
172+
if luma_buff.len() != expected_luma {
173+
arlog_e!(
174+
"ar_detect_marker: AR2VideoBufferT.buff_luma length {} does not match \
175+
expected {} for {}x{} (1 byte/pixel)",
176+
luma_buff.len(),
177+
expected_luma,
178+
ar_handle.xsize,
179+
ar_handle.ysize
180+
);
181+
return Err("AR2VideoBufferT.buff_luma length mismatches xsize*ysize");
182+
}
183+
134184
let thresh = ar_handle.ar_labeling_thresh as u8;
135185
// TODO: port AR_LABELING_THRESH_MODE_*_AUTO variants (bracketing, median, etc.) from C
136186
let label_mode = if ar_handle.ar_labeling_mode == 0 {

0 commit comments

Comments
 (0)