Feat/shape pixel tests#55
Merged
Merged
Conversation
…abelary Mirrors the bwip-js barcode regression infrastructure for the geometric primitives. A pure 2D-canvas renderer in src/lib/shapeRender.ts produces Option-A ZPL-aligned geometry (outline thickness extrudes inward for ^GB/^GE/^GC, downward/rightward for axis-aligned ^GB lines) and the test diffs it pixel-for-pixel against Labelary references. Coverage: 15 active fixtures (boxes outline/filled/thickness sweep, horizontal/vertical lines with reverse-direction cases, ellipse, circle), plus 4 fetched-but-skipped diagonal fixtures awaiting the GD renderer. The Konva canvas in production still uses centred-stroke geometry and therefore mismatches the renderer / Labelary; the follow-up commit aligns KonvaObject/LineObject to renderShape.
Parallelogram with horizontal short sides (top edge flat, sides pointy), thickness extruded downward from the diagonal centreline. Matches Zebra firmware exactly to within rasterisation tolerance; the four previously skipped diagonal fixtures (slash/backslash 45 deg, shallow 30 deg, steep 60 deg) now run as active regression cases. Bumps the per-test diff budget to 1500 px and the pixelmatch per-pixel threshold to 0.3 to absorb the AA halo @napi-rs/canvas produces along diagonal edges (Labelary renders 1-bit binary, so the halo is unavoidable on our side without a custom rasteriser). Axis- aligned cases still finish at sub-50 px diff so the budget is not meaningfully eroded for them.
The Konva canvas previously rendered shape outlines with a centred stroke that straddled the declared bounding box. ZPL extrudes thickness inward from the box for ^GB/^GE/^GC and downward/rightward from (x, y) for axis-aligned ^GB lines, so the designer view drifted from what Labelary and the printer produce. Box / ellipse / circle now inset the body by t/2 (centred stroke fills the outer band exactly), with the firmware's 'clamp to solid when 2t >= min(w, h)' rule applied so very-thick outlines collapse cleanly. Axis-aligned lines shift the visible body by t/2 perpendicular along the ZPL extrusion axis; handles stay at the conceptual band corner. The box selection overlay is decoupled from the body thickness (now constant 1.5 px) so a thick outline no longer gets a thick selection halo. Diagonal lines still fall back to the centred-stroke rendering and remain visibly off vs. ^GD's parallelogram geometry; a follow-up commit replaces them with a closed Konva.Line polygon.
The previous parallelogram straddled the bbox diagonal, putting half the thickness above and half below the centreline. Pixel inspection of Labelary references shows Zebra actually places the conceptual line along the *left long edge* of the band and extrudes thickness in +x only — both endpoints sit on the same side, the band overhangs the declared bbox on the right by t. Updates renderShape.ts and the matching Konva polygon in LineObject.tsx, so the canvas preview reflects what gets printed. Tests now pass at strict tolerance (200 px / threshold 0.1) including all four diagonal fixtures — previously these only passed at the AA-absorbing 1500 / 0.3 budget because the geometry was visibly off at the corners.
The Konva render switched between the axis-aligned band shape and the diagonal parallelogram based on p.angle, which only commits on dragEnd. Dragging a near-horizontal endpoint slightly off-axis therefore showed the body locked to the horizontal band until release, then snapped to the parallelogram in one frame. Reading isHorizontal / isVertical off the live dispX/dispY pair (with a 0.5 px epsilon to absorb constrainLine's auto-snap residue) makes the preview track the geometry that will actually be committed, so the release no longer changes shape.
The diagonal-polygon and outline-inset math previously lived in two places: the @napi-rs pixel-regression renderer (lib/shapeRender.ts) and the Konva canvas components (LineObject.tsx, KonvaObject.tsx). Both described the same ZPL semantics but were maintained independently, so a future change to the firmware-clamp threshold or the ^GD parallelogram shape risked silent drift between the two render paths. New module lib/shapeGeometry.ts owns the pure helpers (outlineInset for ^GB/^GE/^GC bounding-box adjustments, diagonalPolygonPoints for ^GD parallelogram vertices). Both renderers import from there, so the pixel-regression tests transitively validate the Konva canvas too. No behaviour change; all 680 tests still green.
Direct unit tests for the pure helpers — the pixel-regression suite covers the rendered output but logic errors in the helpers would only surface as cross-test diffs there. Cheap to add, faster signal.
Adds a draggable square on the far long edge of the band (bottom edge for horizontal lines, right edge for vertical / diagonal) that resizes thickness in real time. The handle follows the band edge as the user drags, clamped to a 1-dot minimum. Implementation lives entirely in LineObject; thickness during the drag is tracked in component state and committed on dragEnd, so the live preview matches what gets stored — no release-time snap. The flip-on- overshoot affordance the user described earlier (rotate 180 deg to put thickness on the other side, skipping zero) is intentionally deferred to a follow-up.
…rence The 15-dot rotation offset compensated for the previous off-by-t/2 centred-stroke line geometry rather than a real Konva-vs-ZPL pivot mismatch — once the lines render at the correct ZPL position, rotated text sits where it should without any extra shift. Removes the offset constant, the rotationOffsetDelta helper, and the related dead branches in objectToDisplay / displayToObject. The remaining transform is the ^FT baseline correction; ^FO becomes the identity. Tests updated accordingly.
Three smells from a clean-code pass over the branch: 1. LineObject had a thicknessDragRef whose value was written on dragStart and cleared on dragEnd but never read — dragMove computed everything from the live cursor position via the JSX closure. Dead weight, removed. 2. The thickness handle was rendered through an IIFE inside the JSX body just to scope four geometry locals. Hoisted those into the component body and flattened the JSX. 3. diagonalPolygonPoints returned a generic number[], which forced shapeRender to either non-null-assert each index or cast to an 8- tuple. Typed the return as ParallelogramPoints so destructuring is ergonomic without the cast. No behaviour change; all 689 tests still green.
EllipseProps and CircleProps carry differently-named size keys (width/height vs diameter) — sharing one switch arm forced a narrowing ternary on the union, which strictTS in CI rejected. Extract the actual drawing into drawEllipticalOutline so each case arm pushes the type-specific keys through a single normalised signature.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements accurate ZPL shape rendering for boxes, ellipses, circles, and lines by mirroring Zebra firmware semantics, including inward outline extrusion and parallelogram geometry for diagonal lines. It introduces a shared geometry library, a 2D canvas renderer, and a visual regression suite using Labelary references. Review feedback identifies a bug where selection highlights disappear on filled ellipses, suggests optimizing diagonal line point calculations, and notes that the new box renderer currently ignores the rounding property.
1. (HIGH) Ellipse/circle selection visual disappeared whenever renderFilled forced strokeWidth to 0 — i.e. filled shapes and very thick outlines that hit the firmware clamp. Reordered the ternary so isSelected always wins, giving the selection a 1.5 px halo regardless of fill state. 2. (MEDIUM) diagonalPolygonPoints was called twice per render for selected diagonals (body + outline). Hoisted the call to the component body so both <KLine>s share the same vertex array. 3. (MEDIUM) renderShape ignores ^GB rounding. No regression-fixture exercises rounding>0 today, so a speculative implementation would not be validated. Added a TODO that points at the existing KonvaObject rounding formula and outlines the evenodd-roundRect approach for whenever the fixture is added.
Owner
Author
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces standardized geometric rendering for ZPL shape primitives (box, line, ellipse, circle) to ensure consistency between the Konva canvas, ZPL output, and a new pixel-regression test suite. Key changes include the addition of a shapeGeometry library for calculating inward extrusion and parallelogram vertices, a shapeRender utility for 2D canvas rendering, and a visual regression suite that compares local renders against Labelary reference images. Additionally, text position transforms were simplified by removing empirical rotation offsets, and interactive thickness handles were added to the line component. Feedback was provided regarding the error handling in the shape renderer, suggesting a more graceful failure mode for unsupported object types to improve future UI integration.
Reviewer suggested graceful failure for the unsupported-type case, based on the Phase-2 TODO in the file header that anticipated Konva consuming renderShape directly. That refactor took a different shape: both renderShape and the Konva components share lib/shapeGeometry, and renderShape itself stayed test-only. With that scope, a loud throw is the right behaviour — any non-shape object reaching this function is a test-author bug, not a runtime condition. Updated the header comment to reflect the actual architecture and expanded the default-case comment to spell out why the throw stays.
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
1 participant
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.
No description provided.