Fix/zpl gb thickness promotion#69
Merged
Merged
Conversation
…otion User report: filled box `^GB101,92,101` shows as 101×92 in the editor but Labelary renders it 101×101 (Zebra firmware promotes the rendered height up to `thickness` when `t > h`, per the "horizontal line" rule in the ^GB docs). This test fails on main with ~900 pixel diff along the bottom edge of the box, exactly the dimension gap (`thickness - height = 9 dots`) multiplied by the rect width. Fix lands in the follow-up commit.
Zebra ^GB documents a "horizontal line" as `h <= t, w > t` rendered at `w × t` (not `w × h`), and the symmetric rule for vertical lines and filled rects whose thickness exceeds the smaller axis. The renderers were honouring `width × height` literally, so a user-typed `^GB101,92,101` showed up 9 dots short of what Labelary prints. Fold the promotion into `outlineInset` so both the Konva editor render (KonvaObject) and the @napi-rs/canvas test renderer (shapeRender) read their `renderFilled` width/height from one place. The shape regression test added in the previous commit now passes; obj.props stay untouched because ZPL is the source of truth — the canvas only visualises what the firmware would print.
The renderer-side dimension promotion is moot if the parser hands it a clamped thickness. Two changes restore round-trip fidelity for ^GB fields where thickness exceeds the smaller axis: - zplParser.GB: store the original `t` instead of resetting to 3 when `t >= min(w,h)`. The `filled` flag is still derived for the UI, but thickness now carries the firmware's truth. - box.toZPL: emit `t` directly, only floor up to `min(w,h)` when the user toggled `filled` and the stored thickness is below the solid threshold (preserves the "make this solid" intent for UI-driven edits). Together with the renderer fix in the previous commit, the `^GB101,92,101` regression now renders 101 × 101 in the editor and round-trips to `^GB101,92,101` instead of the lossy `^GB101,92,92`.
- shapeGeometry.outlineInset: tighten the JSDoc; the prior wording hinged on the "horizontal line" wording from the ^GB docs, but the parser routes literal horizontal / vertical lines into `type: 'line'` before they reach this helper. The actual scope is filled rects whose thickness exceeds one of the two axes. - box.toZPL: lift the `min(w, h)` solid threshold into a named local so the nested-conditional reads top-to-bottom. - shapeTestCases: add the symmetric `thickness > width` case and the "exceeds both axes → square" case. Without them a regression that only broke one axis or only handled the non-square promotion would slip past the height-only fixture.
There was a problem hiding this comment.
Code Review
This pull request implements Zebra's dimension promotion logic for ^GB rectangles, ensuring that the rendered field expands to the thickness value when it exceeds the specified width or height. The changes include updates to geometry calculations in shapeGeometry.ts, refactoring the rendering logic in shapeRender.ts, and ensuring lossless thickness preservation during ZPL parsing and generation. A review comment suggests passing an additional parameter to the outlineInset function to explicitly enable this promotion logic for boxes.
Gemini review pointed out that ^GE / ^GC follow a different rule than ^GB: per the Zebra docs they collapse to a solid shape at their declared bbox when thickness exceeds the radius / axes, with no per-axis promotion. Applying `max(w,t) × max(h,t)` unconditionally inside `outlineInset` would silently change ellipse / circle behaviour for very thick outlines. Add a `promoteFilled` flag (defaults to false) and switch only the two ^GB call sites (renderShape + KonvaObject) on. Current ellipse and circle tests still pass; future thick-outline ellipse cases stay on the correct firmware-matching branch.
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.
No description provided.