Skip to content

Commit bd1fb73

Browse files
committed
fix(capture-linux): measure the cursor against the window, not its monitor
mutter never renegotiates the format for a window stream: 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, so the file holds the window -- but the portal reports the pointer in STREAM pixels, measured from the monitor's corner, and emit_sample normalised it against the negotiated format. A monitor-relative position divided by monitor dimensions, then painted onto window-sized footage: wrong origin and wrong scale, in every window recording, from the first frame. A 640x480 window at (100, 50) on 1920x1080 put a pointer at the window's centre at 0.219, 0.269. Capture now remembers the rect it actually read -- read_origin at the committed size, so it follows a window that moves and keeps the clamp that stops a shrunken window reading past the buffer -- and content_rect() is the one place that answers "what does the file show". emit_sample takes the origin off the position and reports that rect's dimensions, so the accumulator's x/width lands in the space the compositor assumes. `visible` moves with it. It was tested against the monitor, so a pointer that had left the recorded window still reported visible: true. Full-screen capture is byte-identical: no crop means the content rect is the stream. A cursor-only session opens no encoder and keeps normalising against the stream, which is right -- its video comes from Electron. Not #511. That one is temporal (the video time-compresses under frame drops while the cursor keeps wall-clock time, so the error grows across the take); this one is spatial and constant. They stack.
1 parent 059f4e8 commit bd1fb73

4 files changed

Lines changed: 241 additions & 16 deletions

File tree

electron/native-bridge/cursor/recording/pipeWireCursorAccumulator.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,15 @@ export class PipeWireCursorAccumulator {
150150
addSample(payload: Extract<PipeWireHelperEvent, { event: "cursor-sample" }>) {
151151
this.rememberAsset(payload.asset);
152152

153-
// Normalised against the stream's own dimensions, which the helper repeats
154-
// on every sample. Electron's display bounds are deliberately NOT used:
155-
// they are in DIPs, whereas the portal reports stream pixels, and the
156-
// portal's source is whatever the user picked in its own dialog, which
157-
// need not be the display the app thinks it is recording.
153+
// Normalised against the RECORDED RECTANGLE, which the helper repeats on
154+
// every sample: the crop for a window stream, the whole stream for a
155+
// screen. It reports its own because only it knows — for a window,
156+
// mutter pins the stream to the monitor and carves the window out
157+
// through a crop, so the stream's dimensions describe a rectangle the
158+
// file does not show. Electron's display bounds are deliberately NOT
159+
// used either: they are in DIPs, whereas the portal reports pixels, and
160+
// the portal's source is whatever the user picked in its own dialog,
161+
// which need not be the display the app thinks it is recording.
158162
const width = Math.max(1, payload.width);
159163
const height = Math.max(1, payload.height);
160164

electron/native/pipewire-capture/src/capture.rs

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,17 @@ pub struct Capture {
220220
/// through it rather than replacing it.
221221
committed_width: i32,
222222
committed_height: i32,
223+
/// The rectangle of the SOURCE STREAM the encoder last read, in stream
224+
/// pixels: the live crop origin (clamped by [`Self::read_origin`]) at the
225+
/// committed size.
226+
///
227+
/// Exists for the cursor, not for the video. The portal reports pointer
228+
/// positions in stream pixels, and for a window stream the stream is the
229+
/// whole monitor — mutter pins it there and carves the window out through
230+
/// SPA_META_VideoCrop. Normalising the pointer against the stream would
231+
/// then place it in a rectangle the file does not show. This is the one the
232+
/// file DOES show.
233+
content: shim::CropRect,
223234
}
224235

225236
impl Capture {
@@ -286,6 +297,7 @@ impl Capture {
286297
frames_written: 0,
287298
committed_width: width,
288299
committed_height: height,
300+
content: shim::CropRect { x: 0, y: 0, width, height },
289301
},
290302
selection,
291303
))
@@ -331,6 +343,12 @@ impl Capture {
331343
// path subtracts the x offset from it, which is wrong for any non-zero x
332344
// and is latent there only because no shipping compositor sets one.
333345
let (x, y) = self.read_origin(frame);
346+
self.content = shim::CropRect {
347+
x,
348+
y,
349+
width: self.committed_width,
350+
height: self.committed_height,
351+
};
334352
let offset = (y as usize)
335353
.checked_mul(frame.stride)
336354
.and_then(|rows| rows.checked_add((x as usize) * BYTES_PER_SOURCE_PIXEL))
@@ -364,6 +382,11 @@ impl Capture {
364382
self.epoch.is_some()
365383
}
366384

385+
/// The source rectangle the file is showing. See [`Self::content`].
386+
pub fn content_rect(&self) -> shim::CropRect {
387+
self.content
388+
}
389+
367390
/// Encodes forward to the current clock position. Returns how many frames
368391
/// were written.
369392
pub fn advance(&mut self) -> Result<u32, String> {
@@ -658,6 +681,72 @@ mod tests {
658681
let _ = std::fs::remove_file(&output);
659682
}
660683

684+
/// The rectangle the cursor is measured against has to be the one the FILE
685+
/// shows, which for a window is the crop and not the stream. Getting this
686+
/// wrong put the pointer in monitor coordinates over window-sized footage —
687+
/// a fixed offset and a wrong scale, in every window recording.
688+
#[test]
689+
fn the_content_rect_is_the_window_the_file_shows() {
690+
let output = std::env::temp_dir().join("openscreen-capture-content.mp4");
691+
let (mut capture, _) =
692+
Capture::start(&output, 320, 240, 30, Some(1_000_000), Some(Backend::Software), Vec::new())
693+
.expect("start");
694+
695+
// Before a frame is staged there is nothing cropped yet, so the whole
696+
// committed size sits at the origin.
697+
assert_eq!(
698+
capture.content_rect(),
699+
shim::CropRect { x: 0, y: 0, width: 320, height: 240 }
700+
);
701+
702+
capture
703+
.stage(&cropped_frame(
704+
1920,
705+
1080,
706+
shim::CropRect { x: 100, y: 50, width: 320, height: 240 },
707+
shim::constants().video_format_bgrx,
708+
))
709+
.expect("stage");
710+
assert_eq!(
711+
capture.content_rect(),
712+
shim::CropRect { x: 100, y: 50, width: 320, height: 240 }
713+
);
714+
715+
// The window moved. The file follows its origin at the committed size,
716+
// and so must the cursor.
717+
capture
718+
.stage(&cropped_frame(
719+
1920,
720+
1080,
721+
shim::CropRect { x: 700, y: 400, width: 320, height: 240 },
722+
shim::constants().video_format_bgrx,
723+
))
724+
.expect("stage");
725+
assert_eq!(
726+
capture.content_rect(),
727+
shim::CropRect { x: 700, y: 400, width: 320, height: 240 }
728+
);
729+
730+
// An origin that would read past the buffer is clamped for the pixels,
731+
// so the cursor has to be clamped with it or the two disagree about
732+
// which rectangle was recorded.
733+
capture
734+
.stage(&cropped_frame(
735+
1920,
736+
1080,
737+
shim::CropRect { x: 1800, y: 1000, width: 320, height: 240 },
738+
shim::constants().video_format_bgrx,
739+
))
740+
.expect("stage");
741+
assert_eq!(
742+
capture.content_rect(),
743+
shim::CropRect { x: 1600, y: 840, width: 320, height: 240 }
744+
);
745+
746+
let _ = capture.finish();
747+
let _ = std::fs::remove_file(&output);
748+
}
749+
661750
/// A crop flush against the right edge leaves the last row short of a full
662751
/// stride. The old `stride * height` bounds check rejected exactly those —
663752
/// i.e. every window not touching the left edge.

electron/native/pipewire-capture/src/main.rs

Lines changed: 141 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -808,7 +808,8 @@ fn run<W: Write>(
808808
// The portal's size is in the compositor's coordinate space
809809
// and can differ from the negotiated pixel size on a scaled
810810
// display. Logged rather than used: cursor positions arrive
811-
// in stream pixels, so only the negotiated size normalises them.
811+
// in stream pixels, and `content_rect` places them from
812+
// there.
812813
let _ = emitter.emit(&Event::Debug {
813814
code: "portal-stream".to_owned(),
814815
data: json_map([
@@ -1052,14 +1053,14 @@ fn run<W: Write>(
10521053
// A new sprite ships immediately; positions respect the sample
10531054
// interval so a 120fps compositor cannot flood stdout.
10541055
if asset_is_new || last_emit.elapsed() >= config.sample_interval {
1055-
emit_sample(emitter, &cursor, size, &mut pending_asset);
1056+
emit_sample(emitter, &cursor, content_rect(&capture, size), &mut pending_asset);
10561057
last_emit = Instant::now();
10571058
}
10581059
}
10591060

10601061
Err(RecvTimeoutError::Timeout) => {
10611062
if cursor.is_some() && last_emit.elapsed() >= config.sample_interval {
1062-
emit_sample(emitter, &cursor, size, &mut pending_asset);
1063+
emit_sample(emitter, &cursor, content_rect(&capture, size), &mut pending_asset);
10631064
last_emit = Instant::now();
10641065
}
10651066
// The heartbeat that keeps the output at a constant frame rate
@@ -1148,22 +1149,49 @@ fn finish_capture<W: Write>(
11481149
}
11491150
}
11501151

1152+
/// The rectangle cursor positions are measured against.
1153+
///
1154+
/// The encoder's once a frame has been staged — that is the only rectangle the
1155+
/// file shows. Before then, and for a cursor-only session that opens no encoder
1156+
/// at all, the whole negotiated stream, which is what the consumer of a
1157+
/// cursor-only recording is compositing over.
1158+
fn content_rect(capture: &Option<Capture>, size: Option<(i32, i32)>) -> Option<shim::CropRect> {
1159+
match capture {
1160+
Some(capture) if capture.started() => Some(capture.content_rect()),
1161+
_ => size.map(|(width, height)| shim::CropRect { x: 0, y: 0, width, height }),
1162+
}
1163+
}
1164+
1165+
/// Emits one cursor sample, positioned inside `content`.
1166+
///
1167+
/// `content` is the sub-rectangle of the stream the recording actually shows —
1168+
/// [`Capture::content_rect`] once pixels are being encoded, the whole stream
1169+
/// otherwise (a cursor-only session records no video of its own). The pointer
1170+
/// arrives in STREAM pixels, so for a window stream it is measured from the
1171+
/// corner of the monitor while the file starts at the corner of the window;
1172+
/// subtracting the origin is what puts the two in the same space. The consumer
1173+
/// normalises against the `width`/`height` reported here, so those must be the
1174+
/// content's, not the stream's.
11511175
fn emit_sample<W: Write>(
11521176
emitter: &mut Emitter<W>,
11531177
cursor: &Option<CursorState>,
1154-
size: Option<(i32, i32)>,
1178+
content: Option<shim::CropRect>,
11551179
pending_asset: &mut Option<CursorAsset>,
11561180
) {
1157-
let (Some(state), Some((width, height))) = (cursor, size) else {
1181+
let (Some(state), Some(content)) = (cursor, content) else {
11581182
return;
11591183
};
1160-
let visible = state.x >= 0 && state.y >= 0 && state.x < width && state.y < height;
1184+
let (x, y) = (state.x - content.x, state.y - content.y);
1185+
// A pointer outside the recorded rectangle is REPORTED, not withheld: the
1186+
// consumer clamps the position and carries `visible` so a renderer can hide
1187+
// the sprite rather than pin it to an edge.
1188+
let visible = x >= 0 && y >= 0 && x < content.width && y < content.height;
11611189
let _ = emitter.emit(&Event::CursorSample {
11621190
timestamp_ms: timestamp_ms(),
1163-
x: state.x,
1164-
y: state.y,
1165-
width,
1166-
height,
1191+
x,
1192+
y,
1193+
width: content.width,
1194+
height: content.height,
11671195
visible,
11681196
asset_id: state.asset_id.clone(),
11691197
asset: pending_asset.take(),
@@ -1234,6 +1262,109 @@ fn resolve_microphone_node(label: &str, sources: &[shim::AudioSourceInfo]) -> Op
12341262
candidates.first().map(|s| s.name.clone())
12351263
}
12361264

1265+
#[cfg(test)]
1266+
mod cursor_sample_tests {
1267+
use super::*;
1268+
1269+
fn sample_json(cursor: (i32, i32), content: Option<shim::CropRect>) -> serde_json::Value {
1270+
let mut buffer = Vec::new();
1271+
let mut emitter = Emitter::new(&mut buffer, false);
1272+
emit_sample(
1273+
&mut emitter,
1274+
&Some(CursorState { x: cursor.0, y: cursor.1, asset_id: None }),
1275+
content,
1276+
&mut None,
1277+
);
1278+
let line = String::from_utf8(buffer).expect("utf8");
1279+
serde_json::from_str(line.trim()).expect("json")
1280+
}
1281+
1282+
/// A full-screen capture crops nothing, so the pointer keeps the numbers the
1283+
/// portal gave and is normalised against the whole stream.
1284+
#[test]
1285+
fn a_full_screen_capture_reports_stream_coordinates() {
1286+
let value = sample_json(
1287+
(960, 540),
1288+
Some(shim::CropRect { x: 0, y: 0, width: 1920, height: 1080 }),
1289+
);
1290+
assert_eq!(value["x"], 960);
1291+
assert_eq!(value["y"], 540);
1292+
assert_eq!(value["width"], 1920);
1293+
assert_eq!(value["height"], 1080);
1294+
assert_eq!(value["visible"], true);
1295+
}
1296+
1297+
/// THE WINDOW-CAPTURE BUG. mutter pins a window stream to the whole monitor
1298+
/// and carves the window out through SPA_META_VideoCrop, so the pointer
1299+
/// arrives measured from the monitor's corner while the file starts at the
1300+
/// window's. Reporting the stream's size here normalised a monitor position
1301+
/// against monitor dimensions and handed the compositor a fraction of the
1302+
/// wrong rectangle: the cursor sat at the wrong place in every window
1303+
/// recording, by the crop origin, at the wrong scale.
1304+
#[test]
1305+
fn a_window_capture_reports_coordinates_inside_the_window() {
1306+
// A 1920x1080 monitor carrying a 640x480 window at (100, 50), pointer
1307+
// one quarter into the window.
1308+
let value = sample_json(
1309+
(260, 170),
1310+
Some(shim::CropRect { x: 100, y: 50, width: 640, height: 480 }),
1311+
);
1312+
assert_eq!(value["x"], 160, "the crop origin has to come off the position");
1313+
assert_eq!(value["y"], 120);
1314+
assert_eq!(value["width"], 640, "the consumer normalises against what it is told");
1315+
assert_eq!(value["height"], 480);
1316+
assert_eq!(value["visible"], true);
1317+
}
1318+
1319+
/// Outside the window but still on the monitor. The old test was `x < width`
1320+
/// against the STREAM, which called this visible — and since the consumer
1321+
/// clamps to 0..1, it parked the sprite on the frame's edge for as long as
1322+
/// the pointer was anywhere else on screen.
1323+
#[test]
1324+
fn a_pointer_outside_the_window_is_reported_invisible() {
1325+
let outside = sample_json(
1326+
(1500, 900),
1327+
Some(shim::CropRect { x: 100, y: 50, width: 640, height: 480 }),
1328+
);
1329+
assert_eq!(outside["visible"], false);
1330+
1331+
// Above and to the left of the window, which goes negative rather than
1332+
// past the far edge.
1333+
let before = sample_json(
1334+
(10, 10),
1335+
Some(shim::CropRect { x: 100, y: 50, width: 640, height: 480 }),
1336+
);
1337+
assert_eq!(before["visible"], false);
1338+
}
1339+
1340+
/// No content rectangle means the format has not been negotiated yet. There
1341+
/// is nothing to measure against, so nothing is emitted — a sample stamped
1342+
/// with a guess would be indistinguishable from a real one downstream.
1343+
#[test]
1344+
fn nothing_is_emitted_before_the_format_is_known() {
1345+
let mut buffer = Vec::new();
1346+
let mut emitter = Emitter::new(&mut buffer, false);
1347+
emit_sample(
1348+
&mut emitter,
1349+
&Some(CursorState { x: 10, y: 10, asset_id: None }),
1350+
None,
1351+
&mut None,
1352+
);
1353+
assert!(buffer.is_empty(), "emitted {}", String::from_utf8_lossy(&buffer));
1354+
}
1355+
1356+
/// A cursor-only session opens no encoder, so it falls back to the whole
1357+
/// stream; once pixels are flowing the encoder's rectangle wins.
1358+
#[test]
1359+
fn the_content_rect_prefers_the_encoder_once_it_has_started() {
1360+
assert_eq!(
1361+
content_rect(&None, Some((1920, 1080))),
1362+
Some(shim::CropRect { x: 0, y: 0, width: 1920, height: 1080 })
1363+
);
1364+
assert_eq!(content_rect(&None, None), None);
1365+
}
1366+
}
1367+
12371368
#[cfg(test)]
12381369
mod microphone_resolution_tests {
12391370
use super::*;

technical-documentation/architecture/recording.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,11 @@ Stopping is the part of that boundary that has broken repeatedly (issues #34, #1
5252

5353
`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".
5454

55-
Two consequences follow, and both were once bugs:
55+
Three consequences follow, and all three were once bugs:
5656

5757
- **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.
5858
- **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.
59+
- **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.
5960

6061
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.
6162

0 commit comments

Comments
 (0)