From d7753f7401d8489b2e9806cdaf40aadc72f7c4bf Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 19 Jul 2026 07:22:18 +0200 Subject: [PATCH] fix(playback): keep CSS bounds unrounded until native scaling Review feedback on #1206: measureBounds() rounded the CSS edges in the renderer, before the main-process CSS-to-native conversion, so fractional layout positions could drift by a pixel per scale factor (a 10.49px edge at 200% must land on 21 physical px, not 20). The renderer now sends raw getBoundingClientRect() edges and rounding happens exactly once, after scaling. Also pins process.platform explicitly in the macOS wiring test instead of relying on the suite default. Co-Authored-By: Claude Fable 5 --- .../app/services/embedded-mpv-bounds.util.spec.ts | 11 +++++++++++ .../src/app/services/embedded-mpv-bounds.util.ts | 7 +++++-- .../services/embedded-mpv-native.service.spec.ts | 1 + docs/architecture/embedded-mpv-native.md | 8 +++++--- .../embedded-mpv-format.utils.spec.ts | 9 ++++++--- .../embedded-mpv-format.utils.ts | 15 +++++++++++---- .../embedded-mpv-session-controller.spec.ts | 4 +++- 7 files changed, 42 insertions(+), 13 deletions(-) diff --git a/apps/electron-backend/src/app/services/embedded-mpv-bounds.util.spec.ts b/apps/electron-backend/src/app/services/embedded-mpv-bounds.util.spec.ts index 1996b1567..03d3ee95a 100644 --- a/apps/electron-backend/src/app/services/embedded-mpv-bounds.util.spec.ts +++ b/apps/electron-backend/src/app/services/embedded-mpv-bounds.util.spec.ts @@ -80,6 +80,17 @@ describe('toNativeViewBounds', () => { expect(result).toEqual({ x: 150, y: 75, width: 960, height: 540 }); }); + it('rounds fractional CSS edges only after scaling', () => { + // A 10.49px CSS edge at 200% renders at 21 physical pixels; edges + // rounded before scaling would send 20 and shift the video by 1px. + const result = toNativeViewBounds( + { x: 10.49, y: 0.5, width: 100.02, height: 50 }, + context({ platform: 'win32', displayScaleFactor: 2 }) + ); + + expect(result).toEqual({ x: 21, y: 1, width: 200, height: 100 }); + }); + it('keeps vertically adjacent rects seamless under fractional scales', () => { // 42 × 1.25 and 153 × 1.25 both land on .5/.25 fractions: rounding // x/y/width/height independently would misplace the shared edge by diff --git a/apps/electron-backend/src/app/services/embedded-mpv-bounds.util.ts b/apps/electron-backend/src/app/services/embedded-mpv-bounds.util.ts index 1c539bf70..7c7015dd1 100644 --- a/apps/electron-backend/src/app/services/embedded-mpv-bounds.util.ts +++ b/apps/electron-backend/src/app/services/embedded-mpv-bounds.util.ts @@ -19,8 +19,11 @@ export interface NativeViewBoundsContext { * every platform scales by the zoom factor and win32/linux additionally by * the display scale factor (#1145). * - * Edges are scaled before deriving width/height so rounding cannot open - * 1px seams between the native video window and the surrounding DOM UI. + * Bounds arrive with unrounded CSS edges and are rounded exactly once here, + * after scaling: edges first, then width/height derived from them. Rounding + * any earlier (or per-field) lets fractional CSS layouts drift by a pixel + * per scale factor and open 1px seams between the native video window and + * the surrounding DOM UI. */ export function toNativeViewBounds( bounds: EmbeddedMpvBounds, diff --git a/apps/electron-backend/src/app/services/embedded-mpv-native.service.spec.ts b/apps/electron-backend/src/app/services/embedded-mpv-native.service.spec.ts index 2b89e1888..0568a9f0c 100644 --- a/apps/electron-backend/src/app/services/embedded-mpv-native.service.spec.ts +++ b/apps/electron-backend/src/app/services/embedded-mpv-native.service.spec.ts @@ -487,6 +487,7 @@ describe('EmbeddedMpvNativeService power blocker', () => { it('applies page zoom but not the display scale on macOS', () => { // NSView frames are in points (device-independent pixels): only // the webContents zoom factor separates them from CSS pixels. + Object.defineProperty(process, 'platform', { value: 'darwin' }); screenGetDisplayMatchingMock.mockReturnValue({ scaleFactor: 2 }); mainWindowGetZoomFactorMock.mockReturnValue(1.25); addon.createSession.mockReturnValueOnce('s-zoom'); diff --git a/docs/architecture/embedded-mpv-native.md b/docs/architecture/embedded-mpv-native.md index 2edaa58e7..458368467 100644 --- a/docs/architecture/embedded-mpv-native.md +++ b/docs/architecture/embedded-mpv-native.md @@ -607,9 +607,11 @@ and match physical pixels only at 100% page zoom AND 100% display scale. `EmbeddedMpvNativeService` therefore converts every native-view bounds payload in the main process (`toNativeViewBounds` in `embedded-mpv-bounds.util.ts`): all platforms scale by the webContents zoom factor, win32/linux additionally -by the scale factor of the display hosting the window. Edges are scaled -before deriving width/height so rounding cannot open 1px seams against the -surrounding DOM UI. Skipping this conversion is issue #1145: on scaled +by the scale factor of the display hosting the window. The renderer sends +unrounded CSS edges (`measureBounds` does not round) and the conversion +rounds exactly once, after scaling — edges first, width/height derived from +them — so fractional CSS layouts and fractional scales cannot open 1px +seams against the surrounding DOM UI. Skipping this conversion is issue #1145: on scaled displays (Windows 125%, Linux fractional scaling, HiDPI TVs) the video landed toward the window's top-left corner at `1/scale` of its size, in windowed and fullscreen mode alike. diff --git a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-format.utils.spec.ts b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-format.utils.spec.ts index 9b80e2dfb..0c250d405 100644 --- a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-format.utils.spec.ts +++ b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-format.utils.spec.ts @@ -59,7 +59,10 @@ describe('embedded MPV format utilities', () => { expect(volumeLabel(0.755)).toBe('Volume 76%'); }); - it('rounds host bounds and keeps minimum native view dimensions', () => { + it('preserves fractional host edges and keeps minimum native view dimensions', () => { + // Rounding happens once in the main process, after CSS→native + // scaling — pre-rounded edges would drift by up to 1px per scale + // factor on scaled displays. const host = { getBoundingClientRect: () => ({ left: 10.4, @@ -70,8 +73,8 @@ describe('embedded MPV format utilities', () => { } as HTMLElement; expect(measureBounds(host)).toEqual({ - x: 10, - y: 21, + x: 10.4, + y: 20.6, width: 1, height: 1, }); diff --git a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-format.utils.ts b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-format.utils.ts index 6678c291b..2e8635b6d 100644 --- a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-format.utils.ts +++ b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-format.utils.ts @@ -120,12 +120,19 @@ export function persistVolume(value: number): void { localStorage.setItem('volume', String(value)); } +/** + * Measures the host element in CSS pixels without rounding. The main process + * converts these bounds to native units (page zoom × display scale) and + * rounds exactly once, after scaling — pre-rounding here would bake up to + * ±0.5px of CSS error that the scale factor then amplifies into visible + * off-by-one seams (e.g. a 10.49px edge at 200% renders at 21px, not 20px). + */ export function measureBounds(host: HTMLElement): EmbeddedMpvBounds { const rect = host.getBoundingClientRect(); return { - x: Math.round(rect.left), - y: Math.round(rect.top), - width: Math.max(1, Math.round(rect.width)), - height: Math.max(1, Math.round(rect.height)), + x: rect.left, + y: rect.top, + width: Math.max(1, rect.width), + height: Math.max(1, rect.height), }; } diff --git a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.spec.ts b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.spec.ts index d2a60a690..b70512184 100644 --- a/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.spec.ts +++ b/libs/ui/playback/src/lib/embedded-mpv-player/embedded-mpv-session-controller.spec.ts @@ -141,8 +141,10 @@ describe('EmbeddedMpvSessionController', () => { ); expect(electron.prepareEmbeddedMpv).toHaveBeenCalled(); + // Fractional CSS edges stay unrounded: the main process rounds once, + // after converting them to native units. expect(electron.createEmbeddedMpvSession).toHaveBeenCalledWith( - { x: 11, y: 21, width: 640, height: 360 }, + { x: 10.6, y: 20.5, width: 640, height: 360 }, 'Example Movie', 0.7 );