diff --git a/electron/native/screencapturekit/Sources/OpenScreenCaptureCore/CaptureSizing.swift b/electron/native/screencapturekit/Sources/OpenScreenCaptureCore/CaptureSizing.swift new file mode 100644 index 000000000..9741d9559 --- /dev/null +++ b/electron/native/screencapturekit/Sources/OpenScreenCaptureCore/CaptureSizing.swift @@ -0,0 +1,74 @@ +import CoreGraphics +import Foundation + +/// How big the ScreenCaptureKit output buffer has to be, given the content the stream will +/// actually rasterise. +/// +/// # Why this is not "the size of the display" +/// +/// `SCStreamConfiguration.width`/`height` are **pixels**, and ScreenCaptureKit will not +/// enlarge a smaller source to fill them. `scalesToFit` defaults to `false`, and the SDK +/// header is explicit about what that means: *"When true, the output scales up and down. When +/// false, the output only scales down."* Configure a buffer bigger than the source and the +/// frame is drawn at its native size in one corner of it, with the background — black — +/// everywhere else. The recording is then a small picture inside a large black rectangle, at +/// full file resolution, for its entire duration (issue #418). +/// +/// The size the stream rasterises is a property of the `SCContentFilter`: its `contentRect` +/// (points) times its `pointPixelScale`. That product is the only correct value for +/// `width`/`height`. A display size read from CoreGraphics is not: `CGDisplayPixelsWide` is +/// documented in "pixel units" but tracks the display *mode*, so on a Retina Mac it can report +/// a number that is neither what the filter measures nor what the stream draws — and nothing +/// downstream reconciles the two. On a 1× display the two agree, which is exactly why this +/// only ever showed up on HiDPI machines. +/// +/// # Why the cap is proportional +/// +/// The caller carries a ceiling (the app asks for at most 4K). Clamping each axis on its own +/// changes the *shape* of the buffer, and a buffer whose aspect ratio differs from the source +/// hits the same wall from the other side: `scalesToFit == false` still scales down, but it +/// preserves aspect, so the surplus becomes black bars. A 5120×1440 ultrawide capped to +/// 3840×1440 per axis is drawn as 3840×1080 with 180 black rows above and below it. Scaling +/// both axes by one factor keeps the buffer the same shape as the source, so the downscale +/// fills it. +/// +/// - Parameters: +/// - contentSize: `SCContentFilter.contentRect.size` — the captured region, in points. +/// - pointPixelScale: `SCContentFilter.pointPixelScale` — points-to-pixels for that region. +/// - maxWidth: Ceiling for the returned width, in pixels. +/// - maxHeight: Ceiling for the returned height, in pixels. +/// - Returns: Even, positive pixel dimensions with the source's aspect ratio. +public func captureOutputSize( + contentSize: CGSize, + pointPixelScale: CGFloat, + maxWidth: Int, + maxHeight: Int +) -> (width: Int, height: Int) { + // The ceiling is the one value we can never exceed, so it is also the only sane answer for + // a filter that reports nothing usable — better a correctly-shaped guess than a buffer the + // stream will corner its frame into. + let ceilingWidth = evenFloor(max(2, maxWidth)) + let ceilingHeight = evenFloor(max(2, maxHeight)) + + let scale = pointPixelScale.isFinite && pointPixelScale > 0 ? pointPixelScale : 1 + let pixelWidth = contentSize.width.isFinite ? contentSize.width * scale : 0 + let pixelHeight = contentSize.height.isFinite ? contentSize.height * scale : 0 + guard pixelWidth >= 1, pixelHeight >= 1 else { + return (ceilingWidth, ceilingHeight) + } + + // One factor for both axes: see the note above on why a per-axis clamp reintroduces the + // very padding this function exists to remove. + let fit = min(1, min(CGFloat(ceilingWidth) / pixelWidth, CGFloat(ceilingHeight) / pixelHeight)) + return ( + evenFloor(Int((pixelWidth * fit).rounded())), + evenFloor(Int((pixelHeight * fit).rounded())) + ) +} + +/// Nearest even value at or below `value`, floored at 2 — H.264 chroma subsampling needs both +/// dimensions even, and the encoder rejects zero. +private func evenFloor(_ value: Int) -> Int { + let clamped = max(2, value) + return clamped - (clamped % 2) +} diff --git a/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift b/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift index 47ef67261..75163dee7 100644 --- a/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift +++ b/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift @@ -387,12 +387,16 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { guard let display = content.displays.first(where: { $0.displayID == displayId }) else { throw HelperError.sourceNotFound("No ScreenCaptureKit display found for id \(displayId).") } - let width = Int(CGDisplayPixelsWide(display.displayID)) - let height = Int(CGDisplayPixelsHigh(display.displayID)) + let filter = SCContentFilter(display: display, excludingWindows: []) + let size = captureSize( + for: filter, + fallbackPointSize: display.frame.size, + fallbackDisplayId: display.displayID + ) return CaptureTarget( - filter: SCContentFilter(display: display, excludingWindows: []), - width: clampCaptureDimension(width, fallback: request.video.width), - height: clampCaptureDimension(height, fallback: request.video.height), + filter: filter, + width: size.width, + height: size.height, captureFrame: display.frame ) case "window": @@ -405,13 +409,16 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { let candidateDisplay = content.displays.first { $0.frame.intersects(window.frame) || $0.frame.contains(CGPoint(x: window.frame.midX, y: window.frame.midY)) } - let scaleFactor = Self.scaleFactor(for: candidateDisplay?.displayID ?? CGMainDisplayID()) - let width = Int(window.frame.width) * scaleFactor - let height = Int(window.frame.height) * scaleFactor + let filter = SCContentFilter(desktopIndependentWindow: window) + let size = captureSize( + for: filter, + fallbackPointSize: window.frame.size, + fallbackDisplayId: candidateDisplay?.displayID ?? CGMainDisplayID() + ) return CaptureTarget( - filter: SCContentFilter(desktopIndependentWindow: window), - width: clampCaptureDimension(width, fallback: request.video.width), - height: clampCaptureDimension(height, fallback: request.video.height), + filter: filter, + width: size.width, + height: size.height, captureFrame: window.frame ) default: @@ -423,6 +430,14 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { let configuration = SCStreamConfiguration() configuration.width = outputWidth configuration.height = outputHeight + // Belt and braces for the defect `captureOutputSize` exists to prevent. Left at its + // default of `false`, ScreenCaptureKit "only scales down" (SDK header), so any frame + // smaller than the buffer is drawn at native size in a corner and the rest stays + // background black — a whole recording letterboxed inside its own frame (issue #418). + // With the buffer now derived from the filter the two agree and this changes nothing; + // it is here so that a future source whose size we mispredict comes out scaled rather + // than cornered. + configuration.scalesToFit = true configuration.minimumFrameInterval = CMTime(value: 1, timescale: CMTimeScale(max(1, request.video.fps))) configuration.queueDepth = 6 configuration.showsCursor = !request.video.hideSystemCursor @@ -716,11 +731,36 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { return status == .complete } - private func clampCaptureDimension(_ value: Int, fallback: Int) -> Int { - let requested = max(2, fallback) - let candidate = value > 0 ? value : requested - let clamped = min(candidate, requested) - return max(2, clamped - (clamped % 2)) + /// Output-buffer size for a filter, capped by the resolution the app asked for. + /// + /// The filter is asked first, because `contentRect × pointPixelScale` is by definition the + /// pixel size ScreenCaptureKit is about to rasterise — see `captureOutputSize`, which also + /// explains what a buffer sized any other way does to the frame (issue #418). + /// + /// Those two properties are macOS 14. On 13 the fallback is the region's own point size + /// times the display's scale factor, which is the same product one step further from the + /// source. It is NOT `CGDisplayPixelsWide`/`High`: those follow the display *mode* rather + /// than the filter, and the mismatch between them is the bug. + private func captureSize( + for filter: SCContentFilter, + fallbackPointSize: CGSize, + fallbackDisplayId: CGDirectDisplayID + ) -> (width: Int, height: Int) { + let contentSize: CGSize + let pointPixelScale: CGFloat + if #available(macOS 14.0, *) { + contentSize = filter.contentRect.size + pointPixelScale = CGFloat(filter.pointPixelScale) + } else { + contentSize = fallbackPointSize + pointPixelScale = CGFloat(Self.scaleFactor(for: fallbackDisplayId)) + } + return captureOutputSize( + contentSize: contentSize, + pointPixelScale: pointPixelScale, + maxWidth: request.video.width, + maxHeight: request.video.height + ) } private static func scaleFactor(for displayId: CGDirectDisplayID) -> Int { diff --git a/electron/native/screencapturekit/Tests/OpenScreenCaptureCoreTests/CaptureSizingTests.swift b/electron/native/screencapturekit/Tests/OpenScreenCaptureCoreTests/CaptureSizingTests.swift new file mode 100644 index 000000000..53029a31a --- /dev/null +++ b/electron/native/screencapturekit/Tests/OpenScreenCaptureCoreTests/CaptureSizingTests.swift @@ -0,0 +1,137 @@ +import CoreGraphics +import Foundation +import XCTest + +import OpenScreenCaptureCore + +/// Does the stream get a buffer the shape and size of what it is about to draw? +/// +/// Every case here is the same question asked from a different display, because the answer is +/// only ever wrong on hardware the author did not have: on a 1× display a point size and a +/// pixel size are the same number, so the defect in issue #418 was invisible in every test, +/// every CI run and every dev machine without a Retina panel. What ScreenCaptureKit does with +/// a buffer that does not match — draw the frame in a corner, background the rest black — is +/// quoted in `captureOutputSize`. +final class CaptureSizingTests: XCTestCase { + /// The app's ceiling, from `useScreenRecorder.ts` (TARGET_WIDTH / TARGET_HEIGHT). + private let maxWidth = 3840 + private let maxHeight = 2160 + + private func size(_ w: CGFloat, _ h: CGFloat, scale: CGFloat) -> (width: Int, height: Int) { + captureOutputSize( + contentSize: CGSize(width: w, height: h), + pointPixelScale: scale, + maxWidth: maxWidth, + maxHeight: maxHeight + ) + } + + // MARK: - The reported bug + + /// Issue #418, on the reporter's hardware: a 14" MacBook Pro built-in panel, whose capture + /// region is 1512×949 points at 2×. The buffer has to be the full 3024×1898 — but it has to + /// be that because the *filter* says so, not because a display API happened to agree. + /// + /// The regression this pins is the other direction: a buffer of 3024×1898 around content + /// ScreenCaptureKit rasterises at 1512×949 puts the whole desktop in one corner of a black + /// frame, which is what the reporter saw in the editor — and, being baked into the file, on + /// export too. + func testRetinaDisplayGetsTheFullPixelBuffer() { + let result = size(1512, 949, scale: 2) + XCTAssertEqual(result.width, 3024) + XCTAssertEqual(result.height, 1898) + } + + /// The same panel described in points but reported at 1× — the shape must survive, because + /// a buffer of the right shape is what keeps the frame from being letterboxed into it. + func testNonRetinaDisplayIsUnchanged() { + let result = size(1920, 1080, scale: 1) + XCTAssertEqual(result.width, 1920) + XCTAssertEqual(result.height, 1080) + } + + /// Aspect ratio is the property that decides whether a downscale fills the buffer or leaves + /// bars, so it is asserted directly rather than inferred from the two dimensions. + func testAspectRatioSurvivesEveryScale() { + for scale in [CGFloat(1), 2, 3] { + let result = size(1512, 949, scale: scale) + XCTAssertEqual( + Double(result.width) / Double(result.height), + 1512.0 / 949.0, + accuracy: 0.002, + "scale \(scale) changed the shape of the buffer" + ) + } + } + + // MARK: - The ceiling + + /// A 5K panel at 2× wants 10240×5416, far over the app's 4K ceiling. Capping is fine; capping + /// each axis on its own is not, because it hands the stream a buffer of a different shape + /// than the frame and the surplus comes back as black bars. + func testOversizedContentIsScaledDownProportionally() { + let result = size(2560, 1354, scale: 2) + XCTAssertLessThanOrEqual(result.width, maxWidth) + XCTAssertLessThanOrEqual(result.height, maxHeight) + XCTAssertEqual(result.width, 3840) + XCTAssertEqual( + Double(result.width) / Double(result.height), + 2560.0 / 1354.0, + accuracy: 0.002 + ) + } + + /// The reporter's second monitor: 5120×1440 at 1×. Only the WIDTH is over the ceiling, and + /// this is exactly the case a per-axis clamp gets wrong — it would answer 3840×1440, a 2.67 + /// buffer for a 3.56 frame, and ScreenCaptureKit would draw 3840×1080 with 180 black rows + /// above and below. + func testUltrawideIsNotSquashedByTheWidthCap() { + let result = size(5120, 1440, scale: 1) + XCTAssertEqual(result.width, 3840) + XCTAssertEqual(result.height, 1080) + } + + /// Under the ceiling on both axes, nothing is touched. + func testContentBelowTheCeilingIsNotScaled() { + let result = size(1280, 800, scale: 2) + XCTAssertEqual(result.width, 2560) + XCTAssertEqual(result.height, 1600) + } + + // MARK: - Encoder constraints and degenerate input + + /// H.264 chroma subsampling needs both dimensions even; the encoder rejects the frame + /// otherwise. An odd point size at 1× is the way to get there. + func testDimensionsAreAlwaysEven() { + let result = size(1023, 767, scale: 1) + XCTAssertEqual(result.width % 2, 0) + XCTAssertEqual(result.height % 2, 0) + XCTAssertEqual(result.width, 1022) + XCTAssertEqual(result.height, 766) + } + + /// A filter that reports nothing usable must still yield an encodable buffer rather than a + /// zero-sized one — the ceiling is the only value left that is known good. + func testDegenerateContentFallsBackToTheCeiling() { + for bad in [CGSize.zero, CGSize(width: CGFloat.nan, height: 1080), CGSize(width: -100, height: -100)] { + let result = captureOutputSize( + contentSize: bad, + pointPixelScale: 2, + maxWidth: maxWidth, + maxHeight: maxHeight + ) + XCTAssertEqual(result.width, maxWidth, "\(bad)") + XCTAssertEqual(result.height, maxHeight, "\(bad)") + } + } + + /// A scale of 0 or NaN means "the filter could not tell us", and 1× is the only reading that + /// cannot make the buffer bigger than the frame. + func testUnusableScaleFallsBackToOne() { + for scale in [CGFloat(0), -2, .nan] { + let result = size(1920, 1080, scale: scale) + XCTAssertEqual(result.width, 1920, "scale \(scale)") + XCTAssertEqual(result.height, 1080, "scale \(scale)") + } + } +} diff --git a/technical-documentation/testing/manual-e2e-checklist.md b/technical-documentation/testing/manual-e2e-checklist.md index c7af9d121..a54ca5665 100644 --- a/technical-documentation/testing/manual-e2e-checklist.md +++ b/technical-documentation/testing/manual-e2e-checklist.md @@ -416,6 +416,8 @@ The agent may only call the fixed tool set in [ai-agent.md](../architecture/ai-a - [ ] Confirm local transcription reports the device it actually ran on and completes on a Metal-capable machine. - [ ] Confirm the packaged `.app` contains the compositor addon and that the addon carries no build-machine path. - [ ] Confirm the packaged `.app` bundles its ffmpeg libraries and runs on a machine with no developer toolchain installed. +- [ ] **On a Retina/HiDPI display**, record the screen and confirm the recorded frame is filled edge to edge — not the desktop drawn small in one corner of a black rectangle. Then do the same for a single window. Issue #418 shipped exactly this, invisible on every 1× display because a point size and a pixel size are the same number there; `SCStreamConfiguration` does not scale a frame up to fill an oversized buffer, so the surplus stays background black. Check the frame, not just the file's dimensions — the reporter's `.mp4` was 3024×1898 as expected and still wrong inside. +- [ ] **With a second display attached at a different scale factor**, record each display in turn and confirm both fill their frame. A machine whose displays all share one scale factor cannot catch a units mix-up. ### Linux