From d40441da55557f7dc29838debf00d906cac97869 Mon Sep 17 00:00:00 2001 From: Gabriel Kaszewski Date: Mon, 23 Mar 2026 02:13:10 +0100 Subject: [PATCH] docs: address spec review issues in png exporter design --- .../2026-03-23-png-image-exporter-design.md | 39 ++++++++++++++----- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/docs/superpowers/specs/2026-03-23-png-image-exporter-design.md b/docs/superpowers/specs/2026-03-23-png-image-exporter-design.md index 55f0f61..b1ac5e4 100644 --- a/docs/superpowers/specs/2026-03-23-png-image-exporter-design.md +++ b/docs/superpowers/specs/2026-03-23-png-image-exporter-design.md @@ -31,7 +31,15 @@ Add a `"png"` format to the exporter registry. The output is a PNG image with a All z-layers are stacked vertically in one PNG. Each layer has a "Layer N" header row above its grid. ### Legend -Below all layers: one row per active VoxelType showing a color swatch, the letter code, and the full Minecraft block name (e.g. `B minecraft:stone`). +Below all layers: one row per **active** VoxelType. A VoxelType is active if both: +1. The palette field is `Some(_)` (i.e. `palette.blocks.outline.is_some()` for Outline, `palette.blocks.shadow.is_some()` for Shadow; Body is always active) +2. At least one cell of that type exists in the grid + +Each legend row shows: color swatch, letter code, full Minecraft block name (e.g. `B minecraft:stone`). + +If a VoxelType has cells in the grid but its palette field is `None`, `BlockPalette::resolve` returns `"minecraft:air"`. Such cells are rendered in the grid with the type's color and letter (since the voxel exists), but the legend shows `"minecraft:air"` as the block name — no special suppression. + +If the grid contains no voxels at all (all cells `None`), the legend is omitted and the image contains only the layer label rows on a dark background. ## CLI Interface @@ -41,7 +49,11 @@ New flag added to `crates/bin`: --cell-size Cell size in pixels for PNG export (16 or 32) [default: 16] ``` -The `--format png` flag selects the image exporter. `--format all` includes PNG at 16px (default). +The `--format png` flag selects the image exporter. `--format all` includes PNG using whatever `--cell-size` the user specified (or the default 16). + +The CLI always routes `"png"` through `build_png(&palette, cli.cell_size)` regardless of whether it arrived via `--format png` or `--format all`. The registry entry (16px default) exists solely so `available_names()` lists `"png"` and the help text is accurate; it is never used directly for rendering. + +`--cell-size` is validated at parse time to the range `[8, 64]` via a `clap::value_parser` range constraint (matching the pattern used by `--outline-mode`). Values below 8 would produce a letter scale of 0 (invisible); values above 64 are unreasonably large. The flag is ignored for all non-PNG formats. ## Architecture @@ -64,11 +76,11 @@ impl StructureExporter for PngExporter { ``` Private helpers: -- `image_dimensions(grid, cell_size) -> (u32, u32)` — pure dimension calculation +- `image_dimensions(grid, palette, cell_size) -> (u32, u32)` — computes full image dimensions including legend; takes palette to determine number of active legend rows - `draw_cell(img, px, py, cell_size, color)` — fill a rectangle - `draw_letter(img, px, py, cell_size, ch, color)` — render one char from embedded bitmap font - `draw_layer_label(img, y, z_index, cell_size)` — "Layer N" header row -- `draw_text_row(img, x, y, text, cell_size)` — render a text string +- `draw_text_row(img, x, y, text, scale: u32)` — render a text string at a fixed pixel scale (1 = 5×7 px per char, 2 = 10×14, etc.); used at `scale=1` for legend labels and `scale = cell_size / 8` for layer labels - `draw_legend(img, y_start, palette, cell_size)` — legend section ### Text rendering @@ -90,15 +102,23 @@ When `"png"` appears in `format_names`, the CLI calls `exporters::build_png(&pal ``` gap = 1 - -layer_block_height = (cell_size + 4) + gap + height * (cell_size + gap) - gap +label_height = cell_size + 4 // "Layer N" header row +per_layer_height = height * (cell_size + gap) - gap // 0 if height=0 +layer_block_height = label_height + gap + per_layer_height total_grid_height = depth * layer_block_height + (depth - 1) * gap -img_width = gap + width * (cell_size + gap) -img_height = total_grid_height + gap + legend_height +active_types = count of VoxelTypes active in palette+grid (0–3) +legend_height = if active_types > 0 { active_types * (cell_size + gap) + gap } else { 0 } +img_width = gap + width * (cell_size + gap) // 1 if width=0 +img_height = total_grid_height + (if legend_height > 0 { gap + legend_height } else { 0 }) ``` Row y=0 in `VoxelGrid` is the bottom row; the image renders row `height-1-y` from the top so text reads naturally top-to-bottom. +**Edge cases:** +- `width=0` or `height=0`: `img_width`/`per_layer_height` collapse gracefully to minimal values; no panic. The "empty grid" test case (#4) uses a grid with non-zero dimensions but all-`None` cells — this is the common case and must not panic. +- `depth=1`: `(depth-1)*gap = 0`, only one layer block rendered; correct. +- `--out result.mcfunction --format png`: `set_extension("png")` replaces the existing extension, producing `result.png`. This is existing CLI behavior, unchanged for PNG. + ## Dependencies Only `crates/exporters/Cargo.toml` gains a new dependency: @@ -116,9 +136,10 @@ All in `crates/exporters/src/image_export.rs`: 1. `dimensions_single_layer` — formula correct for 1 layer 2. `dimensions_multi_layer` — height increases per additional layer 3. `export_produces_valid_png` — output starts with PNG magic bytes `[137, 80, 78, 71, ...]` -4. `export_empty_grid_no_panic` — all-air grid produces valid output +4. `export_empty_grid_no_panic` — grid with all-`None` cells (non-zero dimensions) produces valid output without panic 5. `export_multi_layer_larger_than_single` — 3-layer > 1-layer byte count 6. `letter_for_each_voxel_type` — B/O/S mapping is correct +7. `export_palette_with_no_outline` — palette where `outline = None`; grid with Outline voxels still renders without panic; legend shows `"minecraft:air"` for Outline ## Verification