RDP frontend phase 1: renderer prep¶
Master plan: PLAN-rdp-frontend.md.
Planning effort: low. The master plan asks for high, and the Execution notes record the choice to proceed anyway. It asks for high because this phase changes renderer internals that every mode depends on (the display surface, the playback channel, the crate's feature set and public API) and has to coordinate with Kerbside's transcoding spike. No review effort is set by the master plan; review at high effort, because steps 5 and 6 change the published crate's API.
Prompt¶
Read the master plan's Prompt first; everything there applies. For this phase specifically, read before acting:
shakenfist-spice-renderer/src/display/surface.rsandsrc/surface_mirror.rs(damage, steps 1 and 2).shakenfist-spice-renderer/src/session.rs(run_connectionat :193,run_headlessat :646, the fan-out and mirror tasks at :883-935,plan_channel_linksat :1151) andryll/src/main.rsrun_web(:514-1010).ryll/src/web/inputs.rsrun_input_relay(:132-314) and the pointer block inryll/src/app/mod.rs(:4536-4630).shakenfist-spice-renderer/src/channels/playback.rsandsrc/audio_sink.rs.- Every
Cargo.tomlin the workspace, theMakefile(lintat :277), andAGENTS.mdCargo feature gating.
Builds and tests run in Docker through the Makefile (make
build, make test, make lint), never with a host toolchain.
Line references below are against develop 7690e1e; re-find
them rather than trusting them once earlier steps have landed.
Scope¶
In scope, closing the master plan's renderer gaps 1 to 4:
- Damage tracking on
DisplaySurface, with a bounded rectangle region a consumer takes in one call. - A shared pointer translator in the renderer, used by
--weband the GUI. - A shared event-bus helper for the forwarder and
mirror-apply tasks, used by
--weband headless. - A slim renderer build: features for usbredir, webdav, the H.264 encoder, H.264 decode and mozjpeg, so a consumer can link display decode alone.
- Device-free playback: a run-time playback mode that
hands a sink Opus packets, raw PCM and decoded PCM without
opening a cpal device, and
--webswitched to it. - A
make lintcheck that keeps the slim build compiling and slim. - Documentation of the above.
Out of scope:
- Any RDP code or IronRDP dependency (phase 2).
- Mono and 4-bit cursor decode (gap 6, phase 3).
- Multi-monitor (gap 5, future work).
- Changing headless's audio behaviour: it keeps local output. See decision 9.
- Pruning
ryll/Cargo.toml's own unused direct dependencies (see Future work); this phase prunes only the renderer's. - Fixing #398 (H.264 from spice-server's gstreamer never decodes); decision 6 relies on it only as evidence.
- Adding wheel support to
--web, which has none (see What the survey found, item 9).
What the survey found¶
Surveyed on 2026-10-20 against develop 7690e1e. The master
plan's gap list is substantially right. These items correct or
sharpen it; the false claims are fixed at their source in the
master plan and the index.md row in this planning commit, so
later steps need not redo that.
- openh264 is pulled in by H.264 decode, not only by the
encoder.
openh264 = "=0.9.8"is a non-optional dependency ofshakenfist-spice-compression, used byH264VideoDecoder(video.rs:266-477, selected byfor_streamatvideo.rs:174). Gating the renderer's encoder alone leavesopenh264-sys2(a C++ build with cc and nasm) in the tree. Separating it is possible: a build without H.264 decode must also stop advertisingDISPLAY_CODEC_H264, andH264Offer::Offalready does exactly that (link::display_channel_caps, protocollink.rs:744-749;preferred_video_codecs,display.rs:170-176). MJPEG decode does not depend on it. The master plan said only that the encoder pulls openh264; corrected. - The renderer turns on more C builds than the master plan
listed. It depends on the compression crate with default
features plus
encode, so it always buildsmozjpeg(vendored libjpeg-turbo) as well as openh264. The pure-Rustjpeg-decoderfallback already exists, so mozjpeg can be a feature. - Ten renderer dependencies are unused.
rsa,sha1,rand,tokio-rustls,rustls-pemfile,webpki-roots,byteorder,async-channel,thiserrorandhttp-body-utilhave no references in the renderer'ssrc/ortests/(checked by grep), and nor doeshttp.bytesis used only bytests/webrtc_h264_smoke.rs. They can be deleted, no feature needed.nusbis used only undercfg(target_os = "linux")but is an unconditional dependency on every target. - The audio gap is worse than "a tap, not a sink". The
tap fires before decode and whether or not a device opened
(
playback.rs:1156-1221), but the playback channel exists only with theaudiofeature (channels/mod.rs:6,session.rs:456,:1158-1161). So a--no-default-featuresryll serves--webwith no audio at all, and a default build serving--webalso plays the guest's audio out of the server's own speakers, or, with no device, retries cpal every 30 s for nothing (playback.rs:540-554). The sink never sees decoded PCM, START/STOP or format changes (audio_sink.rs:29-42). Its only implementor isWebOpusSink(ryll/src/web/audio.rs:51-117). - The playback channel offers Opus by accident. Playback
links with
DEFAULT_OTHER(protocollink.rs:706-712), whose bits are the main channel's migrate caps; on the playback channel those same bits meanCELT_0_5_1 | OPUS(constants.rs:130-135,:221-234). Ryll cannot decode CELT (playback.rs:1199-1219). Not in the master plan; step 6 gives playback its own caps constant. - The forwarder and mirror-apply tasks exist twice, not
once: in
run_web(main.rs:803-845) and inrun_headless(session.rs:883-935), with two separateEVENT_BROADCAST_CAPACITYconstants (session.rs:87,web/server.rs:34), both 1024. RDP would be the third copy. The master plan named onlyrun_web; corrected. - The two pointer translators disagree. The GUI
(
app/mod.rs:4536-4555) suppresses a repeated position in both mouse modes; the web relay (web/inputs.rs:208-233) suppresses only zero deltas in server mode. The GUI places button events at the last hover position; the web relay takes the button's own coordinates and records them as the next delta origin without telling the guest the pointer moved (web/inputs.rs:237-264). In server mode SPICE'sMOUSE_PRESScarries no position, so that movement is lost. The inputs channel already sendsMOUSE_POSITIONbefore every press and release (inputs.rs:750-830), which covers client mode. - Damage rectangles exist on six events, not four:
ImageReadyChromaandImageReadyAlphacarry them too. The draw ops clamp to the surface and to clip lists insideDisplaySurface(surface.rs:70-402), so that is where the exact written rectangles are known. --webhas no mouse wheel.BrowserMsghas no wheel variant andapp.jssends none; the GUI maps the wheel to button 4/5 press-release pairs (app/mod.rs:4596-4618).docs/multi-mode-parity.mdhas no wheel row, so the gap is unrecorded. Out of scope here (the translator in step 3 gives both frontends the same wheel mapping to call); recorded in Future work.ChannelEvent::UsbDevicesChangedis never emitted (channels/mod.rs:450-452,#[allow(dead_code)]), and it is the only thing tyingusb::UsbDeviceInfointo the always-present event enum.- No CI lane builds the renderer slim.
make lintruns a slim clippy for the compression crate only (Makefile:280). cargo-deny runs with--all-features, so it never sees a slim tree. - Kerbside's side of the hand-over is not merged. The
master plan says Kerbside's phase 3 plan "has been updated
to match". That update is kerbside PR #581, still open on
2026-10-20. Kerbside's spike step 3a has partly reached
ryll already: the LZ4 and JPEG image encoders landed
(
6895658,8dc590b). Its slim-features part has not, so this phase does not collide with it.
Decisions¶
- Damage lives on
DisplaySurface, beside the dirty bool, and is independent of it. Each draw op records the sub-rectangles it actually wrote, after clamping to the surface and the clip list, into a boundedDamageRegion. A new surface starts fully damaged, as it starts dirty.take_damage()returns the region and clears it;consume_dirty()keeps its meaning and does not touch the region, and vice versa.RealFrameSourceand the GUI texture cache are unchanged. Independence costs nothing: the region is bounded, so a surface whose damage nobody takes holds at mostMAX_DAMAGE_RECTSrectangles. Putting damage inDrawOutcomeinstead was rejected: it would hand the merging and bounding to every consumer, which is what the master plan's "goes in the renderer" rules out. - The region merges and bounds deterministically. A new
rectangle contained in an existing one is dropped. Two
rectangles merge when their bounding box's area is no more
than the sum of their areas, so a merge never adds pixels
that neither covered beyond their overlap; merging repeats
until no pair qualifies. Past
MAX_DAMAGE_RECTS(32), the whole region collapses to its bounding box. This is simple, testable and good enough for RDP, which re-diffs in 64x64 tiles anyway (master plan, The RDP side). - A lagged mirror marks everything damaged. When the
mirror's bus subscription reports
Lagged, the shared helper (decision 4) calls a newSurfaceMirror::mark_all_damaged(), which sets every surface fully damaged and dirty. The pixels have already diverged; this makes every consumer resend the whole surface, rather than trusting a region that is missing the draws it never saw. - One event-bus helper, in the renderer, used by
--weband headless now.EventBus::new(capacity)creates the broadcast sender and the mirror and takes the mirror's subscription at once. Callers take their own subscriptions withsubscribe(), then callstart(event_rx), which spawns the forwarder and the mirror-apply task and returns their handles. Splitting construction fromstartpreservesrun_web's rule that every long-lived consumer subscribes before events flow (main.rs:724-735). The two capacity constants become one,session::EVENT_BROADCAST_CAPACITY. - One pointer translator, synchronous, mode passed per
call.
PointerTranslatorin the renderer (src/pointer.rs) holds only the last position. It takes the mouse mode as an argument on each call, so the GUI (synchronous, mode inself.mouse_mode) and the web relay (async, mode in awatch) both use it without sharing a runtime type. The unified behaviour: - A move to the last position sends nothing, in either
mode (the GUI's behaviour). For
--webthis drops duplicateMOUSE_POSITIONmessages in client mode, which the guest ignored anyway. - In server mode the first move after a reset is a zero
delta and sends nothing; later moves send
MouseMotion. - A button event that carries a position different from
the last one, in server mode, is preceded by the
MouseMotiondelta to it, then recorded as the new origin. This changes--webbehaviour, deliberately: today the movement is silently dropped (survey item 7). The existing testa_button_position_becomes_the_next_delta_referenceis updated to expect the delta. Client mode needs no extra event, because the inputs channel already sends a position with every press. - A button event with no position (the GUI) uses the last position.
wheel(clicks)turns signed wheel clicks into button 4 or 5 press-release pairs at the last position, as the GUI does today; the egui-specific click accumulator stays in the GUI.reset()forgets the position (the GUI calls it on reconnect,app/mod.rs:1160).- openh264 decode becomes a feature; a build without it
offers no H.264. The compression crate gains
h264-decode = ["dep:openh264"], default on, andencodegainsdep:openh264explicitly. In a renderer build withouth264-decode,run_connectionandDisplayChannel::newforceH264Offer::Off, which already drops both the cap bit and the codec preference; the protocol crate'sDEFAULT_DISPLAYand its logging test do not change. MJPEG stream decode is untouched. The cost of dropping H.264 decode in a slim build is nil in practice: H.264 from spice-server's gstreamer encoder does not decode in ryll at all today (#398), and the default preference already asks for MJPEG first. - The renderer's feature set. All default-on, so a plain dependency on the crate builds what it builds today:
| Feature | Pulls in | Gates |
|---|---|---|
audio |
cpal, rtrb | local audio output only (narrowed, decision 8) |
usbredir |
nusb (Linux), shakenfist-spice-usbredir | usb, channels::usbredir |
webdav |
dav-server, hyper, hyper-util | webdav, channels::webdav |
encoder |
compression encode (openh264-sys2, jpeg-encoder) |
encoder::h264, encoder::task |
h264-decode |
compression h264-decode (openh264) |
H.264 stream decode |
mozjpeg |
compression mozjpeg (libjpeg-turbo) |
the SIMD JPEG decoder |
digest-decode |
unchanged, default off | unchanged |
The renderer depends on the compression crate with
default-features = false and the decoder features it
always needs (quic, glz, lz, lz4, jpeg). Gating
follows the audio precedent (session.rs:226, :456,
:1158-1161): plain-data types stay ungated so
run_connection and run_headless keep one signature in
every build (UsbCommand, WebdavCommand,
ShareDirConfig, VirtualDiskConfig, every snapshot, every
other ChannelEvent variant), and plan_channel_links
skips a compiled-out channel with a logged reason.
FrameSource, FrameRef, RealFrameSource,
SyntheticFrameSource, EncodedFrame and EncoderControl
stay ungated (the last two move out of the files that need
openh264), because shakenfist-spice-webrtc's production
code needs only those. even_dimensions moves out of the
compression crate's encode module so it is available
without the encoder (web/inputs.rs uses it for viewport
rounding). The dead UsbDevicesChanged variant is deleted
rather than having its types moved. ryll names the
features it needs explicitly (it already uses
default-features = false), and shakenfist-spice-webrtc
takes encoder only as a dev-dependency. These names
become public API on crates.io, which is why they are
settled here rather than by the implementer.
8. Playback is always compiled; only local output is a
feature. opus-decoder is pure Rust and small, while cpal
pulls ALSA, so the split that matters is decode versus
device. PlaybackChannel and opus decode become
unconditional, and audio gates only the cpal audio thread
and its ring. run_connection's
opus_sink: Option<Arc<dyn OpusPacketSink>> parameter
becomes playback: PlaybackOutput, with two variants:
- Local: today's behaviour. In a build without audio,
the channel is not linked, as today.
- Sink(Arc<dyn PlaybackSink>): never spawns the audio
thread and is linked in every build. Notification
suppression keys off this variant instead of
opus_sink.is_some() (playback.rs:1383).
The trait is renamed PlaybackSink, since it is no longer
only about Opus, and gains on_start(PlaybackFormat),
on_stop() and on_decoded_pcm(&[i16], rate, channels),
all with default no-op bodies beside the existing
on_opus_packet and on_pcm_samples. Decode keeps running
in both variants, as it does today, so the control socket's
stats.playback.packets_decoded keeps its meaning
(docs/control-socket-protocol.md). Decoding only on demand
was rejected for that reason; Opus decode of one stereo
stream is cheap. The rename is a breaking API change, which
is acceptable at 0.x and needed anyway by the signature
change.
9. --web switches to Sink; headless and the GUI stay on
Local. That --web should not open a local audio device
is the master plan's requirement. Headless today really does
play audio locally when a device exists, and whether that is
a feature or an accident is not this phase's call. It is
recorded in Future work rather than changed silently.
10. Playback gets its own caps constant. A
DEFAULT_PLAYBACK in the protocol crate, OPUS only, and
default_channel_caps uses it for the playback channel.
The only wire change is dropping the CELT bit, which ryll
cannot decode. Phase 4 will want to choose Opus or PCM per
client, and that starts from a named constant rather than
a borrowed one.
11. The slim build is checked in make lint, which CI
already runs. A new tools/check-slim-renderer.sh, run in
the devcontainer by make lint, does three things. It
clippies the renderer with --no-default-features
--all-targets. It checks the renderer with each feature
enabled on its own. It runs cargo tree -p
shakenfist-spice-renderer --no-default-features -e
normal,build and fails if any of openh264,
openh264-sys2, mozjpeg-sys, nusb, dav-server,
hyper, cpal or alsa-sys appears. A separate CI job
was rejected: the lint job exists and this costs it a
renderer build.
Step plan¶
Steps run in order; each depends on the one before only where stated. Each step is one commit, with the subject given.
| Step | Effort | Model | Isolation | Brief for sub-agent |
|---|---|---|---|---|
| 1 | high | opus | none | Damage accumulator on DisplaySurface. Commit: "Track damage rectangles on display surfaces." See brief 1. |
| 2 | medium | sonnet | none | Shared event-bus helper; switch run_web and run_headless. Needs step 1. Commit: "Share the event bus and mirror tasks." See brief 2. |
| 3 | high | opus | none | Shared pointer translator; switch --web and the GUI. Commit: "Share pointer translation between frontends." See brief 3. |
| 4 | medium | sonnet | none | Dependency hygiene with no behaviour change. Commit: "Prune the renderer's unused dependencies." See brief 4. |
| 5 | high | opus | worktree | Slim feature set. Needs step 4. Commit: "Make the renderer's heavy dependencies optional." See brief 5. |
| 6 | high | opus | worktree | Device-free playback; --web uses it. Needs step 5. Commit: "Add a device-free playback mode." See brief 6. |
| 7 | medium | sonnet | none | Slim-build check in make lint. Needs steps 5 and 6. Commit: "Check the slim renderer build in lint." See brief 7. |
| 8 | medium | sonnet | none | Documentation. Needs all of the above. Commit: "Document the renderer prep for RDP." See brief 8. |
Brief 1: damage accumulator¶
- New module
shakenfist-spice-renderer/src/display/damage.rs, declared indisplay/mod.rs. Types:pub struct DamageRect { pub left: u32, pub top: u32, pub right: u32, pub bottom: u32 }(right and bottom exclusive, matching the(l, t, r, b)tuplessurface.rsalready uses), andpub struct DamageRegionholding aVec<DamageRect>. Methods:add(rect),is_empty(),rects() -> &[DamageRect],bounding_box() -> Option<DamageRect>,full(width, height) -> Self. Constantpub const MAX_DAMAGE_RECTS: usize = 32. - Merge and bound rules exactly as decision 2. Ignore empty rectangles. Keep it allocation-light: no new dependencies.
DisplaySurface(display/surface.rs) gains adamage: DamageRegionfield, initialised to the full surface innew(). Addtake_damage(&mut self) -> DamageRegion(returns and clears) anddamage(&self) -> &DamageRegion(peek). Do not changeconsume_dirty/is_dirty.- In each draw op, add the rectangle(s) actually written,
where the op already sets
dirty = true: blit(:70): the clamped rectangle, left toleft + copy_width/4, top tomin(top + height, self.height).fill_rectandinvert_rect: each clipped sub-rectangle passed tofill_subrect/invert_subrect.copy_bits: each destination sub-rectangle passed tocopy_subrect. The source is not damage.blit_chromaandblit_alpha: the clampedclip_w x clip_hrectangle, only when the op wrote.SurfaceMirror(surface_mirror.rs): addmark_all_damaged(&mut self), which sets each surface's region to full and its dirty bit. Add a crate-internal setter onDisplaySurfacefor that.apply_eventandDrawOutcomedo not change.- Re-export
DamageRect,DamageRegionandMAX_DAMAGE_RECTSfromlib.rs. - Unit tests in
damage.rsandsurface.rs: - a new surface is fully damaged;
take_damageclears;- each op's damage, including
copy_bitsdamaging only the destination, and clip lists producing per-clip rectangles; - an off-surface draw adds nothing;
- containment dropping;
- the merge rule (two adjacent rectangles merge, two distant ones do not);
- 33 disjoint rectangles collapse to the bounding box;
consume_dirtyandtake_damagedo not affect each other;mark_all_damaged.make testandmake lintpass.
Brief 2: event-bus helper¶
- New module
shakenfist-spice-renderer/src/event_bus.rs, exported fromlib.rs.pub struct EventBusowning abroadcast::Sender<ChannelEvent>, anArc<tokio::sync::Mutex<SurfaceMirror>>, and the mirror's pre-takenbroadcast::Receiver. EventBus::new(capacity)creates all three.sender()clones the sender.subscribe()returns a receiver.mirror()clones the mirror handle.start(self, event_rx: mpsc::Receiver<ChannelEvent>, label: &'static str) -> EventBusHandlesspawns the forwarder and the mirror-apply task and returns theirJoinHandles.labelis for logs ("web", "headless").- Behaviour must match the two existing copies:
session.rs:883-935andryll/src/main.rs:803-845. - The forwarder drains the mpsc into the broadcast and ignores send errors.
- The mirror task applies each event under the lock.
- On
Lagged(n)the mirror task warns and then callsmark_all_damaged()(decision 3, from step 1). - On
Closedit logs and exits. starttakesselfby value. IfEventBusitself still holds a sender afterstart, the broadcast never closes, so make sure it does not.run_headlessrelies on the bus closing when the forwarder exits (session.rs:937-940): read that comment.- Switch
run_headlessandrun_webto it. Keeprun_web's ordering: construct the bus, take the cursor subscription (and any other long-lived ones) before spawningrun_connection, thenstart. Use the bus'ssender()whereevent_broadcast_txwas cloned. Deleteryll/src/web/server.rs'sEVENT_BROADCAST_CAPACITYand useshakenfist_spice_renderer::session::EVENT_BROADCAST_CAPACITY. - Test: a tokio test that starts a bus, sends a
SurfaceCreatedand anImageReadythrough the mpsc, and sees them applied to the mirror. A second test: a bus with capacity 1, a flood of events and a slow mirror, ends with every surface fully damaged. If forcingLaggeddeterministically is awkward, testmark_all_damagedthrough a smaller seam and say so. make testandmake lintpass. No behaviour change in either mode beyond decision 3.
Brief 3: pointer translator¶
- New module
shakenfist-spice-renderer/src/pointer.rs, exportingPointerTranslatorfromlib.rs. API, all synchronous: new()andreset();last_pos() -> Option<(u32, u32)>;motion(mouse_mode: u32, x: u32, y: u32) -> Option<InputEvent>;button(mouse_mode: u32, button: u32, down: bool, pos: Option<(u32, u32)>) -> Vec<InputEvent>, at most two events;wheel(clicks: i32) -> Vec<InputEvent>. Coordinates are surface pixels. Compare the mode againstshakenfist_spice_protocol::MOUSE_MODE_SERVER; anything else (client, unknown) is treated as client, as both callers do today.- Semantics exactly as decision 5. Port the comments that
explain why (the ack-window cost of zero deltas,
web/inputs.rs:216-224; the first move as a zero delta; the inputs channel sending a position before each press,inputs.rs:750-760) into the new module rather than losing them. ryll/src/web/inputs.rsrun_input_relay: replacelast_poswith aPointerTranslator.PointerMovecallsmotion.PointerButtoncallsbutton(..., Some((x, y)))and sends each returned event in order. Keep the denormalisation and the no-primary-surface skip.ryll/src/app/mod.rs: replace thelast_mouse_posfield (:374, initialised :982, reset :1160) with aPointerTranslator.- The hover block at :4536-4555 calls
motion. - The button loop at :4568-4594 calls
button(..., None). Keepforwarded_buttonsand thepointer_on_surfacegate exactly as they are. - The wheel block at :4596-4618 calls
wheel(clicks). - The input-suppressed release path at :4623 and the cursor
overlay at :4956 read
last_pos(). - The GUI sends with
try_send; keep that. - Tests:
- Unit tests in
pointer.rsfor both modes: duplicate suppression, the first server-mode move, deltas, the server-mode button catch-up delta, no catch-up in client mode, a button with no position, wheel up and down,reset. - Update
a_button_position_becomes_the_next_delta_referenceinweb/inputs.rsto expect the catch-upMouseMotionbefore the press, and add a client-mode test that a repeated position sends nothing. The other existing web input tests must pass unchanged. make testandmake lintpass.
Brief 4: dependency hygiene¶
No behaviour change, and every build stays identical apart from fewer crates.
shakenfist-spice-renderer/Cargo.toml:- Delete
rsa,sha1,rand,tokio-rustls,rustls-pemfile,webpki-roots,byteorder,async-channel,thiserror,httpandhttp-body-util. Grepsrc/andtests/for each before deleting. - Move
bytesto[dev-dependencies]. - Move
nusbunder[target.'cfg(target_os = "linux")'.dependencies]. - Fix the stale comments:
socket2is used only by the control socket's bind (control/server.rs:158-160), andimageonly by the screenshot verb and display tests. - In
shakenfist-spice-compression, moveeven_dimensionsout ofencode/h264.rs(:83-85) to an ungated module, and re-export it from the crate root unconditionally. Keep the renderer's re-export path (encoder/h264.rs:12,lib.rs:46-49) working. - In the renderer, move
EncodedFrame(encoder/h264.rs:16-26) andEncoderControl(encoder/task.rs:25-32) into a newencoder/types.rsand re-export them from the same paths, so step 5 can gateh264.rsandtask.rswithout taking them along. - Delete
ChannelEvent::UsbDevicesChanged(channels/mod.rs:450-452) and itskind()arm (:601). Grep the workspace, includingdocs/control-socket-protocol.md, for the event name, and remove it from any list of subscribable kinds. make build,make testandmake lintpass, plusmake check-windows, sincenusbmoved.
Brief 5: slim feature set¶
Read decisions 6 and 7 first. They are settled; the work is mechanical but wide, which is why this runs in a worktree.
shakenfist-spice-compression/Cargo.toml. Makeopenh264optional and addh264-decode = ["dep:openh264"]todefault. Adddep:openh264toencode, sinceencode/h264.rs:14-20usesopenh264::encoder.video.rs. GateH264VideoDecoderand its impl (:266-477) and thefor_streamH.264 arm (:174) onh264-decode. Without it, codec 3 falls through toUnsupportedCodec, which the display channel already logs and skips (display.rs:1775-1792). LeaveSPICE_VIDEO_CODEC_TYPE_H264ungated.- Compression tests. Gate the H.264 tests (
video.rs:574, :718, :768, theh264_round_trip_testsmodule at :1022, :1174), andencode/h264.rs:1201-1220, which needs both features. Confirmmake lint's existing slim compression clippy (--no-default-features --features encode) still passes. shakenfist-spice-renderer/Cargo.toml. Add the features in decision 7's table, all indefaultexceptdigest-decode. Depend on compression withdefault-features = false, features = ["quic", "glz", "lz", "lz4", "jpeg"], withencoder,h264-decodeandmozjpegforwarding to it. Makenusb,shakenfist-spice-usbredir,dav-server,hyperandhyper-utiloptional.lz4_flexstays (the display tests use it, and compression pulls it anyway).- Gating, following the
audioprecedent (list everycfg(feature = "audio")site first and copy the shape): lib.rs:pub mod usbunderusbredir,pub mod webdavunderwebdav.channels/mod.rs:usbredirandwebdavand their re-exports.encoder/mod.rs:h264andtaskunderencoder.session.rs: the usbredir arm (:410-431) and the webdav arm (:433-454). Inplan_channel_links(:1151-1175),cfg!skips with reasons like "usbredir feature disabled at compile time". Silence the now-unused parameters assession.rs:226does.- In a build without
h264-decode, forceH264Offer::Offbeforedisplay_channel_capsis called (aroundsession.rs:340) and inDisplayChannel::new(display.rs:1040), with a comment pointing at decision 6. - Renderer tests. Gate the display H.264 tests
(
display.rs:4822-4839helper, :4866, :4888, :4915) onall(feature = "h264-decode", feature = "encoder"). The preference tests at :5876-5925 must pass in the default build; gate the H.264-specific ones. Add[[test]] required-features = ["encoder"]fortests/encoder_smoke.rsandtests/webrtc_h264_smoke.rs. Add aplan_channel_linkstest per new feature, mirroringplayback_follows_the_audio_feature(session.rs:1314). ryll/Cargo.toml. List the renderer features ryll needs explicitly:usbredir,webdav,encoder,h264-decode,mozjpeg.audiokeeps forwarding through ryll's ownaudiofeature. Every existing ryll build (make build,make check-windows, the slim--no-default-features -p ryll) must link the same functionality as before.shakenfist-spice-webrtc/Cargo.toml. No production features. Add the renderer as a dev-dependency withencoder, sincebridge.rs:2141andtests/loopback.rs,tests/lifecycle.rsuseH264Encoder/EncoderTask.- Verify, and paste the output into the step's report:
cargo clippy -p shakenfist-spice-renderer --no-default-features --all-targets -- -D warningsis clean.cargo tree -p shakenfist-spice-renderer --no-default-features -e normal,buildcontains none ofopenh264,openh264-sys2,mozjpeg-sys,nusb,dav-server,hyper.cpalwill still be absent, becauseaudiois off.cargo check -p shakenfist-spice-renderer --no-default-features --features Xpasses for each new feature X.
Run them inside the devcontainer (copy the
DOCKER_RUN_OFFLINE invocation from the Makefile). Then
make build, make test, make lint and
make check-windows.
- Report the full cargo tree output for the slim build; the
management session records it in this plan.
Brief 6: device-free playback¶
Read decisions 8 to 10 and the survey's items 4 and 5 first.
shakenfist-spice-renderer/Cargo.toml.opus-decoderbecomes non-optional, andaudio = ["dep:cpal", "dep:rtrb"].channels/mod.rs. Compileplaybackunconditionally.playback.rs. Gate onlyAudioThread,CallbackState, the ring and their tests onaudio.Resampler,pcm_bytes_to_i16, the TOC parser,OutputStatusand the mode helpers stay ungated, with their tests. Expect to restructurestart_audio_output(:1329-1352) and theaudio_producerhandling so a build withoutaudio, or aSinksession, never constructs the thread.audio_sink.rs. RenameOpusPacketSinktoPlaybackSink(rename the file toplayback_sink.rs). Add:pub struct PlaybackFormat { pub mode: u16, pub sample_rate_hz: u32, pub channels: u8 };fn on_start(&self, format: PlaybackFormat) {}andfn on_stop(&self) {};fn on_decoded_pcm(&self, samples: &[i16], sample_rate_hz: u32, channels: u8) {}. Callon_startafter START (and again after a MODE change, with the new mode) andon_stopon STOP. Callon_decoded_pcmwith the decode buffer slice after each successful Opus decode (push_samples_opus, :1281-1314). Saturate rather than truncate the channel count intou8(todayas u8, :1170-1176).run_connection. Replaceopus_sink: Option<Arc<dyn OpusPacketSink>>withplayback: PlaybackOutput:pub enum PlaybackOutput { Local, Sink(Arc<dyn PlaybackSink>) }. Updateplan_channel_links(:1158-1161):Sinklinks in every build;Locallinks only withaudio, with today's skip reason. Replace theopus_sink.is_some()notification suppression (:1383) with theSinkvariant. Delete thecfg(not(feature = "audio"))silencer at :226 if it is no longer needed.- Callers. There are four:
session.rs:771(headless,Local),ryll/src/main.rs:764(--web,Sink(WebOpusSink)), andryll/src/app/mod.rs:915and:1248(GUI,Local). RenameWebOpusSinktoWebPlaybackSinkand updateryll/src/web/audio.rs's module doc, which describes local playback happening alongside. - Caps (protocol crate). Add
DEFAULT_PLAYBACK(PLAYBACK_OPUSonly) inconstants.rsnext to the playback cap constants (:221-234), and aChannelType::Playbackarm indefault_channel_caps(link.rs:706-712). Update the test atlink.rs:2000-2009, which pins today's accidental value. - Tests. None today drive
PlaybackChannel's DATA dispatch (survey). Usingchannels/test_support.rs, add tests that feed START, MODE(OPUS), DATA and STOP to aSink-mode channel with a recording sink, and assert: on_startarrives before the first packet;- the Opus packet is tapped;
on_decoded_pcmfollows the tap with 48 kHz samples;on_stoparrives;- no audio thread was spawned (expose a test-only
observation, such as a count of thread starts).
Add a RAW-mode case for
on_pcm_samples. Updateplayback_follows_the_audio_featureto cover both variants. - Verify.
make testandmake lintpass.- The slim clippy from brief 5 still passes.
cargo tree -p shakenfist-spice-renderer --no-default-features -e normal,buildhas nocpaloralsa-sys.- Run
ryll --webagainst the local QEMU test server (docs/development.md) and confirm in the log that no audio thread starts. Say in the report if no guest with audio was available to hear it end to end.
Brief 7: slim-build check¶
- New
tools/check-slim-renderer.sh(shellcheck-clean,set -euo pipefail), run inside the devcontainer. It performs decision 11's three checks with--frozen, and on failure prints which crate appeared and thecargo tree -ipath that pulled it in. - Call it from the
linttarget in theMakefile(:277-280), after the existing clippies, via the same$(DOCKER_RUN_OFFLINE) $(RYLL_IMAGE)wrapper. Update the comment abovelint. - Prove it bites. Temporarily make
nusbnon-optional, run it, and confirm it fails namingnusb. Do not commit that. make lintpasses.
Brief 8: documentation¶
docs/development.mdCargo features: a renderer subsection with decision 7's table (in prose or a table), how to build the slim renderer, and thatmake lintchecks it. Correct theaudiobullet: withoutaudiothe playback channel is now still linked for aSinksession.AGENTS.mdCargo feature gating: one or two sentences that the renderer now has its own feature set, the same gate-at-the-use-site rule applies, andmake lintruns the slim check. Nothing else inAGENTS.md.ARCHITECTURE.md:- The trait table row (:114):
OpusPacketSinkbecomesPlaybackSink, now carrying decoded PCM and format events. - The compression crate row (:30): H.264 decode is a feature.
- Add the event bus, pointer translator and damage region to the renderer's file tree. The shape of the system does not otherwise change.
docs/multi-mode-parity.md:- Audio playback row (:109), web cell: no host cpal thread any more.
- Row :207: correct the PCM fallback wording to match what the code does.
- Add a mouse wheel row: GUI available, headless n/a, web missing (survey item 9).
docs/web-mode-internals.md: the event bus helper, the playback sink mode, and the pointer translator, replacing any description of the hand-written tasks.docs/rendering-pipeline.md: a short section on damage tracking. What is recorded per op (copy_bitsdamages its destination), the merge and bound rules, the independence from the dirty bit, and lag marking everything damaged.docs/control-socket-protocol.md: removeUsbDevicesChangedif step 4 has not already.shakenfist-spice-renderer/README.md: the feature list, if it has one; add one if it does not, since crates.io consumers read it.- Check with
grep -rn OpusPacketSink docs/ ARCHITECTURE.md AGENTS.mdthat no stale name remains outsidedocs/plans/.
Risks and mitigations¶
- A missed
cfgbreaks a feature combination nobody builds. Mitigation: step 7's per-feature checks run in everymake lint. The management session reviews step 5 by running the slim clippy itself, not by reading the report. - Feature unification hides a missing feature in ryll. A
workspace build unifies features, so ryll forgetting to
request
webdavwould still build undermake build. Mitigation:make check-windowsbuilds-p ryllalone. The management session also runscargo check -p ryllandcargo check -p ryll --no-default-featuresalone after step 5. - The pointer change regresses the GUI. The GUI's pointer
block is egui-coupled and has no unit tests. Mitigation:
the translator's behaviour for the GUI's calls (
buttonwith no position,motion) is identical to today's by construction. The management session runs the GUI against the local QEMU test server after step 3, and checks hover, click, drag and wheel in both mouse modes (a guest with and without vdagent). - The server-mode catch-up delta surprises a
--webuser. Mitigation: it only fires when a button arrives at a position no move reported, which the browser rarely does. Decision 5 records it as deliberate, and the PR description calls it out. - Damage tracking slows the draw path. One
addper written sub-rectangle, with at most 32 rectangles to scan. Mitigation: none needed beyond keepingaddlinear; the management session rejects a quadratic-per-pixel design in review. - The playback refactor breaks local audio in the GUI. The
cpal thread code moves but should not change. Mitigation:
step 6 runs in a worktree, the existing
playback.rstests move with their code, and the management session plays guest audio through the GUI after step 6. - Kerbside's spike step 3b wants a different damage API. Mitigation: the API (decision 1) is a plain region taken under the mirror lock, which is what spike 3b's brief describes. After this phase merges, the management session points kerbside PR #581 (or its successor) at the merged API.
- Publishing. Feature names and the
PlaybackSinkrename are breaking for crates.io consumers of 0.3.0. Mitigation: acceptable at 0.x; the next release notes must list them.tools/package-check-verify.sh(cargo package --workspace) in CI catches a manifest that does not package.
Definition of done¶
-
tools/check-slim-renderer.shexists,make lintruns it, and it passes. It fails whennusbis made non-optional (verified in step 7, not committed). -
cargo tree -p shakenfist-spice-renderer --no-default-features -e normal,buildlists none ofopenh264,openh264-sys2,mozjpeg-sys,nusb,dav-server,hyper,cpal,alsa-sys. Its output is recorded in Results below. -
grep -rn "fn take_damage" shakenfist-spice-renderer/srcandgrep -rn "pub struct PointerTranslator"andgrep -rn "pub struct EventBus"each find one definition.grep -n "last_mouse_pos\|last_pos" ryll/src/app/mod.rs ryll/src/web/inputs.rsfinds no hand-rolled position state. -
grep -rn "broadcast::channel::<" ryll/src/main.rs shakenfist-spice-renderer/src/session.rsfinds nothing; both modes useEventBus. -
grep -rn "OpusPacketSink" --include=*.rs --include=*.md . | grep -v docs/plans/finds nothing. -
run_webpassesPlaybackOutput::Sink; a--websession against the local test server logs no audio thread start. - Damage tests cover
copy_bitsdestination-only damage, the 32-rectangle bound and the lag path; pointer tests cover both modes and the catch-up delta. -
make build,make test,make lint,make check-windowsandpre-commit run --all-filespass. - The GUI is checked by hand (pointer in both modes, wheel, local audio) and the result is recorded in Results.
- The docs listed in brief 8 are updated, and no doc
outside
docs/plans/describes--webas opening a local audio device. - The master plan's acceptance for phase 1 is met. Where
it differs from this plan (decision 5's
--webbehaviour change), the difference is recorded here.
Results¶
Filled in by the management session as steps land.
- Slim dependency tree after step 5 (
cargo tree -p shakenfist-spice-renderer --no-default-features -e normal,build, 266 lines): - Direct dependencies: anyhow, base64, flate2, image, libc, lz4_flex, serde, serde_json, shakenfist-spice-compression, shakenfist-spice-protocol, socket2, tokio, tokio-util, tracing.
- None of
openh264,openh264-sys2,mozjpeg-sys,nusb,dav-server,hyper,cpaloralsa-sysappear (checked by grep in the management session). - The only
-syscrate left isaws-lc-sys(a C build needing cmake). It does not come from the renderer: the protocol crate'stokio-rustlstakes rustls's default crypto provider, aws-lc-rs. The master plan's acceptance names openh264, nusb, dav-server and cpal, so the criterion is met, but a slim consumer still needs a C toolchain. Recorded in Future work. - GUI pointer check after step 3, 2026-10-20 (AEDT), against the
debian-xfce:13desktop guest on SPICE port 5910 (5900 was in use by the local andris service): - Client mode: Mikal checked hover, click, drag, right-click and wheel; all fine.
- Server mode: buttons, wheel and relative motion work. Getting
the guest into server mode took more than stopping vdagent.
q35's default
vmmouseis an absolute device, so spice-server keeps offering client mode through it. The guest was restarted with-machine q35,vmport=off,-vga virtioandagent-mouse=off, after which the server offered server mode only. - In server mode the host and guest cursors drift apart until
the host cursor leaves the window before reaching the target.
This predates the phase. A scripted closed pointer path,
replayed with python-xlib over each window, moved the guest
cursor about 50 px left per run in both the develop
7690e1ebuild and the step-3 build (runs: −53, −51, −49 px; matching cursor-move counts). The cause is the guest's pointer acceleration on relative motion, with no pointer grab or warp in the GUI to correct it (spice-gtk grabs in server mode). Recorded in Future work. - Audio checks after step 6, same guest, with
alsa-utilsinstalled in it andspeaker-testplaying (it played pink noise, its default, rather than the intended sine): - GUI (
Local): Mikal heard the audio. The log showedMODE: 3(Opus), START, and the local audio thread starting. The develop build sounded the same. --web(Sink): the log showedMODE: 3, START and the Opus decoder starting, and no line about a local audio thread or output device (0 matches for "audio output started", "audio output unavailable" or "audio thread"). No browser was attached, so audio arriving in a browser was not checked.- Final tree:
make build,make test(1505 passed, 0 failed),make lint(including the slim check),make check-windows,make package-checkandpre-commit run --all-filesall pass.
Execution notes¶
- Run on 2026-10-20 (AEDT), steps in plan order.
- The plan was written at low planning effort, not the high effort the master plan asks for; Mikal chose to proceed with it as written.
- Steps 5 and 6 ran in worktrees made by hand from the phase branch, then fast-forwarded into it, because the Agent tool's own worktree isolation branches from the default branch.
- Step 4 ran while the desktop guest was booting for the step 3 GUI check, which used a binary copied out of the tree first.
- The desktop guest ran on SPICE port 5910 via
tools/start-desktop-qemu.sh, because the local andris service holds 5900, somake test-qemu-desktopcould not be used as is. - Review changes beyond the briefs:
- Step 4: gated run_headless's mirror handle, which step 2 had left unused on Windows.
- Steps 5 and 8: corrected two comment and doc claims.
- Step 8: added the GUI's server-mode drift to the parity matrix. Each commit message says what review changed.
- The PR's first automated review changed decision 11: the
per-feature builds in
tools/check-slim-renderer.shnow run clippy with warnings denied rather thancargo check, so a gating mistake that only warns in one feature's build fails lint. It also made a sink'son_stopfollow anon_startonly, and made raw PCM conversion reuse one buffer.
Future work¶
- Headless audio. Headless still plays audio locally when
a device exists, and retries cpal when none does. With
PlaybackOutputin place, a headless discard or sink mode is a one-line choice; whether headless should play audio is a product decision (decision 9). ryll/Cargo.tomldeclares crates it never uses directly: nusb, dav-server, hyper, hyper-util, http, http-body-util, lz4_flex, image, jpeg-decoder, flate2, socket2, rsa, sha1, byteorder, tokio-rustls, rustls-pemfile, webpki-roots and async-channel (per the survey;http::hits inryll/srcareaxum::http). Pruning them does not affect a renderer consumer, but tidies ryll's own tree.- Web mouse wheel (#594, survey item 9): add a wheel message to
app.jsand the relay, throughPointerTranslator::wheel. - GUI server-mode pointer grab (#592). In server mouse mode the GUI sends relative deltas from host hover positions without grabbing or warping the pointer, so guest acceleration makes the two cursors drift until parts of the guest display cannot be reached (see Results). spice-gtk grabs the pointer in server mode. This predates the phase; the RDP frontend will meet the same problem in server mode.
- The protocol crate's TLS provider (#593).
shakenfist-spice-protocoldepends ontokio-rustlswith default features, which pulls in aws-lc-rs andaws-lc-sys(a cmake C build), while ryll itself installsringas its rustls provider. Choosing one provider in the protocol crate (probablyring, withdefault-features = false) would take the last C build out of the slim renderer and stop ryll building two crypto libraries. Check what Kerbside's proxy expects before changing it. - cargo-deny on the slim tree: it runs with
--all-features, so it audits a superset. Probably fine; noted in case a licence ever differs between the two trees.
Back brief¶
Before executing any step of this plan, back brief the operator as to your understanding of the plan and how the work you intend to do aligns with it. Step 5 is the step that is cheap to propose and expensive to redo: before its sub-agent starts, the management session confirms that decision 7's feature table still matches the tree after step 4.