Repository navigation
Resolution-aware QR label printing (per-device DPI) - #56
Merged
Merged
Conversation
Quick-spec deliverable: decouple QR label rendering from the hardcoded 300 DPI by introducing named printer/device presets (Thermal 203, Brother P-Touch 180, Laser/Inkjet 300/600/1200) and embedding DPI metadata in the PNG, fixing wrong physical print sizes on non-300-dpi printers (thermal clipping, laser mis-scaling). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fix float DPI-metadata assertions, banker's-rounding test bug, missed _draw_wifi_header_row internal call sites, byte-identical backward-compat claim, low-DPI scannability caveat, stale 300-DPI doc lines, non-testable AC4, device-validation test isolation, invalid-device preview clamp, and oversized-guard flash assertion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve dpi-required vs TestFitText contradiction (default dpi=300), fix manual_testing line numbers, add graceful preview-error handling for the oversized-guard trip, split AC8 into automated/manual, fully specify view-test payloads, document the existing WiFi dropdown, add ETag/caching analysis, clarify the device-clamp rationale, and tidy landmark citations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add automated preview-side oversized-400 test (AC5 coverage gap), exact device-preset count test, slow-render cost note, ImageMagick-free DPI check fallback, 50MP memory-budget justification, JS handler attach-point, and staff.md line-range consistency. Third review found no real Critical/High defects — spec has converged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reorder Task 8 test items 5/6 and tighten the download log-line edit instruction. 4th review verdict: converged, ready for development (no blocking issues; existing POST tests verified to stay green via the SelectField default). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Decouple QR label physical size from the hardcoded 300 DPI so labels print at the correct physical dimensions on any printer. Introduce named device/DPI presets, render each label at inches × selected-device-DPI, and embed DPI metadata in the PNG so the OS/CUPS "actual size" path reproduces the intended physical size. - qr_service: add QRDevicePreset/QR_DEVICE_PRESETS/QR_DEVICES_BY_KEY (5 presets, default laser_300); replace the _DPI global with a per-call dpi threaded through _px, _pt_to_px, render_qr_png, and all nested _draw_*/_fit_text call sites; embed dpi=(d,d) PNG metadata; add a 50 MP oversized-canvas guard and a dpi<=0 guard - forms: add a device SelectField to QRGenerateForm - views: resolve device->DPI in qr() and qr_preview() (400 on unknown device), clamp invalid device on re-render, encode the device key in the download filename, log the selected device - template/JS: device selector, device in initial + live preview URL, device-aware caption, graceful preview-failure message via onerror/onload - tests: device-preset structure, DPI-driven dimensions, embedded-metadata round-trip (180/203/300/600/1200), oversized guard, and view-level device dropdown/download/preview coverage - docs: staff.md and manual_testing.md section 11 device steps; regenerated qr-generation.png screenshot Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request makes QR label rendering resolution-aware by introducing named printer/device DPI presets, rendering PNGs at physical_inches × selected_dpi, and embedding DPI metadata so labels print at the intended physical size across different printer resolutions.
Changes:
- Added device/DPI presets and threaded
dpithrough QR rendering, including an oversized-canvas guard and embedded PNG DPI metadata. - Wired device selection through the QR form, download/preview views, template, and live preview URL generation.
- Expanded tests and updated staff/manual-testing documentation to cover per-device sizing/metadata and guard behavior.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
esb/services/qr_service.py |
Adds device presets, per-call DPI rendering, embedded PNG DPI metadata, and an oversized-canvas safeguard. |
esb/forms/equipment_forms.py |
Introduces the new device SelectField backed by device presets. |
esb/views/equipment.py |
Resolves selected device → DPI for download/preview, clamps invalid device on re-render, and encodes device in filename/logging. |
esb/templates/equipment/qr.html |
Adds the device dropdown and includes device in the initial preview URL plus a preview error message element. |
esb/static/js/app.js |
Adds device to live-preview query params and toggles a human-readable preview error message via onerror/onload. |
tests/test_services/test_qr_service.py |
Adds coverage for device presets, DPI-driven dimensions, embedded DPI metadata, and oversized-canvas guard behavior. |
tests/test_views/test_equipment_views.py |
Adds view/template coverage for device dropdown, device-aware download/preview, invalid-device handling, and oversized guard UX. |
docs/staff.md |
Updates staff-facing instructions to explain the device selector and low-DPI tradeoffs. |
docs/manual_testing.md |
Updates manual verification steps for per-device pixel sizing, DPI metadata verification, and oversized guard behavior. |
_bmad-output/implementation-artifacts/tech-spec-resolution-aware-qr-printing.md |
Adds an implementation tech spec describing the approach, risks, and acceptance criteria. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add role="alert" and aria-live="assertive" to #qr-preview-error so screen readers announce the preview-failure message when it is toggled visible. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…test - equipment_forms: use the service's DEFAULT_DEVICE_KEY for the device field default instead of hardcoding 'laser_300', avoiding drift if it changes. - test_qr_service: replace the heavy letter@600 full render (~101 MB canvas) with an arithmetic check of the oversized-guard pixel-count condition. Keep the single sticker_4@1200 render as the lone end-to-end large-canvas test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…vice param - Pass DEFAULT_DEVICE_KEY into the qr.html render context and use it for the initial preview src fallback instead of the literal 'laser_300', so the preview can't drift from the canonical default device. - Document the device query param in the qr_preview() docstring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Summary
QR labels were rendered at a hardcoded 300 DPI with no DPI metadata, so they printed at the wrong physical size on any printer that isn't 300 DPI — a 1200 DPI laser shrank a "4×4" label to ~1″, and a 203 dpi thermal printer overflowed and clipped it.
This change decouples physical size from a fixed DPI:
Laser/Inkjet (300/600/1200 dpi),Thermal Label (203 dpi),Brother P-Touch (180 dpi). Default islaser_300(backward-compatible: same pixel dimensions as before).inches × selected-device-DPI, and the DPI is embedded in the PNG (pHYschunk) so the OS/CUPS "actual size" print path reproduces the intended physical dimensions. Correct pixel count fixes thermal clipping; embedded DPI fixes desktop mis-scaling.dpi <= 0defensive guard.Changes
qr_service.py:QRDevicePreset/QR_DEVICE_PRESETS/QR_DEVICES_BY_KEY; replaced the_DPIglobal with a per-calldpithreaded through_px,_pt_to_px,render_qr_png, and every nested_draw_*/_fit_textcall site; embedded PNG DPI metadata; canvas guards.equipment_forms.py: newdeviceSelectField.equipment.py: bothqr()andqr_preview()resolve device→DPI (400 on unknown device), clamp invalid device on re-render, encode the device key in the download filename, and log the selected device.qr.html+app.js: device selector, device in the initial + live-preview URL, device-aware caption, and a graceful preview-failure message viaonerror/onload.staff.mdandmanual_testing.md§11 device steps; regeneratedqr-generation.pngscreenshot.Testing
make lintclean.Notes
🤖 Generated with Claude Code