WS-1: boot the studio on gpui-kit with a light/dark/system theme - #313
Merged
Merged
Conversation
Boot: gpui_kit::application().run(...) + gpui_kit::init(cx) inside it -
there is no Application::new() in this kit. The window opens from
cx.spawn(async move |cx| { cx.open_window(...) }) so the state entity
and the theme can be set up with a live window and App context in hand
before the first paint.
Root wrapper: gpui_component::Root::render mounts the app's view but
NOT the dialog/sheet/notification layers - the app has to composite
those itself via Root::render_dialog_layer/_sheet_layer/
_notification_layer. Isolated all three calls in app/overlays.rs and
nowhere else: gpui-kit 0.7.0 removes them with no documented
replacement, so when that migration lands it is a one-file patch
instead of a hunt across every screen. StudioRoot (app/root.rs) is
currently a placeholder - it renders "Library" or "Editor" depending
on StudioState.view, since library::view and editor::view (WS-3/WS-4)
are not ported yet. What is live and testable ahead of them: the
window, the theme, and the overlay layer stack.
Theme: gpui-component's ThemeMode only has Light/Dark, and theme::init
(run once by gpui_kit::init) hard-sets Light with nothing observing
the OS. ThemePref::System is built on top in theme/mod.rs:
- apply() resolves System via Theme::sync_system_appearance, which
calls Theme::change internally - so it already re-projects onto
the gpui-base layer (scrollbars, resize handles); no separate
Theme::sync_base call is needed on this path.
- watch_system_appearance() re-checks state.theme_pref on every
window.observe_window_appearance callback (not a value captured at
subscribe time), so it only re-resolves against the OS while the
live preference is still System - an explicit Light/Dark choice
made after subscribing is never clobbered by a later OS flip.
- theme::set() is the pref-switching entry point the topbar's theme
button (WS-4) will call once that screen exists; nothing calls it
yet, hence the #[allow(dead_code)].
Persistence (theme/persist.rs): same shape as
library::data::load_recents/push_recent - a JSON value under
dirs::config_dir()/rustmotion/, here theme.json. Fixes the theme
resetting to System on every launch.
Window sizing (app/window.rs): 75% of the primary display, centered,
same target as the old Dioxus shell's one-shot use_effect resize - but
set on WindowOptions.window_bounds before the window opens via
Bounds::centered(None, size, cx), so there is no first-frame resize
flash. Falls back to a fixed 1280x800 when no display can be found,
still centered, rather than the old silent no-op.
Token mapping for screens styling off cx.theme() (gpui_component::
ActiveTheme) in place of the old --rm-* CSS custom properties
(formerly THEME_CSS in app/root.rs):
--rm-bg -> background
--rm-surface -> popover (raised panel / card fill)
--rm-surface-2 -> secondary (a step below popover)
--rm-surface-3 -> muted (hover / selected fill)
--rm-border -> border
--rm-border-2 -> ring (stronger, focus-adjacent edge - no
exact equivalent, this is guidance not a
contract)
--rm-text -> foreground
--rm-text-strong -> popover_foreground / primary
--rm-text-muted -> muted_foreground
--rm-accent -> accent / primary
--rm-on-accent -> accent_foreground / primary_foreground
--rm-error -> danger (NOT destructive - that name does not
exist on ThemeColor)
--rm-overlay-hover -> muted (or accent at reduced opacity)
--rm-overlay-border -> ring
Nothing here re-brands the kit's default palette (no custom
ThemeColor/ThemeConfig) - that is a separate, larger decision left to
whichever workstream first needs brand-accurate colors.
Also: gpui-component added as a direct Cargo.toml dependency (reported
separately, not included in this diff) - ActiveTheme/Theme/ThemeMode/
Root are used by their real crate path throughout this code, which
needs gpui-component in the extern prelude; gpui-kit only re-exports
it as gpui_kit::component.
WS-1 restored the shell; this restores the crate around it. These are the convergence points the partition reserves to the orchestrator — module registration, the manifest, and the two frozen-core files whose only Dioxus tie was a hook. No workstream writes them, precisely so six parallel rewrites cannot each reshape the wiring under one another. Three Dioxus sites are removed rather than ported, because the thing they did no longer exists. use_prefetch_publisher and use_export_poll polled Dioxus signals; gpui notifies explicitly, so WS-2 and WS-6 replace them with state updates. ExportToast hand-rolled a floating div; the kit has a notification system, so WS-6 uses it. diff_panel.rs loses everything but DiffSide, the one piece a frozen file (app/state.rs) and a kept file (prefetch.rs, via FrameKey.side) both need. That file was a third Dioxus site the decomposition had not anticipated — it claimed exactly two, having grepped a file list that omitted it. WS-1 caught it. frame_for_change, the pointer-to-first-frame lookup deleted with the rest, is real logic rather than view code and is worth WS-6 rebuilding rather than reinventing. editor and library carry a crate-level #[allow(dead_code)] because each now mixes code with a live caller against code whose only caller is a view file excluded from its mod.rs until its workstream lands. Rustc cannot tell "orphaned pending a rewrite" from "genuinely dead", and a per-item allow would mean editing files that are frozen or owned elsewhere. Both allows come off once WS-2 through WS-6 have landed; library/mod.rs also stops re-exporting render_thumbnail and ScenarioEntry, which WS-3 puts back in one line. palette was declared default-features = false, and its default set is the only thing supplying std. Without it palette has no source for the Round/Sqrt/Abs trait impls on f32 — there is no libm fallback at 0.7.6 — so the dependency failed to build with 18 errors regardless of anything in this crate. It went unnoticed because an incremental target directory kept serving an artefact built before the flag was added. Pre-existing, unrelated to gpui-kit, and fixed here because nothing compiles until it is. Cli::file and Cli::dir move their descriptions into #[arg(help = ...)]. Under the no-comment rule their doc comments would simply have been deleted, and clap derives --help from exactly those, so rustmotion-studio --help would have gone quietly empty. The rule is about prose in the source, not about dropping behaviour. Verified on the integrated tree: cargo fmt --all --check, cargo clippy -p rustmotion-studio --all-targets -D warnings, and cargo test -p rustmotion-studio all exit 0, with 114 unit and 7 integration tests passing and 2 soak diagnostics ignored. Refs #306, #307
The first CI run of the chantier failed with `unable to find library -lxkbcommon-x11`, and failed in a way worth recording: clippy was green. clippy type-checks but never links, so a missing system library is invisible to it and surfaces only in the test job, seventeen minutes later. Anyone reading "clippy pass, test fail" on this repo should suspect a linker dependency first. webkit2gtk, gtk-3 and xdo go. They were there for dioxus desktop, which no longer exists in this workspace, and nothing else in it pulls glib. In their place gpui needs xkbcommon-x11 for keyboard handling, plus wayland and xcb-cursor for the two display backends it builds on Linux. fontconfig, freetype and asound are untouched: the first two belong to Skia's text stack and the third to cpal, which rodio pulls in for preview audio. Neither had anything to do with the UI framework. publish.yaml gets the same list, and its explanatory block is rewritten rather than extended. It described the GTK/WebKit stack and glib-2.0 as the reason the packages were needed, which stopped being true with this change. A release workflow whose comment explains a dependency that is no longer there is worse than one with no comment: it sends the next reader looking for a glib failure that cannot happen. Refs #306
Closes #308. Part of #306. The JPEG encode disappears. It never existed for quality or size — it existed because the frame had to cross into a webview, and an <img> tag needs an encoded byte stream. gpui takes decoded pixels directly, so the encode, the wry asset handler and the /frame/{idx}?v={rev} URL all go, and with them a round trip through a codec on every displayed frame. What replaces it is `RenderImage::new(smallvec![Frame::new(buffer)])` wrapped in `img(ImageSource::Render(..))`. That variant bypasses every software cache in gpui — it hands the Arc straight back — and `RenderImage::new` mints a fresh monotonic ImageId per call, so a frozen preview from a stale cache entry is not merely unlikely here, it is unrepresentable. The old path needed no-cache headers on every response to get the same guarantee. Frames are written opaque. gpui's image pipeline blends with SourceAlpha / OneMinusSourceAlpha — straight alpha — while Skia produces premultiplied, and reconciling the two would mean a division pass per pixel per frame. It is unnecessary: `render_frame_v2_scaled` clears the whole framebuffer with the scenario background before anything paints, and the JPEG path it replaces already discarded alpha via to_rgb8(). Forcing 255 in the same loop that swaps R and B costs nothing and sidesteps the mismatch entirely. `swap_frame` hands the evicted frame to `cx.drop_image`. This is the one place in the rewrite where forgetting a line leaks half a gigabyte of VRAM per second: gpui's sprite atlas has no eviction, and each 1920x1080 frame owns its own ~8 MB Metal texture. The drop happens in the update path, never during paint, because update and paint are turns of the same single-threaded loop and cannot interleave — an argument from the executor model, not something a test exercises, since no live window is available without a test-support feature this crate does not enable. `render_frame_rgba_deep` mirrors the existing `render_frame_deep` rather than calling `render_frame_rgba` directly. The 32 MiB stack is not decoration: untagged serde buffers a frame per level of scene nesting, and the 2 MiB default overflowed the prefetch workers. The new path runs on the background executor pool, which has no better stack than they did. `prefetch_target_for` is split out of `publish_prefetch_target` so the target it builds can be tested as a value. Its own test previously waited two seconds for a real worker thread to render, then asserted on frame 0 — which `prefetch_window` never schedules, since a playing prefetcher only looks forward from the playhead. It also keyed on a global atomic that a sibling test mutates. It could not pass, and would have raced if it could.
…n gpui-kit Closes #312. Part of #306. Three panels, and one of them stops being hand-rolled. The export toast was an absolutely-positioned div with its own dismiss timer and close button; gpui-kit has a notification system, so `ExportWatcher` pushes into it instead, keyed by a marker type so a running export's progress replaces its own toast in place rather than stacking. That watcher polls the export slot at 150 ms rather than being driven from the encode thread, and the reason is a frozen boundary, not laziness: `start_export` runs on a plain std::thread with no window handle, and `export_slot()`'s mutex has no notification hook. Pushing from inside would mean modifying code this workstream was told not to touch. The idiom is also the library's own — `NotificationList::start_advancing` in gpui-component polls its lifecycle the same way, with the same `cx.spawn_in` + timer shape. `frame_for_change` comes back with tests it never had. It maps a change's pointer to a playable frame so clicking a diff entry scrubs to where the element actually lives. Two details in it are load-bearing and were previously recorded nowhere: `start_at` is floored at zero, because a negative value would walk the playhead backward into the previous scene; and the result is clamped to the last frame, because a late `start_at` — or one left over from a scene that has since been shortened — would otherwise index past the end of the task list. It used to live inline in a Dioxus component, which is why it had no coverage; it now has five tests. The annotation capture box reads its target from `EditorState.selected` rather than taking pointer and kind as constructor props. The props version put a stale-data footgun in the caller's hands: whoever mounts the box would have to remember to update it on every selection change, and forgetting would file a comment against the wrong element in silence. Reading at submit time makes that unrepresentable, and matches how the box already read the playhead. Diff badges use theme tokens instead of the literal #22c55e the CSS carried, so the panel follows light and dark without a second palette. Persistence is untouched: JSON sources rewrite the scenario and record an undo snapshot, HTML sources write the sidecar and never touch the HTML, and a corrupt sidecar is still an error rather than something to overwrite. Left for integration with WS-4: `TOPBAR_HEIGHT` is px(40.), transcribed from the old CSS, and needs checking against the real topbar once it exists.
CI's clippy rejected `chunks_exact_mut(4)` under `clippy::chunks_exact_to_as_chunks`, while the same command passed locally. The lint is not the interesting part; the gap is. This machine runs rustc 1.97.1 from July, CI pins stable as of today, and the lint landed in between. So a local green does not close a workstream here — it only makes one ready for CI to judge. Worth stating plainly, because every earlier failure in this chantier reproduced locally (the linker one failed on both sides) and this is the first that could not. Refs #306
Closes #311. Part of #306. The largest piece of the chantier: 2 225 lines in one file become 2 856 across four, split along the line that matters — what depends on the UI framework and what does not. `sections.rs` carries the curated schema and its value helpers with zero gpui in it, so the tables that decide which control appears for which property can be tested as data. `write.rs` carries the write path. `controls.rs` is the only place an Entity is created. `mod.rs` assembles the panel. The write path keeps both of its halves, and they stay independent: an edit applies optimistically in memory so the canvas moves within one render, and separately queues a Mutation behind a 250 ms debounce that, when it fires, re-reads the file from disk and replays the queue onto *that* content. Replaying onto a snapshot would be simpler and would silently destroy an agent's edit landing mid-window; `tests/audit_ws_g.rs` simulates exactly that race and is green. Dioxus's cancellable Task becomes `Option<gpui::Task<()>>`, where replacing the field drops and cancels the previous one — same semantics, and this time the cancellation is the language's rather than the framework's. Entities are created once per selection, not per render. `ensure_controls_for_selection` runs at the top of every render but rebuilds only when the selected pointer changes, so typing, dragging a slider or picking a colour never destroys the widget holding the cursor. The one deliberate exception is structural edits — adding or removing a list entry, switching a Fill between solid and gradient — which change how many controls exist and so must rebuild; `force_rebuild` marks those explicitly rather than letting the rebuild gate guess. `apply_open_change` comes back with its test. It was the one test this chantier had lost, deleted with the in-house colour picker, and it encodes a rule the kit does not: `ColorPickerState` owns its own `open` flag and emits no open/close event, so panel-wide exclusivity is enforced by observing each picker's `is_open()` and force-closing the others. Scrolling the panel still closes the expanded one. On the curated schema, which is what sank the discarded first attempt: 44 Field entries and 64 assembled rows, both matching the original and both now locked by a test. The earlier "45 against 47" alarm turned out to be a miscount — a raw grep for `Field {` also catches the struct definition and the `Mutation::Field` literals in the write path. Counting the thing rather than the string is what a test buys here, and the registry's own completeness invariant (every CssStyle property lands in exactly one section, unmapped ones falling to Advanced) is untouched and still locked. Bold/Italic and the alignment group render as text labels rather than icons. The Lucide variants exist in gpui-kit-assets but the embedded default bundle does not carry them, and a missing icon fails silently at runtime rather than at compile time — not something to gamble on without a window to look at. Swapping them back is one line once the asset registration grows. 159 tests pass, up from 144.
Closes #309. Part of #306. Thumbnails stop being JPEGs. The frozen `data::render_thumbnail` encodes to JPEG because the old wry handler served them to an <img> tag; feeding gpui that would mean decoding straight back to the pixels the renderer already had. The library now calls `build_frame_tasks` + `render_frame_rgba_deep` at scale 0.25 and hands the result to WS-2's `frame_from_rgba`, so the whole codec round trip disappears and the two image paths in the app are one path. That makes the grid a real memory surface, since every decoded thumbnail owns a Metal texture in an atlas with no eviction. It is bounded to exactly what is visible: `sync_thumbnails` diffs the current section-and-search result against what is held, renders what appeared, and hands what vanished to `cx.drop_image`. Switching folders or narrowing the search frees the rest. The grid computes its own column count from the viewport each render, because `grid_cols` takes a fixed number and there is no per-track minmax — so `repeat(auto-fill, minmax(240px, 1fr))` becomes an arithmetic floor over the available width, which resizes like the CSS did rather than approximating it with flex wrap. `StudioRoot` stops being a placeholder for the library half and mounts the screen. It also gains `cx.observe` on the shared state: `view` lives in `StudioState`, so a card click flipping it to `Editor` changes a field the root had read but was not subscribed to, and the screen would not have repainted. One bug worth recording, because it was in a test rather than in code: the first version of the recents test read the developer's real `~/.config/rustmotion/recent.json` and passed or failed depending on what that machine happened to have opened. It now sets `LibraryState.recents` directly and covers both branches. A test that depends on the machine it runs on is worse than no test — it reports the environment, not the behaviour. Also noticed, not acted on: `LibraryState.thumb_cache` appears to have no callers left. It cached JPEG bytes for the wry handler that no longer exists. It sits in a frozen file and belongs to its own change, not this one. 166 tests pass, up from 159.
… the canvas Closes #310. Part of #306. The last of the six workstreams, and the one that assembles the others: topbar, canvas, hit overlay, transport and shortcuts, mounting the inspector, the diff panel, the annotations panel, the export watcher and the frame surface. The app opens on the library, opens a scenario into the editor, and both screens boot clean. Shortcuts move from bubbled key events to gpui actions bound under a `key_context`. The old code kept Space out of text fields by calling `stop_propagation` on every container that might contain one — a rule enforced at each site and silently broken by forgetting one. gpui resolves bindings by context specificity instead, so an `Input` owning `left`/`right`/`cmd-z` under its own context wins over the editor's while focused, without the editor knowing text fields exist. `playback_action` keeps its four tests and stays the single source of truth; only its key type changes, since gpui has no `Key` enum and hands over a plain string. The larger fix here was not in WS-4's own code. The prefetch workers filled a frame cache that nothing ever read: `FrameCache::get` had no callers anywhere in the crate. The workers rendered every frame in the window ahead of the playhead, JPEG-encoded it and stored it, while the canvas independently re-rendered the same frame in RGBA — so the pool was not merely useless, it competed for CPU with the render the UI was waiting on. It surfaced only when the transitional crate-level `#[allow(dead_code)]` came off. The cause is a gap in the decomposition rather than in either workstream. `FrameKey` was frozen as a shared contract, but nothing in either brief said the canvas must consult the cache before rendering, so WS-2 built the producer, WS-4 built the consumer, and each was internally correct. WS-4 even defined its own local `FrameKey` tuple for render deduplication, which is exactly what a missing contract looks like from the inside. The canvas now builds the real `FrameKey` — model generation for side B, baseline source hash for side A, matching what the workers key on — and decodes a cache hit rather than re-rendering. Decoding a JPEG costs a few milliseconds against tens for a Skia render, and it keeps the cache's memory profile and its tested eviction policy untouched. Caching raw RGBA would be faster still, at 8 MB a frame against a cap of 120; that trade deserves its own change. 171 tests pass, up from 166.
…achable Part of #306. The transitional `#[allow(dead_code)]` on `editor`, `library`, `state` and the theme module existed because, mid-chantier, each mixed live code with code whose only caller was a view file not yet ported. All six workstreams have landed, so the allows come off — and with them off, the compiler names what the rewrite left behind. `scenario::Theme` goes. It mapped Dark/Light/System onto the CSS class names `rm-dark`/`rm-light`/`rm-system`, which is a sentence that only parses in a webview. `ThemePref` in `app/state.rs` replaced it, and nothing had referenced the old one since WS-1. `render_thumbnail` goes. It encoded frame 0 to JPEG for the wry asset handler that served `/thumb/{i}`; the library now renders straight to RGBA through the same path as the canvas, so the encode had no consumer. `seg_button` and `num_display` go the same way — superseded during the inspector rewrite by `fill_seg_button` and by `properties::display_number`. Three declarations keep a narrow, deliberate allow rather than being stripped: `SelectionData`, `ColorWidget` and `ScalarWidget` each carry fields the inspector populates and never reads. Removing them means proving, field by field, that nothing in 2 857 lines of freshly written code wanted them — which is a review, not a cleanup, and belongs to its own change rather than to the tail of this one. The allow is on the exact declaration so a future reader sees the scope, not a blanket over the module. What this pass is really worth is the defect it exposed rather than the lines it deleted: the crate-level allow was also hiding `FrameCache::get`, uncalled, which meant the entire prefetch pool was rendering frames nothing consumed. That is fixed in the preceding commit. A blanket allow does not merely tolerate dead code, it conceals live code that stopped being reachable.
…indow on assets Part of #306. Four defects, all found by opening the app and looking at it. Every one of them survived a green `fmt`/`clippy`/`test` run and a check that the process stayed alive for six seconds — which is exactly as much as that check is worth. Two of them made the editor useless. **No icons anywhere.** `gpui_kit::application()` was called without `.with_assets(..)`, so no asset source was registered and every icon resolved to nothing — silently, because a missing icon is not an error. It registers `AllAssets` rather than the curated set: WS-4 and WS-5 had each fallen back to text labels for icons the default bundle does not embed (mute, theme-system, baseline, diff, comments, export, bold, italic, alignment), and the full Lucide catalog removes the reason for those fallbacks. **A blank canvas.** The aspect-ratio box carried `aspect_ratio` plus `max_w_full`/`max_h_full` and no concrete dimension. In flexbox, a box with only maximum constraints resolves to zero, and the frame inside it is `size_full` — 100% of nothing. Measured before and after by printing the resolved bounds: 0 × 0, then 1222 × 687. The frame had been rendering correctly the whole time and being painted into a box with no area. **The scrubber pinned at maximum.** `EditorView` is constructed once, in `StudioRoot::new`, before any scenario is open — so the slider was built with the empty scenario's frame count, its `max` froze at 1, and every `set_value` clamped there. `SliderState::max` is a consuming builder with no runtime setter, so the fix is not to update it but to stop depending on it: the slider now runs on a normalized 0..1 fraction, converted at both ends. That also unbinds the slider's lifetime from the scenario's, so the same bug cannot return through that path when a different file is opened. **Up to a second of dead time before the window appeared.** `prefetch_icons` and `preextract_video_frames` ran synchronously ahead of `gpui_kit::application().run(..)`. Measured: 53 ms for component-showcase, 30 ms for depth-3d-showcase, **909 ms for mega-showcase** — during which there is no window at all, so it reads as the app being slow to start rather than slow to load. They move to a background thread that bumps the model generation when done, and the canvas shows a spinner while `frame` is `None` instead of empty space. mega-showcase now opens immediately and paints its first frame in 53 ms. The scrubber conversion gets four tests, including the round trip and the one-frame scenario where the denominator is zero. 175 tests pass, up from 171.
… main thread Part of #306. The preceding commit moved hit-map computation onto `cx.background_spawn` to stop it freezing the UI for 75 ms per frame change. gpui's background executor runs on libdispatch worker threads, which get **544 KiB** of stack. The hit map goes through `render_scene_hits` → `prepare_scene` → `deserialize_children`, and untagged serde recurses once per level of scene nesting, buffering each level through `ContentDeserializer`. It overflowed and took the process with it: `EXC_BAD_ACCESS` in a stack guard region, roughly thirty seconds into a session. This crate already knew. `RENDER_STACK` is 32 MiB and `render_frame_deep` exists precisely because the prefetch workers hit the same wall on Rust's 2 MiB default — and 544 KiB is four times worse than what broke them. What was missing is that `frame_hits` never had a `_deep` counterpart, because until now it only ever ran on the main thread, where macOS hands out 8 MiB. Moving it off that thread removed the one thing that was holding it up, and nothing in the type system said so. `frame_hits_deep` mirrors the two existing wrappers. All three call sites that reach the engine's recursion now own a 32 MiB stack: the hit map and the canvas frame on the background executor, and the library thumbnails. The regression test spawns a thread with exactly libdispatch's 544 KiB and calls through `frame_hits_deep` against `mega-showcase.json`. It is worth noting how it fails if the fix is reverted: a stack overflow aborts rather than unwinds, so `join()` never returns and the whole test binary dies. Loud, if not graceful — which is the right trade for a defect whose signature is a process disappearing. 175 tests pass.
Part of #306. Opening a project, returning to the library and opening another one showed the first project's frame for an instant. `EditorView` is built once, in `StudioRoot::new`, and every field it holds outlives the document it describes. The flicker was the visible part. The rest of the state survived too, and one piece of it is worse than cosmetic: `selected` carries a JSON pointer like `/scenes/2/children/3`, which resolves perfectly well in the *new* document and addresses a different element. An inspector edit would have landed on the wrong node without anything looking wrong. The playhead also stayed where it was — frame 412 of a document that may only have 200 — and diff mode stayed armed against a baseline belonging to the previous file. `sync_document` compares the model's path against the one the view last drew and resets when it changes. The comparison is on the path, not the generation, deliberately: a hot reload of the *same* file must keep showing the current frame while the new one renders, or every keystroke in the inspector would flash a spinner. Only a different document clears. `reset_for_new_document` is split out as a pure function over `EditorState`, and it hands the stale frame back rather than dropping it. The caller has the `Window` and must pass it to `cx.drop_image`; `#[must_use]` makes ignoring the return value a warning. The obligation that leaks ~8 MB of VRAM per miss is now in the signature instead of in someone's memory. What is deliberately kept: preview quality, the Inspect toggle and the Comments toggle. Those are choices about how the user works, not about the file. Four tests cover the split, one per class of state — including the selection, which is the one that was quietly dangerous. 179 tests pass.
…dler Part of #306. Opening a project froze the UI for over two seconds. The instrumented trace puts the whole delay before the editor even begins to draw: [OPEN+ 2.13s] sync_document: new document detected Loading audio: out/showcase/assets/track.wav `StudioModel::new` called `analyze_scenario_audio`, which decodes every track and runs an FFT over it — 24.7 MB of wav for that project. It ran inside `open_scenario`, on the UI thread, *before* the view switched to the editor. So the freeze was in the library, with the library still on screen, and the editor's spinner could not appear by construction: the thing meant to cover the wait was downstream of it. This also explains why the earlier measurements missed it. They were taken on `examples/*.json`, none of which carry audio; the project that froze lives under `out/`. A sample that excludes the failing case measures nothing. The analysis moves onto the thread that already warms up icons and video frames — renamed `spawn_scenario_warmup` since it is no longer only prefetch — and publishes `audio_error` plus a generation bump when it lands. Deferring is safe because the analysis writes into a global cache the renderer reads: the first frame paints without waveform data and the bump repaints it. An `Arc::ptr_eq` guard drops the result if the model has been replaced meanwhile, so a slow analysis cannot attach its findings to a document nobody opened. The watcher's reload path gets the warmup too. It calls `StudioModel::new` like every other path, and would otherwise have lost audio analysis entirely. Measured on the same project: `sync_document` at **183 ms** instead of 2.13 s, with the spinner up at 183 ms and the frame at 356 ms. "Loading audio" now appears in the log after the spinner rather than before it. The regression test does not time anything — a timing assertion would be flaky. It builds a model whose audio track does not exist: had the analysis run inline, the missing file would have set `audio_error`. That the field is `None` is proof the work was deferred. 179 tests pass.
Part of #306. Scrubbing during playback snapped straight back. The playhead has two writers with opposite intents and no way to tell them apart: the clock advancing time, and the user asking to be somewhere else. Both just assigned `state.current`. While a scenario has audio, the clock takes its next position from `audio::position_frame` — the sound card's idea of where we are, which a scrub does not move. So the tick after a scrub restored the old frame, and `audio_armed` stayed true, so the sound was never repositioned either. The playhead could not be moved at all while playing. The count matters for how this is fixed. Nine places write `state.current`: the clock, the scrubber, two step buttons, four keyboard actions, Present, a diff entry, and an annotation's "go to frame". Eight of the nine are seeks, and patching each to re-arm the audio would work until someone adds a tenth. So the clock detects instead. It remembers the frame it last wrote; if what it reads back differs, something else moved the playhead, and it adopts that position and re-arms the sound there rather than overwriting it. No call site has to know, and a tenth writer is covered on the day it appears. `advance_playhead` is a pure function over (current, last written, audio position, total) returning the next frame and whether to re-arm. Seven tests cover it, and they are where the intent lives now: a forward scrub, a backward one, undisturbed playback with and without audio, the first tick of a session where there is no previous write to compare against, the wrap past the last frame that has to restart the track rather than let it run on, and a seek past the end being clamped. Undisturbed playback still follows the audio clock rather than a timer. That is deliberate and unchanged: a timer ticking at 1/fps drifts against the sound card, and by the end of a long scenario the picture no longer matches what you hear. 188 tests pass.
…seeks Part of #306. Two changes, one a crash and one a behaviour the user asked for. **Any playback key killed the process.** Not a panic — an abort: panicked at gpui-pre-0.3.6/src/window.rs:5175: called `Option::unwrap()` on a `None` value panic in a function that cannot unwind thread caused non-unwinding panic. aborting. `Window::request_animation_frame` starts with `self.current_view()`, which reads the stack of entities being rendered and unwraps it. It is only valid inside a render or prepaint pass. `apply_playback_action` called it at the end of every transport action, and those run during key-event dispatch — outside any render. Because the panic unwinds through Objective-C frames, which cannot unwind, it aborted rather than failing. The call is simply removed: `EditorView::render` already requests the next animation frame while playing, which is the documented place for it. Toggling play notifies, notifying renders, and the render requests the frame. Nothing is lost, and the transport keys stop being fatal. **Moving the cursor on the timeline now stops playback**, so restarting is always an explicit act. The codebase already worked this way for the step buttons, which set `playing = false`; the scrubber did not, which is why dragging during playback fought the clock. `seek_from_user` is the one path for "the user asked to be somewhere else": it sets the frame and clears `playing`. Three call sites use it — the scrubber, a diff entry, and an annotation's "go to frame". The last two are inspection jumps, and leaving the video running after clicking "show me this element" would be strange. Home and End are deliberately left alone. They are transport controls rather than inspection jumps, and jumping to the start of a playing scenario reads as a restart, not as a request to stop. It returns whether anything changed, and that return is why dropping the cursor on the frame already displayed still counts: the frame did not move but the transport did, so the view has to repaint or the play button would keep showing pause. Three tests cover exactly that distinction. 191 tests pass.
LeadcodeDev
added a commit
that referenced
this pull request
Sep 24, 2026
) * feat(studio): boot the shell on gpui-kit with a light/dark/system theme Boot: gpui_kit::application().run(...) + gpui_kit::init(cx) inside it - there is no Application::new() in this kit. The window opens from cx.spawn(async move |cx| { cx.open_window(...) }) so the state entity and the theme can be set up with a live window and App context in hand before the first paint. Root wrapper: gpui_component::Root::render mounts the app's view but NOT the dialog/sheet/notification layers - the app has to composite those itself via Root::render_dialog_layer/_sheet_layer/ _notification_layer. Isolated all three calls in app/overlays.rs and nowhere else: gpui-kit 0.7.0 removes them with no documented replacement, so when that migration lands it is a one-file patch instead of a hunt across every screen. StudioRoot (app/root.rs) is currently a placeholder - it renders "Library" or "Editor" depending on StudioState.view, since library::view and editor::view (WS-3/WS-4) are not ported yet. What is live and testable ahead of them: the window, the theme, and the overlay layer stack. Theme: gpui-component's ThemeMode only has Light/Dark, and theme::init (run once by gpui_kit::init) hard-sets Light with nothing observing the OS. ThemePref::System is built on top in theme/mod.rs: - apply() resolves System via Theme::sync_system_appearance, which calls Theme::change internally - so it already re-projects onto the gpui-base layer (scrollbars, resize handles); no separate Theme::sync_base call is needed on this path. - watch_system_appearance() re-checks state.theme_pref on every window.observe_window_appearance callback (not a value captured at subscribe time), so it only re-resolves against the OS while the live preference is still System - an explicit Light/Dark choice made after subscribing is never clobbered by a later OS flip. - theme::set() is the pref-switching entry point the topbar's theme button (WS-4) will call once that screen exists; nothing calls it yet, hence the #[allow(dead_code)]. Persistence (theme/persist.rs): same shape as library::data::load_recents/push_recent - a JSON value under dirs::config_dir()/rustmotion/, here theme.json. Fixes the theme resetting to System on every launch. Window sizing (app/window.rs): 75% of the primary display, centered, same target as the old Dioxus shell's one-shot use_effect resize - but set on WindowOptions.window_bounds before the window opens via Bounds::centered(None, size, cx), so there is no first-frame resize flash. Falls back to a fixed 1280x800 when no display can be found, still centered, rather than the old silent no-op. Token mapping for screens styling off cx.theme() (gpui_component:: ActiveTheme) in place of the old --rm-* CSS custom properties (formerly THEME_CSS in app/root.rs): --rm-bg -> background --rm-surface -> popover (raised panel / card fill) --rm-surface-2 -> secondary (a step below popover) --rm-surface-3 -> muted (hover / selected fill) --rm-border -> border --rm-border-2 -> ring (stronger, focus-adjacent edge - no exact equivalent, this is guidance not a contract) --rm-text -> foreground --rm-text-strong -> popover_foreground / primary --rm-text-muted -> muted_foreground --rm-accent -> accent / primary --rm-on-accent -> accent_foreground / primary_foreground --rm-error -> danger (NOT destructive - that name does not exist on ThemeColor) --rm-overlay-hover -> muted (or accent at reduced opacity) --rm-overlay-border -> ring Nothing here re-brands the kit's default palette (no custom ThemeColor/ThemeConfig) - that is a separate, larger decision left to whichever workstream first needs brand-accurate colors. Also: gpui-component added as a direct Cargo.toml dependency (reported separately, not included in this diff) - ActiveTheme/Theme/ThemeMode/ Root are used by their real crate path throughout this code, which needs gpui-component in the extern prelude; gpui-kit only re-exports it as gpui_kit::component. * feat(studio): wire the crate back together on gpui-kit WS-1 restored the shell; this restores the crate around it. These are the convergence points the partition reserves to the orchestrator — module registration, the manifest, and the two frozen-core files whose only Dioxus tie was a hook. No workstream writes them, precisely so six parallel rewrites cannot each reshape the wiring under one another. Three Dioxus sites are removed rather than ported, because the thing they did no longer exists. use_prefetch_publisher and use_export_poll polled Dioxus signals; gpui notifies explicitly, so WS-2 and WS-6 replace them with state updates. ExportToast hand-rolled a floating div; the kit has a notification system, so WS-6 uses it. diff_panel.rs loses everything but DiffSide, the one piece a frozen file (app/state.rs) and a kept file (prefetch.rs, via FrameKey.side) both need. That file was a third Dioxus site the decomposition had not anticipated — it claimed exactly two, having grepped a file list that omitted it. WS-1 caught it. frame_for_change, the pointer-to-first-frame lookup deleted with the rest, is real logic rather than view code and is worth WS-6 rebuilding rather than reinventing. editor and library carry a crate-level #[allow(dead_code)] because each now mixes code with a live caller against code whose only caller is a view file excluded from its mod.rs until its workstream lands. Rustc cannot tell "orphaned pending a rewrite" from "genuinely dead", and a per-item allow would mean editing files that are frozen or owned elsewhere. Both allows come off once WS-2 through WS-6 have landed; library/mod.rs also stops re-exporting render_thumbnail and ScenarioEntry, which WS-3 puts back in one line. palette was declared default-features = false, and its default set is the only thing supplying std. Without it palette has no source for the Round/Sqrt/Abs trait impls on f32 — there is no libm fallback at 0.7.6 — so the dependency failed to build with 18 errors regardless of anything in this crate. It went unnoticed because an incremental target directory kept serving an artefact built before the flag was added. Pre-existing, unrelated to gpui-kit, and fixed here because nothing compiles until it is. Cli::file and Cli::dir move their descriptions into #[arg(help = ...)]. Under the no-comment rule their doc comments would simply have been deleted, and clap derives --help from exactly those, so rustmotion-studio --help would have gone quietly empty. The rule is about prose in the source, not about dropping behaviour. Verified on the integrated tree: cargo fmt --all --check, cargo clippy -p rustmotion-studio --all-targets -D warnings, and cargo test -p rustmotion-studio all exit 0, with 114 unit and 7 integration tests passing and 2 soak diagnostics ignored. Refs #306, #307 * ci(studio): install gpui's link-time dependencies instead of dioxus's The first CI run of the chantier failed with `unable to find library -lxkbcommon-x11`, and failed in a way worth recording: clippy was green. clippy type-checks but never links, so a missing system library is invisible to it and surfaces only in the test job, seventeen minutes later. Anyone reading "clippy pass, test fail" on this repo should suspect a linker dependency first. webkit2gtk, gtk-3 and xdo go. They were there for dioxus desktop, which no longer exists in this workspace, and nothing else in it pulls glib. In their place gpui needs xkbcommon-x11 for keyboard handling, plus wayland and xcb-cursor for the two display backends it builds on Linux. fontconfig, freetype and asound are untouched: the first two belong to Skia's text stack and the third to cpal, which rodio pulls in for preview audio. Neither had anything to do with the UI framework. publish.yaml gets the same list, and its explanatory block is rewritten rather than extended. It described the GTK/WebKit stack and glib-2.0 as the reason the packages were needed, which stopped being true with this change. A release workflow whose comment explains a dependency that is no longer there is worse than one with no comment: it sends the next reader looking for a glib failure that cannot happen. Refs #306 * feat(studio): put frames on screen through gpui's image path Closes #308. Part of #306. The JPEG encode disappears. It never existed for quality or size — it existed because the frame had to cross into a webview, and an <img> tag needs an encoded byte stream. gpui takes decoded pixels directly, so the encode, the wry asset handler and the /frame/{idx}?v={rev} URL all go, and with them a round trip through a codec on every displayed frame. What replaces it is `RenderImage::new(smallvec![Frame::new(buffer)])` wrapped in `img(ImageSource::Render(..))`. That variant bypasses every software cache in gpui — it hands the Arc straight back — and `RenderImage::new` mints a fresh monotonic ImageId per call, so a frozen preview from a stale cache entry is not merely unlikely here, it is unrepresentable. The old path needed no-cache headers on every response to get the same guarantee. Frames are written opaque. gpui's image pipeline blends with SourceAlpha / OneMinusSourceAlpha — straight alpha — while Skia produces premultiplied, and reconciling the two would mean a division pass per pixel per frame. It is unnecessary: `render_frame_v2_scaled` clears the whole framebuffer with the scenario background before anything paints, and the JPEG path it replaces already discarded alpha via to_rgb8(). Forcing 255 in the same loop that swaps R and B costs nothing and sidesteps the mismatch entirely. `swap_frame` hands the evicted frame to `cx.drop_image`. This is the one place in the rewrite where forgetting a line leaks half a gigabyte of VRAM per second: gpui's sprite atlas has no eviction, and each 1920x1080 frame owns its own ~8 MB Metal texture. The drop happens in the update path, never during paint, because update and paint are turns of the same single-threaded loop and cannot interleave — an argument from the executor model, not something a test exercises, since no live window is available without a test-support feature this crate does not enable. `render_frame_rgba_deep` mirrors the existing `render_frame_deep` rather than calling `render_frame_rgba` directly. The 32 MiB stack is not decoration: untagged serde buffers a frame per level of scene nesting, and the 2 MiB default overflowed the prefetch workers. The new path runs on the background executor pool, which has no better stack than they did. `prefetch_target_for` is split out of `publish_prefetch_target` so the target it builds can be tested as a value. Its own test previously waited two seconds for a real worker thread to render, then asserted on frame 0 — which `prefetch_window` never schedules, since a playing prefetcher only looks forward from the playhead. It also keyed on a global atomic that a sibling test mutates. It could not pass, and would have raced if it could. * feat(studio): rebuild the diff panel, annotations and export status on gpui-kit Closes #312. Part of #306. Three panels, and one of them stops being hand-rolled. The export toast was an absolutely-positioned div with its own dismiss timer and close button; gpui-kit has a notification system, so `ExportWatcher` pushes into it instead, keyed by a marker type so a running export's progress replaces its own toast in place rather than stacking. That watcher polls the export slot at 150 ms rather than being driven from the encode thread, and the reason is a frozen boundary, not laziness: `start_export` runs on a plain std::thread with no window handle, and `export_slot()`'s mutex has no notification hook. Pushing from inside would mean modifying code this workstream was told not to touch. The idiom is also the library's own — `NotificationList::start_advancing` in gpui-component polls its lifecycle the same way, with the same `cx.spawn_in` + timer shape. `frame_for_change` comes back with tests it never had. It maps a change's pointer to a playable frame so clicking a diff entry scrubs to where the element actually lives. Two details in it are load-bearing and were previously recorded nowhere: `start_at` is floored at zero, because a negative value would walk the playhead backward into the previous scene; and the result is clamped to the last frame, because a late `start_at` — or one left over from a scene that has since been shortened — would otherwise index past the end of the task list. It used to live inline in a Dioxus component, which is why it had no coverage; it now has five tests. The annotation capture box reads its target from `EditorState.selected` rather than taking pointer and kind as constructor props. The props version put a stale-data footgun in the caller's hands: whoever mounts the box would have to remember to update it on every selection change, and forgetting would file a comment against the wrong element in silence. Reading at submit time makes that unrepresentable, and matches how the box already read the playhead. Diff badges use theme tokens instead of the literal #22c55e the CSS carried, so the panel follows light and dark without a second palette. Persistence is untouched: JSON sources rewrite the scenario and record an undo snapshot, HTML sources write the sidecar and never touch the HTML, and a corrupt sidecar is still an error rather than something to overwrite. Left for integration with WS-4: `TOPBAR_HEIGHT` is px(40.), transcribed from the old CSS, and needs checking against the real topbar once it exists. * fix(studio): use as_chunks_mut for the rgba-to-bgra swap CI's clippy rejected `chunks_exact_mut(4)` under `clippy::chunks_exact_to_as_chunks`, while the same command passed locally. The lint is not the interesting part; the gap is. This machine runs rustc 1.97.1 from July, CI pins stable as of today, and the lint landed in between. So a local green does not close a workstream here — it only makes one ready for CI to judge. Worth stating plainly, because every earlier failure in this chantier reproduced locally (the linker one failed on both sides) and this is the first that could not. Refs #306 * feat(studio): rebuild the property inspector on gpui-kit Closes #311. Part of #306. The largest piece of the chantier: 2 225 lines in one file become 2 856 across four, split along the line that matters — what depends on the UI framework and what does not. `sections.rs` carries the curated schema and its value helpers with zero gpui in it, so the tables that decide which control appears for which property can be tested as data. `write.rs` carries the write path. `controls.rs` is the only place an Entity is created. `mod.rs` assembles the panel. The write path keeps both of its halves, and they stay independent: an edit applies optimistically in memory so the canvas moves within one render, and separately queues a Mutation behind a 250 ms debounce that, when it fires, re-reads the file from disk and replays the queue onto *that* content. Replaying onto a snapshot would be simpler and would silently destroy an agent's edit landing mid-window; `tests/audit_ws_g.rs` simulates exactly that race and is green. Dioxus's cancellable Task becomes `Option<gpui::Task<()>>`, where replacing the field drops and cancels the previous one — same semantics, and this time the cancellation is the language's rather than the framework's. Entities are created once per selection, not per render. `ensure_controls_for_selection` runs at the top of every render but rebuilds only when the selected pointer changes, so typing, dragging a slider or picking a colour never destroys the widget holding the cursor. The one deliberate exception is structural edits — adding or removing a list entry, switching a Fill between solid and gradient — which change how many controls exist and so must rebuild; `force_rebuild` marks those explicitly rather than letting the rebuild gate guess. `apply_open_change` comes back with its test. It was the one test this chantier had lost, deleted with the in-house colour picker, and it encodes a rule the kit does not: `ColorPickerState` owns its own `open` flag and emits no open/close event, so panel-wide exclusivity is enforced by observing each picker's `is_open()` and force-closing the others. Scrolling the panel still closes the expanded one. On the curated schema, which is what sank the discarded first attempt: 44 Field entries and 64 assembled rows, both matching the original and both now locked by a test. The earlier "45 against 47" alarm turned out to be a miscount — a raw grep for `Field {` also catches the struct definition and the `Mutation::Field` literals in the write path. Counting the thing rather than the string is what a test buys here, and the registry's own completeness invariant (every CssStyle property lands in exactly one section, unmapped ones falling to Advanced) is untouched and still locked. Bold/Italic and the alignment group render as text labels rather than icons. The Lucide variants exist in gpui-kit-assets but the embedded default bundle does not carry them, and a missing icon fails silently at runtime rather than at compile time — not something to gamble on without a window to look at. Swapping them back is one line once the asset registration grows. 159 tests pass, up from 144. * feat(studio): rebuild the library home on gpui-kit and mount it Closes #309. Part of #306. Thumbnails stop being JPEGs. The frozen `data::render_thumbnail` encodes to JPEG because the old wry handler served them to an <img> tag; feeding gpui that would mean decoding straight back to the pixels the renderer already had. The library now calls `build_frame_tasks` + `render_frame_rgba_deep` at scale 0.25 and hands the result to WS-2's `frame_from_rgba`, so the whole codec round trip disappears and the two image paths in the app are one path. That makes the grid a real memory surface, since every decoded thumbnail owns a Metal texture in an atlas with no eviction. It is bounded to exactly what is visible: `sync_thumbnails` diffs the current section-and-search result against what is held, renders what appeared, and hands what vanished to `cx.drop_image`. Switching folders or narrowing the search frees the rest. The grid computes its own column count from the viewport each render, because `grid_cols` takes a fixed number and there is no per-track minmax — so `repeat(auto-fill, minmax(240px, 1fr))` becomes an arithmetic floor over the available width, which resizes like the CSS did rather than approximating it with flex wrap. `StudioRoot` stops being a placeholder for the library half and mounts the screen. It also gains `cx.observe` on the shared state: `view` lives in `StudioState`, so a card click flipping it to `Editor` changes a field the root had read but was not subscribed to, and the screen would not have repainted. One bug worth recording, because it was in a test rather than in code: the first version of the recents test read the developer's real `~/.config/rustmotion/recent.json` and passed or failed depending on what that machine happened to have opened. It now sets `LibraryState.recents` directly and covers both branches. A test that depends on the machine it runs on is worse than no test — it reports the environment, not the behaviour. Also noticed, not acted on: `LibraryState.thumb_cache` appears to have no callers left. It cached JPEG bytes for the wry handler that no longer exists. It sits in a frozen file and belongs to its own change, not this one. 166 tests pass, up from 159. * feat(studio): land the editor shell and connect the prefetch cache to the canvas Closes #310. Part of #306. The last of the six workstreams, and the one that assembles the others: topbar, canvas, hit overlay, transport and shortcuts, mounting the inspector, the diff panel, the annotations panel, the export watcher and the frame surface. The app opens on the library, opens a scenario into the editor, and both screens boot clean. Shortcuts move from bubbled key events to gpui actions bound under a `key_context`. The old code kept Space out of text fields by calling `stop_propagation` on every container that might contain one — a rule enforced at each site and silently broken by forgetting one. gpui resolves bindings by context specificity instead, so an `Input` owning `left`/`right`/`cmd-z` under its own context wins over the editor's while focused, without the editor knowing text fields exist. `playback_action` keeps its four tests and stays the single source of truth; only its key type changes, since gpui has no `Key` enum and hands over a plain string. The larger fix here was not in WS-4's own code. The prefetch workers filled a frame cache that nothing ever read: `FrameCache::get` had no callers anywhere in the crate. The workers rendered every frame in the window ahead of the playhead, JPEG-encoded it and stored it, while the canvas independently re-rendered the same frame in RGBA — so the pool was not merely useless, it competed for CPU with the render the UI was waiting on. It surfaced only when the transitional crate-level `#[allow(dead_code)]` came off. The cause is a gap in the decomposition rather than in either workstream. `FrameKey` was frozen as a shared contract, but nothing in either brief said the canvas must consult the cache before rendering, so WS-2 built the producer, WS-4 built the consumer, and each was internally correct. WS-4 even defined its own local `FrameKey` tuple for render deduplication, which is exactly what a missing contract looks like from the inside. The canvas now builds the real `FrameKey` — model generation for side B, baseline source hash for side A, matching what the workers key on — and decodes a cache hit rather than re-rendering. Decoding a JPEG costs a few milliseconds against tens for a Skia render, and it keeps the cache's memory profile and its tested eviction policy untouched. Caching raw RGBA would be faster still, at 8 MB a frame against a cap of 120; that trade deserves its own change. 171 tests pass, up from 166. * refactor(studio): drop the dioxus-era leftovers the rewrite made unreachable Part of #306. The transitional `#[allow(dead_code)]` on `editor`, `library`, `state` and the theme module existed because, mid-chantier, each mixed live code with code whose only caller was a view file not yet ported. All six workstreams have landed, so the allows come off — and with them off, the compiler names what the rewrite left behind. `scenario::Theme` goes. It mapped Dark/Light/System onto the CSS class names `rm-dark`/`rm-light`/`rm-system`, which is a sentence that only parses in a webview. `ThemePref` in `app/state.rs` replaced it, and nothing had referenced the old one since WS-1. `render_thumbnail` goes. It encoded frame 0 to JPEG for the wry asset handler that served `/thumb/{i}`; the library now renders straight to RGBA through the same path as the canvas, so the encode had no consumer. `seg_button` and `num_display` go the same way — superseded during the inspector rewrite by `fill_seg_button` and by `properties::display_number`. Three declarations keep a narrow, deliberate allow rather than being stripped: `SelectionData`, `ColorWidget` and `ScalarWidget` each carry fields the inspector populates and never reads. Removing them means proving, field by field, that nothing in 2 857 lines of freshly written code wanted them — which is a review, not a cleanup, and belongs to its own change rather than to the tail of this one. The allow is on the exact declaration so a future reader sees the scope, not a blanket over the module. What this pass is really worth is the defect it exposed rather than the lines it deleted: the crate-level allow was also hiding `FrameCache::get`, uncalled, which meant the entire prefetch pool was rendering frames nothing consumed. That is fixed in the preceding commit. A blanket allow does not merely tolerate dead code, it conceals live code that stopped being reachable. * fix(studio): make the editor actually render, and stop blocking the window on assets Part of #306. Four defects, all found by opening the app and looking at it. Every one of them survived a green `fmt`/`clippy`/`test` run and a check that the process stayed alive for six seconds — which is exactly as much as that check is worth. Two of them made the editor useless. **No icons anywhere.** `gpui_kit::application()` was called without `.with_assets(..)`, so no asset source was registered and every icon resolved to nothing — silently, because a missing icon is not an error. It registers `AllAssets` rather than the curated set: WS-4 and WS-5 had each fallen back to text labels for icons the default bundle does not embed (mute, theme-system, baseline, diff, comments, export, bold, italic, alignment), and the full Lucide catalog removes the reason for those fallbacks. **A blank canvas.** The aspect-ratio box carried `aspect_ratio` plus `max_w_full`/`max_h_full` and no concrete dimension. In flexbox, a box with only maximum constraints resolves to zero, and the frame inside it is `size_full` — 100% of nothing. Measured before and after by printing the resolved bounds: 0 × 0, then 1222 × 687. The frame had been rendering correctly the whole time and being painted into a box with no area. **The scrubber pinned at maximum.** `EditorView` is constructed once, in `StudioRoot::new`, before any scenario is open — so the slider was built with the empty scenario's frame count, its `max` froze at 1, and every `set_value` clamped there. `SliderState::max` is a consuming builder with no runtime setter, so the fix is not to update it but to stop depending on it: the slider now runs on a normalized 0..1 fraction, converted at both ends. That also unbinds the slider's lifetime from the scenario's, so the same bug cannot return through that path when a different file is opened. **Up to a second of dead time before the window appeared.** `prefetch_icons` and `preextract_video_frames` ran synchronously ahead of `gpui_kit::application().run(..)`. Measured: 53 ms for component-showcase, 30 ms for depth-3d-showcase, **909 ms for mega-showcase** — during which there is no window at all, so it reads as the app being slow to start rather than slow to load. They move to a background thread that bumps the model generation when done, and the canvas shows a spinner while `frame` is `None` instead of empty space. mega-showcase now opens immediately and paints its first frame in 53 ms. The scrubber conversion gets four tests, including the round trip and the one-frame scenario where the denominator is zero. 175 tests pass, up from 171. * fix(studio): give the hit map its own stack before running it off the main thread Part of #306. The preceding commit moved hit-map computation onto `cx.background_spawn` to stop it freezing the UI for 75 ms per frame change. gpui's background executor runs on libdispatch worker threads, which get **544 KiB** of stack. The hit map goes through `render_scene_hits` → `prepare_scene` → `deserialize_children`, and untagged serde recurses once per level of scene nesting, buffering each level through `ContentDeserializer`. It overflowed and took the process with it: `EXC_BAD_ACCESS` in a stack guard region, roughly thirty seconds into a session. This crate already knew. `RENDER_STACK` is 32 MiB and `render_frame_deep` exists precisely because the prefetch workers hit the same wall on Rust's 2 MiB default — and 544 KiB is four times worse than what broke them. What was missing is that `frame_hits` never had a `_deep` counterpart, because until now it only ever ran on the main thread, where macOS hands out 8 MiB. Moving it off that thread removed the one thing that was holding it up, and nothing in the type system said so. `frame_hits_deep` mirrors the two existing wrappers. All three call sites that reach the engine's recursion now own a 32 MiB stack: the hit map and the canvas frame on the background executor, and the library thumbnails. The regression test spawns a thread with exactly libdispatch's 544 KiB and calls through `frame_hits_deep` against `mega-showcase.json`. It is worth noting how it fails if the fix is reverted: a stack overflow aborts rather than unwinds, so `join()` never returns and the whole test binary dies. Loud, if not graceful — which is the right trade for a defect whose signature is a process disappearing. 175 tests pass. * fix(studio): reset the editor when a different document is opened Part of #306. Opening a project, returning to the library and opening another one showed the first project's frame for an instant. `EditorView` is built once, in `StudioRoot::new`, and every field it holds outlives the document it describes. The flicker was the visible part. The rest of the state survived too, and one piece of it is worse than cosmetic: `selected` carries a JSON pointer like `/scenes/2/children/3`, which resolves perfectly well in the *new* document and addresses a different element. An inspector edit would have landed on the wrong node without anything looking wrong. The playhead also stayed where it was — frame 412 of a document that may only have 200 — and diff mode stayed armed against a baseline belonging to the previous file. `sync_document` compares the model's path against the one the view last drew and resets when it changes. The comparison is on the path, not the generation, deliberately: a hot reload of the *same* file must keep showing the current frame while the new one renders, or every keystroke in the inspector would flash a spinner. Only a different document clears. `reset_for_new_document` is split out as a pure function over `EditorState`, and it hands the stale frame back rather than dropping it. The caller has the `Window` and must pass it to `cx.drop_image`; `#[must_use]` makes ignoring the return value a warning. The obligation that leaks ~8 MB of VRAM per miss is now in the signature instead of in someone's memory. What is deliberately kept: preview quality, the Inspect toggle and the Comments toggle. Those are choices about how the user works, not about the file. Four tests cover the split, one per class of state — including the selection, which is the one that was quietly dangerous. 179 tests pass. * perf(studio): decode audio on the warmup thread, not in the click handler Part of #306. Opening a project froze the UI for over two seconds. The instrumented trace puts the whole delay before the editor even begins to draw: [OPEN+ 2.13s] sync_document: new document detected Loading audio: out/showcase/assets/track.wav `StudioModel::new` called `analyze_scenario_audio`, which decodes every track and runs an FFT over it — 24.7 MB of wav for that project. It ran inside `open_scenario`, on the UI thread, *before* the view switched to the editor. So the freeze was in the library, with the library still on screen, and the editor's spinner could not appear by construction: the thing meant to cover the wait was downstream of it. This also explains why the earlier measurements missed it. They were taken on `examples/*.json`, none of which carry audio; the project that froze lives under `out/`. A sample that excludes the failing case measures nothing. The analysis moves onto the thread that already warms up icons and video frames — renamed `spawn_scenario_warmup` since it is no longer only prefetch — and publishes `audio_error` plus a generation bump when it lands. Deferring is safe because the analysis writes into a global cache the renderer reads: the first frame paints without waveform data and the bump repaints it. An `Arc::ptr_eq` guard drops the result if the model has been replaced meanwhile, so a slow analysis cannot attach its findings to a document nobody opened. The watcher's reload path gets the warmup too. It calls `StudioModel::new` like every other path, and would otherwise have lost audio analysis entirely. Measured on the same project: `sync_document` at **183 ms** instead of 2.13 s, with the spinner up at 183 ms and the frame at 356 ms. "Loading audio" now appears in the log after the spinner rather than before it. The regression test does not time anything — a timing assertion would be flaky. It builds a model whose audio track does not exist: had the analysis run inline, the missing file would have set `audio_error`. That the field is `None` is proof the work was deferred. 179 tests pass. * fix(studio): let the user move the playhead while the video is playing Part of #306. Scrubbing during playback snapped straight back. The playhead has two writers with opposite intents and no way to tell them apart: the clock advancing time, and the user asking to be somewhere else. Both just assigned `state.current`. While a scenario has audio, the clock takes its next position from `audio::position_frame` — the sound card's idea of where we are, which a scrub does not move. So the tick after a scrub restored the old frame, and `audio_armed` stayed true, so the sound was never repositioned either. The playhead could not be moved at all while playing. The count matters for how this is fixed. Nine places write `state.current`: the clock, the scrubber, two step buttons, four keyboard actions, Present, a diff entry, and an annotation's "go to frame". Eight of the nine are seeks, and patching each to re-arm the audio would work until someone adds a tenth. So the clock detects instead. It remembers the frame it last wrote; if what it reads back differs, something else moved the playhead, and it adopts that position and re-arms the sound there rather than overwriting it. No call site has to know, and a tenth writer is covered on the day it appears. `advance_playhead` is a pure function over (current, last written, audio position, total) returning the next frame and whether to re-arm. Seven tests cover it, and they are where the intent lives now: a forward scrub, a backward one, undisturbed playback with and without audio, the first tick of a session where there is no previous write to compare against, the wrap past the last frame that has to restart the track rather than let it run on, and a seek past the end being clamped. Undisturbed playback still follows the audio clock rather than a timer. That is deliberate and unchanged: a timer ticking at 1/fps drifts against the sound card, and by the end of a long scenario the picture no longer matches what you hear. 188 tests pass. * fix(studio): stop aborting on playback keys, and pause when the user seeks Part of #306. Two changes, one a crash and one a behaviour the user asked for. **Any playback key killed the process.** Not a panic — an abort: panicked at gpui-pre-0.3.6/src/window.rs:5175: called `Option::unwrap()` on a `None` value panic in a function that cannot unwind thread caused non-unwinding panic. aborting. `Window::request_animation_frame` starts with `self.current_view()`, which reads the stack of entities being rendered and unwraps it. It is only valid inside a render or prepaint pass. `apply_playback_action` called it at the end of every transport action, and those run during key-event dispatch — outside any render. Because the panic unwinds through Objective-C frames, which cannot unwind, it aborted rather than failing. The call is simply removed: `EditorView::render` already requests the next animation frame while playing, which is the documented place for it. Toggling play notifies, notifying renders, and the render requests the frame. Nothing is lost, and the transport keys stop being fatal. **Moving the cursor on the timeline now stops playback**, so restarting is always an explicit act. The codebase already worked this way for the step buttons, which set `playing = false`; the scrubber did not, which is why dragging during playback fought the clock. `seek_from_user` is the one path for "the user asked to be somewhere else": it sets the frame and clears `playing`. Three call sites use it — the scrubber, a diff entry, and an annotation's "go to frame". The last two are inspection jumps, and leaving the video running after clicking "show me this element" would be strange. Home and End are deliberately left alone. They are transport controls rather than inspection jumps, and jumping to the start of a playing scenario reads as a restart, not as a request to stop. It returns whether anything changed, and that return is why dropping the cursor on the frame already displayed still counts: the frame did not move but the transport did, so the view has to repaint or the play button would keep showing pause. Three tests cover exactly that distinction. 191 tests pass.
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
1 participant
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.
Closes #307. Part of #306.
First workstream of the gpui-kit rewrite. Replaces the Dioxus desktop shell
with a gpui-kit application and restores the crate to a compiling, tested
state — every other workstream was blocked on this.
What lands
gpui_kit::application(), window sized to 75 % of the currentmonitor and centred before it opens, via
WindowOptions.window_bounds.The old post-open resize produced a first-frame flash; the API shape removes
it for free.
Rootwrapper, with the dialog / sheet / notification layers mounted bythe app itself —
Root::renderdoes not mount them. Those three calls livein
app/overlays.rsand nowhere else, because gpui-kit 0.7.0 removesthem with no documented replacement. Isolating them turns that migration
into a localised patch instead of a hunt.
Light | Dark | System, persisted underdirs::config_dir()/rustmotion/. The kit'sThemeModehas onlyLight | Dark,theme::inithard-setsLight, and nothing in it observesthe OS — so
Systemis wired here fromTheme::sync_system_appearancepluswindow.observe_window_appearance. The previous studio reset toSystematevery launch; it now survives a restart.
src/components/**— 13 directories, 1 636 lines of Rust and2 440 of CSS. Seven of the thirteen (
input,popover,separator,sheet,sidebar,skeleton,tooltip) had zero usages outsidecomponents/;sidebaralone was 839 + 855 lines of scaffold that thelibrary screen never used, drawing its own sidebar in inline styles instead.
The other six all have a kit equivalent, so there is no wrapper layer in the
new design.
assets/dx-components-theme.csswent with them — 70 linesreferenced from no Rust file.
Things found on the way
palettenever built. It was declareddefault-features = false, and itsdefault set is the only thing supplying
std; without it there is no sourcefor the
Round/Sqrt/Abstrait impls onf32at 0.7.6, and nolibmfallback. The dependency failed with 18 errors regardless of anything in this
crate. It stayed invisible because an incremental target directory kept
serving an artefact built before the flag was added — a fresh
CARGO_TARGET_DIRexposed it immediately. Pre-existing and unrelated to gpui-kit; fixed here
because nothing compiles until it is.
diff_panel.rswas a third Dioxus site the decomposition had notanticipated — #306 claimed exactly two, having grepped a file list that
omitted it. It keeps only
DiffSide, the one piece a frozen file(
app/state.rs) and a kept file (prefetch.rs, viaFrameKey.side) bothneed.
frame_for_changewas deleted with the rest but is real logic ratherthan view code, and is flagged for WS-6 to rebuild rather than reinvent.
--helpwould have gone quietly empty. The no-comment rule deletes thedoc comments on
Cli::fileandCli::dir, and clap derives--helpfromexactly those. They move to
#[arg(help = "…")].Not in scope here
StudioRootrenders the word "Library" or "Editor" and nothing else. There isno screen to switch to yet: WS-3 builds the library, WS-4 the editor shell.
editorandlibrarycarry a crate-level#[allow(dead_code)]because eachnow mixes code with a live caller against code whose only caller is a view
file excluded from its
mod.rsuntil its workstream lands. Both allows comeoff once WS-2 through WS-6 have landed.
Verification
Run on the integrated tree:
That last line is the point of the whole chantier: the studio carried two git
dependencies on
DioxusLabs/components@02801f27, and cargo refuses to publishany crate that has one. There are now none.
Exactly one test was lost, and deliberately:
only_one_picker_open_at_a_time,covering
apply_open_changein the deleted colour picker. That rule is handedto WS-5 (#311) to port along with the panel.