Skip to content

fix(dc_measurements): retract the QR in-plane rotation limitation - #359

Merged
Minipada merged 2 commits into
jazzyfrom
feature/297-zxing-c-fails-to-decode-some-qr-codes-ro
Aug 18, 2026
Merged

fix(dc_measurements): retract the QR in-plane rotation limitation#359
Minipada merged 2 commits into
jazzyfrom
feature/297-zxing-c-fails-to-decode-some-qr-codes-ro

Conversation

@Minipada

Copy link
Copy Markdown
Owner

What this is

#297 asked three questions in order: does a newer ZXing-C++ fix the reported in-plane rotation failure, does it matter in practice, and if so is a bounded rotation-retry worth its per-frame cost.

The answers are no, no, and moot — the reported limitation does not exist. #123 and the camera.md warning it left behind were both measuring the harness, not the library.

What was actually happening

The benchmark rotated the demo texture inside its own 290x365 bounds. qrcode_0001.png's module area is 210 px at (45, 100) — the printed label takes the rest of the height, so the code sits above the centre the rotation turns about. Swing that and a corner of the module area, finder pattern and all, leaves the frame.

Forward-mapping the four module corners through the same getRotationMatrix2D the harness used says exactly when:

angle module corners outside the frame decodes
0/4
15° 0/4
30° 1/4
45° 1/4
60° 1/4
75° 1/4
90° 0/4

That is, to the angle, the reported pass/fail pattern. A QR the frame has cut a finder pattern off is unreadable at any angle by any decoder. The causes #297 ruled out — interpolation blur, module size, every Binarizer, tryDownscale — were all ruled out correctly; the one variable nobody varied was the frame.

Confirmed against the production library, not a binding

A standalone C++ program calling ZXing::ReadBarcodes the way camera.cpp does, compiled inside localhost/dc-workspace:latest against the exact libzxing-dev 2.2.1-3 that rosdep resolves on Noble/Jazzy:

clipped (rotated in the texture's own bounds)  0:ok 15:ok 30:MISS 45:MISS 60:MISS 75:MISS 90:ok
fully framed (padded to the diagonal)          0:ok 15:ok 30:ok   45:ok   60:ok   75:ok   90:ok

The PyPI bindings at 2.3.0 and 3.1.1 give identical results in both columns. So there is nothing for a newer release to fix — no vendoring, no rosdep pin — and the rotation-retry mitigation would not have worked anyway: re-rotating an image whose corner was already cropped cannot put the pixels back.

What changed

  • dc_measurements/test/test_barcode_rotation.cpp (new) — two tests over the real reader, with no dc_simulation asset dependency; it reconstructs the texture's layout to the pixel with ZXing's own writer, the way test_code_pose already does.
    • DecodesACodeAtEveryInPlaneRotation — fully framed, decodes at every 5° from 0° to 90°. This is the assertion the retracted claim would fail.
    • LosesACodeTheFrameClips — rotated inside the texture's own bounds, asserts decode succeeds iff all four module corners stay inside the frame, so the test pins the cause and not just the symptom.
  • doc/src/dc/measurements/camera.md — the admonish block goes from a warning describing a library defect to an info describing the framing requirement that actually matters: keep the whole code plus its quiet zone in the image, and in-plane angle is a non-issue. It also notes rotation_angle is unrelated, since whole quarter turns never crop. The perspective/tilt half of the Move QR/barcode detection to a more reliable C++ library (ZXing-C++) #123 comparison (100% vs. 0% for zbar at 35% skew) was independently measured and stands unchanged.
  • No change to camera.cpp — nothing to mitigate, so no new parameter and no per-frame cost.

Verification

  • colcon build --packages-up-to dc_measurements in localhost/dc-workspace:latest with this worktree bind-mounted: clean.
  • colcon test --packages-select dc_measurements --ctest-args -R "test_barcode_rotation |test_code_pose"10 tests, 0 errors, 0 failures, 0 skipped (8 gtest cases across the two binaries plus their 2 ctest entries), against libzxing 2.2.1.
  • prek run --all-files --skip build-doc → all 24 hooks pass, including clang-format and the REUSE licence check on the new file.

Closes #297

🤖 Generated with Claude Code

https://claude.ai/code/session_01L6PGKrx3v8rc8E1b6yrNpd

@Minipada
Minipada force-pushed the feature/297-zxing-c-fails-to-decode-some-qr-codes-ro branch from 1cb505e to 64774ef Compare August 18, 2026 07:59
Minipada added a commit that referenced this pull request Aug 18, 2026
…cally

Two halves of the same gap, both found by #359's Format failure.

CI ran prek over the whole tree on every PR, so a PR paid for files it never
touched. It now runs `--from-ref origin/<base> --to-ref HEAD`, checking only what
the PR changed; a push to jazzy still re-checks everything, so nothing a PR never
touched can rot unnoticed. The checkout needs full history for the base ref to
exist to diff against.

Locally, `prek run --all-files` means all *tracked* files -- a new file that is not
staged yet is skipped silently, which is exactly how #359's unformatted test file
passed a local run and failed CI. CLAUDE.md's documented command stages first.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L6PGKrx3v8rc8E1b6yrNpd
Signed-off-by: David Bensoussan <d.bensoussan@proton.me>
@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.20%. Comparing base (f4cad58) to head (f75c6d8).

Additional details and impacted files
@@           Coverage Diff           @@
##            jazzy     #359   +/-   ##
=======================================
  Coverage   67.20%   67.20%           
=======================================
  Files          96       96           
  Lines        5981     5981           
=======================================
  Hits         4019     4019           
  Misses       1962     1962           
Flag Coverage Δ
cpp-jazzy 67.20% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Minipada and others added 2 commits August 18, 2026 10:35
30-45 degrees in-plane. There is no such failure. The benchmark behind it rotated
dc_simulation's demo texture inside the texture's own 290x365 bounds; the printed
label offsets the 210 px code from the centre it turns about, so between 30 and 75
degrees a corner of the code -- finder pattern and all -- leaves the frame. A QR the
frame has clipped is unreadable at any angle by any decoder.

Confirmed against libzxing-dev 2.2.1-3, the version rosdep resolves on Noble/Jazzy,
calling ZXing::ReadBarcodes the way camera.cpp does: fully framed, it decodes at
every angle from 0 to 90 degrees; rotated inside the texture's bounds it misses at
exactly the angles that clip a corner. The PyPI bindings at 2.3.0 and 3.1.1 behave
identically, so there is nothing for a newer release to fix, and the rotation-retry
mitigation the issue floated would not have worked -- re-rotating an image whose
corner was cropped cannot put the pixels back.

test_barcode_rotation locks both halves down: decoding at every 5 degrees when fully
framed, and decoding iff all four module corners stay inside the frame when not, so
the test pins the cause rather than the symptom. camera.md's warning about a library
defect becomes an info block about the framing requirement that does matter. No
change to camera.cpp: nothing to mitigate, so no new parameter and no per-frame cost.

Closes #297

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L6PGKrx3v8rc8E1b6yrNpd
Signed-off-by: David Bensoussan <d.bensoussan@proton.me>
…cally

Two halves of the same gap, both found by #359's Format failure.

CI ran prek over the whole tree on every PR, so a PR paid for files it never
touched. It now runs `--from-ref origin/<base> --to-ref HEAD`, checking only what
the PR changed; a push to jazzy still re-checks everything, so nothing a PR never
touched can rot unnoticed. The checkout needs full history for the base ref to
exist to diff against.

Locally, `prek run --all-files` means all *tracked* files -- a new file that is not
staged yet is skipped silently, which is exactly how #359's unformatted test file
passed a local run and failed CI. CLAUDE.md's documented command stages first.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L6PGKrx3v8rc8E1b6yrNpd
Signed-off-by: David Bensoussan <d.bensoussan@proton.me>
@Minipada
Minipada force-pushed the feature/297-zxing-c-fails-to-decode-some-qr-codes-ro branch from 1c42efe to f75c6d8 Compare August 18, 2026 08:35
@Minipada
Minipada merged commit 83d8894 into jazzy Aug 18, 2026
9 checks passed
@Minipada
Minipada deleted the feature/297-zxing-c-fails-to-decode-some-qr-codes-ro branch September 2, 2026 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant