diff --git a/electron/native-bridge/cursor/recording/pipeWireCursorAccumulator.ts b/electron/native-bridge/cursor/recording/pipeWireCursorAccumulator.ts index 84a13130e..cc89ce7c3 100644 --- a/electron/native-bridge/cursor/recording/pipeWireCursorAccumulator.ts +++ b/electron/native-bridge/cursor/recording/pipeWireCursorAccumulator.ts @@ -150,11 +150,15 @@ export class PipeWireCursorAccumulator { addSample(payload: Extract) { this.rememberAsset(payload.asset); - // Normalised against the stream's own dimensions, which the helper repeats - // on every sample. Electron's display bounds are deliberately NOT used: - // they are in DIPs, whereas the portal reports stream pixels, and the - // portal's source is whatever the user picked in its own dialog, which - // need not be the display the app thinks it is recording. + // Normalised against the RECORDED RECTANGLE, which the helper repeats on + // every sample: the crop for a window stream, the whole stream for a + // screen. It reports its own because only it knows — for a window, + // mutter pins the stream to the monitor and carves the window out + // through a crop, so the stream's dimensions describe a rectangle the + // file does not show. Electron's display bounds are deliberately NOT + // used either: they are in DIPs, whereas the portal reports pixels, and + // the portal's source is whatever the user picked in its own dialog, + // which need not be the display the app thinks it is recording. const width = Math.max(1, payload.width); const height = Math.max(1, payload.height); diff --git a/electron/native/pipewire-capture/src/capture.rs b/electron/native/pipewire-capture/src/capture.rs index 17c42f801..be7526579 100644 --- a/electron/native/pipewire-capture/src/capture.rs +++ b/electron/native/pipewire-capture/src/capture.rs @@ -220,6 +220,17 @@ pub struct Capture { /// through it rather than replacing it. committed_width: i32, committed_height: i32, + /// The rectangle of the SOURCE STREAM the encoder last read, in stream + /// pixels: the live crop origin (clamped by [`Self::read_origin`]) at the + /// committed size. + /// + /// Exists for the cursor, not for the video. The portal reports pointer + /// positions in stream pixels, and for a window stream the stream is the + /// whole monitor — mutter pins it there and carves the window out through + /// SPA_META_VideoCrop. Normalising the pointer against the stream would + /// then place it in a rectangle the file does not show. This is the one the + /// file DOES show. + content: shim::CropRect, } impl Capture { @@ -286,6 +297,7 @@ impl Capture { frames_written: 0, committed_width: width, committed_height: height, + content: shim::CropRect { x: 0, y: 0, width, height }, }, selection, )) @@ -331,6 +343,12 @@ impl Capture { // path subtracts the x offset from it, which is wrong for any non-zero x // and is latent there only because no shipping compositor sets one. let (x, y) = self.read_origin(frame); + self.content = shim::CropRect { + x, + y, + width: self.committed_width, + height: self.committed_height, + }; let offset = (y as usize) .checked_mul(frame.stride) .and_then(|rows| rows.checked_add((x as usize) * BYTES_PER_SOURCE_PIXEL)) @@ -364,6 +382,11 @@ impl Capture { self.epoch.is_some() } + /// The source rectangle the file is showing. See [`Self::content`]. + pub fn content_rect(&self) -> shim::CropRect { + self.content + } + /// Encodes forward to the current clock position. Returns how many frames /// were written. pub fn advance(&mut self) -> Result { @@ -658,6 +681,72 @@ mod tests { let _ = std::fs::remove_file(&output); } + /// The rectangle the cursor is measured against has to be the one the FILE + /// shows, which for a window is the crop and not the stream. Getting this + /// wrong put the pointer in monitor coordinates over window-sized footage — + /// a fixed offset and a wrong scale, in every window recording. + #[test] + fn the_content_rect_is_the_window_the_file_shows() { + let output = std::env::temp_dir().join("openscreen-capture-content.mp4"); + let (mut capture, _) = + Capture::start(&output, 320, 240, 30, Some(1_000_000), Some(Backend::Software), Vec::new()) + .expect("start"); + + // Before a frame is staged there is nothing cropped yet, so the whole + // committed size sits at the origin. + assert_eq!( + capture.content_rect(), + shim::CropRect { x: 0, y: 0, width: 320, height: 240 } + ); + + capture + .stage(&cropped_frame( + 1920, + 1080, + shim::CropRect { x: 100, y: 50, width: 320, height: 240 }, + shim::constants().video_format_bgrx, + )) + .expect("stage"); + assert_eq!( + capture.content_rect(), + shim::CropRect { x: 100, y: 50, width: 320, height: 240 } + ); + + // The window moved. The file follows its origin at the committed size, + // and so must the cursor. + capture + .stage(&cropped_frame( + 1920, + 1080, + shim::CropRect { x: 700, y: 400, width: 320, height: 240 }, + shim::constants().video_format_bgrx, + )) + .expect("stage"); + assert_eq!( + capture.content_rect(), + shim::CropRect { x: 700, y: 400, width: 320, height: 240 } + ); + + // An origin that would read past the buffer is clamped for the pixels, + // so the cursor has to be clamped with it or the two disagree about + // which rectangle was recorded. + capture + .stage(&cropped_frame( + 1920, + 1080, + shim::CropRect { x: 1800, y: 1000, width: 320, height: 240 }, + shim::constants().video_format_bgrx, + )) + .expect("stage"); + assert_eq!( + capture.content_rect(), + shim::CropRect { x: 1600, y: 840, width: 320, height: 240 } + ); + + let _ = capture.finish(); + let _ = std::fs::remove_file(&output); + } + /// A crop flush against the right edge leaves the last row short of a full /// stride. The old `stride * height` bounds check rejected exactly those — /// i.e. every window not touching the left edge. diff --git a/electron/native/pipewire-capture/src/main.rs b/electron/native/pipewire-capture/src/main.rs index 8ec4078c2..4c4611369 100644 --- a/electron/native/pipewire-capture/src/main.rs +++ b/electron/native/pipewire-capture/src/main.rs @@ -808,7 +808,8 @@ fn run( // The portal's size is in the compositor's coordinate space // and can differ from the negotiated pixel size on a scaled // display. Logged rather than used: cursor positions arrive - // in stream pixels, so only the negotiated size normalises them. + // in stream pixels, and `content_rect` places them from + // there. let _ = emitter.emit(&Event::Debug { code: "portal-stream".to_owned(), data: json_map([ @@ -1052,14 +1053,14 @@ fn run( // A new sprite ships immediately; positions respect the sample // interval so a 120fps compositor cannot flood stdout. if asset_is_new || last_emit.elapsed() >= config.sample_interval { - emit_sample(emitter, &cursor, size, &mut pending_asset); + emit_sample(emitter, &cursor, content_rect(&capture, size), &mut pending_asset); last_emit = Instant::now(); } } Err(RecvTimeoutError::Timeout) => { if cursor.is_some() && last_emit.elapsed() >= config.sample_interval { - emit_sample(emitter, &cursor, size, &mut pending_asset); + emit_sample(emitter, &cursor, content_rect(&capture, size), &mut pending_asset); last_emit = Instant::now(); } // The heartbeat that keeps the output at a constant frame rate @@ -1148,22 +1149,49 @@ fn finish_capture( } } +/// The rectangle cursor positions are measured against. +/// +/// The encoder's once a frame has been staged — that is the only rectangle the +/// file shows. Before then, and for a cursor-only session that opens no encoder +/// at all, the whole negotiated stream, which is what the consumer of a +/// cursor-only recording is compositing over. +fn content_rect(capture: &Option, size: Option<(i32, i32)>) -> Option { + match capture { + Some(capture) if capture.started() => Some(capture.content_rect()), + _ => size.map(|(width, height)| shim::CropRect { x: 0, y: 0, width, height }), + } +} + +/// Emits one cursor sample, positioned inside `content`. +/// +/// `content` is the sub-rectangle of the stream the recording actually shows — +/// [`Capture::content_rect`] once pixels are being encoded, the whole stream +/// otherwise (a cursor-only session records no video of its own). The pointer +/// arrives in STREAM pixels, so for a window stream it is measured from the +/// corner of the monitor while the file starts at the corner of the window; +/// subtracting the origin is what puts the two in the same space. The consumer +/// normalises against the `width`/`height` reported here, so those must be the +/// content's, not the stream's. fn emit_sample( emitter: &mut Emitter, cursor: &Option, - size: Option<(i32, i32)>, + content: Option, pending_asset: &mut Option, ) { - let (Some(state), Some((width, height))) = (cursor, size) else { + let (Some(state), Some(content)) = (cursor, content) else { return; }; - let visible = state.x >= 0 && state.y >= 0 && state.x < width && state.y < height; + let (x, y) = (state.x - content.x, state.y - content.y); + // A pointer outside the recorded rectangle is REPORTED, not withheld: the + // consumer clamps the position and carries `visible` so a renderer can hide + // the sprite rather than pin it to an edge. + let visible = x >= 0 && y >= 0 && x < content.width && y < content.height; let _ = emitter.emit(&Event::CursorSample { timestamp_ms: timestamp_ms(), - x: state.x, - y: state.y, - width, - height, + x, + y, + width: content.width, + height: content.height, visible, asset_id: state.asset_id.clone(), asset: pending_asset.take(), @@ -1234,6 +1262,109 @@ fn resolve_microphone_node(label: &str, sources: &[shim::AudioSourceInfo]) -> Op candidates.first().map(|s| s.name.clone()) } +#[cfg(test)] +mod cursor_sample_tests { + use super::*; + + fn sample_json(cursor: (i32, i32), content: Option) -> serde_json::Value { + let mut buffer = Vec::new(); + let mut emitter = Emitter::new(&mut buffer, false); + emit_sample( + &mut emitter, + &Some(CursorState { x: cursor.0, y: cursor.1, asset_id: None }), + content, + &mut None, + ); + let line = String::from_utf8(buffer).expect("utf8"); + serde_json::from_str(line.trim()).expect("json") + } + + /// A full-screen capture crops nothing, so the pointer keeps the numbers the + /// portal gave and is normalised against the whole stream. + #[test] + fn a_full_screen_capture_reports_stream_coordinates() { + let value = sample_json( + (960, 540), + Some(shim::CropRect { x: 0, y: 0, width: 1920, height: 1080 }), + ); + assert_eq!(value["x"], 960); + assert_eq!(value["y"], 540); + assert_eq!(value["width"], 1920); + assert_eq!(value["height"], 1080); + assert_eq!(value["visible"], true); + } + + /// THE WINDOW-CAPTURE BUG. mutter pins a window stream to the whole monitor + /// and carves the window out through SPA_META_VideoCrop, so the pointer + /// arrives measured from the monitor's corner while the file starts at the + /// window's. Reporting the stream's size here normalised a monitor position + /// against monitor dimensions and handed the compositor a fraction of the + /// wrong rectangle: the cursor sat at the wrong place in every window + /// recording, by the crop origin, at the wrong scale. + #[test] + fn a_window_capture_reports_coordinates_inside_the_window() { + // A 1920x1080 monitor carrying a 640x480 window at (100, 50), pointer + // one quarter into the window. + let value = sample_json( + (260, 170), + Some(shim::CropRect { x: 100, y: 50, width: 640, height: 480 }), + ); + assert_eq!(value["x"], 160, "the crop origin has to come off the position"); + assert_eq!(value["y"], 120); + assert_eq!(value["width"], 640, "the consumer normalises against what it is told"); + assert_eq!(value["height"], 480); + assert_eq!(value["visible"], true); + } + + /// Outside the window but still on the monitor. The old test was `x < width` + /// against the STREAM, which called this visible — and since the consumer + /// clamps to 0..1, it parked the sprite on the frame's edge for as long as + /// the pointer was anywhere else on screen. + #[test] + fn a_pointer_outside_the_window_is_reported_invisible() { + let outside = sample_json( + (1500, 900), + Some(shim::CropRect { x: 100, y: 50, width: 640, height: 480 }), + ); + assert_eq!(outside["visible"], false); + + // Above and to the left of the window, which goes negative rather than + // past the far edge. + let before = sample_json( + (10, 10), + Some(shim::CropRect { x: 100, y: 50, width: 640, height: 480 }), + ); + assert_eq!(before["visible"], false); + } + + /// No content rectangle means the format has not been negotiated yet. There + /// is nothing to measure against, so nothing is emitted — a sample stamped + /// with a guess would be indistinguishable from a real one downstream. + #[test] + fn nothing_is_emitted_before_the_format_is_known() { + let mut buffer = Vec::new(); + let mut emitter = Emitter::new(&mut buffer, false); + emit_sample( + &mut emitter, + &Some(CursorState { x: 10, y: 10, asset_id: None }), + None, + &mut None, + ); + assert!(buffer.is_empty(), "emitted {}", String::from_utf8_lossy(&buffer)); + } + + /// A cursor-only session opens no encoder, so it falls back to the whole + /// stream; once pixels are flowing the encoder's rectangle wins. + #[test] + fn the_content_rect_prefers_the_encoder_once_it_has_started() { + assert_eq!( + content_rect(&None, Some((1920, 1080))), + Some(shim::CropRect { x: 0, y: 0, width: 1920, height: 1080 }) + ); + assert_eq!(content_rect(&None, None), None); + } +} + #[cfg(test)] mod microphone_resolution_tests { use super::*; diff --git a/technical-documentation/architecture/recording.md b/technical-documentation/architecture/recording.md index bb6e143aa..a191ec0c7 100644 --- a/technical-documentation/architecture/recording.md +++ b/technical-documentation/architecture/recording.md @@ -52,10 +52,11 @@ Stopping is the part of that boundary that has broken repeatedly (issues #34, #1 `org.freedesktop.portal.ScreenCast.SelectSources` takes exactly `(session, cursor_mode, types, multiple, restore_token, persist_mode)`. There is no window id, monitor id, or node id a caller may supply, so the compositor's own picker is the only thing that can choose a source — the app cannot ask for one and cannot override the answer. The helper reports what it was given back on `stream-started` as `sourceKind` (`"monitor"`, `"window"` or `"virtual"`); that reply is the only knowledge the app ever has about what is being recorded, and an absent `sourceKind` means unknown, not "monitor". -Two consequences follow, and both were once bugs: +Three consequences follow, and all three were once bugs: - **The HUD shows no source button on Linux.** An in-app picker cannot steer the portal, and the one that existed raised a *second* portal dialog of its own through `desktopCapturer.getSources()` whose grant was then discarded — which is why choosing a window there changed nothing. - **No portal restore token is persisted.** Replaying one used to suppress the picker on later runs. Because a token is bound to the source it was minted for, an approved monitor came back on every subsequent recording and the picker — the only source chooser Wayland offers — never reappeared, so "record this window" recorded the whole screen. Answering the picker each time is the cost of being able to choose at all. +- **A window stream is a monitor stream with a crop, and the cursor has to be measured against the crop.** mutter never renegotiates the format for a window: it pins the stream to the window's monitor and carves the window out through `SPA_META_VideoCrop`, which can move on any buffer as the window does. The encoder already reads through that rect (`Capture::read_origin`), so the file holds the window — but the portal reports the pointer in *stream* pixels, measured from the monitor's corner. Normalising it against the stream's dimensions therefore described a rectangle the file does not show, and the overlay drew the cursor offset by the crop origin and scaled by the ratio of monitor to window, in every window recording. `Capture::content_rect` is the rect the file actually holds, and `emit_sample` is the one place that turns a pointer into a fraction of it. Electron resolves selected sources, devices, and paths before launching the helper. The helper does not guess a DirectShow camera: Windows receives the resolved selection. A helper error is reported explicitly rather than silently switching a Windows native feature to browser capture.