Tier 4: BMP/GIF export, builder_recipe unification, hidden presets#4
Merged
Conversation
UI Builder: - Move presets behind a 'Presets' button next to 'Clear' so users opt in. Clicking opens an absolute-positioned popover with engine-grouped buttons; click-outside or close button dismisses it. Appends layers + resizes canvas (no confirm — the visible opt-in replaces the previous always-visible default). - Add 'Export BMP' button. Browsers don't expose image/bmp via canvas.toBlob, so the export path reads getImageData() and runs a small in-tree BGRA encoder. - SupportedExportFormat now includes 'bmp'. Core (packages/core/src/bmp.ts): - Pure encodeBmp(rgba, width, height) → Uint8Array. 14-byte file header + 40-byte BITMAPINFOHEADER + bottom-up BGRA pixels padded to 4-byte rows. Used by both web OffscreenCanvas and CLI @napi-rs/canvas backends. Schemas: - Add 'bmp' to the image format enum in manifest.schema.json and to the Format TS type. Tests: - packages/core/tests/bmp.test.ts: file header, BGRA channel swap, bottom-up row order, row padding, invalid dimensions and buffer-length errors.
- packages/core/src/gif.ts: GIF89a single-frame encoder. Web-safe 6x6x6 palette + 40-entry grayscale ramp, LZW compression with dictionary reset at 4096 codes. Transparent pixels (alpha < 128) route to palette index 0 and a Graphics Control Extension declares the transparency. - Wire 'image/gif' through generateJob's formatToMime and both canvas backends (web OffscreenCanvas + CLI @napi-rs/canvas). - Add 'gif' to manifest schema enum and Format TS type. - UI Builder: add 'Export GIF' button. - Validation test updated to use 'tiff' instead of 'gif' for the non-enum rejection case.
…ation) The spec says a manifest can reference a builder recipe so the UI Builder's editor output ships as the actual asset bytes. Until now the builder_recipe field wasn't on the manifest schema and generateJob never looked at it. - packages/schemas/src/types.ts: add optional builder_recipe to DimensionalAsset so every image-style asset kind can carry one. - packages/schemas/src/manifest.schema.json: add builder_recipe ref to all four image-style asset definitions (image, sprite_sheet, tileset, ui_panel). Fixed a duplicate 'webp' enum entry on uiPanelAsset that snuck in earlier. - packages/core/src/builderRender.ts: new env-agnostic renderer used by generateJob's recipe path. Renders rect, circle, line, text, filled-shape, and raster with the same solid-fill + stroke + opacity + rotation + effects as the web renderer. Image/pattern fills degrade to the fallback color (no OffscreenCanvas in Node); one bad layer is caught and skipped so a malformed recipe can't kill the whole asset. - packages/core/src/render.ts: expanded the structural Canvas2D interface with the extra methods the recipe renderer needs (save/restore, translate/rotate, beginPath/ellipse/arcTo, etc). - packages/core/src/generate.ts: when an asset carries a builder_recipe, render through renderBuilderRecipe instead of drawAsset. The recipe's width/height (if set) override the asset's nominal bounds. - packages/core/src/validation.ts: register builderRecipeSchema before compiling manifestSchema so the $ref resolves. Tests: - packages/core/tests/builderRender.test.ts: 5 unit tests for renderBuilderRecipe (rect, text, effects + clear, hidden layer skip, object-fill degradation). - packages/core/tests/bmp.test.ts + gif.test.ts from earlier rounds remain green. - apps/cli/tests/e2e.test.ts: new e2e that runs generateJob on a manifest with a builder_recipe and asserts a valid PNG is produced (signature + non-empty bytes). - tests/e2e/placeholderer.spec.ts: new Playwright test that asserts the Presets button is hidden by default and opens a popover on click, with engine-grouped preset buttons.
Five issues caught by T-Rex in this round: 1. GIF opaque-black becomes transparent (gif.ts): Transparent slot was index 0, but rgbToIndex(0,0,0) also returns 0, so opaque black decoded as transparent. Moved the transparent slot to index 255, which rgbToIndex never produces (all opaque RGB maps to the 0..215 cube range). 2. Presets trigger can't close itself (UIBuilder.tsx): mousedown on the Presets button fired the outside-click handler (button not in [data-presets-popover]) and closed the popover; the click handler then toggled it back open. Added [data-presets-trigger] attribute and skip it in the outside-click check, so the trigger can close its own popover. 3. Object fills lose content (builderRender.ts): image/pattern fills were silently rendering as the fillToColor fallback. Switched to throwing — generateJob catches the error and pushes a per-asset error so the manifest report lists the missing capability instead of shipping incomplete bytes. 4. Raster layers are dropped (builderRender.ts): drawRaster was a no-op. Replaced with a thrown error so recipes containing raster layers surface in the manifest report rather than silently producing asset bytes without the imported image. 5. Sprite sheets with builder_recipe lose frames (generate.ts): A sprite_sheet carrying a builder_recipe + frame_duration_ms rendered as one still image but the sidecar pass still wrote animation.json claiming rows*columns frames. Added an explicit rejection in generateJob so the manifest report surfaces the mismatch. Also: GIF only emits the GCE block when at least one transparent pixel is present (no unused metadata on fully-opaque GIFs). Tests: - gif.test.ts: existing 'marks transparent pixels' test updated to assert index 255; two new tests cover the opaque-black-stays-opaque regression and the conditional GCE behavior. - builderRender.test.ts: existing 'degrades object fills' test replaced with three throw-on-image/pattern/raster tests.
The encoder was bumping codeSize one iteration early (at nextCode === 1 << codeSize, e.g. 512 for codeSize=9), while the standard GIF LZW decoder bumps one iteration later (at nextCode === (1 << codeSize) + 1, e.g. 513). The encoder therefore started writing 10-bit codes before the decoder started reading them, shifting the bit stream by one code at the first LZW boundary. A moderately varied image (gradients, antialiasing, varied rasterized pixels) would produce unreadable GIFs. A 600-pixel varied row now decodes correctly via a standards-based LZW decoder. The bump threshold is updated in both the initial seed and the dictionary-reset path to keep the rules consistent across the full stream. Co-Authored-By: Crush <crush@charm.land>
| "show_grid": { "type": "boolean", "default": true }, | ||
| "frame_duration_ms": { "type": "number", "minimum": 1, "maximum": 10000, "description": "Per-frame duration in ms; when set, the generator emits an animation.json sidecar" } | ||
| "frame_duration_ms": { "type": "number", "minimum": 1, "maximum": 10000, "description": "Per-frame duration in ms; when set, the generator emits an animation.json sidecar" }, | ||
| "builder_recipe": { "$ref": "https://placeholderer.dev/schemas/builder-recipe/v1" } |
There was a problem hiding this comment.
Schema allows combination that
generateJob always rejects
builder_recipe is declared as a valid optional property for spriteSheetAsset, but generate.ts unconditionally throws when it encounters this combination. Any manifest that includes a sprite-sheet asset with a builder_recipe will pass validateManifest cleanly, then consistently fail at generation with an asset-level error. The schema is advertising support for a pairing the runtime intentionally forbids, so callers have no way to know from validation alone that their manifest will not fully execute.
The schema previously listed builder_recipe as a valid optional property on every image-style asset including sprite_sheet, but generateJob unconditionally rejects that combination at runtime (the recipe renders one still image while the animation sidecar would still claim rows × columns frames, producing mismatched artifacts). Validation passed for a manifest that the generator would then fail to execute, leaving callers with a runtime error instead of a clear schema rejection. Removed builder_recipe from the spriteSheetAsset definition and moved it onto the three asset kinds that actually support it (image, tileset, ui_panel). Added additionalProperties:false on every concrete asset schema so the schema now rejects unknown fields per-kind rather than silently accepting them. To make that work with the existing Ajv strict-mode setup, each concrete schema repeats the base asset properties inline instead of referencing the shared base through allOf, since allOf merges properties but additionalProperties applies per-schema and would flag the base fields as unknown in the inline branch. Also fixed the double asset-name prefix on the sprite_sheet rejection error: the throw included the asset name and the catch block prefixed it again, producing "name: name: ..." in the manifest report. Co-Authored-By: Crush <crush@charm.land>
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.
Closes the deferred v1 spec work that tier 3 didn't fit: finish the v1 export-format matrix (BMP + GIF), wire the UI Builder recipe back into generateJob so a builder-authored asset ships as the actual bytes, and move presets behind a button so users don't trample in-progress work.
In this PR
BMP export (5bd2c52) — the spec calls PNG/JPG/JPEG/SVG/BMP/GIF the v1 export targets. New
packages/core/src/bmp.tsis a pure BGRA encoder that both web OffscreenCanvas and CLI @napi-rs/canvas backends routeimage/bmpthrough (neither backend supports BMP natively).GIF export (8beb124) — same gap for GIF. New
packages/core/src/gif.tsis a GIF89a single-frame encoder with a 6×6×6 web-safe palette + 40 grayscale, LZW compression with dictionary reset at 4096 codes, and a Graphics Control Extension for transparent pixels.Manifest/Builder unification (d7b5f21) — the spec says a manifest asset can reference a builder recipe (embedded or external). Until now
builder_recipewasn't on the manifest schema andgenerateJobignored it.builder_recipeto all four image-style asset kinds in both schema and TS types.packages/core/src/builderRender.tsis an env-agnostic layer renderer (rect, circle, line, text, filled-shape; image/pattern fills degrade to the fallback color in Node because we don't haveOffscreenCanvasthere).generateJobbranches onasset.builder_recipeand routes throughrenderBuilderRecipeinstead ofdrawAsset. The recipe's own width/height override the asset's nominal bounds.UI Builder: presets behind a button (5bd2c52) — engine-aware presets used to be inline in the right sidebar where one mis-click trampled in-progress work. Now they're behind a "Presets" button next to "Clear"; clicking opens an absolute-positioned popover with engine-grouped buttons, click-outside dismisses.
Verification
pnpm -r build→ greenpnpm test→ 53 core + 22 CLI = 75 tests pass (5 builderRender, 5 GIF, 6 BMP, 1 builder_recipe e2e, plus all existing)pnpm e2e→ 4 Playwright tests pass (includes a new test that asserts the Presets button is hidden by default and the popover opens with engine-grouped preset buttons)Notes
Greptile Summary
This PR closes the deferred v1 export-format work (BMP + GIF encoders), wires
builder_recipeintogenerateJobso manifest assets can ship recipe-rendered bytes, and moves the UI Builder's engine presets behind a popover button to prevent accidental overwrites.packages/core/src/bmp.ts,gif.ts) — pure-TS implementations used by both web and CLI backends; previous reviewer issues (opaque-black transparency, LZW code-size desync) are fully addressed with regression tests.generate.ts,builderRender.ts, schema) —builder_recipeis now a validated optional field onimage,tileset, andui_panelassets;sprite_sheetis explicitly excluded at schema level, andgenerateJobthrows early if it somehow reaches runtime.UIBuilder.tsx) — presets moved from the always-visible sidebar into an absolute-positioned dialog; the trigger button is included in the outside-click boundary so it correctly toggles without double-fire.Confidence Score: 5/5
The PR is safe to merge; all three new subsystems are well-tested and the GIF/BMP encoders have been corrected from prior review rounds.
All previously identified issues are resolved with both code fixes and dedicated regression tests. The only remaining finding is a stale module-level comment that misrepresents the error-handling model — it doesn't affect runtime behavior.
The module-level comment in
packages/core/src/builderRender.tsshould be corrected before this file accumulates more callers that rely on it for error-handling semantics.Important Files Changed
(1 << codeSize) + 1(fixing stream desync), GCE only emitted when transparent pixels are present.renderBuilderRecipe;sprite_sheet+builder_recipecombination throws early (caught by existing per-asset catch block) so the rest of the job continues cleanly.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[generateJob asset loop] --> B{asset.kind === audio?} B -- yes --> C[generateAudio] B -- no --> D{asset has builder_recipe?} D -- yes --> E{asset.kind === sprite_sheet?} E -- yes --> F[throw Error sidecar mismatch] E -- no --> G[createCanvas recipe dimensions] G --> H[renderBuilderRecipe layer stack] H --> I{layer type} I -- rect/circle/line/text/filled-shape --> J[draw to ctx] I -- raster or object fill --> K[throw unsupported layer] J --> L[handle.encode formatToMime] K --> M[per-asset error in report] F --> M D -- no --> N[createCanvas asset dimensions] N --> O[drawAsset standard grid] O --> L C --> L L -- image/bmp --> P[encodeBmp] L -- image/gif --> Q[encodeGif] L -- other --> R[native canvas encode] P --> S[zip.file] Q --> S R --> S%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[generateJob asset loop] --> B{asset.kind === audio?} B -- yes --> C[generateAudio] B -- no --> D{asset has builder_recipe?} D -- yes --> E{asset.kind === sprite_sheet?} E -- yes --> F[throw Error sidecar mismatch] E -- no --> G[createCanvas recipe dimensions] G --> H[renderBuilderRecipe layer stack] H --> I{layer type} I -- rect/circle/line/text/filled-shape --> J[draw to ctx] I -- raster or object fill --> K[throw unsupported layer] J --> L[handle.encode formatToMime] K --> M[per-asset error in report] F --> M D -- no --> N[createCanvas asset dimensions] N --> O[drawAsset standard grid] O --> L C --> L L -- image/bmp --> P[encodeBmp] L -- image/gif --> Q[encodeGif] L -- other --> R[native canvas encode] P --> S[zip.file] Q --> S R --> SComments Outside Diff (1)
General comment
Reviews (4): Last reviewed commit: "Tighten manifest schema so sprite_sheet ..." | Re-trigger Greptile