Skip to content

Commit 4cbcdd3

Browse files
zackeesclaude
andauthored
fix(viewer): recover screen maps created after setup() (#screenmap-250) (#264)
Sketches that never call setScreenMap() in setup() rendered an empty canvas. The background worker read getScreenMapData() exactly once, right after extern_setup() returned, but FastLED creates its default layouts lazily on the first exported frame (jsFillInMissingScreenMaps), so that single read came back without a layout for the strip and the renderer had nothing to draw. - Add modules/core/screenmap_sync.ts with pure helpers that match a frame entry's strip_id against the cached screenMaps dictionary. - Share one fetchScreenMapsFromWasm()/applyScreenMaps() pair between the post-setup read and a new refreshScreenMapsIfIncomplete(), which runs after extractFrameData() and before the frame is posted or drawn, capped at 5 re-fetches per strip. - Merge rather than replace the cache, so layouts delivered by the push-based screenmap_update path survive a partial late re-read, and only mark the cache dirty when a strip is actually recovered. - Add tests/fixtures/wasm_no_screenmap plus a wheel-smoke render check that greps the viewer log for the recovery marker, and widen the _build.yml log upload glob to pick up the second viewer log. Refs #250 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 4367202 commit 4cbcdd3

6 files changed

Lines changed: 435 additions & 39 deletions

File tree

‎.github/workflows/_build.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ jobs:
138138
uses: actions/upload-artifact@v4
139139
with:
140140
name: wheel-smoke-log-${{ inputs.runs-on }}-${{ inputs.rust-target || 'native' }}
141-
path: ${{ runner.temp }}/fastled-wheel-smoke/artifacts/viewer.log
141+
path: ${{ runner.temp }}/fastled-wheel-smoke/artifacts/*.log
142142
if-no-files-found: warn
143143

144144
- name: Upload wheel

‎ci/smoke_installed_wheel.sh‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ set -euo pipefail
1010

1111
SMOKE_ROOT="$RUNNER_TEMP/fastled-wheel-smoke"
1212
rm -rf "$SMOKE_ROOT"
13-
mkdir -p "$SMOKE_ROOT/home" "$SMOKE_ROOT/sketch/data" "$SMOKE_ROOT/artifacts" "$SMOKE_ROOT/asset-origin"
13+
mkdir -p "$SMOKE_ROOT/home" "$SMOKE_ROOT/sketch/data" "$SMOKE_ROOT/artifacts" "$SMOKE_ROOT/asset-origin" "$SMOKE_ROOT/no-screenmap"
1414
uv venv "$SMOKE_ROOT/venv" --python "$UV_PYTHON"
1515
uv pip install --python "$SMOKE_ROOT/venv/bin/python" dist/*.whl
1616
test -x "$SMOKE_ROOT/venv/bin/uv"
@@ -92,6 +92,39 @@ if width <= 0 or height <= 0:
9292
raise SystemExit(f"viewer screenshot has invalid dimensions: {width}x{height}")
9393
PY
9494

95+
# Regression check for issue #250: a sketch with no setScreenMap() call in
96+
# setup() used to render an empty canvas, because the worker only read
97+
# getScreenMapData() once, immediately after extern_setup() returned, before
98+
# FastLED had lazily created its default layout on the first exported frame.
99+
# The worker now re-reads layouts after a frame and logs the recovery marker
100+
# below when it picks up the late screen map.
101+
cp -R "$GITHUB_WORKSPACE/tests/fixtures/wasm_no_screenmap/." "$SMOKE_ROOT/no-screenmap/"
102+
env -i \
103+
HOME="$SMOKE_ROOT/home" \
104+
PATH="/usr/bin:/bin:/usr/sbin:/sbin" \
105+
"$SMOKE_ROOT/venv/bin/fastled" "$SMOKE_ROOT/no-screenmap" \
106+
--check \
107+
--test-wait-secs=2 \
108+
--test-timeout-secs=240 \
109+
--test-ready-timeout-secs=45 \
110+
--test-screenshot="$SMOKE_ROOT/artifacts/no-screenmap.png" \
111+
--test-log="$SMOKE_ROOT/artifacts/viewer-no-screenmap.log"
112+
grep -F "[fastled] late screenmap recovered" "$SMOKE_ROOT/artifacts/viewer-no-screenmap.log"
113+
"$SMOKE_ROOT/venv/bin/python" - "$SMOKE_ROOT/artifacts/no-screenmap.png" <<'PY'
114+
from pathlib import Path
115+
import sys
116+
117+
image = Path(sys.argv[1]).read_bytes()
118+
if image[:8] != b"\x89PNG\r\n\x1a\n":
119+
raise SystemExit("viewer screenshot is not a PNG")
120+
if image[12:16] != b"IHDR" or len(image) < 24:
121+
raise SystemExit("viewer screenshot is missing its IHDR header")
122+
width = int.from_bytes(image[16:20], "big")
123+
height = int.from_bytes(image[20:24], "big")
124+
if width <= 0 or height <= 0:
125+
raise SystemExit(f"viewer screenshot has invalid dimensions: {width}x{height}")
126+
PY
127+
95128
if [ "${FASTLED_SAFARI_SMOKE:-0}" = "1" ]; then
96129
# The viewer run above left the compiled sketch in sketch/fastled_js; the
97130
# asset origin is still serving, so Safari loads the same program.

‎src/fastled/frontend/modules/core/fastled_background_worker.ts‎

Lines changed: 170 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424

2525
/* global postMessage, self, performance, requestAnimationFrame, cancelAnimationFrame */
2626

27+
import { findStripsMissingLayout } from './screenmap_sync.ts';
28+
2729
// CRITICAL FIX: Workers don't support import maps, but Three.js jsm files use bare "three" imports.
2830
// Solution: Use local vendor files with patched relative imports to three.module.js
2931

@@ -99,6 +101,8 @@ async function loadThreeJSModules() {
99101
* @property {number} frameCaptureInterval - Milliseconds between frame captures
100102
* @property {number} lastFrameCaptureTime - Timestamp of last frame capture
101103
* @property {Object} screenMaps - Dictionary of screenmaps (stripId → screenmap, push-based from C++)
104+
* @property {Object} screenMapRefreshAttempts - Late screenmap re-fetch counters per strip id
105+
* @property {boolean} screenMapRecoveryReported - Whether the late-recovery notice was already emitted
102106
*/
103107

104108
/**
@@ -142,6 +146,13 @@ const workerState = {
142146
screenMapsDirty: false, // screenMaps changed and the main thread has not been told yet (main-thread rendering)
143147
renderOnMainThread: false, // frames are posted to the main thread instead of drawn on an OffscreenCanvas
144148

149+
// Late screenmap recovery (#250): FastLED fills in default layouts lazily on
150+
// the first exported frame (jsFillInMissingScreenMaps), which happens after
151+
// the single post-setup getScreenMapData() read in handleStart(). Strips seen
152+
// in frame data without a layout are re-fetched for a few frames.
153+
screenMapRefreshAttempts: {}, // stripId (string) -> late re-fetches performed for that strip
154+
screenMapRecoveryReported: false, // stdout notice emitted once per session
155+
145156
// Audio sample queue - samples buffered here from onmessage, flushed to WASM at frame start
146157
audioSampleQueue: [],
147158
audioSampleBufferedOnce: false,
@@ -159,6 +170,9 @@ const performanceMonitor = {
159170

160171
};
161172

173+
/** Maximum late getScreenMapData() re-fetches attempted per strip (#250). */
174+
const MAX_SCREENMAP_REFRESH_ATTEMPTS = 5;
175+
162176
/**
163177
* Debug logging in worker context
164178
* @param {string} level - Log level (LOG, ERROR, TRACE)
@@ -608,6 +622,68 @@ async function initializeGraphicsManager() {
608622
}
609623
}
610624

625+
/**
626+
* Reads the current screenmap dictionary out of the WASM module.
627+
* Shared by the post-setup read in handleStart() and the late refresh in
628+
* refreshScreenMapsIfIncomplete() (#250).
629+
* @returns {Object|null} Parsed screenmap dictionary, or null when unavailable
630+
*/
631+
function fetchScreenMapsFromWasm() {
632+
const Module = workerState.fastledModule;
633+
if (!Module || !Module.cwrap) {
634+
return null;
635+
}
636+
637+
// Bind WASM functions if not already bound (same binding set as extractFrameData)
638+
if (!workerState.wasmFunctions) {
639+
workerState.wasmFunctions = {
640+
getFrameData: Module.cwrap('getFrameData', 'number', ['number']),
641+
getScreenMapData: Module.cwrap('getScreenMapData', 'number', ['number']),
642+
getStripPixelData: Module.cwrap('getStripPixelData', 'number', ['number', 'number']),
643+
freeFrameData: Module.cwrap('freeFrameData', null, ['number'])
644+
};
645+
}
646+
647+
const screenMapSizePtr = Module._malloc(4);
648+
try {
649+
const screenMapDataPtr = workerState.wasmFunctions.getScreenMapData(screenMapSizePtr);
650+
if (!screenMapDataPtr) {
651+
return null;
652+
}
653+
try {
654+
const screenMapSize = Module.getValue(screenMapSizePtr, 'i32');
655+
const screenMapJson = Module.UTF8ToString(screenMapDataPtr, screenMapSize);
656+
return JSON.parse(screenMapJson);
657+
} finally {
658+
workerState.wasmFunctions.freeFrameData(screenMapDataPtr);
659+
}
660+
} catch (error) {
661+
workerLog('ERROR', 'BACKGROUND_WORKER', 'Failed to read screenmap data from WASM', error);
662+
return null;
663+
} finally {
664+
Module._free(screenMapSizePtr);
665+
}
666+
}
667+
668+
/**
669+
* Caches a screenmap dictionary and pushes it to the graphics manager.
670+
* @param {Object} screenMapData - Dictionary stripId -> screenmap
671+
* @param {string} reason - Why the update happened (logging only)
672+
*/
673+
function applyScreenMaps(screenMapData, reason) {
674+
workerState.screenMaps = screenMapData;
675+
workerState.screenMapsDirty = true;
676+
if (workerState.graphicsManager && workerState.graphicsManager.updateScreenMap) {
677+
workerState.graphicsManager.updateScreenMap(screenMapData);
678+
workerLog('LOG', 'BACKGROUND_WORKER', 'ScreenMaps sent to graphics manager', {
679+
reason,
680+
screenMapCount: Object.keys(screenMapData || {}).length
681+
});
682+
} else {
683+
workerLog('WARN', 'BACKGROUND_WORKER', 'Graphics manager not ready, screenMaps cached for initialization', { reason });
684+
}
685+
}
686+
611687
/**
612688
* Handles animation start request
613689
* @param {Object} _payload - Start parameters (unused)
@@ -639,47 +715,16 @@ async function handleStart(_payload) {
639715
workerState.externFunctions.externSetup();
640716
workerLog('LOG', 'BACKGROUND_WORKER', 'FastLED setup completed');
641717

642-
// Poll for screenmap data after setup (C++ setup() has registered screenmaps)
643-
// EM_JS push mechanism has linking issues, so we use polling instead
718+
// Read the screenmaps registered by C++ setup(). Layouts FastLED creates
719+
// lazily on the first exported frame are picked up later by
720+
// refreshScreenMapsIfIncomplete() (#250).
644721
try {
645-
const Module = workerState.fastledModule;
646-
647-
// Bind getScreenMapData if not already bound
648-
if (!workerState.wasmFunctions) {
649-
workerState.wasmFunctions = {
650-
getFrameData: Module.cwrap('getFrameData', 'number', ['number']),
651-
getScreenMapData: Module.cwrap('getScreenMapData', 'number', ['number']),
652-
getStripPixelData: Module.cwrap('getStripPixelData', 'number', ['number', 'number']),
653-
freeFrameData: Module.cwrap('freeFrameData', null, ['number'])
654-
};
655-
}
656-
657-
// Fetch screenmap data from C++
658-
const screenMapSizePtr = Module._malloc(4);
659-
const screenMapDataPtr = workerState.wasmFunctions.getScreenMapData(screenMapSizePtr);
660-
661-
if (screenMapDataPtr !== 0) {
662-
const screenMapSize = Module.getValue(screenMapSizePtr, 'i32');
663-
const screenMapJson = Module.UTF8ToString(screenMapDataPtr, screenMapSize);
664-
const screenMapData = JSON.parse(screenMapJson);
665-
666-
// Update worker state and notify graphics manager
667-
workerState.screenMaps = screenMapData;
668-
workerState.screenMapsDirty = true;
669-
if (workerState.graphicsManager && workerState.graphicsManager.updateScreenMap) {
670-
workerState.graphicsManager.updateScreenMap(screenMapData);
671-
workerLog('LOG', 'BACKGROUND_WORKER', 'ScreenMaps fetched and sent to graphics manager', {
672-
screenMapCount: Object.keys(screenMapData).length
673-
});
674-
}
675-
676-
// Free the allocated memory
677-
workerState.wasmFunctions.freeFrameData(screenMapDataPtr);
722+
const screenMapData = fetchScreenMapsFromWasm();
723+
if (screenMapData) {
724+
applyScreenMaps(screenMapData, 'post-setup');
678725
} else {
679726
workerLog('WARN', 'BACKGROUND_WORKER', 'No screenmap data available after setup');
680727
}
681-
682-
Module._free(screenMapSizePtr);
683728
} catch (error) {
684729
workerLog('ERROR', 'BACKGROUND_WORKER', 'Failed to fetch screenmap data', error);
685730
// Non-fatal - continue with animation
@@ -977,6 +1022,89 @@ function handleScreenMapUpdate(payload) {
9771022
}
9781023
}
9791024

1025+
/**
1026+
* Picks up layouts that FastLED creates after setup() (#250).
1027+
*
1028+
* FastLED fills in default screenmaps lazily when a frame is exported
1029+
* (jsFillInMissingScreenMaps in FastLED's src/platforms/wasm/js_bindings.cpp.hpp),
1030+
* which is after handleStart()'s single post-setup read. Without this, a sketch
1031+
* that never calls setScreenMap() (stock Blink) renders an empty canvas.
1032+
*
1033+
* Only fetches while a strip in the current frame still has no layout, and gives
1034+
* up on a strip after MAX_SCREENMAP_REFRESH_ATTEMPTS fetches so a genuinely
1035+
* layout-less strip cannot cost a JSON parse every frame. The push-based
1036+
* screenmap_update path (handleScreenMapUpdate) is unaffected.
1037+
*
1038+
* NOTE: this worker is mirrored in FastLED at
1039+
* src/platforms/wasm/compiler/modules/core/fastled_background_worker.ts; that
1040+
* copy needs the same change (out of scope for this repo).
1041+
*
1042+
* @param {Array} frameData - Strip data from extractFrameData()
1043+
* @returns {boolean} True when a refresh fetch was performed
1044+
*/
1045+
function refreshScreenMapsIfIncomplete(frameData) {
1046+
if (!Array.isArray(frameData) || frameData.length === 0) {
1047+
return false;
1048+
}
1049+
1050+
const missing = findStripsMissingLayout(frameData, workerState.screenMaps);
1051+
if (missing.length === 0) {
1052+
return false; // every strip already has a layout - nothing to do
1053+
}
1054+
1055+
const attempts = workerState.screenMapRefreshAttempts;
1056+
const retryable = missing.filter((stripId) => (attempts[stripId] || 0) < MAX_SCREENMAP_REFRESH_ATTEMPTS);
1057+
if (retryable.length === 0) {
1058+
return false; // already retried these strips; stop polling
1059+
}
1060+
for (const stripId of retryable) {
1061+
attempts[stripId] = (attempts[stripId] || 0) + 1;
1062+
}
1063+
1064+
const screenMapData = fetchScreenMapsFromWasm();
1065+
if (!screenMapData || typeof screenMapData !== 'object') {
1066+
return false;
1067+
}
1068+
1069+
// Merge rather than replace: layouts pushed earlier through the
1070+
// screenmap_update path must survive a late re-read that happens to return
1071+
// fewer entries (or an empty dictionary).
1072+
const merged = Object.assign({}, workerState.screenMaps, screenMapData);
1073+
const stillMissing = findStripsMissingLayout(frameData, merged);
1074+
const recovered = missing.filter((stripId) => !stillMissing.includes(stripId));
1075+
if (recovered.length === 0) {
1076+
// Nothing new arrived; leave the cache (and its dirty flag) untouched so we
1077+
// do not re-push identical layouts to the graphics manager every frame.
1078+
if (stillMissing.length > 0) {
1079+
workerLog('WARN', 'BACKGROUND_WORKER', 'Strips still have no layout after screenmap refresh', {
1080+
strips: stillMissing
1081+
});
1082+
}
1083+
return true;
1084+
}
1085+
1086+
applyScreenMaps(merged, 'late-screenmap-refresh');
1087+
1088+
if (!workerState.screenMapRecoveryReported) {
1089+
workerState.screenMapRecoveryReported = true;
1090+
workerLog('LOG', 'BACKGROUND_WORKER', 'Late screenMaps recovered after first frame', {
1091+
strips: recovered,
1092+
frameNumber: workerState.frameCount
1093+
});
1094+
// Surface it on the viewer's stdout log so `--test` runs can assert it.
1095+
postMessage({
1096+
type: 'stdout',
1097+
payload: { text: `[fastled] late screenmap recovered for strips ${recovered.join(',')}` }
1098+
});
1099+
}
1100+
if (stillMissing.length > 0) {
1101+
workerLog('WARN', 'BACKGROUND_WORKER', 'Strips still have no layout after screenmap refresh', {
1102+
strips: stillMissing
1103+
});
1104+
}
1105+
return true;
1106+
}
1107+
9801108
/**
9811109
* Handles audio samples from main thread and pushes them to C++ WASM ring buffer.
9821110
* AudioManager runs on the main thread (needs window/document), but Module.ccall()
@@ -1133,6 +1261,11 @@ async function executeFrameLoop(currentTime) {
11331261
const frameData = extractFrameData();
11341262

11351263
if (frameData) {
1264+
// Layouts FastLED creates lazily on the first exported frame (#250) only
1265+
// become visible after getFrameData(); pick them up before rendering so
1266+
// the dirty screenmaps ride along with this frame.
1267+
refreshScreenMapsIfIncomplete(frameData);
1268+
11361269
if (workerState.renderOnMainThread) {
11371270
// No OffscreenCanvas here: hand the frame to the main thread to draw
11381271
postFrameToMainThread(frameData);

0 commit comments

Comments
 (0)