Skip to content

Commit 22882e6

Browse files
clebertclaude
andcommitted
Prune and tighten comments; fix stale comments and docs
Audit every code comment and markdown doc against the project's comment philosophy: keep only math, non-obvious algorithms, cited constants, and genuine gotchas; make every comment standalone and current-solution-only. - Fix out-of-sync comments: Crop's "unit circle" (the crop radius is viewport.scale - 1, not 1), Spectrum's stale "(replaces atan2)", and the pebble plan's single-band render() description (render now supersamples). - Remove trivial comments that restate the code: test-assertion narration, doc-comments echoing the symbol name, the duplicated localStorage-catch notes, and textbook lerp citations. - Shorten verbose doc/module comments while keeping their load-bearing gotchas (wasm32 overflow guard, arena alignment, dither error-buffer invariant, downsample in-place safety). - Trim docs/pebble-port-plan.md: drop duplicated binary-model/risk prose and a boilerplate C listing. - Add 14"/16" wallpaper export examples to the README. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
1 parent f72067c commit 22882e6

24 files changed

Lines changed: 110 additions & 231 deletions

README.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,13 @@ zig-out/bin/png 1024 7 14 logo.png --grain --supersample
4949
zig build png -Doptimize=ReleaseFast && \
5050
zig-out/bin/png 260 7 14 pebble.png --dither --supersample
5151
```
52+
53+
```bash
54+
zig build png -Doptimize=ReleaseFast && \
55+
zig-out/bin/png 1964 7 14 wallpaper-14-inch.png --grain --supersample
56+
```
57+
58+
```bash
59+
zig build png -Doptimize=ReleaseFast && \
60+
zig-out/bin/png 2234 7 14 wallpaper-16-inch.png --grain --supersample
61+
```

bin/png/main.zig

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ pub fn main(init: std.process.Init) !void {
1818

1919
var config = try lib.Config.init(allocator);
2020

21-
// The config default may enable grain; the PNG tool ignores it and applies a
22-
// texture only when explicitly requested via --grain or --dither.
21+
// Ignore config default; apply texture only on --grain/--dither.
2322
config.texture = if (args.dither) .dither else if (args.grain) .grain else .none;
2423

2524
config.supersample_enabled = args.supersample;
@@ -66,7 +65,7 @@ const Args = struct {
6665
fn parseArgs(process_args: std.process.Args) ?Args {
6766
var arguments = process_args.iterate();
6867

69-
_ = arguments.next(); // skip program name
68+
_ = arguments.next();
7069

7170
var positional: [4][]const u8 = undefined;
7271
var positional_count: usize = 0;

bin/png/png.zig

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
// PNG encoder that compresses the image data with std.compress.flate.Compress,
2-
// the deflate compressor introduced in Zig 0.16.0, at its best level.
1+
// PNG encoder; compresses image data with deflate at its best level.
32

43
const std = @import("std");
54

@@ -69,8 +68,7 @@ fn filterScanlines(
6968
return data;
7069
}
7170

72-
/// Compresses data into a zlib stream (RFC 1950) using deflate at the best
73-
/// compression level. The returned slice is owned by the caller.
71+
/// Compresses data into a zlib stream RFC 1950. Returned slice owned by caller.
7472
fn zlibDeflate(allocator: std.mem.Allocator, data: []const u8) ![]u8 {
7573
const window = try allocator.alloc(u8, std.compress.flate.max_window_len);
7674

bin/wasm/main.zig

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,9 @@ const std = @import("std");
33
const lib = @import("lib");
44

55
comptime {
6-
// render() returns a pointer that JS reinterprets as a flat RGBA byte buffer
7-
// (a Uint8ClampedArray over WASM memory; see src/renderer.ts). That contract
8-
// requires lib.Srgb to be exactly 4 bytes laid out as r, g, b, a with no
9-
// padding. lib.Srgb has the default (auto) layout, which the language does
10-
// not guarantee — pin the assumption here so a layout change fails the build
11-
// instead of silently producing garbled pixels.
6+
// JS reads render()'s result as a flat RGBA Uint8ClampedArray (src/renderer.ts),
7+
// so Srgb must be exactly r, g, b, a with no padding. Auto layout does not
8+
// guarantee this, so pin it:
129
std.debug.assert(@sizeOf(lib.Srgb) == 4);
1310
std.debug.assert(@offsetOf(lib.Srgb, "r") == 0);
1411
std.debug.assert(@offsetOf(lib.Srgb, "g") == 1);
@@ -44,12 +41,9 @@ fn getConfig(config_json_byte_length: u32) ?lib.Config {
4441
return cached_config;
4542
}
4643

47-
// A single grow-only arena for the render buffers, backed directly by
48-
// @wasmMemoryGrow. Growing by exact page counts (rather than through the general
49-
// allocator, which rounds large allocations up to a power of two) lets a native
50-
// full-resolution frame fit tightly — e.g. a 6K canvas needs ~407 MB, not ~642 MB.
51-
// One contiguous region reused across frames means peak memory equals the largest
52-
// frame ever rendered, with no fragmentation or resize accumulation.
44+
// A single grow-only contiguous arena backed by @wasmMemoryGrow, reused across
45+
// frames. Exact page-count growth avoids the allocator's power-of-two rounding so a
46+
// native full-res frame fits tightly (a 6K canvas takes ~407 MB, not ~642 MB).
5347
var arena_base: usize = 0;
5448
var arena_bytes: usize = 0;
5549
var arena_initialized: bool = false;
@@ -92,19 +86,15 @@ export fn render(
9286

9387
const supersample_factor = lib.frame.supersampleFactor(config);
9488

95-
// usize is u32 on wasm32 and this module is built without runtime safety, so the
96-
// buffer-size products below would wrap silently for an oversize frame, under-sizing
97-
// the arena while the slices still span the true pixel count — a heap overflow. Reject
98-
// any frame whose arena footprint does not fit usize; the JS caller treats a null return
99-
// as "render failed, keep the previous frame".
89+
// usize is u32 on wasm32 and this module runs without runtime safety, so an oversize
90+
// frame would wrap these size products silently and under-size the arena (a heap
91+
// overflow). Use checked arithmetic and return null on overflow.
10092
const pixel_count = std.math.mul(usize, image_width, image_height) catch return null;
10193
const supersampled_count = std.math.mul(usize, pixel_count, supersample_factor * supersample_factor) catch return null;
10294
const error_count = lib.dither.errorBufferSize(image_width);
10395

104-
// Lay the three buffers out consecutively in the arena: Linear (16 B, the strictest
105-
// alignment) first, then Srgb (4 B), then the f32 error rows — each offset is a multiple
106-
// of the next type's size, so every slice stays naturally aligned. The Linear scratch
107-
// holds the full supersampled render; downsampling rewrites its front in place.
96+
// Lay the buffers out by descending alignment so every offset stays naturally
97+
// aligned: Linear (16 B) first, then Srgb (4 B), then the f32 error rows.
10898
const linear_bytes = std.math.mul(usize, supersampled_count, @sizeOf(lib.Linear)) catch return null;
10999
const srgb_bytes = std.math.mul(usize, pixel_count, @sizeOf(lib.Srgb)) catch return null;
110100
const error_bytes = std.math.mul(usize, error_count, @sizeOf(f32)) catch return null;

docs/pebble-port-plan.md

Lines changed: 33 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,11 @@ export** binary (`bin/png/`). Two properties of the library make a Pebble port r
1919
Floyd–Steinberg error diffusion — its only state is a small two-row error buffer carried forward
2020
between strips; see [the Pebble dither](#dither-palette).
2121

22-
`lib/frame.zig`'s `render()` renders the **full image as a single band**
23-
(`image.band(Linear, buffer, image.height, 0)`), and both shells call it that way. Driving the
24-
pipeline strip-by-strip on-device is therefore _new shell code_, not new library code — the banded
25-
primitives already exist and are tested.
22+
`lib/frame.zig`'s `render()` renders the whole frame in one shot: it builds a supersampled band
23+
(`supersampled.band(Linear, linear_buffer, supersampled.height, 0)`), runs `Watchface.render` into
24+
it, box-averages it down, then constructs the target-resolution band over the front of the same
25+
buffer. Driving the pipeline strip-by-strip on-device is therefore _new shell code_, not new library
26+
code — the banded primitives already exist and are tested.
2627

2728
## Feasibility
2829

@@ -59,7 +60,7 @@ approach has direct prior art.
5960
1. Whether Zig 0.16 / LLVM emits relocations for `thumbv7m`/`thumbv7em` that the Pebble app loader
6061
applies correctly for any mutable globals/statics. Build a minimal Zig-on-Pebble proof first and
6162
keep global state at zero.
62-
2. Soft-float f32 plus scalarized `@Vector` SIMD (≈59 uses across `lib/`; neither Cortex-M4F nor
63+
2. Soft-float f32 plus scalarized `@Vector` SIMD (used throughout `lib/`; neither Cortex-M4F nor
6364
base Cortex-M33 has packed SIMD) — code size and render time within the budget. Fine in principle
6465
for a once-per-minute redraw, but must be measured on the emulator.
6566
3. `compiler_rt` soft-float routines linking against newlib-nano without duplicate `__aeabi_*`
@@ -85,13 +86,10 @@ rectangular 64-colour model is a straightforward second target.
8586

8687
Notes:
8788

88-
- The SoC is the **SiFli SF32LB52J**, a big.LITTLE pair of Cortex-M33 STAR-MC1 cores (240 MHz HCPU +
89-
24 MHz LCPU, 512 KB SRAM). The 16 MB PSRAM exists on-chip but is **not enabled in PebbleOS**, so
90-
don't count on it.
91-
- Round 2 is **real and pre-orderable** (shown at CES, January 2026) but **has not shipped to
92-
customers**. Plan to validate entirely on the **emulator**.
93-
- **FPU:** the Cortex-M33 FPU is optional and not confirmed for the SF32LB52J from primary sources —
94-
but it's **moot**, because the Pebble app ABI is soft-float regardless (see below).
89+
- The 16 MB PSRAM exists on-chip but is **not enabled in PebbleOS**, so don't count on it. Round 2
90+
has not shipped to customers (2026-05-30); plan to validate entirely on the **emulator**.
91+
- **FPU:** the Cortex-M33 FPU is optional and unconfirmed for the SF32LB52J, but **moot** — the
92+
Pebble app ABI is soft-float regardless (see below).
9593

9694
### Platform names vs. board codenames
9795

@@ -153,25 +151,21 @@ The Floyd–Steinberg dither's only extra scratch is a two-row error buffer
153151
(`dither.errorBufferSize(width)` = `width × 3 × 2` f32; ~6 KB at 260 px), carried forward between
154152
strips (see below).
155153

156-
Supersampling (`config.supersample_enabled`; factor `N = 2` via `frame.supersampleFactor`) renders
157-
the continuous image at `` and box-averages it down in linear light before quantizing, which
158-
antialiases the prism, hand, and rainbow edges. The downsample is purely local — each output pixel
159-
reads only its own `N × N` source block — so it stays band-compatible: a `band_height = 1` strip
160-
needs `N` supersampled rows of `N × width` linear scratch (`N² × width × 16 B`; ~16 KB at `N = 2`,
161-
260 px wide), still far under budget. The cost is render time, which grows with `` — measure it on
162-
the emulator against the once-a-minute redraw before enabling it.
154+
Supersampling (`config.supersample_enabled`; factor `N = 2` via `frame.supersampleFactor`)
155+
antialiases the prism, hand, and rainbow edges. It stays band-compatible because each output pixel
156+
reads only its own `N × N` source block, so a `band_height = 1` strip needs only `N² × width × 16 B`
157+
of linear scratch (~16 KB at `N = 2`, 260 px); the cost is render time, which grows with ``
158+
measure it on the emulator.
163159

164160
The **framebuffer itself is owned by the firmware**`graphics_capture_frame_buffer` hands you the
165161
real 8-bit `GColor8` buffer (~66 KB for 260×260), which the OS already allocated. The app only pays
166162
for the band scratch above, comfortably within budget.
167163

168-
The Floyd–Steinberg dither carries error between rows, so it is **order-dependent**: strips must be
169-
applied top-to-bottom and the caller must persist the two-row error buffer across `dither.apply`
170-
calls (it is zeroed on the first band, where `y_offset == 0`). Done that way, strip rendering
171-
reproduces the single-pass output bit-for-bit (proven by the
172-
`multi-band apply matches single-band apply` test). This is the one cross-band dependency the
173-
renderer has; everything else is per-pixel. Drive a few `band_height` values and pick the smallest
174-
that renders fast enough.
164+
The dither is the renderer's one cross-band dependency (top-to-bottom order, persisted error buffer;
165+
see [the Pebble dither](#dither-palette)); everything else is per-pixel. Driven that way, strip
166+
rendering reproduces the single-pass output bit-for-bit (the
167+
`multi-band apply matches single-band apply` test). Drive a few `band_height` values and pick the
168+
smallest that renders fast enough.
175169

176170
## Pixel format: `GColor8`
177171

@@ -347,16 +341,14 @@ each strip into the framebuffer via `gbitmap_get_data_row_info`.
347341
348342
## C app shell
349343

350-
Standard Pebble watchface lifecycle:
344+
Standard Pebble watchface lifecycle (`window_create` → a `Layer` with an update proc →
345+
`tick_timer_service_subscribe(MINUTE_UNIT, …)` whose handler calls `layer_mark_dirty`
346+
`app_event_loop`); scaffold it from `pebble new-project`. The only port-specific part is the band
347+
loop in the update proc:
351348

352349
```c
353-
#include <pebble.h>
354-
355350
extern void pebbleRenderBand(uint8_t *out, uint16_t band_index, uint8_t hour, uint8_t minute);
356351

357-
static Window *s_window;
358-
static Layer *s_canvas;
359-
360352
static void canvas_update_proc(Layer *layer, GContext *ctx) {
361353
GBitmap *fb = graphics_capture_frame_buffer(ctx);
362354
if (!fb) return;
@@ -376,30 +368,6 @@ static void canvas_update_proc(Layer *layer, GContext *ctx) {
376368

377369
graphics_release_frame_buffer(ctx, fb);
378370
}
379-
380-
static void tick_handler(struct tm *tick_time, TimeUnits units) {
381-
layer_mark_dirty(s_canvas);
382-
}
383-
384-
static void window_load(Window *window) {
385-
Layer *root = window_get_root_layer(window);
386-
s_canvas = layer_create(layer_get_bounds(root));
387-
layer_set_update_proc(s_canvas, canvas_update_proc);
388-
layer_add_child(root, s_canvas);
389-
}
390-
391-
static void window_unload(Window *window) { layer_destroy(s_canvas); }
392-
393-
int main(void) {
394-
s_window = window_create();
395-
window_set_window_handlers(s_window, (WindowHandlers){
396-
.load = window_load, .unload = window_unload,
397-
});
398-
window_stack_push(s_window, true);
399-
tick_timer_service_subscribe(MINUTE_UNIT, tick_handler);
400-
app_event_loop();
401-
window_destroy(s_window);
402-
}
403371
```
404372
405373
## Build integration
@@ -478,30 +446,21 @@ band-by-band and `pebble screenshot` to compare against the PNG export.
478446

479447
## Open questions
480448

449+
The relocation, soft-float perf/size, and `compiler_rt` linker risks are tracked in
450+
[Risks to retire early](#feasibility). The remaining open questions:
451+
481452
- **Exact app build flags.** Read the installed SDK `waftools` for the precise `-mcpu`, `-mthumb`,
482453
`-fPIC`/`-fPIE`, `-mfloat-abi=soft`, `-msingle-pic-base`/`-mpic-register` — don't infer the PIC
483454
model from ARM convention.
484-
- **Relocations.** Empirically confirm Zig 0.16/LLVM emits data relocations the Pebble loader
485-
applies correctly for `thumbv7m`/`thumbv7em` under `relocation-model=pic`. Build a minimal proof
486-
first.
487-
- **Performance & size.** Measure soft-float render time and `.pbw` code/heap size on the
488-
`gabbro`/`emery` emulator with `@Vector` ops scalarized, against the ~128 KB budget.
489-
- **Linker hygiene.** Confirm `compiler_rt` soft-float routines link against newlib-nano with no
490-
duplicate `__aeabi_*` symbols.
491455
- **Manifest.** Confirm the exact `sdkVersion` string and `targetPlatforms` list the appstore
492456
accepts for a 4.9.x watchface; re-read per-app heap numbers from PebbleOS headers.
493457
- <a id="panel-gamma"></a>**Panel gamma — mostly resolved; not a blocker.** `GColor8` is a _nominal_
494-
colour space: levels expand linearly to {0, 85, 170, 255} (no gamma), and the QEMU emulator
495-
renders them linearly (`* 255 / 3`, no curve), so the dither — which quantizes in the sRGB domain
496-
assuming the four levels are evenly spaced — is **exactly** emulator-accurate. PebbleOS adds
497-
**no** gamma/colour-correction LUT for `getafix`/`obelix` — the SiFli driver
498-
(`src/fw/drivers/display/sf32lb/display_jdi.c`) only does a mechanical 222→332 bit-repack (its
499-
LCDC layer is `RGB332`); the `GColor8`/ARGB2222 model is identical to `basalt`/`chalk`. The
500-
**only** residual unknown is the physical reflective JDI panel + the closed SiFli vendor HAL
501-
(`bf0_hal_lcdc.c`), measurable only on real hardware. If its response diverges from sRGB it
502-
degrades dither **quality** (the threshold lands at slightly wrong brightnesses — most visible on
503-
the rainbow gradient), never output validity; the fix applies a correction curve before
504-
quantization. So it cannot gate pre-hardware work.
458+
colour space (levels expand linearly to {0, 85, 170, 255}, no gamma) and QEMU renders them
459+
linearly, so the sRGB-domain dither is **exactly** emulator-accurate; PebbleOS adds no
460+
gamma/colour LUT for `getafix`/`obelix`. The only residual unknown is the physical reflective JDI
461+
panel's response, measurable only on hardware — and a divergence degrades dither **quality**, not
462+
validity (fixable with a correction curve before quantization), so it can't gate pre-hardware
463+
work.
505464
- **Shipping reality.** Confirm Round 2 hardware actually ships before relying on anything beyond
506465
the emulator.
507466

index.html

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,7 @@
137137
left: 0;
138138
width: 100%;
139139
height: 100%;
140-
/* The backing store is antialiased at device resolution; "pixelated" would
141-
nearest-neighbor fractional-DPI/zoom scaling and reintroduce aliasing. */
140+
/* Backing store is antialiased at device resolution; "pixelated" would nearest-neighbor fractional-DPI/zoom scaling and reintroduce aliasing. */
142141
image-rendering: auto;
143142
}
144143

lib/Clock.zig

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ const vector = @import("vector.zig");
88

99
const Self = @This();
1010

11-
const hour_arc: f32 = std.math.pi / 6.0; // 2π/12 = π/6 ≈ 30° (one hour moves 30°)
12-
const apex_angle: f32 = -std.math.pi / 2.0; // −π/2 (−90°, 12 o'clock)
11+
const hour_arc: f32 = std.math.pi / 6.0; // 30° per hour-step on the dial
12+
const apex_angle: f32 = -std.math.pi / 2.0; // 12 o'clock (apex, top of dial)
1313
const rainbow_max_spread_radians: f32 = std.math.pi / 6.0;
1414

1515
prism: Prism,

lib/Crop.zig

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,7 @@ fn antialiasAtBoundary(
8585
}
8686

8787
fn antialiasNearEdge(self: Self, row: []Srgb, center_x: f32, dy: f32, radius: f32) void {
88-
// For rows just outside the circle, find the horizontal range where
89-
// pixels have partial coverage (top/bottom of the circle).
88+
// Rows just outside the circle still have partially covered pixels near the top/bottom arc.
9089
const outer = radius + 0.5;
9190
const dx_max_squared = outer * outer - dy * dy;
9291
if (dx_max_squared < 0.0) return;
@@ -142,10 +141,10 @@ test "apply sets pixels outside circle to outside color" {
142141

143142
crop.apply(band, viewport);
144143

145-
// Corner pixel (0,0) is outside the unit circle — should be white
144+
// Corner pixel (0,0) is outside the crop circle — should be white
146145
try std.testing.expectEqual(Srgb.white, buffer[0]);
147146

148-
// Center pixel (5,5) is inside the unit circle — should stay black
147+
// Center pixel (5,5) is inside the crop circle — should stay black
149148
try std.testing.expectEqual(Srgb.black, buffer[5 * 10 + 5]);
150149
}
151150

@@ -160,10 +159,8 @@ test "apply sets transparent outside color" {
160159

161160
crop.apply(band, viewport);
162161

163-
// Corner pixel should be transparent
164162
try std.testing.expectEqual(@as(u8, 0), buffer[0].a);
165163

166-
// Center pixel should remain opaque (default alpha=255)
167164
try std.testing.expectEqual(@as(u8, 255), buffer[5 * 10 + 5].a);
168165
}
169166

@@ -178,13 +175,11 @@ test "apply handles wide image" {
178175

179176
crop.apply(band, viewport);
180177

181-
// Far left pixel (0,5) is outside circle in a wide image (circle radius = 5)
178+
// Far left pixel (0,5) is outside the crop circle in this wide image
182179
try std.testing.expectEqual(Srgb.white, buffer[5 * 20 + 0]);
183180

184-
// Far right pixel (19,5) is also outside
185181
try std.testing.expectEqual(Srgb.white, buffer[5 * 20 + 19]);
186182

187-
// Center pixel (10,5) is inside
188183
try std.testing.expectEqual(Srgb.black, buffer[5 * 20 + 10]);
189184
}
190185

@@ -209,14 +204,13 @@ test "multi-band crop matches single-band crop" {
209204

210205
const crop = Self{ .outside_color = .{ .r = 20, .g = 30, .b = 40 } };
211206

212-
// Reference: single-band (full height)
213207
var reference = input;
214208

215209
const full_band = try image.band(Srgb, &reference, height, 0);
216210

217211
crop.apply(full_band, viewport);
218212

219-
// Test with band heights: 1 (extreme), 2 (even), 3 (odd), 4 (even), 8, 16
213+
// Cover extreme (1), odd, and even band heights
220214
const band_heights = [_]usize{ 1, 2, 3, 4, 8, 16 };
221215

222216
for (band_heights) |band_height| {
@@ -265,14 +259,11 @@ test "antialias produces intermediate alpha at circle edge" {
265259

266260
crop.apply(band, viewport);
267261

268-
// Center pixel should remain fully opaque
269262
try std.testing.expectEqual(@as(u8, 255), buffer[10 * 20 + 10].a);
270263

271-
// Corner pixel should be fully transparent
272264
try std.testing.expectEqual(@as(u8, 0), buffer[0].a);
273265

274-
// Find an edge pixel with intermediate alpha (proves AA is working).
275-
// Scan the middle row for a pixel with 0 < alpha < 255.
266+
// An edge pixel with 0 < alpha < 255 proves antialiasing ran.
276267
var found_intermediate = false;
277268

278269
for (0..20) |x| {

0 commit comments

Comments
 (0)