Skip to content

fix(testdata): normalize diff artifacts to 813x1626 canvas - #38

Open
GOODBOY008 wants to merge 3 commits into
mainfrom
fix/normalize-diff-artifacts-canvas
Open

fix(testdata): normalize diff artifacts to 813x1626 canvas#38
GOODBOY008 wants to merge 3 commits into
mainfrom
fix/normalize-diff-artifacts-canvas

Conversation

@GOODBOY008

Copy link
Copy Markdown
Owner

问题

testdata/diffs/ 中部分 diff 产物尺寸不一致,根源有两类:

1. 过期残留(gb_normal / gb_0_width / gb_0_height / postnl_qr / qr_code_offset)

这三个标签以及 postnl_qr、qr_code_offset 的 *_diff.png400×640 的旧画布时代(50×80mm 默认画布)产物。这些标签现在与 Labelary 渲染完全一致(PERFECT 0.00%),管线不再生成 diff 图,旧文件就永远留在仓库里。font_p/font_q/font_s 也有同样的 813×1626 时代残留。

2. 引用 PNG 尺寸不一致(7 个标签)

cf_font_designatorcf_font_no_orientationcode128_mode_d_fnc1cp850_hex_charsfo_lenient_coordmaxicode_default_mode2maxicode_mode4 的 Labelary 引用 PNG 是 812×1624(服务端浮点舍入),其他全部是 813×1626。导致每次 diff 报告都生成 1629×1626 对比图、812×1624 diff 图,并多计 1px 列的"伪差异"。

修复

  • 将 7 个 unit 引用 PNG 补白为 813×1626(bootstrap 对新抓取的 PNG 本来就做同样的归一化)
  • 删除 8 个过期 *_diff.png
  • e2e_diff_report 增加自愈:某标签无 diff 时自动清理遗留的旧尺寸 diff 图,防止画布变更后残留

验证

  • 所有 _diff.png 统一为 813×1626,所有对比图为 1630×1626,报告中无尺寸不一致行
  • 7 个标签 diff% 下降(多计的 1px 列不再计入):
    标签 之前 之后
    fo_lenient_coord 0.27% 0.02%
    maxicode_default_mode2 0.84% 0.59%
    maxicode_mode4 0.83% 0.58%
    code128_mode_d_fnc1 0.48% 0.23%
    cp850_hex_chars 0.47% 0.23%
    cf_font_designator 0.38% 0.13%
    cf_font_no_orientation 0.43% 0.18%
  • 完整 e2e_golden 109 个测试全过,unit_golden_png_dimensions 通过

Copilot AI lite review requested due to automatic review settings September 2, 2026 15:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new “perfect match” cleanup path is keyed off diff_image == None, which can also occur on dimensions-only mismatches, potentially deleting useful artifacts and mislabeling a mismatch as perfect.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR addresses inconsistent image dimensions in testdata/diffs/ artifacts by normalizing diff outputs to the standard 813×1626 canvas and preventing legacy diff images from lingering when a label now renders without pixel differences.

Changes:

  • Add a self-healing cleanup step in e2e_diff_report to remove stale *_diff.png artifacts when no diff image is produced.
  • Introduce remove_diff_image() helper in tests/common/image_compare.rs to delete previously saved diff images.
  • Regenerate testdata/diffs/diff_report_unit.txt to reflect normalized dimensions and updated diff percentages.
File summaries
File Description
tests/e2e_diff_report.rs Removes stale diff artifacts when the comparison produces no diff image.
tests/common/image_compare.rs Adds helper to delete previously saved diff images in testdata/diffs/.
testdata/diffs/diff_report_unit.txt Updates the unit diff report after normalization (dims and diff% changes).
Review details
  • Files reviewed: 3/32 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/e2e_diff_report.rs
Comment on lines 145 to 151
if let Some(ref diff_img) = result.diff_image {
image_compare::save_diff_image(&name, diff_img);
} else {
// Perfect match — drop any stale diff image left over from an older canvas
// size (the pipeline no longer produces one, so it could never be refreshed).
image_compare::remove_diff_image(&name);
}
@GOODBOY008
GOODBOY008 force-pushed the fix/normalize-diff-artifacts-canvas branch 2 times, most recently from db0235d to 6d2abaa Compare September 2, 2026 15:35
Several diff artifacts in testdata/diffs/ were stale or mismatched:
- gb_normal/gb_0_width/gb_0_height, postnl_qr, qr_code_offset: leftover
  400x640 diff images from an old canvas era; these labels are now
  PERFECT so the pipeline never regenerated them
- cf_font_designator, cf_font_no_orientation, code128_mode_d_fnc1,
  cp850_hex_chars, fo_lenient_coord, maxicode_default_mode2,
  maxicode_mode4: Labelary reference PNGs were 812x1624, so every
  diff run produced 1629x1626 comparisons and 812x1624 diffs

Fix: pad the 7 unit references to 813x1626 (same normalization the
bootstrap already applies to new fetches), drop the stale diff images,
and make the diff report self-healing — when a label has no diff,
remove any leftover diff image so old canvases can't linger.

Diff percentages drop for the 7 labels since the spurious 1px column
no longer counts (e.g. fo_lenient_coord 0.27% -> 0.02%, maxicode
labels 0.84%/0.83% -> 0.59%/0.58%). All e2e_golden tests still pass.
@GOODBOY008
GOODBOY008 force-pushed the fix/normalize-diff-artifacts-canvas branch from 6d2abaa to fdb6512 Compare September 2, 2026 15:52
Labels improved:
- font_p: 0.54% → 0.16% (GOOD <1%)

Root cause: ^APN,,30 was treated as height=width=30 via the generic
scalable-font path, but Labelary renders font P (Letter Gothic) at a
15-dot default em with the width parameter setting the character
advance linearly (~0.44 x w).

Changes:
- src/elements/font.rs: font P defaults to 15 dots when h/w omitted
  (measured: ^APN,,15 == ^APN,,; ^APN,,30 == 2x width); advance ratio
  0.90 (Labelary ~0.44em advances vs DejaVu Mono Bold's 0.518em)
- src/drawers/renderer.rs: cap scale 1.10 + low-cap anchor 0.30em,
  calibrated against the Labelary reference (sweep 0.80-0.95 ratio,
  1.0-1.4 cap, 0.0-0.30em anchor)
- docs/DIFF_THRESHOLDS.md: add font_p row (0.16 / tolerance 1.0)
- testdata/diffs: regenerated font_p comparison + diff images
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.

2 participants