fix: include view data in pipeline responses and skip zero-delta commits#159
Merged
streamer45 merged 7 commits intovideofrom Mar 14, 2026
Merged
Conversation
Root cause: two interacting bugs caused stale compositor layout data. A) Missing initial view data: The compositor server emits resolved layout on startup, and the engine stores it in node_view_data, but pipeline API responses never included this snapshot. New clients fell back to config-parsed positions which could disagree with the server's aspect-fit adjusted layout. B) Zero-delta click commits: handlePointerUp fired throttledConfigChange unconditionally — even for click-to-select with zero movement. This sent stale config-parsed positions for ALL layers, overwriting the server's correct resolved layout. Server-side: - Add Session::get_node_view_data() to expose engine's stored view data - Add Pipeline.view_data field (runtime-only, like Node.state) - Populate view_data in both REST and WebSocket get_pipeline handlers Client-side: - Extract pipeline.view_data into sessionStore.nodeViewData on receipt - Guard handlePointerUp: skip config commit when pointer delta is zero - Regenerate TypeScript types for the new Pipeline.view_data field Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
The 'sync from props' effect re-parses layers from params on every params change. In Monitor view, this overwrites the server's resolved layout (e.g. aspect-fit rects) with config-derived positions (e.g. rect:null → full canvas), causing a visible revert ~2s after dragging. Fix: add preserveGeometry option to mergeOverlayState. When sessionId is set (Monitor view), existing layer geometry (x, y, width, height) is preserved from current state (set by useServerLayoutSync) instead of being overwritten by parsed params. New/removed layers still sync correctly, and non-geometric fields (opacity, zIndex, visibility, text content) continue to flow through from params. Also fix test fixture: use camelCase property names matching LayerState interface and include all required fields. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
The previous preserveGeometry fix only kept x, y, width, height from existing state while rebuilding the overlay from parsed params. This lost runtime-only fields (measuredTextWidth, measuredTextHeight) and allowed stale param-derived values for opacity, rotation, z-index, and mirror flags to overwrite the server's resolved state. Now when preserveGeometry is true (Monitor view), mergeOverlayState starts from the full existing overlay and only overlays type-specific config fields (text, fontSize, fontName, color, dataBase64) from the parsed params. This preserves all server-resolved spatial fields AND any runtime measurements applied by useServerLayoutSync. Adds unit tests for mergeOverlayState covering: - OverlayBase field preservation from existing state - Config field updates from parsed params - Runtime field (measuredTextWidth) retention - Referential equality when nothing changed - Image overlay support - New/removed overlay handling Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
Seven integration tests that exercise the full data flow between useServerLayoutSync (server-driven layout) and the 'sync from props' effect (config-driven state) in Monitor view: 1. Server-resolved layer positions survive params echo-back 2. Server text measurements (measuredTextWidth) survive echo-back 3. Config changes (text, fontSize) are picked up while preserving server geometry 4. Server-resolved opacity/rotation/zIndex survive echo-back 5. Layer focus changes do not reset video layer positions 6. Layer focus changes do not reset text overlay size/position 7. Design view (control) still uses parsed positions as source of truth These tests would have caught all four variants of the stale data regression (bugs A–D in #159). Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
…r view Without the comparator, dataBase64 changes from other clients are silently dropped in Monitor view because the change-detection logic sees no OverlayBase field differences and returns the old array unchanged. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
…itor view Covers the bug where missing hasExtraChanges comparator caused dataBase64 updates from other clients to be silently dropped in Monitor view. Signed-off-by: StreamKit Devin <devin@streamkit.dev> Co-Authored-By: Claudio Costa <cstcld91@gmail.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.
Summary
Fixes a regression in the compositor node UI (Monitor view) where stale layout data appeared on the client and was sent back to the server on the next interaction (e.g. click-to-select).
Root cause — four interacting bugs:
A) Missing initial view data: The compositor server emits resolved layout on startup and the engine stores it in
node_view_data, but pipeline API responses (both REST and WebSocket) never included this snapshot. New clients fell back to config-parsed positions which could disagree with the server's aspect-fit adjusted layout.B) Zero-delta click commits stale data:
handlePointerUpfiredthrottledConfigChangeunconditionally — even for click-to-select with zero pointer movement. This sent stale config-parsed positions for ALL layers back to the server, overwriting the server's correct resolved layout.C) "Sync from props" effect overwrites server geometry: When the server echoes back updated params, the "sync from props" effect in
useCompositorLayersre-parses all layer positions from params viaparseLayers(). For layers withrect: null(auto-layout), this produces{x:0, y:0, width:canvasWidth, height:canvasHeight}— overwriting the server's aspect-fit adjusted positions thatuseServerLayoutSynchad applied. This caused a visible revert ~2 seconds after any drag.D)
preserveGeometryonly preserved x/y/w/h — not all server-resolved fields: The initial fix (C) only keptx, y, width, heightfrom existing state, but rebuilt the overlay by spreading{ ...parsed, ... }. This lost runtime-only fields (measuredTextWidth,measuredTextHeight) and allowed stale param-derived values foropacity,rotationDegrees,zIndex, and mirror flags to overwrite the server's resolved state — causing text overlays to revert size/position on focus change.Fix:
Server-side:
Session::get_node_view_data()to expose the engine's stored view dataPipeline.view_datafield (runtime-only, followsNode.statepattern)view_datain both REST and WebSocketget_pipelinehandlersClient-side:
pipeline.view_dataintosessionStore.nodeViewDataon receipt (bothsetPipelineandbatchSetPipelines)handlePointerUp: skip config commit when pointer delta is zeromergeOverlayStateMonitor-view merge: whenpreserveGeometryis true, start from the full existing overlay (preserving ALL server-resolved OverlayBase fields + runtime measurements likemeasuredTextWidth), then overlay only type-specific config fields (text,fontSize,fontName,color,dataBase64) from parsed params. UsespickConfigFieldshelper +OVERLAY_BASE_KEYSset to cleanly separate server-owned vs config-only fields.Pipeline.view_datafieldTests:
mergeOverlayStatepreserveGeometry behavior (9 tests): OverlayBase field preservation, config field updates, runtime field retention, referential equality, image overlay support, new/removed overlay handlingsetPipelineview_data extraction (initial load, merge, null, batch)Review & Testing Checklist for Human
Notes
crates/nodes/src/video/compositor/kernel.rsandtests.rsare NOT introduced by this PR — they fail on thevideobranch without any changes.Pipeline.view_datafield usesskip_serializing_if = "Option::is_none"andserde(default)so it's backward-compatible with existing pipeline definitions (YAML configs never include it).preserveGeometryflag only applies in Monitor view (whensessionIdis set). Design view (nosessionId) continues to use config-parsed positions as the source of truth, which is the correct behavior for editing.Link to Devin session: https://staging.itsdev.in/sessions/a5c78299b1204298b83f286a29918a25
Requested by: @streamer45