feat(api): add frame notification and display buffer access - #1149
Merged
Conversation
The API gave embedders no way to know when the display was repainted. An embedder mirroring the screen elsewhere had to poll, which is wrong in both directions: the interpreter posts a frame only when the running app draws something, so a settled SceneGraph app posts nothing for seconds while a busy one posts at the 60fps ceiling. Polling is late on the first and wasteful on the second. Two opt-in additions, both no-ops unless enabled: - `setFrameNotify(enabled)` gates a new `frame` event, emitted at the end of `drawBufferImage()` — the single real paint into the display canvas, and after the canvas has content, so a consumer that copies on the event always sees a complete frame. The payload is a bare monotonic counter, so a 60fps path allocates nothing. - `getDisplayBuffer()` returns `bufferCanvas`, the canvas the visible display is drawn from. It is always at the display mode's native resolution and carries no overscan guidelines, where the visible canvas is sized to the window and may be considerably smaller — copying that one streams a blurry upscale. Blanking the display is reported as a separate `cleared` event rather than as a frame, because it cannot be served from the buffer: `clearDisplay()` deliberately never touches `bufferCanvas`, so after an app exits the buffer still holds that app's final image. A consumer told "new frame" there would copy it and show an app that had already quit. `updateBuffer()` and `drawBufferImage()`'s else-branch now use a non-notifying `clearCanvas()`, so the transient blank before a queued repaint is not reported at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review follow-ups on the frame notification API, all found before release,
so the event rename is not a breaking change.
Blanked display was reported as a frame. When the display is disabled the
app keeps drawing, so the buffer holds fresh content while the screen shows
black: `drawBufferImage()` emitted `frame` there, making a mirroring
embedder stream content the display was not showing. It now emits the
cleared event instead. `setDisplayState()` also acts on the transition,
because a settled app never repaints on its own: turning the display off
blanks and notifies, turning it back on queues a repaint from the buffer.
Both defer to the video loop when it is driving repaints. Consecutive
blanks collapse into a single event.
A throwing observer could permanently freeze rendering. Display events are
raised from inside the requestAnimationFrame loop, which reschedules itself
*after* notifying, so an exception unwound past the reschedule and no
further frame was ever queued. Both `notifyAll` implementations now isolate
each callback.
The display buffer is not at the display mode's resolution: it tracks the
app's screen size, so `CreateObject("roScreen", true, 640, 480)` renders
640x480 in any mode. Docs corrected, and the resize is now a shared helper
so the splash path reports `resolution` before changing dimensions like
`updateBuffer()` does (it also clears explicitly, since the helper only
resets the canvas on an actual size change).
Remaining fixes: the frame counter restarts on every enable, as documented,
not only on the false -> true edge; the events are renamed to
`framePainted`/`frameCleared` to stop colliding with the Node library's
`frame` event, which carries an `ImageData` instead of a counter; and the
relay's event list is hoisted out of the per-event path.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
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.



Why
The API gave embedders no way to know when the display was repainted, so anything mirroring the screen elsewhere had to poll. That is wrong in both directions: the interpreter posts a frame only when the running app draws something (
RoSGScreen.finishDraw()/RoScreenare gated on an internalisDirtyflag), so a settled SceneGraph app posts zero frames while a busy one posts at the 60fps ceiling. Polling is late on the first and wasteful on the second.This surfaced in
brs-desktop's new Remote Screen service: a static menu app took several seconds to show a keypress remotely, while a grid/video app streamed smoothly.What
Two opt-in additions, both no-ops unless a consumer enables them — an embedder that doesn't stream pays nothing.
setFrameNotify(enabled)getDisplayBuffer()bufferCanvas, ornullbefore initframePaintedeventnumber)frameClearedeventnull)framePaintedis emitted at the end ofdrawBufferImage()— the single real paint into the display canvas. It fires after the canvas has content, so a consumer that copies on the event always sees a complete frame, and it covers the video path (drawVideoFrame) which bypassesupdateBuffer()entirely. The payload is a bare number rather than an object, so a 60fps path allocates nothing. The names are deliberately distinct from thebrs-nodelibrary'sframeevent, which carries anImageData: an embedder porting code between the two libraries would otherwise readdata.widthoff a counter.getDisplayBuffer()returns the canvas the visible display is drawn from. It carries no overscan guidelines and is at the resolution the app actually renders at, where the visible canvas is sized to the window (CSS size × dpr) andredrawDisplay()lets it be smaller than the frame — copying that one streams a blurry upscale from a small window.bufferCanvasis created once and only ever resized, never reassigned, so a getter returning it stays valid for the session. Documented as read-only; returning a copy would defeat the purpose.Note the buffer is at the app's screen size, not the display mode's:
CreateObject("roScreen", true, 640, 480)renders 640x480 in any mode, and even the default 480proScreenis 854x480 while the mode is 720x540. Consumers must re-read the dimensions each frame; aresolutionevent precedes every change, which is why the resize is a shared helper — the splash-screen path used to resize silently.Why
frameClearedis a separate eventBlanking the display cannot be served from the buffer.
clearDisplay()deliberately never touchesbufferCanvas, so after an app exits the buffer still holds that app's final image — a consumer told "new frame" there would copy it and leave the viewer looking at an app that had already quit. Hence two events rather than one, with the asymmetry documented at the emit site.The same reasoning covers a disabled display (
roAppManager.setDisplayDisabled(true)): the app keeps drawing, so the buffer holds fresh content while the screen shows black. That path reportsframeCleared, notframePainted.setDisplayState()acts on the transition too, since a settled app never repaints on its own — turning the display off blanks and notifies, turning it back on queues a repaint from the buffer so the screen doesn't stay black. Both defer to the video loop when it is already driving repaints, and consecutive blanks collapse into one event so a disabled display doesn't emit 60/s.Relatedly,
updateBuffer()anddrawBufferImage()'selsebranch call a non-notifyingclearCanvas(): those blanks are transient, with the repaint already queued, so they must not be reported at all.Observer isolation
Display events are raised from inside the
requestAnimationFrameloop, anddrawVideoFrame()reschedules itself after the notification. An embedder callback that threw would unwind past the reschedule, leavingvideoLooptrue and no frame queued — the display would freeze for the rest of the session with no way for host code to recover. BothnotifyAllimplementations now isolate each callback and log the failure to the console (routing it through the event system would recurse into the same faulty observer).Testing
src/api/has no test coverage in this repo (no DOM harness;vitest.config.mtsisenvironment: "node"), and adding jsdom for this would be a large dependency for one module. Verified by running the desktop app against this build:frameClearedpath).getDisplayBuffer()win).npm run lintclean;npx prettier --check src/api/display.ts src/api/index.tsclean. (npm run prettierreports pre-existing failures in generatedpackages/*/types/*.d.tsandpackages/scenegraph/lib/*.js, unrelated to this change.)npm run build:apiandnpm run build:clisucceed, the bundle exports both new methods, and the full suite passes (200 files / 2539 tests)..d.tsfiles are generated, so the JSDoc here is the published contract — written accordingly.docs/engine-api.mdgains two Methods rows and two Events rows;docs/using-node-library.mdnotes that itsframeevent is unrelated.Consumer
brs-desktopPR (Remote Screen becomes event-driven, ~100 lines of polling heuristics deleted) depends on this shipping in a release.🤖 Generated with Claude Code