Repository navigation
fix(scene): carry display zones across the driver-scoped fingerprint change - #414
Draft
hyperb1iss wants to merge 4 commits into
Draft
hyperb1iss wants to merge 4 commits into
hyperb1iss wants to merge 4 commits into
Conversation
When a display's physical identity is remapped, the zone authored against its old identity lands on a display that may already own an empty surface minted for the new identity. Scene validation rejects two display zones for one device, so the remap now keeps exactly one. The zone carrying layers survives. When both carry layers, the one whose newest layer was assigned most recently wins, read from the UUIDv7 layer ids that every face assignment mints; when that is not knowable the remapped zone wins. Each collapse is logged at info with the reason. Runtime default faces drop a retargeted duplicate in favor of the default already registered, matching how the display preference store resolves the same collision. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…prints
Adding the driver segment to physical fingerprints rehashed every
physical DeviceId once. Layout reconciliation never noticed displays
like the Push 2 and the iCUE LINK LCD, because their layout ids read the
same in both encodings, and the sweep only ran when a layout id moved.
Scenes kept display zones pointing at dead ids while fresh empty
surfaces were minted for the live ones, so configured faces fell off
the screen.
The pre-scoping id is a pure function of the current fingerprint:
{namespace}:{driver}:{key} was {namespace}:{key}. The sweep now derives
it for every present device and maps it to the live id whenever a
persisted display zone still targets it. An id two present devices
derive, or one a present device still owns, is left alone. The sweep
runs when only physical identities move, through the existing journal
and transaction, so every participant and the scene store are rewritten
by their usual writers, and the scene remap collapses the resulting
duplicate display zones. Once nothing targets an old id the pass plans
nothing, so later sweeps are no-ops.
Device settings are deliberately left out: a claimed USB device's
settings key is its portable claim, which reads exactly like its old
fingerprint, so mapping those keys would move canonical rows away.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Drives two complete discovery sweeps over a named scene shaped like the rig that lost its LCD face: display zones authored against Push 2 and iCUE LINK LCD ids minted before driver-scoped fingerprints, the LCD's carrying its face, plus empty surfaces for the ids discovery reports now. The LCD is discovered with a portable serial claim, as on real hardware, which keeps it out of the shape-based reconciler. The first sweep must leave one display zone per display, the face intact on the live LCD, the scene valid and the journal cleared. The second sweep must not advance the scene revision or change the scene. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The device binding migration published its scene commit with an empty event batch, so the revision advanced while nothing told observers. The web UI and the TUI refetch scenes only on scene event hints or a reconnect, so when displays were already connected and an earlier sweep had deferred the migration, open clients kept the collapsed zones and the old display targets and failed their next write against them. The layout identity remap shared the same silent publication. Preparing the migration now diffs the live scene state against the candidate it installs and carries the result through admission and persistence to the commit ticket, which releases it in admission order like any other mutation: ZoneChanged for each zone the rewrite updated, created, or removed, ZoneChanged against the active scene for runtime default faces as their overlay path announces them, and SceneLibraryChanged for every saved scene the rewrite touched. ZoneChanged and SceneLibraryChanged carry no revision field, so the sweep test ties the batch to its commit by asserting the migration is exactly one scene commit and that a settled sweep emits nothing. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Comment |
This branch has not been deployed
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.
Since the August change that added the driver segment to every device fingerprint (
31e0d5d51), two displays on the owner's rig have been split in two. The Ableton Push 2 and the Corsair iCUE LINK LCD each kept a display zone pointing at the device id their old fingerprint hashed to, and the daemon minted a fresh, empty zone for their new ids. The old LCD zone still carried the configured face (Pulse Temp with custom fonts), so that face silently fell off the pump screen while the scene went on listing four display zones for two displays. After this PR, the next clean discovery sweep moves those zones to the live ids, collapses each duplicate pair onto the zone that carries a face, and tells connected clients, so the LCD gets its face back without anyone touching it.What this changes
core/src/scene/display_collisions.rscollapses display zones that end up targeting the same display. A zone with layers beats an empty one. When both have layers, the zone whose newest layer was assigned most recently wins (layer ids are UUIDv7 and every face assignment mints one). Otherwise the remapped legacy zone wins. Each collapse is logged at info. Fallback references to a removed zone are redirected to the survivor.ZoneChangedfor each zone updated or removed,ZoneChangedfor runtime default faces, andSceneLibraryChangedfor each saved scene the rewrite touched, released through the commit ticket in order. Before this, binding and layout-id migrations advanced the scene revision with an empty event batch, so an open UI or TUI kept showing removed zones and failed its next write with 404 or 412.Why
Display zones are retained across disconnects on purpose, so a device that comes back finds its face waiting. These two zones could never be matched again, because nothing would ever produce their ids, so retention preserved them forever without ever applying them. Fixing the migration, rather than pruning orphans, recovers the configuration the owner actually set.
Found while rehearsing the demo film: the rehearsal listed the extra zones, and the pump screen showed no face.
Verification
just verifypasses locally (Rust fmt + lint + test)just denypasses (required for dependency or license changes)just ui-testandjust ui-buildpass (required forcrates/hypercolor-ui/)just sdk-lint,just sdk-check, andjust sdk-buildpass (required forsdk/)just python-verifypasses (required forpython/)just compat-checkpasses (required fordata/drivers/vendors/*.toml)just docs-buildpasses (required for docs or README changes)cd docs && zola checkpasses (required for docs link/content changes)scripts/orpackaging/)just e2e-buildpasses with the normal Servo stack (required for daemon/UI/effect integration changes)just e2e-build-cpupasses when validating the CPU smoke fallbackjust e2epasses against the Servo stack (required for end-to-end behavior changes; starts daemon/browser)The regression test reproduces the live shape through a full discovery sweep: saved display zones pointing at the legacy hashes of a Push 2 and a serial-claimed LCD (one carrying a face), fresh empty zones for the current ids, and devices discovered under driver-scoped fingerprints. It asserts the remap, the collapse that keeps the face, the exact ordered event batch with a single revision bump, and a second sweep that changes and emits nothing. On the base this test fails, because base reconciliation plans no remap for these layouts and returns early, leaving all four zones.
discovery_tests, single-threaded--lib, single-threadedinternal_api_surface_testsscene_device_binding_remap_testshtml_loader_testsneed generated effect files and fail without them, unrelated)-D warningson core and daemon (--all-targets),cargo check --workspace, fmtThe full
just verifygate deadlocks on this machine's GPU tests in parallel, so the daemon suite ran single-threaded instead. The computed ids match the four observed on the rig. The live rig itself has not run this build: the migration needs a clean full sweep with no scan errors, so it applies on the next daemon start that sees every device.Notes for reviewers
domain/device_binding.rs. An independent review checked it against every fingerprint shape inDeviceIdentifier::fingerprint(USB serial, USB and HID path fallback, SMBus, network MAC, generic bridges) and found no wrong-device mapping. It does not cover drivers whose fingerprint changed format in other ways in the same commit: Hue, Nanoleaf, and the ROLIblocksdtorolirename. None of those exposes a display today.🤖 Generated with Claude Code