Forward registered images across the renderer channel - #26
Conversation
Found by running the full widget roster under the cutover: packets carry only image id + fingerprint references — the pixels ride the in-process side channel (uploadGpuSurfaceImage, ahead of the packet that references them), which never crossed the process boundary. A device-less Noro rendered everything except its album art: text, strokes, and controls intact, the entire art region blank (captures archived in the weaver repo's receipts directory). - kWeaverRendererMachMsgImageUpload: one registered image per message, pixels as an out-of-line descriptor, pixels_len == 0 is a removal. The 1 MiB ceiling mirrors canvas_limits' max_registered_canvas_image_pixel_bytes (stock profile; the widget profile's 256 KiB is enforced client-side by the SDK). The reply is the completion signal, same send/reply tripwires as frames. - One storage implementation for both worlds: the store/remove/ texture-for-key bodies move to shared statics; NativeSdkAppKitHost's methods delegate, and the headless per-client renderer gets its own mirror stores. The view reads through activeCanvasImageStore / activeTextureForImageKey so in-process and render-host paths cannot drift. - The device-less client forwards uploads and removals instead of storing locally (its raster paths never run); the host stores into the client's renderer, constructing it on the spot when an image precedes the first frame — the export completion now binds lazily so that order works. - Fixes the frame-budget contains-check left pinning the pre-review wall-clock pattern by #24 (would have failed the next weaver pin bump's CI). Live receipts (Mac15,6, noro-shell through weaverd with the cutover): before, art region blank (stddev 0 across the top half); after, full album art with Noro's grain overlay and scrim intact. Noro's widget process: 43.1 MB phys_footprint flat, ZERO owned-unmapped-graphics regions (in-process baseline ~155-166 MB with the 85 MB arena). test-canvas 841/841; all five shared-renderer contains-checks pass.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Greptile SummaryThis change adds macOS shared-renderer support for registered image and font resources, including per-client storage, removal, and replay after reconnecting to the renderer. The malformed unknown-resource removal concern was disproved: the focused harness showed that the previous behavior accepted the removal-shaped message, while the current validator rejects it. The same run completed image and font upload, removal, reconnect replay, and first-frame resource-resolution checks. T-Rex validation blockedNative macOS runtime verification could not run because the required Confidence Score: 5/5
What T-Rex did
Reviews (7): Last reviewed commit: "Reject unknown resource kinds before the..." | Re-trigger Greptile |
…es from the store Review is right that a replacement render host started with an empty per-client store while the runtime's cache (correctly) kept emitting retain actions — the first post-crash frame referenced images the host never received, and the draws were silently skipped: art gone until the image changed. Two halves, both needed: - The client connection keeps each registered image's bytes and replays them after every reconnect (the mach analog of the Windows client re-supplying retained content to a replacement renderer). Bounded by the SDK's registry: 16 ids, 256 KiB each in the widget profile. - A retain action whose view-cache entry is missing but whose pixels are in the store installs from the store — a fresh renderer sees retain for content it never installed, and the draw survives instead of silently skipping. This also hardens the in-process path against the same cache/store drift. Live receipt: noro-shell through weaverd, host kill -9 mid-run, weaverd respawn — album art region byte-equivalent before and after (stddev 20.9/21.3/21.1 both sides). test-canvas 841/841; the images contains-check pins both halves.
|
@greptileai review |
Image messages now pass the same shape-before-content rule as frames — non-complex (a removal) or complex with exactly one out-of-line descriptor; anything else is destroyed whole, so a smuggled port right where pixels belong is disposed instead of leaked. And a replay failure mid-loop disconnects rather than stranding a session the host half knows: the next attempt reconnects and replays from the top.
|
@greptileai review |
A refused image (invalid by the host's validator) stayed in the replay set and poisoned every reconnect into a disconnect loop. The image send is now tri-state: accepted; refused by a live host (the entry is dropped — it can never be accepted, and the draws referencing it skip exactly like an unregistered image in-process); or transport failure (session torn down, entry kept for the next reconnect's replay). Crash drill re-verified on this build: art region byte-equivalent across host kill -9 + weaverd respawn.
|
@greptileai review |
Found live by Dara running the cutover side by side with an in-process Noro: text drew in the system face instead of the widget's registered font (Cozette), wider and truncated. Same class as images — native_sdk_appkit_register_font is the other pre-packet side channel, and text rasterizes in the render host, which never saw the face. - The image message generalizes to a resource upload (image | font), same ool payload, same removal convention, same tripwires; the font ceiling matches the packet tripwire (faces are megabyte-scale; Noro's subset is ~100 KiB). - Host side, faces land in a PER-CLIENT font table swapped in around each present (NativeSdkRenderHostSetActiveFontTable): the process table would let two widgets' font ids collide in the shared host. The font-size cache becomes a bounded NSCache with client-tagged keys. - Client side, registration is record-and-best-effort: fonts register at widget startup, often before the host is reachable, and the reconnect replay (fonts first, then images) delivers them the moment a session exists. The widget also keeps its local registration so text MEASUREMENT resolves the same face the host draws with. Live receipts: shared-renderer Noro's title/time strip now renders in Cozette, matching the in-process instance glyph for glyph (was system face + truncation); after a host kill -9 + weaverd respawn the title band is byte-identical — faces replay with the session. test-canvas 841/841; the resource contains-check pins the font half.
|
Additional roster finding (caught live by Dara, same class as the album art): registered FONTS are the other pre-packet side channel — text rasterized in the host with the system face instead of the widget's registered font. The image message is now a generalized resource upload (image | font); faces land in a per-client font table swapped in around each present so widgets' font ids can never collide in the shared host, and they replay after reconnect (verified: title band byte-identical across a host kill -9). @greptileai review |
|
@greptileai review |
A removal with an unrecognized kind validated and fell into the image removal path; kind membership is now checked before anything else.
|
@greptileai review |
Cutover blocker, found by widening the acceptance net. The slice 1–4 receipts all used clock widgets. Running the full roster surfaced this: NSGP packets carry only image id + fingerprint references — pixels ride the in-process side channel (
uploadGpuSurfaceImage, sent ahead of the packet that references them), which never crossed the process boundary. A device-less Noro rendered text, strokes, and controls perfectly with the entire album-art region blank. SunkenInTime/weaver#47 should not merge before this does (it needs a pin bump to include this commit).What changed
kWeaverRendererMachMsgImageUpload— one registered image per message, pixels as an ool descriptor,pixels_len == 0is a removal. The 1 MiB ceiling mirrorscanvas_limits.max_registered_canvas_image_pixel_bytes(stock profile; the widget profile's 256 KiB is enforced client-side by the SDK before bytes reach the channel). One reply per request with the same send/reply tripwires as frames.NativeSdkAppKitHost's methods delegate to them, and the headless per-client renderer carries mirror stores. The view reads throughactiveCanvasImageStore/activeTextureForImageKey, so the in-process and render-host paths cannot drift.Receipts (Mac15,6, noro-shell through weaverd with the full cutover config)
noro-broken-no-image-forwarding.pngin the weaver receipts dir (stddev 0 across the top half; everything else correct).noro-fixed-image-forwarding.png).test-macos-shared-renderer-images.