From 00504a43d5e76ec508ae2a9fa0c165c7365daa0b Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 22 Aug 2026 00:36:15 +0000 Subject: [PATCH 01/30] chore(release): bump to 1.10.0-rc.1 [skip ci] --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index e0ea6e09c..a6f2c7f4d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "openscreen", - "version": "1.9.6", + "version": "1.10.0-rc.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "openscreen", - "version": "1.9.6", + "version": "1.10.0-rc.1", "dependencies": { "@fix-webm-duration/fix": "^1.0.1", "@langchain/anthropic": "^1.3.26", diff --git a/package.json b/package.json index 412f6dfb2..a1ba6b4b8 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "openscreen", "private": true, - "version": "1.9.6", + "version": "1.10.0-rc.1", "description": "Record your screen and polish the demo", "homepage": "https://getopenscreen.com/", "license": "MIT", From b67d70234549ef71e50163a4971389e7bcfa1a96 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sat, 22 Aug 2026 11:45:26 +0200 Subject: [PATCH 02/30] fix(editor): make the gradient picker and a couple of dialogs follow the light theme MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two contrast bugs reported directly against the gradient picker: the "remove color" icon read as invisible until hover, and the brightness slider's thumb disappeared into the popover background in light mode. Both buttons/icons in gradient-editor.tsx were colored for the dark color-wheel canvas above them, but the brightness-slider row has no canvas of its own — it sits directly on the (theme-dependent) popover surface, where the near-white thumb (`#f5f5f5`) all but vanished against the light theme's near-white surface. Gave that row its own dark backing, consistent with the canvas and angle-knob beside it (deliberately dark-by-design, like most pro color pickers), and bumped the icon buttons from 60% to 80% white so they don't need hover to read. A light-theme sweep for the same class of bug found two more hardcoded surfaces: ShortcutsConfigDialog.tsx (`bg-[#09090b] text-white`, so the whole dialog stayed dark regardless of theme) and App.tsx's editor Suspense fallback (same dark hex, flashes on every editor load before the real light UI paints). Both now use the design-tokens.css custom properties already used everywhere else in the app. While auditing accent-colored surfaces for the same sweep, found a separate but related bug: a few primary buttons (`.exportBtn`, `.bigRecBtn`, `.previewEmptyPrimaryButton`, and VirtualPreview's icon-button hover state) put white/near-white text directly on the mint `--accent`/`--brand` background instead of `--accent-on` — the token design-tokens.css defines specifically for text on that background. White-on-mint measures under WCAG's contrast minimums in both themes; switched all four to `--accent-on`, matching the pattern already used correctly everywhere else (NewEditorShell.module.css's `.paneTabs button.isActive`, `.btnPrimary`, Modals.tsx, ExportDialog.tsx). Finally, the default gradient presets: 16 swatches built from four grays plus the single mint accent, so half the grid paired a gray with the same green, reading as "we only have one color." Recolored with the same hue spread already offered in the solid-color tab just below it (blue, purple, pink, orange, green) with a couple of mint blends so the brand color still shows up without dominating. Co-Authored-By: Claude Sonnet 5 --- src/App.tsx | 6 +-- .../ai-edition/NewEditorShell.module.css | 2 +- src/components/ai-edition/RightPanes.tsx | 29 +++++++------- .../ai-edition/VirtualPreview.module.css | 1 + .../ai-edition/v4/EditorShellV4.module.css | 4 +- src/components/ui/gradient-editor.tsx | 17 +++++---- .../video-editor/ShortcutsConfigDialog.tsx | 38 +++++++++---------- 7 files changed, 52 insertions(+), 45 deletions(-) diff --git a/src/App.tsx b/src/App.tsx index 517ad7abd..aa7619d26 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -116,9 +116,9 @@ export default function App() { +
- {tEditor("loadingEditor")} + {tEditor("loadingEditor")}
} > diff --git a/src/components/ai-edition/NewEditorShell.module.css b/src/components/ai-edition/NewEditorShell.module.css index 155b665eb..c842a535a 100644 --- a/src/components/ai-edition/NewEditorShell.module.css +++ b/src/components/ai-edition/NewEditorShell.module.css @@ -594,7 +594,7 @@ padding: var(--sp-3) var(--sp-4); border-radius: var(--r-sm); background: var(--primary, #34B27B); - color: #fff; + color: var(--accent-on); border: none; font-weight: 500; font-size: 0.875rem; diff --git a/src/components/ai-edition/RightPanes.tsx b/src/components/ai-edition/RightPanes.tsx index 5f76e355d..d12fe248a 100644 --- a/src/components/ai-edition/RightPanes.tsx +++ b/src/components/ai-edition/RightPanes.tsx @@ -143,22 +143,25 @@ function Pane({ title, icon, helpText, children }: PaneProps) { // keep the gradient palette small and curated — every block renders // in the picker and gets serialized to legacyEditor on save. +// Spans the same hues as COLOR_PALETTE below rather than leaning on the +// brand mint for half the grid — a wall of green reads as "we only +// have one color" rather than "pick a gradient." const GRAD_PRESETS: readonly string[] = [ "linear-gradient(135deg, #eaebed, #bcc0c6)", - "linear-gradient(135deg, #10b981, #eaebed)", - "linear-gradient(135deg, #6b7280, #bcc0c6)", - "linear-gradient(135deg, #eaebed, #10b981)", - "linear-gradient(135deg, #16171d, #6b7280)", - "linear-gradient(135deg, #bcc0c6, #16171d)", - "linear-gradient(135deg, #10b981, #6b7280)", - "linear-gradient(135deg, #eaebed, #10b981)", + "linear-gradient(135deg, #3b82f6, #8b5cf6)", + "linear-gradient(135deg, #8b5cf6, #ec4899)", + "linear-gradient(135deg, #f97316, #ec4899)", + "linear-gradient(135deg, #f59e0b, #f97316)", + "linear-gradient(135deg, #10b981, #3b82f6)", + "linear-gradient(135deg, #22c55e, #10b981)", "linear-gradient(135deg, #6b7280, #16171d)", - "linear-gradient(135deg, #bcc0c6, #10b981)", - "linear-gradient(135deg, #16171d, #6b7280)", - "linear-gradient(135deg, #eaebed, #bcc0c6)", - "linear-gradient(135deg, #10b981, #bcc0c6)", - "linear-gradient(135deg, #eaebed, #16171d)", - "linear-gradient(135deg, #6b7280, #10b981)", + "linear-gradient(135deg, #ec4899, #ef4444)", + "linear-gradient(135deg, #3b82f6, #22c55e)", + "linear-gradient(135deg, #8b5cf6, #3b82f6)", + "linear-gradient(135deg, #f59e0b, #ef4444)", + "linear-gradient(135deg, #16171d, #1e293b)", + "linear-gradient(135deg, #34d399, #3b82f6)", + "linear-gradient(135deg, #ef4444, #8b5cf6)", "linear-gradient(135deg, #bcc0c6, #eaebed)", ]; diff --git a/src/components/ai-edition/VirtualPreview.module.css b/src/components/ai-edition/VirtualPreview.module.css index 78d84a06f..6177a9f52 100644 --- a/src/components/ai-edition/VirtualPreview.module.css +++ b/src/components/ai-edition/VirtualPreview.module.css @@ -60,6 +60,7 @@ .iconButton:hover:not(:disabled) { background: var(--accent); + color: var(--accent-on); } .iconButton:disabled { diff --git a/src/components/ai-edition/v4/EditorShellV4.module.css b/src/components/ai-edition/v4/EditorShellV4.module.css index 342cedd4c..6d47a11aa 100644 --- a/src/components/ai-edition/v4/EditorShellV4.module.css +++ b/src/components/ai-edition/v4/EditorShellV4.module.css @@ -389,7 +389,7 @@ padding: 0 14px; border-radius: 9px; background: var(--accent); - color: #fff; + color: var(--accent-on); border: 1px solid var(--accent); font-size: 13px; font-weight: 600; @@ -1126,7 +1126,7 @@ padding: 0 26px; border-radius: 16px; border: 1px solid var(--accent); - color: #fff; + color: var(--accent-on); background: var(--accent); font-size: 14px; font-weight: 600; diff --git a/src/components/ui/gradient-editor.tsx b/src/components/ui/gradient-editor.tsx index a29e1b923..c9e2525dc 100644 --- a/src/components/ui/gradient-editor.tsx +++ b/src/components/ui/gradient-editor.tsx @@ -462,7 +462,7 @@ export default function GradientEditor({ onChange }: GradientEditorProps) { aria-valuemin={0} aria-valuemax={360} aria-valuenow={Math.round(((mainAngle % 360) + 360) % 360)} - className="absolute w-11 h-11 rounded-full cursor-grab shadow-[0_0_0_4px_#f5f5f5,0_4px_14px_rgba(0,0,0,0.35)] transition-shadow z-10 active:cursor-grabbing focus:outline-none focus:ring-2 focus:ring-[#34B27B]" + className="absolute w-11 h-11 rounded-full cursor-grab shadow-[0_0_0_4px_#f5f5f5,0_4px_14px_rgba(0,0,0,0.35)] transition-shadow z-10 active:cursor-grabbing focus:outline-none focus:ring-2 focus:ring-[var(--brand)]" style={{ left: `${mainX}%`, top: `${mainY}%`, @@ -478,7 +478,7 @@ export default function GradientEditor({ onChange }: GradientEditorProps) { type="button" onClick={addPoint} disabled={totalColors >= MAX_COLORS} - className={`w-6 h-6 rounded-full flex items-center justify-center border-none bg-transparent transition-all text-white/60 hover:text-white ${ + className={`w-6 h-6 rounded-full flex items-center justify-center border-none bg-transparent transition-all text-white/80 hover:text-white ${ totalColors >= MAX_COLORS ? "opacity-40 cursor-not-allowed" : "hover:bg-white/10 cursor-pointer" @@ -492,7 +492,7 @@ export default function GradientEditor({ onChange }: GradientEditorProps) { type="button" onClick={removePoint} disabled={totalColors <= 1} - className={`w-6 h-6 rounded-full flex items-center justify-center border-none bg-transparent transition-all text-white/60 hover:text-white ${ + className={`w-6 h-6 rounded-full flex items-center justify-center border-none bg-transparent transition-all text-white/80 hover:text-white ${ totalColors <= 1 ? "opacity-40 cursor-not-allowed" : "hover:bg-white/10 cursor-pointer" @@ -506,7 +506,7 @@ export default function GradientEditor({ onChange }: GradientEditorProps) { type="button" onClick={cycleHarmony} disabled={eligibleHarmonies.length <= 1} - className={`w-6 h-6 rounded-full flex items-center justify-center border-none bg-transparent transition-all text-white/60 hover:text-white ${ + className={`w-6 h-6 rounded-full flex items-center justify-center border-none bg-transparent transition-all text-white/80 hover:text-white ${ eligibleHarmonies.length <= 1 ? "opacity-40 cursor-not-allowed" : "hover:bg-white/10 cursor-pointer" @@ -520,7 +520,10 @@ export default function GradientEditor({ onChange }: GradientEditorProps) { {/* Bottom controls: Brightness + Angle */}
- {/* Brightness wavy slider */} + {/* Brightness wavy slider. Its thumb/track are colored for the dark + canvas above, so it needs its own dark backing here too — this row + otherwise sits directly on the (theme-dependent) popover background, + where the light thumb disappears in light mode. */}
diff --git a/src/components/video-editor/ShortcutsConfigDialog.tsx b/src/components/video-editor/ShortcutsConfigDialog.tsx index add21681b..118e6e716 100644 --- a/src/components/video-editor/ShortcutsConfigDialog.tsx +++ b/src/components/video-editor/ShortcutsConfigDialog.tsx @@ -138,17 +138,17 @@ export function ShortcutsConfigDialog() { if (!open) handleClose(); }} > - + - + {t("title")}
-

+

{t("configurable")}

{SHORTCUT_ACTIONS.map((action) => { @@ -156,8 +156,8 @@ export function ShortcutsConfigDialog() { const hasConflict = conflict?.forAction === action; return (
-
- {t(`actions.${action}`)} +
+ {t(`actions.${action}`)}
{hasConflict && conflict?.conflictWith.type === "configurable" && ( -
- +
+ ⚠{" "} {t("alreadyUsedBy", { action: t(`actions.${conflict.conflictWith.action}`), @@ -189,14 +189,14 @@ export function ShortcutsConfigDialog() { @@ -209,32 +209,32 @@ export function ShortcutsConfigDialog() {
-

+

{t("fixed")}

{FIXED_SHORTCUTS.map(({ i18nKey, label, display }) => (
- + {t(`fixedActions.${i18nKey}`, { defaultValue: label })} - + {display}
))}
-

{t("helpText")}

+

{t("helpText")}

{hasConflict && conflict?.conflictWith.type === "configurable" && ( -
+
⚠{" "} {t("alreadyUsedBy", { @@ -189,7 +189,7 @@ export function ShortcutsConfigDialog() { From cfcb63b406f1aab472e69c9afd81b45c5c877dce Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sat, 22 Aug 2026 11:27:50 +0200 Subject: [PATCH 04/30] fix(editor): close the clip picker on an outside click It previously only closed by picking a clip or re-clicking the trigger, matching the mousedown-outside pattern EditorTopBar's LangButton already uses. Covers it with an e2e test. --- .../ai-edition/v4/FloatingInspector.tsx | 15 +++- tests/e2e/v4-shell.spec.ts | 73 ++++++++++++++++--- 2 files changed, 76 insertions(+), 12 deletions(-) diff --git a/src/components/ai-edition/v4/FloatingInspector.tsx b/src/components/ai-edition/v4/FloatingInspector.tsx index 2d3651a15..ac162d322 100644 --- a/src/components/ai-edition/v4/FloatingInspector.tsx +++ b/src/components/ai-edition/v4/FloatingInspector.tsx @@ -14,7 +14,7 @@ import { ZoomIn, } from "lucide-react"; import type { ComponentProps } from "react"; -import { useMemo, useRef, useState } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; import { toast } from "sonner"; import { parseCustomPlaybackSpeedInput } from "@/components/video-editor/customPlaybackSpeed"; import { @@ -101,6 +101,17 @@ export function FloatingInspector({ const ts = useScopedT("settings"); const te = useScopedT("editor"); const [clipPickerOpen, setClipPickerOpen] = useState(false); + const clipPickerRef = useRef(null); + useEffect(() => { + if (!clipPickerOpen) return; + const onDocMouseDown = (e: MouseEvent) => { + if (clipPickerRef.current && !clipPickerRef.current.contains(e.target as Node)) { + setClipPickerOpen(false); + } + }; + document.addEventListener("mousedown", onDocMouseDown); + return () => document.removeEventListener("mousedown", onDocMouseDown); + }, [clipPickerOpen]); const selection = tl.selection; const effectiveOpen = open || selection !== null; return ( @@ -136,7 +147,7 @@ export function FloatingInspector({ ))} -
+
- - ))} + {options.map((option) => { + const Icon = option.icon; + return ( + + ); + })}
); } diff --git a/src/i18n/locales/ar/settings.json b/src/i18n/locales/ar/settings.json index b0beb268d..c19b80bce 100644 --- a/src/i18n/locales/ar/settings.json +++ b/src/i18n/locales/ar/settings.json @@ -302,11 +302,15 @@ "positionTop": "أعلى", "positionMiddle": "الوسط", "positionBottom": "أسفل", + "positionLeft": "الموضع الأيسر", + "positionCenter": "الموضع الأوسط", + "positionRight": "الموضع الأيمن", + "textAlign": "محاذاة النص", "alignLeft": "يسار", "alignCenter": "توسيط", "alignRight": "يمين", - "verticalOffset": "الإزاحة الرأسية", - "horizontalOffset": "الإزاحة الأفقية", + "verticalOffset": "الموضع الرأسي", + "horizontalOffset": "الموضع الأفقي", "width": "العرض", "lineLength": "طول السطر", "minWords": "أقل عدد كلمات في السطر", diff --git a/src/i18n/locales/en/settings.json b/src/i18n/locales/en/settings.json index 071fd4850..25430a23e 100644 --- a/src/i18n/locales/en/settings.json +++ b/src/i18n/locales/en/settings.json @@ -308,11 +308,15 @@ "positionTop": "Top", "positionMiddle": "Middle", "positionBottom": "Bottom", + "positionLeft": "Position left", + "positionCenter": "Position center", + "positionRight": "Position right", + "textAlign": "Text align", "alignLeft": "Left", "alignCenter": "Center", "alignRight": "Right", - "verticalOffset": "Vertical offset", - "horizontalOffset": "Horizontal offset", + "verticalOffset": "Vertical position", + "horizontalOffset": "Horizontal position", "width": "Width", "lineLength": "Line length", "minWords": "Min words per line", diff --git a/src/i18n/locales/es/settings.json b/src/i18n/locales/es/settings.json index 099928b5d..93ac340d9 100644 --- a/src/i18n/locales/es/settings.json +++ b/src/i18n/locales/es/settings.json @@ -302,11 +302,15 @@ "positionTop": "Arriba", "positionMiddle": "Centro", "positionBottom": "Abajo", + "positionLeft": "Posición izquierda", + "positionCenter": "Posición central", + "positionRight": "Posición derecha", + "textAlign": "Alineación del texto", "alignLeft": "Izquierda", "alignCenter": "Centro", "alignRight": "Derecha", - "verticalOffset": "Desplazamiento vertical", - "horizontalOffset": "Desplazamiento horizontal", + "verticalOffset": "Posición vertical", + "horizontalOffset": "Posición horizontal", "width": "Ancho", "lineLength": "Longitud de línea", "minWords": "Mín. palabras por línea", diff --git a/src/i18n/locales/fr/settings.json b/src/i18n/locales/fr/settings.json index 1b7f5fa23..6a8ed73d3 100644 --- a/src/i18n/locales/fr/settings.json +++ b/src/i18n/locales/fr/settings.json @@ -302,11 +302,15 @@ "positionTop": "Haut", "positionMiddle": "Milieu", "positionBottom": "Bas", + "positionLeft": "Position à gauche", + "positionCenter": "Position au centre", + "positionRight": "Position à droite", + "textAlign": "Alignement du texte", "alignLeft": "Gauche", "alignCenter": "Centre", "alignRight": "Droite", - "verticalOffset": "Décalage vertical", - "horizontalOffset": "Décalage horizontal", + "verticalOffset": "Position verticale", + "horizontalOffset": "Position horizontale", "width": "Largeur", "lineLength": "Longueur des lignes", "minWords": "Mots min. par ligne", diff --git a/src/i18n/locales/it/settings.json b/src/i18n/locales/it/settings.json index 587aae961..37a585b2d 100644 --- a/src/i18n/locales/it/settings.json +++ b/src/i18n/locales/it/settings.json @@ -302,11 +302,15 @@ "positionTop": "Alto", "positionMiddle": "Centro", "positionBottom": "Basso", + "positionLeft": "Posizione a sinistra", + "positionCenter": "Posizione centrale", + "positionRight": "Posizione a destra", + "textAlign": "Allineamento testo", "alignLeft": "Sinistra", "alignCenter": "Centro", "alignRight": "Destra", - "verticalOffset": "Scostamento verticale", - "horizontalOffset": "Scostamento orizzontale", + "verticalOffset": "Posizione verticale", + "horizontalOffset": "Posizione orizzontale", "width": "Larghezza", "lineLength": "Lunghezza riga", "minWords": "Parole min. per riga", diff --git a/src/i18n/locales/ja-JP/settings.json b/src/i18n/locales/ja-JP/settings.json index f856cde46..02f27575e 100644 --- a/src/i18n/locales/ja-JP/settings.json +++ b/src/i18n/locales/ja-JP/settings.json @@ -302,11 +302,15 @@ "positionTop": "上", "positionMiddle": "中央", "positionBottom": "下", + "positionLeft": "左配置", + "positionCenter": "中央配置", + "positionRight": "右配置", + "textAlign": "文字揃え", "alignLeft": "左", "alignCenter": "中央", "alignRight": "右", - "verticalOffset": "垂直オフセット", - "horizontalOffset": "水平オフセット", + "verticalOffset": "垂直位置", + "horizontalOffset": "水平位置", "width": "幅", "lineLength": "行の長さ", "minWords": "1 行の最小単語数", diff --git a/src/i18n/locales/ko-KR/settings.json b/src/i18n/locales/ko-KR/settings.json index ee083c9b4..69e4b35e1 100644 --- a/src/i18n/locales/ko-KR/settings.json +++ b/src/i18n/locales/ko-KR/settings.json @@ -302,11 +302,15 @@ "positionTop": "위", "positionMiddle": "가운데", "positionBottom": "아래", + "positionLeft": "왼쪽 배치", + "positionCenter": "가운데 배치", + "positionRight": "오른쪽 배치", + "textAlign": "텍스트 정렬", "alignLeft": "왼쪽", "alignCenter": "가운데", "alignRight": "오른쪽", - "verticalOffset": "세로 오프셋", - "horizontalOffset": "가로 오프셋", + "verticalOffset": "세로 위치", + "horizontalOffset": "가로 위치", "width": "너비", "lineLength": "줄 길이", "minWords": "줄당 최소 단어 수", diff --git a/src/i18n/locales/pt-BR/settings.json b/src/i18n/locales/pt-BR/settings.json index 6512686ba..acbd178b5 100644 --- a/src/i18n/locales/pt-BR/settings.json +++ b/src/i18n/locales/pt-BR/settings.json @@ -302,11 +302,15 @@ "positionTop": "Topo", "positionMiddle": "Meio", "positionBottom": "Base", + "positionLeft": "Posição à esquerda", + "positionCenter": "Posição central", + "positionRight": "Posição à direita", + "textAlign": "Alinhamento do texto", "alignLeft": "Esquerda", "alignCenter": "Centro", "alignRight": "Direita", - "verticalOffset": "Deslocamento vertical", - "horizontalOffset": "Deslocamento horizontal", + "verticalOffset": "Posição vertical", + "horizontalOffset": "Posição horizontal", "width": "Largura", "lineLength": "Comprimento da linha", "minWords": "Mín. de palavras por linha", diff --git a/src/i18n/locales/ru/settings.json b/src/i18n/locales/ru/settings.json index dcd82de34..b070f0b95 100644 --- a/src/i18n/locales/ru/settings.json +++ b/src/i18n/locales/ru/settings.json @@ -302,11 +302,15 @@ "positionTop": "Сверху", "positionMiddle": "По центру", "positionBottom": "Снизу", + "positionLeft": "Положение слева", + "positionCenter": "Положение по центру", + "positionRight": "Положение справа", + "textAlign": "Выравнивание текста", "alignLeft": "Слева", "alignCenter": "По центру", "alignRight": "Справа", - "verticalOffset": "Смещение по вертикали", - "horizontalOffset": "Смещение по горизонтали", + "verticalOffset": "Положение по вертикали", + "horizontalOffset": "Положение по горизонтали", "width": "Ширина", "lineLength": "Длина строки", "minWords": "Мин. слов в строке", diff --git a/src/i18n/locales/tr/settings.json b/src/i18n/locales/tr/settings.json index b3ccf10db..c5f1791e1 100644 --- a/src/i18n/locales/tr/settings.json +++ b/src/i18n/locales/tr/settings.json @@ -302,11 +302,15 @@ "positionTop": "Üst", "positionMiddle": "Orta", "positionBottom": "Alt", + "positionLeft": "Sol konum", + "positionCenter": "Orta konum", + "positionRight": "Sağ konum", + "textAlign": "Metin hizalama", "alignLeft": "Sol", "alignCenter": "Orta", "alignRight": "Sağ", - "verticalOffset": "Dikey ofset", - "horizontalOffset": "Yatay ofset", + "verticalOffset": "Dikey konum", + "horizontalOffset": "Yatay konum", "width": "Genişlik", "lineLength": "Satır uzunluğu", "minWords": "Satır başına en az kelime", diff --git a/src/i18n/locales/vi/settings.json b/src/i18n/locales/vi/settings.json index 2274559b5..b87203235 100644 --- a/src/i18n/locales/vi/settings.json +++ b/src/i18n/locales/vi/settings.json @@ -302,11 +302,15 @@ "positionTop": "Trên", "positionMiddle": "Giữa", "positionBottom": "Dưới", + "positionLeft": "Vị trí trái", + "positionCenter": "Vị trí giữa", + "positionRight": "Vị trí phải", + "textAlign": "Căn chỉnh văn bản", "alignLeft": "Trái", "alignCenter": "Giữa", "alignRight": "Phải", - "verticalOffset": "Độ lệch dọc", - "horizontalOffset": "Độ lệch ngang", + "verticalOffset": "Vị trí dọc", + "horizontalOffset": "Vị trí ngang", "width": "Chiều rộng", "lineLength": "Độ dài dòng", "minWords": "Số từ tối thiểu mỗi dòng", diff --git a/src/i18n/locales/zh-CN/settings.json b/src/i18n/locales/zh-CN/settings.json index cd2856f78..7ab2ceef9 100644 --- a/src/i18n/locales/zh-CN/settings.json +++ b/src/i18n/locales/zh-CN/settings.json @@ -302,11 +302,15 @@ "positionTop": "顶部", "positionMiddle": "中间", "positionBottom": "底部", + "positionLeft": "左侧位置", + "positionCenter": "居中位置", + "positionRight": "右侧位置", + "textAlign": "文字对齐", "alignLeft": "左对齐", "alignCenter": "居中", "alignRight": "右对齐", - "verticalOffset": "垂直偏移", - "horizontalOffset": "水平偏移", + "verticalOffset": "垂直位置", + "horizontalOffset": "水平位置", "width": "宽度", "lineLength": "行长", "minWords": "每行最少词数", diff --git a/src/i18n/locales/zh-TW/settings.json b/src/i18n/locales/zh-TW/settings.json index b8606a2e7..147d71233 100644 --- a/src/i18n/locales/zh-TW/settings.json +++ b/src/i18n/locales/zh-TW/settings.json @@ -303,11 +303,15 @@ "positionTop": "上", "positionMiddle": "中", "positionBottom": "下", + "positionLeft": "靠左位置", + "positionCenter": "置中位置", + "positionRight": "靠右位置", + "textAlign": "文字對齊", "alignLeft": "靠左", "alignCenter": "置中", "alignRight": "靠右", - "verticalOffset": "垂直位移", - "horizontalOffset": "水平位移", + "verticalOffset": "垂直位置", + "horizontalOffset": "水平位置", "width": "寬度", "lineLength": "行長", "minWords": "每行最少字數", diff --git a/src/lib/ai-edition/captions/captions.test.ts b/src/lib/ai-edition/captions/captions.test.ts index 73fe2a180..a4d12f257 100644 --- a/src/lib/ai-edition/captions/captions.test.ts +++ b/src/lib/ai-edition/captions/captions.test.ts @@ -3,9 +3,12 @@ import type { AxcutDocument, AxcutTranscript } from "../schema"; import { captionCuesToTextRegions, deriveCaptionCues } from "./cues"; import type { CaptionSettings, CaptionSettingsPatch } from "./settings"; import { + activeHorizontalPositionPreset, + activeVerticalPositionPreset, CAPTION_BAND_HEIGHT_PCT, captionBackgroundCss, captionBandRect, + captionHorizontalPositionOffset, captionInkHeightPct, captionOffsetRange, DEFAULT_CAPTION_SETTINGS, @@ -251,6 +254,67 @@ describe("caption settings", () => { }); }); +describe("caption position presets", () => { + it("reads a vertical preset as active only while there's no nudge off it", () => { + for (const verticalPosition of ["top", "middle", "bottom"] as const) { + const settings = { ...ON, verticalPosition, offsetY: 0 }; + expect(activeVerticalPositionPreset(settings)).toBe(verticalPosition); + // Any nudge at all — even one too small to see — means the band is no + // longer exactly at the preset, so nothing should read as "active". + expect(activeVerticalPositionPreset({ ...settings, offsetY: 5 })).toBeNull(); + } + }); + + it("reads left/center/right off offsetX, and null off the preset grid", () => { + expect(activeHorizontalPositionPreset(ON)).toBe("center"); + const range = captionOffsetRange(ON); + expect(activeHorizontalPositionPreset({ ...ON, offsetX: range.x.min })).toBe("left"); + expect(activeHorizontalPositionPreset({ ...ON, offsetX: range.x.max })).toBe("right"); + expect(activeHorizontalPositionPreset({ ...ON, offsetX: range.x.min / 2 })).toBeNull(); + }); + + it("collapses to center when the band is full-width, since left/right have nowhere to go", () => { + expect(activeHorizontalPositionPreset({ ...ON, width: 100, offsetX: 0 })).toBe("center"); + }); + + it("sets offsetX to the true frame edge for left/right, matching the reachable range", () => { + const range = captionOffsetRange(ON); + expect(captionHorizontalPositionOffset(ON, "left")).toBeCloseTo(range.x.min, 6); + expect(captionHorizontalPositionOffset(ON, "center")).toBe(0); + expect(captionHorizontalPositionOffset(ON, "right")).toBeCloseTo(range.x.max, 6); + }); + + it("reaches the true left and right frame edges through the left/right presets", () => { + const left = captionBandRect({ ...ON, offsetX: captionHorizontalPositionOffset(ON, "left") }); + expect(left.x).toBeCloseTo(0, 6); + const right = captionBandRect({ + ...ON, + offsetX: captionHorizontalPositionOffset(ON, "right"), + }); + expect(right.x + right.width).toBeCloseTo(100, 6); + }); + + it("de-activates the horizontal preset when width moves the band, without touching offsetX", () => { + // The real asymmetry against the vertical axis: `range.x` moves with + // `width` (`captionAnchor.x` depends on it), so a preset that was flush + // can stop being flush purely because the band got narrower or wider. + // `offsetY === 0` has no such dependency, so a vertical preset never does + // this — it's intended, not a regression. + const atWidth80 = { + ...ON, + width: 80, + offsetX: captionOffsetRange({ ...ON, width: 80 }).x.min, + }; + expect(activeHorizontalPositionPreset(atWidth80)).toBe("left"); + expect(captionBandRect(atWidth80).x).toBeCloseTo(0, 6); + + const narrowed = { ...atWidth80, width: 50 }; + expect(activeHorizontalPositionPreset(narrowed)).toBeNull(); + expect(narrowed.offsetX).toBe(atWidth80.offsetX); + expect(captionBandRect(narrowed).x).toBeCloseTo(15, 6); + }); +}); + describe("deriveCaptionCues", () => { it("returns nothing while the layer is hidden", () => { expect(deriveCaptionCues(doc(), DEFAULT_CAPTION_SETTINGS, {})).toEqual([]); diff --git a/src/lib/ai-edition/captions/index.ts b/src/lib/ai-edition/captions/index.ts index 6ed325c71..c238654bc 100644 --- a/src/lib/ai-edition/captions/index.ts +++ b/src/lib/ai-edition/captions/index.ts @@ -9,6 +9,7 @@ export { } from "./cues"; export type { CaptionBandRect, + CaptionHorizontalPosition, CaptionOffsetRange, CaptionSettings, CaptionSettingsPatch, @@ -16,10 +17,14 @@ export type { CaptionVerticalPosition, } from "./settings"; export { + activeHorizontalPositionPreset, + activeVerticalPositionPreset, CAPTION_BAND_HEIGHT_PCT, CAPTION_EDGE_MARGIN_PCT, + CAPTION_POSITION_PRESET_EPSILON, captionBackgroundCss, captionBandRect, + captionHorizontalPositionOffset, captionInkHeightPct, captionOffsetRange, DEFAULT_CAPTION_SETTINGS, diff --git a/src/lib/ai-edition/captions/settings.ts b/src/lib/ai-edition/captions/settings.ts index 13cacdc94..3f80bd05b 100644 --- a/src/lib/ai-edition/captions/settings.ts +++ b/src/lib/ai-edition/captions/settings.ts @@ -18,6 +18,13 @@ export type CaptionVerticalPosition = "top" | "middle" | "bottom"; /** Horizontal alignment of the text inside the (always centred) caption band. */ export type CaptionTextAlign = "left" | "center" | "right"; +/** Horizontal position preset for the caption band itself — a different axis of + * meaning from `CaptionTextAlign`, which aligns the text *inside* the band. + * Not a stored field: it is derived from `offsetX` (see + * `activeHorizontalPositionPreset`) and set by writing `offsetX` directly (see + * `captionHorizontalPositionOffset`). */ +export type CaptionHorizontalPosition = "left" | "center" | "right"; + export interface CaptionSettings { /** Master show/hide for the whole caption layer (preview AND export). */ enabled: boolean; @@ -85,6 +92,12 @@ export const CAPTION_BAND_HEIGHT_PCT = 22; /** Margin between the band and the frame edge for the top/bottom anchors, in %. */ export const CAPTION_EDGE_MARGIN_PCT = 3; +/** Tolerance for "is this offset at a preset's clean value", in % of frame. + * Deliberately not `Number.EPSILON` (already used below as `sliderStep`'s + * divide-by-zero floor, and far too small to absorb real float noise) — + * matches the `toBeCloseTo(x, 6)` tolerance this file's own tests use. */ +export const CAPTION_POSITION_PRESET_EPSILON = 1e-6; + /** Reference frame height the px-valued settings are authored against, matching * `annotationScale.ts` — `fontSize` is "pixels at a 1080-high frame". */ const CAPTION_REFERENCE_FRAME_HEIGHT = 1080; @@ -175,6 +188,55 @@ export function captionOffsetRange(settings: CaptionSettings): CaptionOffsetRang }; } +/** + * Which vertical preset, if any, the current settings match exactly. + * + * `verticalPosition` is always a real stored value, but a preset button should + * only read as "active" while the user hasn't nudged away from it — otherwise + * clicking a slider would leave a preset highlighted that no longer describes + * where the band actually is. `offsetY` is the nudge *from* the anchor, so + * "at the preset" is exactly "no nudge". + */ +export function activeVerticalPositionPreset( + settings: CaptionSettings, +): CaptionVerticalPosition | null { + return Math.abs(settings.offsetY) < CAPTION_POSITION_PRESET_EPSILON + ? settings.verticalPosition + : null; +} + +/** + * Which horizontal position preset, if any, the current settings match exactly. + * + * There is no stored `horizontalPosition` field — `offsetX` is already an + * absolute-feeling value centred on 0 with a range that reaches both frame + * edges (see `captionOffsetRange`), so "left"/"center"/"right" are just names + * for three points on that existing range, checked in this order because all + * three coincide at `offsetX === 0` when the band is full-width (no travel). + */ +export function activeHorizontalPositionPreset( + settings: CaptionSettings, +): CaptionHorizontalPosition | null { + const range = captionOffsetRange(settings); + const { offsetX } = settings; + if (Math.abs(offsetX) < CAPTION_POSITION_PRESET_EPSILON) return "center"; + if (Math.abs(offsetX - range.x.min) < CAPTION_POSITION_PRESET_EPSILON) return "left"; + if (Math.abs(offsetX - range.x.max) < CAPTION_POSITION_PRESET_EPSILON) return "right"; + return null; +} + +/** The `offsetX` that puts the band at a given horizontal preset, for a preset + * button's click handler to write. `left`/`right` reach the true frame edge — + * the same span `activeHorizontalPositionPreset` reads back against. */ +export function captionHorizontalPositionOffset( + settings: CaptionSettings, + preset: CaptionHorizontalPosition, +): number { + if (preset === "center") return 0; + const range = captionOffsetRange(settings); + return preset === "left" ? range.x.min : range.x.max; +} + const VERTICAL_POSITIONS: readonly CaptionVerticalPosition[] = ["top", "middle", "bottom"]; const TEXT_ALIGNS: readonly CaptionTextAlign[] = ["left", "center", "right"]; From 19ca0db4ff1b849d880b0aa39bdf359361e43a50 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sat, 22 Aug 2026 12:14:55 +0200 Subject: [PATCH 12/30] fix(captions): pick the closest position preset, not the first one checked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit activeHorizontalPositionPreset checked "is this centered?" before "is this flush left/right?" — fine normally, but as width approaches 100 the whole reachable range shrinks toward 0 right along with it, so a band sitting exactly at the true left/right edge could fall inside the center check's epsilon too and get reported as centered. Comparing all three candidates and keeping the nearest one is correct regardless of how narrow the range gets. Found via code review (CodeRabbit hit its OSS rate limit on the PR, so this ran as a subagent review instead). Reachable today only through a hand-edited or externally-generated project file — legacyEditor.captions has no schema validation — not through the shipped integer-stepped width slider. (cherry picked from commit a7536fdaa7efc296a81799413deb99d23492324a) --- src/lib/ai-edition/captions/captions.test.ts | 13 ++++++++++ src/lib/ai-edition/captions/settings.ts | 27 +++++++++++++++----- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/lib/ai-edition/captions/captions.test.ts b/src/lib/ai-edition/captions/captions.test.ts index a4d12f257..d6529489e 100644 --- a/src/lib/ai-edition/captions/captions.test.ts +++ b/src/lib/ai-edition/captions/captions.test.ts @@ -6,6 +6,7 @@ import { activeHorizontalPositionPreset, activeVerticalPositionPreset, CAPTION_BAND_HEIGHT_PCT, + CAPTION_POSITION_PRESET_EPSILON, captionBackgroundCss, captionBandRect, captionHorizontalPositionOffset, @@ -277,6 +278,18 @@ describe("caption position presets", () => { expect(activeHorizontalPositionPreset({ ...ON, width: 100, offsetX: 0 })).toBe("center"); }); + it("still picks left/right over center when a near-full-width band squeezes them inside the epsilon", () => { + // At width this close to 100, range.x.min/max themselves fall inside + // CAPTION_POSITION_PRESET_EPSILON of 0 — checking "is this near center?" + // first would wrongly claim an offset that is exactly at the true edge. + const squeezed = { ...ON, width: 100 - 1e-7 }; + const range = captionOffsetRange(squeezed); + expect(Math.abs(range.x.min)).toBeLessThan(CAPTION_POSITION_PRESET_EPSILON); + expect(activeHorizontalPositionPreset({ ...squeezed, offsetX: range.x.min })).toBe("left"); + expect(activeHorizontalPositionPreset({ ...squeezed, offsetX: range.x.max })).toBe("right"); + expect(activeHorizontalPositionPreset({ ...squeezed, offsetX: 0 })).toBe("center"); + }); + it("sets offsetX to the true frame edge for left/right, matching the reachable range", () => { const range = captionOffsetRange(ON); expect(captionHorizontalPositionOffset(ON, "left")).toBeCloseTo(range.x.min, 6); diff --git a/src/lib/ai-edition/captions/settings.ts b/src/lib/ai-edition/captions/settings.ts index 3f80bd05b..469c67aaf 100644 --- a/src/lib/ai-edition/captions/settings.ts +++ b/src/lib/ai-edition/captions/settings.ts @@ -211,18 +211,33 @@ export function activeVerticalPositionPreset( * There is no stored `horizontalPosition` field — `offsetX` is already an * absolute-feeling value centred on 0 with a range that reaches both frame * edges (see `captionOffsetRange`), so "left"/"center"/"right" are just names - * for three points on that existing range, checked in this order because all - * three coincide at `offsetX === 0` when the band is full-width (no travel). + * for three points on that existing range. All three coincide at `offsetX===0` + * when the band is full-width (no travel) — and can also *nearly* coincide + * for a band merely close to full-width, where `range.x.min`/`max` shrink + * toward 0 as well. Picking the CLOSEST candidate (not the first one within + * epsilon) is what keeps that near-degenerate case from reporting "center" + * for an offset that is actually sitting exactly on `range.x.min`/`max`. */ export function activeHorizontalPositionPreset( settings: CaptionSettings, ): CaptionHorizontalPosition | null { const range = captionOffsetRange(settings); const { offsetX } = settings; - if (Math.abs(offsetX) < CAPTION_POSITION_PRESET_EPSILON) return "center"; - if (Math.abs(offsetX - range.x.min) < CAPTION_POSITION_PRESET_EPSILON) return "left"; - if (Math.abs(offsetX - range.x.max) < CAPTION_POSITION_PRESET_EPSILON) return "right"; - return null; + const candidates: ReadonlyArray<[CaptionHorizontalPosition, number]> = [ + ["center", 0], + ["left", range.x.min], + ["right", range.x.max], + ]; + let closest: CaptionHorizontalPosition | null = null; + let closestDistance = CAPTION_POSITION_PRESET_EPSILON; + for (const [preset, target] of candidates) { + const distance = Math.abs(offsetX - target); + if (distance < closestDistance) { + closest = preset; + closestDistance = distance; + } + } + return closest; } /** The `offsetX` that puts the band at a given horizontal preset, for a preset From ae2d166b6bf7fcd67f65663a4effceba8dba7244 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 22 Aug 2026 11:22:08 +0000 Subject: [PATCH 13/30] chore(release): bump to 1.10.0-rc.2 [skip ci] --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index a6f2c7f4d..fb722503e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "openscreen", - "version": "1.10.0-rc.1", + "version": "1.10.0-rc.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "openscreen", - "version": "1.10.0-rc.1", + "version": "1.10.0-rc.2", "dependencies": { "@fix-webm-duration/fix": "^1.0.1", "@langchain/anthropic": "^1.3.26", diff --git a/package.json b/package.json index a1ba6b4b8..83b7210cd 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "openscreen", "private": true, - "version": "1.10.0-rc.1", + "version": "1.10.0-rc.2", "description": "Record your screen and polish the demo", "homepage": "https://getopenscreen.com/", "license": "MIT", From a2a411b7774b0d6d82fc3f5eec13b01dfd799390 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sat, 22 Aug 2026 14:18:21 +0200 Subject: [PATCH 14/30] fix(editor): give the timeline clip's delete icon the same dark-chip treatment as its label MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit .tlClipDelete sits on the same fixed dark frosted-glass chip as .tlClipLabel right next to it (both `color-mix(in srgb, #080a0d 55%, transparent)`, deliberately theme-independent since they overlay an arbitrary video thumbnail) — but its icon used `color: var(--muted)`, the app's own theme-dependent secondary-text token, instead of the "light text on a dark overlay" token the sibling chip's own text already uses (`.tlClipName` is a flat `#fff` for the same reason). In light theme `--muted` is a medium slate gray, close enough in tone to the chip's blended backdrop to read as nearly invisible — reported directly against a real clip's delete button in the timeline. Switched to `--overlay-text`, the token design-tokens.css defines for exactly this pairing (already used for scene/PiP overlay captions). Co-Authored-By: Claude Sonnet 5 --- src/components/ai-edition/v4/EditorShellV4.module.css | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/components/ai-edition/v4/EditorShellV4.module.css b/src/components/ai-edition/v4/EditorShellV4.module.css index df6af761a..1e313e34c 100644 --- a/src/components/ai-edition/v4/EditorShellV4.module.css +++ b/src/components/ai-edition/v4/EditorShellV4.module.css @@ -1626,7 +1626,12 @@ display: grid; place-items: center; border-radius: 7px; - color: var(--muted); + /* The chip itself is a fixed dark frosted-glass overlay regardless of + theme (it sits on top of an arbitrary video thumbnail), so its icon + needs the "light text on a dark overlay" token, not --muted — which + is tuned for the app's own light/dark surfaces and reads as + near-invisible against this chip in light theme. */ + color: var(--overlay-text); background: color-mix(in srgb, #080a0d 55%, transparent); border: 1px solid rgba(255, 255, 255, 0.08); backdrop-filter: blur(8px); From ce212e5184a8be06f1dc4f6672747a9ad51a1303 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sat, 22 Aug 2026 15:14:44 +0200 Subject: [PATCH 15/30] feat(compositor): let a text region pin an edge instead of centring its block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds one optional field to the text payload, `verticalAlign`, threaded through the scene into all three rasterizers. Absent means centred — so annotations, which never emit it, render byte for byte as before. Centring is what made caption placement incoherent: a centred block moves BOTH its edges when it grows, so a caption drifted vertically whenever the text wrapped to another line, and no setting could hold it still. An anchored block keeps its anchored edge exactly where it was put, at any line count. The Linux test asserts precisely that, which is the assertion the old geometry could not express. Option and not an enum, for the same reason as `space`: serde rejects an unknown unit variant, so a future value would cost the whole scene on an older binary rather than one misplaced caption. Windows needed the layout box inset vertically by the plate margin and the draw origin offset to match, or a bottom anchor puts the glyphs flush against the box and the plate's lower margin gets clipped. That arithmetic cancels exactly for the centred case; it is now a pure function with a test pinning it to where it was, because it was the one calculation on the Windows path no test covered. Nothing emits the field yet. (cherry picked from commit c1fb929d8a18fa6a8af2c28eb49bac48bb0f2cac) --- crates/compositor/src/compositor_linux.rs | 3 + crates/compositor/src/compositor_macos.rs | 3 + crates/compositor/src/compositor_windows.rs | 3 + crates/compositor/src/scene.rs | 12 ++ crates/compositor/src/text_linux.rs | 93 +++++++++++- crates/compositor/src/text_macos.rs | 67 ++++++++- crates/compositor/src/text_windows.rs | 154 ++++++++++++++++++-- 7 files changed, 314 insertions(+), 21 deletions(-) diff --git a/crates/compositor/src/compositor_linux.rs b/crates/compositor/src/compositor_linux.rs index 8b950f753..03a0aa6b9 100644 --- a/crates/compositor/src/compositor_linux.rs +++ b/crates/compositor/src/compositor_linux.rs @@ -1497,6 +1497,9 @@ impl Compositor { italic: text.font_style == "italic", underline: text.text_decoration == "underline", align: text.text_align.clone(), + // Absent = "center", le comportement historique : les + // annotations ne changent pas d'un pixel. + valign: text.vertical_align.clone().unwrap_or_default(), box_px: [ quad_px[0].round().max(1.0) as u32, quad_px[1].round().max(1.0) as u32, diff --git a/crates/compositor/src/compositor_macos.rs b/crates/compositor/src/compositor_macos.rs index 2cee70e30..cffc9395d 100644 --- a/crates/compositor/src/compositor_macos.rs +++ b/crates/compositor/src/compositor_macos.rs @@ -1146,6 +1146,9 @@ impl Compositor { italic: text.font_style == "italic", underline: text.text_decoration == "underline", align: text.text_align.clone(), + // Absent = "center", le comportement historique : les + // annotations ne changent pas d'un pixel. + valign: text.vertical_align.clone().unwrap_or_default(), box_px: [quad_px[0].round() as u32, quad_px[1].round() as u32], }; let key = spec.cache_key(); diff --git a/crates/compositor/src/compositor_windows.rs b/crates/compositor/src/compositor_windows.rs index dbaf6f855..51c3a3a28 100644 --- a/crates/compositor/src/compositor_windows.rs +++ b/crates/compositor/src/compositor_windows.rs @@ -1777,6 +1777,9 @@ impl Compositor { italic: text.font_style == "italic", underline: text.text_decoration == "underline", align: text.text_align.clone(), + // Absent = "center", le comportement historique : les + // annotations ne changent pas d'un pixel. + valign: text.vertical_align.clone().unwrap_or_default(), box_px: [quad_px[0].round() as u32, quad_px[1].round() as u32], }; let key = spec.cache_key(); diff --git a/crates/compositor/src/scene.rs b/crates/compositor/src/scene.rs index bf9260bef..2d20e233b 100644 --- a/crates/compositor/src/scene.rs +++ b/crates/compositor/src/scene.rs @@ -261,6 +261,18 @@ pub struct SceneAnnotationText { pub font_style: String, pub text_decoration: String, pub text_align: String, + /// Quelle arête du bloc de texte est épinglée à sa boîte : `"top"` / `"center"` + /// / `"bottom"`. Absent = `"center"`, le comportement historique — les + /// annotations n'émettent jamais la clé et ne bougent donc pas d'un pixel. + /// Les sous-titres l'émettent pour que l'arête ancrée tienne quand le texte + /// gagne une ligne (un bloc centré voit ses deux arêtes se déplacer). + /// + /// `Option` et pas une enum, pour la même raison que `space` : serde + /// rejette une variante d'unité inconnue, donc une valeur future ferait + /// échouer `Scene::from_json` *en entier* sur un binaire plus ancien, au lieu + /// de coûter un seul sous-titre mal placé. + #[serde(default)] + pub vertical_align: Option, #[serde(default)] pub animation: Option, } diff --git a/crates/compositor/src/text_linux.rs b/crates/compositor/src/text_linux.rs index 3e7b45b2b..d990c3dbe 100644 --- a/crates/compositor/src/text_linux.rs +++ b/crates/compositor/src/text_linux.rs @@ -37,6 +37,12 @@ pub struct TextSpec { pub underline: bool, /// "left" | "center" | "right". pub align: String, + /// "top" | "center" | "bottom" -- quelle arete du bloc de texte est epinglee + /// a la boite. "center" est le comportement historique (et celui des + /// annotations, qui reproduisent `alignItems: center` de l'overlay web) ; les + /// sous-titres passent "bottom" ou "top" pour que l'arete ancree ne bouge pas + /// quand le texte gagne une ligne. + pub valign: String, /// Taille de la boite en px de sortie. pub box_px: [u32; 2], } @@ -61,6 +67,10 @@ impl TextSpec { } mix(&[self.bold as u8, self.italic as u8, self.underline as u8]); mix(self.align.as_bytes()); + // Juste apres `align`, memes octets et meme position que sur les deux + // autres backends : deux specs ne differant que par l'alignement vertical + // rendraient sinon les pixels l'une de l'autre depuis le cache. + mix(self.valign.as_bytes()); mix(&self.box_px[0].to_le_bytes()); mix(&self.box_px[1].to_le_bytes()); h @@ -194,7 +204,18 @@ impl TextRasterizer { .fold(0.0f32, f32::max); // `max(0)` : un texte plus haut que sa boite reste ancre en haut plutot // que de sortir par le dessus, ou il serait entierement rogne. - let y_offset = (((h as f32) - text_h) * 0.5).max(0.0).round() as i32; + // + // ANCRAGE. `center` est le comportement historique, et reste celui des + // annotations. Les sous-titres epinglent une arete : c'est la seule facon + // que l'arete ancree ne bouge pas quand le texte gagne une ligne, parce + // qu'un bloc centre voit ses DEUX aretes se deplacer. + let slack_y = ((h as f32) - text_h).max(0.0); + let y_offset = match spec.valign.as_str() { + "top" | "start" => 0.0, + "bottom" | "end" => slack_y, + _ => slack_y * 0.5, + } + .round() as i32; // LA PLAQUE EPOUSE LE BLOC, PAS LA BOITE. Miroir de // `text_macos::block_layout` (en coordonnees descendantes ici, CoreText @@ -385,6 +406,7 @@ mod tests { italic: false, underline: false, align: align.to_owned(), + valign: "center".to_owned(), box_px: [400, 200], } } @@ -566,6 +588,75 @@ mod tests { ); } + #[test] + fn the_anchored_edge_holds_still_when_the_text_gains_a_line() { + // L'INVARIANT de la refonte du placement des sous-titres, en une + // assertion — et celle que l'ancienne architecture ne pouvait pas ecrire. + // + // Un bloc centre voit ses DEUX aretes bouger quand il grandit : c'est + // exactement pourquoi elargir la bande deplacait verticalement le + // sous-titre. Ancre en bas, l'arete basse ne doit pas bouger d'un pixel, + // que le texte tienne sur une ligne ou en reclame trois. + let raster = TextRasterizer::new().expect("rasterizer"); + let (w, h) = (400usize, 200usize); + let long = "un texte assez long pour devoir se replier sur plusieurs lignes"; + + let one = |valign: &str, content: &str| { + let mut s = spec(content, "center"); + s.valign = valign.to_owned(); + let atlas = raster.build_atlas(&s).expect("atlas").pixels; + let rows = ink_rows(&atlas, w, 0, w); + assert!(!rows.is_empty(), "aucune encre pour {valign:?}"); + (rows[0], *rows.last().unwrap()) + }; + + let (_, short_bottom) = one("bottom", "Hx"); + let (_, long_bottom) = one("bottom", long); + assert!( + (short_bottom as i32 - long_bottom as i32).abs() <= 1, + "ancrage bas : l'arete basse a bouge de {short_bottom} a {long_bottom} \ + en passant d'une ligne a plusieurs" + ); + + // Et le miroir, pour que « haut » ne soit pas juste « pas bas ». + let (short_top, _) = one("top", "Hx"); + let (long_top, _) = one("top", long); + assert!( + (short_top as i32 - long_top as i32).abs() <= 1, + "ancrage haut : l'arete haute a bouge de {short_top} a {long_top}" + ); + + // Le texte long doit vraiment occuper plus de hauteur, sinon les deux + // assertions ci-dessus passeraient sur deux rendus identiques. + let (lt, lb) = one("bottom", long); + let (st, sb) = one("bottom", "Hx"); + assert!( + (lb - lt) > (sb - st), + "le texte « long » ne s'est pas replie : le test ne prouve rien" + ); + + // Enfin, les trois ancrages doivent poser l'encre a trois endroits + // differents dans la boite — sinon `valign` n'est pas applique du tout. + let (top_t, _) = one("top", "Hx"); + let (ctr_t, _) = one("center", "Hx"); + let (bot_t, _) = one("bottom", "Hx"); + assert!( + top_t < ctr_t && ctr_t < bot_t, + "les trois ancrages ne se distinguent pas : haut={top_t} centre={ctr_t} bas={bot_t}" + ); + assert!(bot_t > h / 2, "l'ancrage bas laisse l'encre dans la moitie haute"); + } + + #[test] + fn the_vertical_anchor_changes_the_cache_key() { + // Le piege du cache : la cle est partagee entre plateformes, et deux specs + // ne differant que par `valign` rendraient les pixels l'une de l'autre si + // le champ n'y entrait pas. + let mut bottom = spec("Hx", "center"); + bottom.valign = "bottom".to_owned(); + assert_ne!(bottom.cache_key(), spec("Hx", "center").cache_key()); + } + #[test] fn centering_moves_the_ink_off_the_left_edge() { // `spec.align` n'etait jamais applique : tout sortait ferre a gauche diff --git a/crates/compositor/src/text_macos.rs b/crates/compositor/src/text_macos.rs index 57385efe0..7e076f348 100644 --- a/crates/compositor/src/text_macos.rs +++ b/crates/compositor/src/text_macos.rs @@ -52,6 +52,11 @@ pub struct TextSpec { pub underline: bool, /// "left" | "center" | "right". pub align: String, + /// "top" | "center" | "bottom" — quelle arête du bloc est épinglée à la boîte. + /// "center" est le comportement historique (et celui des annotations) ; les + /// sous-titres passent "bottom" ou "top" pour que l'arête ancrée ne bouge pas + /// quand le texte gagne une ligne. + pub valign: String, /// Taille de la boîte en px de sortie — la mise en page en dépend (retours à la ligne). pub box_px: [u32; 2], } @@ -79,6 +84,10 @@ impl TextSpec { } mix(&[self.bold as u8, self.italic as u8, self.underline as u8]); mix(self.align.as_bytes()); + // Juste après `align`, mêmes octets et même position que sur les deux + // autres backends : deux specs ne différant que par l'alignement vertical + // rendraient sinon les pixels l'une de l'autre depuis le cache. + mix(self.valign.as_bytes()); mix(&self.box_px[0].to_le_bytes()); mix(&self.box_px[1].to_le_bytes()); h @@ -308,6 +317,7 @@ fn block_layout( text_w: CGFloat, text_h: CGFloat, align: u8, + valign: &str, font_px: CGFloat, ) -> (CGRect, CGRect) { let (pad_x, pad_y) = plate_padding(font_px); @@ -317,7 +327,17 @@ fn block_layout( // la mesure. On l'étend d'un pixel vers le BAS — donc en abaissant l'origine `y`, pas // en montant le sommet — pour que le haut du texte ne bouge pas d'un poil. const GUARD: CGFloat = 1.0; - let top = ((box_h - text_h) * 0.5).max(0.0); + // ANCRAGE. `center` reste le comportement historique (et celui des annotations, + // qui reproduisent `alignItems: center` de l'overlay web). Les sous-titres + // épinglent une arête : un bloc centré voit ses DEUX arêtes bouger quand il + // gagne une ligne, ce qui déplaçait le sous-titre. `top` est ici une distance + // depuis le HAUT de la boîte, en coordonnées descendantes. + let slack_y = (box_h - text_h).max(0.0); + let top = match valign { + "top" | "start" => 0.0, + "bottom" | "end" => slack_y, + _ => slack_y * 0.5, + }; let frame_x = (box_w - avail_w) * 0.5; let frame = CGRect { origin: CGPoint { @@ -593,7 +613,7 @@ impl TextRasterizer { let text_h = measured.height.ceil().max(0.0); let (frame_rect, plate_rect) = - block_layout(box_w, box_h, text_w, text_h, alignment, font_px); + block_layout(box_w, box_h, text_w, text_h, alignment, &spec.valign, font_px); // --- plaque de fond, sous le texte --- if spec.background[3] > 0.0 && plate_rect.size.width > 0.0 && plate_rect.size.height > 0.0 @@ -651,6 +671,7 @@ mod tests { italic: false, underline: false, align: "center".into(), + valign: "center".into(), box_px: [256, 256], } } @@ -828,7 +849,7 @@ mod tests { /// Géométrie pure — pas de GPU, pas de CoreText. #[test] fn block_layout_centres_the_frame_and_sizes_the_plate() { - let (frame, plate) = block_layout(1536.0, 238.0, 500.0, 56.0, 2, 48.0); + let (frame, plate) = block_layout(1536.0, 238.0, 500.0, 56.0, 2, "center", 48.0); // Cadre centré : autant de vide au-dessus qu'en dessous (repère CG, y vers le haut). let above = 238.0 - (frame.origin.y + frame.size.height); let below = frame.origin.y; @@ -843,11 +864,49 @@ mod tests { fn block_layout_never_lets_the_plate_leave_the_box() { for align in [0u8, 1, 2] { // Bloc plus large et plus haut que la boîte : la plaque doit se contenter d'elle. - let (_, plate) = block_layout(200.0, 60.0, 400.0, 200.0, align, 48.0); + let (_, plate) = block_layout(200.0, 60.0, 400.0, 200.0, align, "center", 48.0); assert!(plate.origin.x >= 0.0, "align={align} : x={}", plate.origin.x); assert!(plate.origin.y >= 0.0, "align={align} : y={}", plate.origin.y); assert!(plate.origin.x + plate.size.width <= 200.0 + 0.01, "align={align}"); assert!(plate.origin.y + plate.size.height <= 60.0 + 0.01, "align={align}"); } } + + /// L'invariant de la refonte du placement des sous-titres, en géométrie pure. + /// Un bloc centré voit ses DEUX arêtes bouger quand il grandit ; ancré, l'arête + /// ancrée ne bouge pas. Repère CoreGraphics : `y` monte. + #[test] + fn block_layout_pins_the_anchored_edge_whatever_the_block_height() { + let (box_w, box_h) = (1536.0, 238.0); + let edges = |valign: &str, text_h: f64| { + let (frame, _) = block_layout(box_w, box_h, 500.0, text_h, 2, valign, 48.0); + // (bas, haut) en distance depuis le bas de la boîte. + (frame.origin.y, frame.origin.y + frame.size.height) + }; + + // Ancrage bas : l'arête basse est la même à une et à trois lignes. + let (one_bottom, _) = edges("bottom", 56.0); + let (three_bottom, _) = edges("bottom", 168.0); + assert!( + (one_bottom - three_bottom).abs() < 0.01, + "ancrage bas : l'arête basse a bougé de {one_bottom} à {three_bottom}" + ); + + // Ancrage haut : l'arête haute est la même. + let (_, one_top) = edges("top", 56.0); + let (_, three_top) = edges("top", 168.0); + assert!( + (one_top - three_top).abs() < 0.01, + "ancrage haut : l'arête haute a bougé de {one_top} à {three_top}" + ); + + // Et le centrage, lui, fait bien bouger les deux — c'est le comportement + // historique qu'on préserve pour les annotations. + let (c1_bottom, c1_top) = edges("center", 56.0); + let (c3_bottom, c3_top) = edges("center", 168.0); + assert!( + (c1_bottom - c3_bottom).abs() > 1.0 && (c1_top - c3_top).abs() > 1.0, + "le centrage devrait déplacer les deux arêtes" + ); + } } diff --git a/crates/compositor/src/text_windows.rs b/crates/compositor/src/text_windows.rs index 93a5f6df8..b9f6f9e10 100644 --- a/crates/compositor/src/text_windows.rs +++ b/crates/compositor/src/text_windows.rs @@ -33,8 +33,9 @@ use windows::Win32::Graphics::DirectWrite::{ DWriteCreateFactory, IDWriteFactory, DWRITE_FACTORY_TYPE_SHARED, DWRITE_FONT_STRETCH_NORMAL, DWRITE_FONT_STYLE_ITALIC, DWRITE_FONT_STYLE_NORMAL, DWRITE_FONT_WEIGHT_BOLD, DWRITE_FONT_WEIGHT_NORMAL, DWRITE_PARAGRAPH_ALIGNMENT_CENTER, - DWRITE_TEXT_ALIGNMENT_CENTER, DWRITE_TEXT_ALIGNMENT_LEADING, DWRITE_TEXT_ALIGNMENT_TRAILING, - DWRITE_TEXT_METRICS, DWRITE_TEXT_RANGE, + DWRITE_PARAGRAPH_ALIGNMENT_FAR, DWRITE_PARAGRAPH_ALIGNMENT_NEAR, DWRITE_TEXT_ALIGNMENT_CENTER, + DWRITE_TEXT_ALIGNMENT_LEADING, DWRITE_TEXT_ALIGNMENT_TRAILING, DWRITE_TEXT_METRICS, + DWRITE_TEXT_RANGE, }; use windows::Win32::Graphics::Dxgi::Common::DXGI_FORMAT_B8G8R8A8_UNORM; use windows::Win32::Graphics::Dxgi::Common::DXGI_SAMPLE_DESC; @@ -56,6 +57,11 @@ pub struct TextSpec { pub underline: bool, /// "left" | "center" | "right". pub align: String, + /// "top" | "center" | "bottom" — quelle arête du bloc est épinglée à la boîte. + /// "center" est le comportement historique (et celui des annotations) ; les + /// sous-titres passent "bottom" ou "top" pour que l'arête ancrée ne bouge pas + /// quand le texte gagne une ligne. + pub valign: String, /// Taille de la boîte en px de sortie — la mise en page en dépend (retours à la ligne). pub box_px: [u32; 2], } @@ -79,6 +85,10 @@ impl TextSpec { } mix(&[self.bold as u8, self.italic as u8, self.underline as u8]); mix(self.align.as_bytes()); + // Juste après `align`, mêmes octets et même position que sur les deux + // autres backends : deux specs ne différant que par l'alignement vertical + // rendraient sinon les pixels l'une de l'autre depuis le cache. + mix(self.valign.as_bytes()); mix(&self.box_px[0].to_le_bytes()); mix(&self.box_px[1].to_le_bytes()); h @@ -90,6 +100,37 @@ fn wide(s: &str) -> Vec { s.encode_utf16().chain(std::iter::once(0)).collect() } +/// La plaque de fond, en `[left, top, right, bottom]` px dans la boîte, à partir des +/// métriques DirectWrite du bloc mis en page. +/// +/// Fonction pure — et volontairement extraite du chemin de dessin : le rasteriseur +/// Windows exige un device D3D, donc tout ce qui reste inline dans `rasterize` n'est +/// couvert par aucun test. macOS a `block_layout` pour la même raison ; ceci met les +/// deux backends au même niveau, sur le calcul qui décide si la plaque se fait rogner. +/// +/// Les deux annulations qui portent tout : +/// * horizontalement, le texte est dessiné à `pad_x` et commence donc à `pad_x + m.left` : +/// la plaque part de `m.left`, l'inset de la boîte de mise en page et la marge de +/// plaque se compensent exactement, quel que soit l'alignement ; +/// * verticalement, le texte est dessiné à `pad_y` dans une boîte de mise en page rentrée +/// de `2*pad_y`, donc son haut réel vaut `pad_y + m.top` et la plaque va de `m.top` à +/// `m.top + m.height + 2*pad_y`. Ancré en bas (`DWRITE_PARAGRAPH_ALIGNMENT_FAR`), ce +/// second terme tombe pile sur `box_h` : la marge basse tient tout juste au lieu d'être +/// rognée par le `.min()`. +/// +/// Le bornage à la boîte est ce qui empêche la plaque d'être coupée net par le bord de +/// la texture, où elle perdrait ses coins arrondis. +fn plate_rect(metrics: [f32; 4], box_px: [f32; 2], pad_x: f32, pad_y: f32) -> [f32; 4] { + let [m_left, m_top, m_width, m_height] = metrics; + let [box_w, box_h] = box_px; + [ + m_left.max(0.0), + m_top.max(0.0), + (m_left + m_width + pad_x * 2.0).min(box_w), + (m_top + m_height + pad_y * 2.0).min(box_h), + ] +} + pub struct TextRasterizer { d2d: ID2D1Factory, dwrite: IDWriteFactory, @@ -173,8 +214,14 @@ impl TextRasterizer { "right" => DWRITE_TEXT_ALIGNMENT_TRAILING, _ => DWRITE_TEXT_ALIGNMENT_CENTER, })?; - // Centrage vertical : l'overlay web met `alignItems: center` sur le conteneur. - format.SetParagraphAlignment(DWRITE_PARAGRAPH_ALIGNMENT_CENTER)?; + // ANCRAGE vertical. `center` reproduit `alignItems: center` de l'overlay web et + // reste le comportement des annotations ; les sous-titres épinglent une arête, + // parce qu'un bloc centré voit ses DEUX arêtes bouger quand il gagne une ligne. + format.SetParagraphAlignment(match spec.valign.as_str() { + "top" | "start" => DWRITE_PARAGRAPH_ALIGNMENT_NEAR, + "bottom" | "end" => DWRITE_PARAGRAPH_ALIGNMENT_FAR, + _ => DWRITE_PARAGRAPH_ALIGNMENT_CENTER, + })?; let text: Vec = spec.content.encode_utf16().collect(); // La boîte de mise en page est rentrée de la marge de plaque (cf. `text_plate`), et @@ -184,9 +231,15 @@ impl TextRasterizer { let font_px = spec.font_size_px.max(1.0); let (pad_x, pad_y) = crate::text_plate::padding(font_px); let layout_w = crate::text_plate::layout_width(w as f32, font_px); + // La boîte de mise en page est aussi rentrée VERTICALEMENT de la marge de plaque, + // et le texte se dessine à `pad_y`. Sans ça, un ancrage bas colle les glyphes au + // bord de la boîte et le `.min(h)` de la plaque, plus bas, rogne net sa marge + // basse. Le centrage est rigoureusement inchangé par cette paire (l'inset et le + // décalage s'annulent), donc les annotations ne bougent pas d'un pixel. + let layout_h = ((h as f32) - pad_y * 2.0).max(1.0); let layout = self .dwrite - .CreateTextLayout(&text, &format, layout_w, h as f32)?; + .CreateTextLayout(&text, &format, layout_w, layout_h)?; if spec.underline { layout.SetUnderline( true, @@ -214,16 +267,9 @@ impl TextRasterizer { a: spec.background[3], }; let bg_brush = rt.CreateSolidColorBrush(&bg, None)?; - // Le texte commence à `pad_x + m.left`, donc la plaque à `m.left` — l'inset de la - // boîte de mise en page et la marge de plaque s'annulent exactement, quel que soit - // l'alignement. Elle est ensuite bornée à la boîte : au-delà, elle serait coupée - // net par le bord de la texture et perdrait ses coins arrondis. - let rect = D2D_RECT_F { - left: m.left.max(0.0), - top: (m.top - pad_y).max(0.0), - right: (m.left + m.width + pad_x * 2.0).min(w as f32), - bottom: (m.top + m.height + pad_y).min(h as f32), - }; + let [pl, pt, pr, pb] = + plate_rect([m.left, m.top, m.width, m.height], [w as f32, h as f32], pad_x, pad_y); + let rect = D2D_RECT_F { left: pl, top: pt, right: pr, bottom: pb }; let radius = crate::text_plate::radius( font_px, (rect.right - rect.left).max(0.0), @@ -239,7 +285,7 @@ impl TextRasterizer { ); } rt.DrawTextLayout( - D2D_POINT_2F { x: pad_x, y: 0.0 }, + D2D_POINT_2F { x: pad_x, y: pad_y }, &layout, &brush, D2D1_DRAW_TEXT_OPTIONS_NONE, @@ -269,10 +315,81 @@ mod tests { italic: false, underline: false, align: "center".into(), + valign: "center".into(), box_px: [400, 120], } } + /// Métriques DirectWrite telles que `SetParagraphAlignment` les produit, pour un + /// bloc de `text_h` px dans une boîte de `box_h` : la mise en page se fait dans + /// `box_h - 2*pad_y` (cf. `rasterize`), et l'alignement décide de `m.top` dedans. + fn metrics_for(valign: &str, box_h: f32, text_h: f32, pad_y: f32) -> [f32; 4] { + let layout_h = (box_h - pad_y * 2.0).max(1.0); + let slack = (layout_h - text_h).max(0.0); + let top = match valign { + "top" => 0.0, + "bottom" => slack, + _ => slack * 0.5, + }; + [0.0, top, 200.0, text_h] + } + + #[test] + fn the_plate_survives_the_bottom_anchor_instead_of_being_clipped() { + // LE risque de la bascule d'ancrage sous Windows. Avec l'ancienne mise en page + // (boîte pleine hauteur, dessin à y=0), `FAR` collait les glyphes au bord et le + // `.min(box_h)` rognait net la marge basse de la plaque. Ici elle doit tomber + // pile sur le bord, marge comprise. + let (box_w, box_h, pad_y) = (400.0f32, 120.0f32, 4.8f32); + let m = metrics_for("bottom", box_h, 56.0, pad_y); + let [_, top, _, bottom] = plate_rect(m, [box_w, box_h], 9.6, pad_y); + + assert!( + (bottom - box_h).abs() < 0.01, + "la plaque ancrée en bas devrait finir sur le bord de la boîte, pas à {bottom}" + ); + assert!(top >= 0.0, "plaque hors boîte par le haut : {top}"); + // Et elle fait bien la hauteur du bloc plus ses deux marges — donc rien n'a été rogné. + assert!( + ((bottom - top) - (56.0 + pad_y * 2.0)).abs() < 0.01, + "la marge de la plaque a été rognée : {}px pour un bloc de 56 + 2*{pad_y}", + bottom - top + ); + } + + #[test] + fn the_centred_plate_is_exactly_where_it_was_before_the_anchor_landed() { + // La bascule d'ancrage a rentré la boîte de mise en page de 2*pad_y ET décalé le + // dessin de pad_y. Les deux DOIVENT s'annuler pour le centrage, sinon toutes les + // annotations existantes bougent. Référence : l'ancien calcul, boîte pleine + // hauteur, `top = m.top - pad_y`, `bottom = m.top + m.height + pad_y`. + let (box_w, box_h, pad_y, text_h) = (400.0f32, 120.0f32, 4.8f32, 56.0f32); + + let legacy_top = (box_h - text_h) * 0.5 - pad_y; + let legacy_bottom = (box_h - text_h) * 0.5 + text_h + pad_y; + + let m = metrics_for("center", box_h, text_h, pad_y); + let [_, top, _, bottom] = plate_rect(m, [box_w, box_h], 9.6, pad_y); + + assert!( + (top - legacy_top).abs() < 0.01 && (bottom - legacy_bottom).abs() < 0.01, + "le centrage a bougé : ({top}, {bottom}) au lieu de ({legacy_top}, {legacy_bottom})" + ); + } + + #[test] + fn the_plate_never_leaves_the_box() { + // Un bloc plus grand que sa boîte : la plaque se contente de la boîte plutôt que + // d'être coupée net par le bord de la texture (elle y perdrait ses coins arrondis). + let (box_w, box_h) = (200.0f32, 60.0f32); + for valign in ["top", "center", "bottom"] { + let m = metrics_for(valign, box_h, 400.0, 4.8); + let [l, t, r, b] = plate_rect(m, [box_w, box_h], 9.6, 4.8); + assert!(l >= 0.0 && t >= 0.0, "{valign} : coin haut-gauche hors boîte ({l}, {t})"); + assert!(r <= box_w + 0.01 && b <= box_h + 0.01, "{valign} : plaque hors boîte"); + } + } + #[test] fn identical_specs_share_a_cache_key() { assert_eq!(spec("Bonjour").cache_key(), spec("Bonjour").cache_key()); @@ -296,6 +413,11 @@ mod tests { other.align = "left".into(); assert_ne!(other.cache_key(), base, "alignement"); other = spec("Bonjour"); + // Sans ça, deux sous-titres ne différant que par l'ancrage se partageraient + // une texture et rendraient les pixels l'un de l'autre. + other.valign = "bottom".into(); + assert_ne!(other.cache_key(), base, "ancrage vertical"); + other = spec("Bonjour"); // La taille de boîte compte : elle décide des retours à la ligne, donc des pixels. other.box_px = [401, 120]; assert_ne!(other.cache_key(), base, "boîte"); From c6103188469762c40228fc0ad0e12b0734ad3632 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sat, 22 Aug 2026 15:37:25 +0200 Subject: [PATCH 16/30] fix(captions): place captions by anchor and margin, not by an invisible band MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the whole placement model. Every control now names the edge it measures from, and there is exactly one per axis: anchorV + insetY bottom | top, and a distance from that edge anchorH + insetX left | center | right, and a distance from that edge Deleted: verticalPosition, offsetY, offsetX, width, textAlign — and the machinery that existed only to compensate for the old geometry (the fixed 22% band, the ink-height estimate, the overhang, the reachable-offset range, the preset-vs-slider epsilon). The old model drew every caption inside an invisible fixed-height box and let each rasterizer centre the ink in it, while the only thing on screen — the background plate — hugs the text. So `width` changed nothing visible until the text happened to wrap; the horizontal offset moved a band the text floated inside; text-align fought that offset for the same outcome; wrapping grew a centred block from both edges, which moved the caption vertically when nothing vertical had been touched; and the vertical offset had to be signed and clamped against an estimate, which is where "-7.3%" came from. All five are the same decision, so this replaces the decision rather than the controls. `width` becomes a derived column instead of a control (BBC's line-length table: 68% landscape, 90% vertical). How much text is on screen is already a legible question elsewhere — min/max words per line. The default inset follows the output aspect, because 5% on a 9:16 export is under the platform's own chrome. Migration reproduces the PIXELS, not the fields: the old band's geometry is known, so the drawn block's edges are recoverable, and the nearer one becomes the anchor. A migrated project does not move on screen. Line breaks do change for a project with a non-default width, since that WAS the wrap column. Tests assert the invariant as a property — the anchored edge lands at 100−insetY (or insetY) for every font size, background state and inset — rather than pinning numbers a future change would just have to update. (cherry picked from commit b4810d14e798fb0e6445d8d2e1256c7c71e570b5) --- .../CaptionsPane.placement.test.tsx | 180 +++--- src/components/ai-edition/CaptionsPane.tsx | 208 +++---- src/i18n/locales/ar/settings.json | 18 +- src/i18n/locales/en/settings.json | 18 +- src/i18n/locales/es/settings.json | 18 +- src/i18n/locales/fr/settings.json | 18 +- src/i18n/locales/it/settings.json | 18 +- src/i18n/locales/ja-JP/settings.json | 18 +- src/i18n/locales/ko-KR/settings.json | 18 +- src/i18n/locales/pt-BR/settings.json | 18 +- src/i18n/locales/ru/settings.json | 18 +- src/i18n/locales/tr/settings.json | 18 +- src/i18n/locales/vi/settings.json | 18 +- src/i18n/locales/zh-CN/settings.json | 18 +- src/i18n/locales/zh-TW/settings.json | 18 +- src/lib/ai-edition/captions/captions.test.ts | 340 ++++++----- src/lib/ai-edition/captions/cues.ts | 28 +- src/lib/ai-edition/captions/index.ts | 22 +- src/lib/ai-edition/captions/settings.ts | 532 ++++++++++-------- src/lib/ai-edition/store/useCaptions.test.ts | 4 +- src/native/sceneDescription.ts | 21 +- 21 files changed, 767 insertions(+), 802 deletions(-) diff --git a/src/components/ai-edition/CaptionsPane.placement.test.tsx b/src/components/ai-edition/CaptionsPane.placement.test.tsx index b7d41cec1..f1ab095ac 100644 --- a/src/components/ai-edition/CaptionsPane.placement.test.tsx +++ b/src/components/ai-edition/CaptionsPane.placement.test.tsx @@ -1,21 +1,18 @@ // @vitest-environment jsdom -// The placement sliders take their bounds from `captionOffsetRange`, the same -// function the geometry clamps with. That shared range is the fix for the dead -// travel in #396 — the vertical slider used to advertise ±45 while the bottom -// anchor could only honour −45…+3 — so what these tests pin is the agreement -// between what the slider offers and what the band can do, not any one number. +// The placement controls after the anchor redesign. What these pin is the property +// the previous UI could not hold: every control names the edge it measures from, and +// nothing it can produce is a signed number or a dead affordance. +// +// The pane it replaced had four controls that overlapped — a band width nothing drew, +// an offset measured against that invisible band, and a text alignment fighting the +// offset for the same visual outcome — so the tests here are as much about what is +// ABSENT as about what is present. import "@testing-library/jest-dom"; import { cleanup, fireEvent, render, screen } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { I18nProvider } from "@/contexts/I18nContext"; -import { - captionBandRect, - captionInkHeightPct, - captionOffsetRange, - DEFAULT_CAPTION_SETTINGS, - getCaptionSettings, -} from "@/lib/ai-edition/captions"; +import { getCaptionSettings } from "@/lib/ai-edition/captions"; import type { AxcutAsset, AxcutDocument } from "@/lib/ai-edition/schema"; import { useProjectStore } from "@/lib/ai-edition/store/projectStore"; import { useTranscriptionStore } from "@/lib/ai-edition/store/transcriptionStore"; @@ -81,6 +78,8 @@ function sliderFor(label: string): HTMLInputElement { return input; } +const button = (name: string) => screen.getByRole("button", { name }); + function show(captions: Record) { const document = documentWith(captions); useProjectStore.setState({ @@ -108,119 +107,96 @@ afterEach(() => { }); describe("caption placement controls", () => { - it("offers both axes", () => { + it("offers one anchor and one distance per axis", () => { show({}); - expect(sliderFor("Vertical position")).toBeInTheDocument(); - expect(sliderFor("Horizontal position")).toBeInTheDocument(); + expect(button("Bottom")).toHaveAttribute("aria-pressed", "true"); + expect(button("Top")).toHaveAttribute("aria-pressed", "false"); + expect(button("Center")).toHaveAttribute("aria-pressed", "true"); + expect(sliderFor("Distance from bottom")).toBeInTheDocument(); }); - it.each([ - "top", - "middle", - "bottom", - ] as const)("bounds the %s anchor's slider by what the band can actually reach", (verticalPosition) => { - const settings = show({ verticalPosition }); - const range = captionOffsetRange(settings); - const slider = sliderFor("Vertical position"); - expect(Number(slider.min)).toBeCloseTo(range.y.min, 6); - expect(Number(slider.max)).toBeCloseTo(range.y.max, 6); - }); + it("names the edge the distance is measured from, and follows the anchor", () => { + // The old label said "Vertical offset" and the value could read "-7.3%", which + // corresponds to nothing in any subtitle format and to nothing a user can see. + show({ anchorV: "bottom" }); + expect(screen.getByText("Distance from bottom")).toBeInTheDocument(); + expect(screen.queryByText("Distance from top")).not.toBeInTheDocument(); - it("puts both ends of the range on a step, so the edges stay reachable", () => { - // A fixed step of 1 would leave `max` off-grid for these fractional bounds and - // the caption would stop just short of the frame edge — the #396 complaint. - const settings = show({ verticalPosition: "bottom" }); - const slider = sliderFor("Vertical position"); - const [min, max, step] = [slider.min, slider.max, slider.step].map(Number); - const steps = (max - min) / step; - expect(steps).toBeCloseTo(Math.round(steps), 6); - - // And landing on `max` really does put the ink on the frame's bottom edge — - // with the empty part of the band hanging off it, which is what buys the reach. - const band = captionBandRect({ ...settings, offsetY: max }); - expect(band.y + band.height / 2 + captionInkHeightPct(settings) / 2).toBeCloseTo(100, 6); - expect(band.y + band.height).toBeGreaterThan(100); + fireEvent.click(button("Top")); + expect(screen.getByText("Distance from top")).toBeInTheDocument(); + expect(screen.queryByText("Distance from bottom")).not.toBeInTheDocument(); }); - it("disables the horizontal slider only when the band fills the frame", () => { - show({ width: 100 }); - expect(sliderFor("Horizontal position")).toBeDisabled(); - cleanup(); - show({ width: DEFAULT_CAPTION_SETTINGS.width }); - expect(sliderFor("Horizontal position")).toBeEnabled(); - }); -}); - -describe("caption position presets", () => { - const preset = (label: string) => screen.getByRole("button", { name: label }); - - it("shows the default settings' presets pressed: Bottom and Position center", () => { + it("never offers a negative distance", () => { show({}); - expect(preset("Bottom")).toHaveAttribute("aria-pressed", "true"); - expect(preset("Top")).toHaveAttribute("aria-pressed", "false"); - expect(preset("Position center")).toHaveAttribute("aria-pressed", "true"); - expect(preset("Position left")).toHaveAttribute("aria-pressed", "false"); + expect(Number(sliderFor("Distance from bottom").min)).toBe(0); + fireEvent.click(button("Left")); + expect(Number(sliderFor("Distance from left").min)).toBe(0); }); - it("clicking a vertical preset resets the vertical slider and lights that preset up", () => { - show({ verticalPosition: "bottom", offsetY: -20 }); - expect(preset("Bottom")).toHaveAttribute("aria-pressed", "false"); + it("keeps the distance when the anchor flips, mirroring to the opposite edge", () => { + // The inset means the same thing on both anchors, so there is nothing to reset — + // unlike the old presets, which had to zero an offset that meant something else. + show({ anchorV: "bottom", insetY: 12 }); + fireEvent.click(button("Top")); + expect(sliderFor("Distance from top")).toHaveValue("12"); + }); - fireEvent.click(preset("Top")); + it("hides the horizontal distance when centred instead of disabling it", () => { + // A centred block has no edge to measure from. A dead slider reads as a bug, so + // the control is absent rather than greyed out. + show({ anchorH: "center" }); + expect(screen.queryByText("Distance from left")).not.toBeInTheDocument(); + expect(screen.queryByText("Distance from right")).not.toBeInTheDocument(); - expect(sliderFor("Vertical position")).toHaveValue("0"); - expect(preset("Top")).toHaveAttribute("aria-pressed", "true"); - expect(preset("Bottom")).toHaveAttribute("aria-pressed", "false"); + fireEvent.click(button("Right")); + expect(sliderFor("Distance from right")).toBeEnabled(); }); - it("dragging the vertical slider clears every vertical preset's pressed state", () => { + it("leaves no control disabled once a document is open", () => { show({}); - expect(preset("Bottom")).toHaveAttribute("aria-pressed", "true"); - - fireEvent.change(sliderFor("Vertical position"), { target: { value: "-10" } }); - - expect(preset("Bottom")).toHaveAttribute("aria-pressed", "false"); - expect(preset("Top")).toHaveAttribute("aria-pressed", "false"); - expect(preset("Middle")).toHaveAttribute("aria-pressed", "false"); + for (const name of ["Bottom", "Top", "Left", "Center", "Right"]) { + expect(button(name)).toBeEnabled(); + } + expect(sliderFor("Distance from bottom")).toBeEnabled(); }); - it("clicking Position left/right moves the horizontal slider to the true frame edge", () => { - const settings = show({}); - const range = captionOffsetRange(settings); - - fireEvent.click(preset("Position left")); - expect(Number(sliderFor("Horizontal position").value)).toBeCloseTo(range.x.min, 6); - expect(preset("Position left")).toHaveAttribute("aria-pressed", "true"); + it("writes the anchor and the inset straight through to the document", () => { + show({}); + fireEvent.click(button("Top")); + fireEvent.change(sliderFor("Distance from top"), { target: { value: "18.5" } }); - fireEvent.click(preset("Position right")); - expect(Number(sliderFor("Horizontal position").value)).toBeCloseTo(range.x.max, 6); - expect(preset("Position right")).toHaveAttribute("aria-pressed", "true"); - expect(preset("Position left")).toHaveAttribute("aria-pressed", "false"); + const stored = useProjectStore.getState().document as AxcutDocument; + expect(getCaptionSettings(stored)).toMatchObject({ anchorV: "top", insetY: 18.5 }); }); - it("dragging the horizontal slider clears the horizontal preset row", () => { + it("no longer offers the controls the redesign removed", () => { + // Band width drew nothing until the text happened to wrap; the separate text + // alignment fought the horizontal position for the same outcome. show({}); - fireEvent.change(sliderFor("Horizontal position"), { target: { value: "3" } }); - - expect(preset("Position center")).toHaveAttribute("aria-pressed", "false"); - expect(preset("Position left")).toHaveAttribute("aria-pressed", "false"); - expect(preset("Position right")).toHaveAttribute("aria-pressed", "false"); + expect(screen.queryByText("Width")).not.toBeInTheDocument(); + expect(screen.queryByText("Text align")).not.toBeInTheDocument(); + expect(screen.queryByText("Vertical offset")).not.toBeInTheDocument(); + expect(screen.queryByText("Horizontal offset")).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Middle" })).not.toBeInTheDocument(); }); - it("disables the horizontal preset row exactly when the horizontal slider is disabled", () => { - show({ width: 100 }); - expect(preset("Position left")).toBeDisabled(); - cleanup(); - show({ width: DEFAULT_CAPTION_SETTINGS.width }); - expect(preset("Position left")).toBeEnabled(); + it("explains which way a long caption grows", () => { + show({ anchorV: "bottom" }); + expect(screen.getByText(/grow upward/i)).toBeInTheDocument(); + fireEvent.click(button("Top")); + expect(screen.getByText(/grow downward/i)).toBeInTheDocument(); }); +}); - it("gives the text-align row its own section label, separate from Position", () => { - show({}); - expect(screen.getByText("Text align")).toBeInTheDocument(); - // The words "Left"/"Center"/"Right" belong to text-align; "Position left" etc. - // belong to the new row — both must resolve without ambiguity. - expect(preset("Left")).toBeInTheDocument(); - expect(preset("Position left")).toBeInTheDocument(); +describe("migrating a pre-anchor project into the pane", () => { + it("opens an old document on the anchor that reproduces where it was drawn", () => { + // A default bottom caption from the old model: band at 75%, ink centred in it, + // drawn block ending at 92.67% — so a 7.33% inset from the bottom. + show({ verticalPosition: "bottom", offsetY: 0, width: 80, textAlign: "center" }); + expect(button("Bottom")).toHaveAttribute("aria-pressed", "true"); + // The migrated value is the real distance, not a value snapped to the slider's + // step — the step governs dragging, not what a document may already hold. + expect(Number(sliderFor("Distance from bottom").value)).toBeCloseTo(7.333, 2); }); }); diff --git a/src/components/ai-edition/CaptionsPane.tsx b/src/components/ai-edition/CaptionsPane.tsx index 6fc342a74..6d6a776c9 100644 --- a/src/components/ai-edition/CaptionsPane.tsx +++ b/src/components/ai-edition/CaptionsPane.tsx @@ -9,28 +9,13 @@ // translation is stored beside the transcript, keyed by segment id, and picking // "Original" goes straight back to the SSOT text. -import type { LucideIcon } from "lucide-react"; -import { - AlignHorizontalJustifyCenter, - AlignHorizontalJustifyEnd, - AlignHorizontalJustifyStart, - Captions as CaptionsIcon, - Languages, - Loader2, - Trash2, -} from "lucide-react"; +import { Captions as CaptionsIcon, Languages, Loader2, Trash2 } from "lucide-react"; import { useMemo, useState } from "react"; import { useScopedT } from "@/contexts/I18nContext"; -import type { - CaptionHorizontalPosition, - CaptionTextAlign, - CaptionVerticalPosition, -} from "@/lib/ai-edition/captions"; +import type { CaptionAnchorH, CaptionAnchorV } from "@/lib/ai-edition/captions"; import { - activeHorizontalPositionPreset, - activeVerticalPositionPreset, - captionHorizontalPositionOffset, - captionOffsetRange, + CAPTION_INSET_X_MAX, + CAPTION_INSET_Y_MAX, untranslatedUnits, } from "@/lib/ai-edition/captions"; import { useProjectStore } from "@/lib/ai-edition/store/projectStore"; @@ -44,16 +29,6 @@ import { ColorField } from "./ColorField"; import styles from "./NewEditorShell.module.css"; import { SliderCell, Toggle } from "./RightPanes"; -/** A hundred stops across whatever span the offset currently has. - * - * The bounds are geometry, so they are rarely round numbers. A fixed `step` of 1 - * would leave `max` unreachable whenever the span isn't a whole number of steps — - * the caption would stop just short of the frame edge, which is the very thing - * #396 is about. Deriving the step from the span puts both ends exactly on a stop. */ -function sliderStep(range: { min: number; max: number }): number { - return Math.max((range.max - range.min) / 100, Number.EPSILON); -} - /** The families `src/index.css` already loads for on-canvas text — anything else * would render in the preview but fall back to a default in the export canvas. */ const CAPTION_FONTS = [ @@ -142,11 +117,6 @@ export function CaptionsPane() { const disabled = !hasDocument; const languageOptions = useMemo(() => Object.values(translations), [translations]); - // The reach depends on the anchor, the width and the font size, so it moves as the - // user works. Taking the sliders' bounds from the same function the geometry clamps - // with is what keeps every position on them a position the band can actually take. - const offsetRange = useMemo(() => captionOffsetRange(settings), [settings]); - const handleTranslate = async () => { const doc = useProjectStore.getState().document; if (!doc) return; @@ -478,98 +448,84 @@ export function CaptionsPane() { ) : null} {/* ── Placement ──────────────────────────────────────────── */} + {/* One control per axis, each naming the edge it measures from. The old pane + had four that overlapped: a band width nothing drew, an offset measured + against that invisible band, and a text alignment fighting the offset for + the same visual outcome. */}
{t("captions.position")}
- - value={activeVerticalPositionPreset(settings)} + + value={settings.anchorV} disabled={disabled} options={[ - { value: "top", label: t("captions.positionTop") }, - { value: "middle", label: t("captions.positionMiddle") }, - { value: "bottom", label: t("captions.positionBottom") }, + { value: "bottom", label: t("captions.anchorBottom") }, + { value: "top", label: t("captions.anchorTop") }, ]} - // A preset button is a shortcut to a clean position, not a nudge on top - // of one — resetting the offset is what makes clicking it feel like - // "go here" instead of "go here, plus whatever was left over". - onChange={(verticalPosition) => void set({ verticalPosition, offsetY: 0 })} - /> - - value={activeHorizontalPositionPreset(settings)} - // Mirrors the horizontal slider's own disabled condition just below: a - // full-width band has nowhere left or right to go. - disabled={disabled || offsetRange.x.max <= offsetRange.x.min} - options={[ - { - value: "left", - label: t("captions.positionLeft"), - icon: AlignHorizontalJustifyStart, - }, - { - value: "center", - label: t("captions.positionCenter"), - icon: AlignHorizontalJustifyCenter, - }, - { - value: "right", - label: t("captions.positionRight"), - icon: AlignHorizontalJustifyEnd, - }, - ]} - onChange={(preset) => - void set({ offsetX: captionHorizontalPositionOffset(settings, preset) }) - } + // No offset to reset: the inset means the same thing on both anchors, so + // flipping mirrors the caption to the same distance from the opposite edge. + onChange={(anchorV) => void set({ anchorV })} /> +

+ {settings.anchorV === "bottom" + ? t("captions.anchorHintBottom") + : t("captions.anchorHintTop")} +

setLive({ offsetY: v })} - onCommit={() => void commit()} - /> - setLive({ offsetX: v })} - onCommit={() => void commit()} - /> - setLive({ width: v })} + onChange={(v) => setLive({ insetY: v })} onCommit={() => void commit()} />
- {/* ── Text align (inside the band — a different axis from Position) ── */} -
{t("captions.textAlign")}
- - value={settings.textAlign} + + value={settings.anchorH} disabled={disabled} options={[ { value: "left", label: t("captions.alignLeft") }, { value: "center", label: t("captions.alignCenter") }, { value: "right", label: t("captions.alignRight") }, ]} - onChange={(textAlign) => void set({ textAlign })} + onChange={(anchorH) => void set({ anchorH })} /> + {/* Centre has no edge to measure from, so the control is ABSENT rather than + disabled — a dead slider reads as a bug. */} + {settings.anchorH === "center" ? null : ( +
+ setLive({ insetX: v })} + onCommit={() => void commit()} + /> +
+ )} {/* ── Line length ────────────────────────────────────────── */}
{t("captions.lineLength")}
@@ -636,39 +592,25 @@ function Segmented({ disabled, onChange, }: { - /** `null` means no option is currently active — e.g. a free-dragged slider - * has moved off every preset this row offers. */ - value: T | null; - options: ReadonlyArray<{ - value: T; - label: string; - /** Renders in place of the text label when given (with `label` still used - * as the accessible name and hover title) — for a row that would otherwise - * repeat another row's words for a different axis of meaning. */ - icon?: LucideIcon; - }>; + value: T; + options: ReadonlyArray<{ value: T; label: string }>; disabled?: boolean; onChange: (next: T) => void; }) { return (
- {options.map((option) => { - const Icon = option.icon; - return ( - - ); - })} + {options.map((option) => ( + + ))}
); } diff --git a/src/i18n/locales/ar/settings.json b/src/i18n/locales/ar/settings.json index c19b80bce..2e025e9bd 100644 --- a/src/i18n/locales/ar/settings.json +++ b/src/i18n/locales/ar/settings.json @@ -299,19 +299,17 @@ "backgroundColor": "لون الخلفية", "backgroundOpacity": "العتامة", "position": "الموضع", - "positionTop": "أعلى", - "positionMiddle": "الوسط", - "positionBottom": "أسفل", - "positionLeft": "الموضع الأيسر", - "positionCenter": "الموضع الأوسط", - "positionRight": "الموضع الأيمن", - "textAlign": "محاذاة النص", + "anchorBottom": "أسفل", + "anchorTop": "أعلى", + "anchorHintBottom": "الترجمات الطويلة تمتد إلى أعلى — الحافة السفلية لا تتحرك.", + "anchorHintTop": "الترجمات الطويلة تمتد إلى أسفل — الحافة العلوية لا تتحرك.", + "distanceFromBottom": "المسافة من الأسفل", + "distanceFromTop": "المسافة من الأعلى", + "distanceFromLeft": "المسافة من اليسار", + "distanceFromRight": "المسافة من اليمين", "alignLeft": "يسار", "alignCenter": "توسيط", "alignRight": "يمين", - "verticalOffset": "الموضع الرأسي", - "horizontalOffset": "الموضع الأفقي", - "width": "العرض", "lineLength": "طول السطر", "minWords": "أقل عدد كلمات في السطر", "maxWords": "أكثر عدد كلمات في السطر" diff --git a/src/i18n/locales/en/settings.json b/src/i18n/locales/en/settings.json index 25430a23e..f6149858d 100644 --- a/src/i18n/locales/en/settings.json +++ b/src/i18n/locales/en/settings.json @@ -305,19 +305,17 @@ "backgroundColor": "Background color", "backgroundOpacity": "Opacity", "position": "Position", - "positionTop": "Top", - "positionMiddle": "Middle", - "positionBottom": "Bottom", - "positionLeft": "Position left", - "positionCenter": "Position center", - "positionRight": "Position right", - "textAlign": "Text align", + "anchorBottom": "Bottom", + "anchorTop": "Top", + "anchorHintBottom": "Long captions grow upward — the bottom edge stays put.", + "anchorHintTop": "Long captions grow downward — the top edge stays put.", + "distanceFromBottom": "Distance from bottom", + "distanceFromTop": "Distance from top", + "distanceFromLeft": "Distance from left", + "distanceFromRight": "Distance from right", "alignLeft": "Left", "alignCenter": "Center", "alignRight": "Right", - "verticalOffset": "Vertical position", - "horizontalOffset": "Horizontal position", - "width": "Width", "lineLength": "Line length", "minWords": "Min words per line", "maxWords": "Max words per line" diff --git a/src/i18n/locales/es/settings.json b/src/i18n/locales/es/settings.json index 93ac340d9..4628154f6 100644 --- a/src/i18n/locales/es/settings.json +++ b/src/i18n/locales/es/settings.json @@ -299,19 +299,17 @@ "backgroundColor": "Color del fondo", "backgroundOpacity": "Opacidad", "position": "Posición", - "positionTop": "Arriba", - "positionMiddle": "Centro", - "positionBottom": "Abajo", - "positionLeft": "Posición izquierda", - "positionCenter": "Posición central", - "positionRight": "Posición derecha", - "textAlign": "Alineación del texto", + "anchorBottom": "Abajo", + "anchorTop": "Arriba", + "anchorHintBottom": "Los subtítulos largos crecen hacia arriba: el borde inferior no se mueve.", + "anchorHintTop": "Los subtítulos largos crecen hacia abajo: el borde superior no se mueve.", + "distanceFromBottom": "Distancia desde abajo", + "distanceFromTop": "Distancia desde arriba", + "distanceFromLeft": "Distancia desde la izquierda", + "distanceFromRight": "Distancia desde la derecha", "alignLeft": "Izquierda", "alignCenter": "Centro", "alignRight": "Derecha", - "verticalOffset": "Posición vertical", - "horizontalOffset": "Posición horizontal", - "width": "Ancho", "lineLength": "Longitud de línea", "minWords": "Mín. palabras por línea", "maxWords": "Máx. palabras por línea" diff --git a/src/i18n/locales/fr/settings.json b/src/i18n/locales/fr/settings.json index 6a8ed73d3..e9521d9d2 100644 --- a/src/i18n/locales/fr/settings.json +++ b/src/i18n/locales/fr/settings.json @@ -299,19 +299,17 @@ "backgroundColor": "Couleur du fond", "backgroundOpacity": "Opacité", "position": "Position", - "positionTop": "Haut", - "positionMiddle": "Milieu", - "positionBottom": "Bas", - "positionLeft": "Position à gauche", - "positionCenter": "Position au centre", - "positionRight": "Position à droite", - "textAlign": "Alignement du texte", + "anchorBottom": "Bas", + "anchorTop": "Haut", + "anchorHintBottom": "Les sous-titres longs s'étendent vers le haut — le bord bas ne bouge pas.", + "anchorHintTop": "Les sous-titres longs s'étendent vers le bas — le bord haut ne bouge pas.", + "distanceFromBottom": "Distance depuis le bas", + "distanceFromTop": "Distance depuis le haut", + "distanceFromLeft": "Distance depuis la gauche", + "distanceFromRight": "Distance depuis la droite", "alignLeft": "Gauche", "alignCenter": "Centre", "alignRight": "Droite", - "verticalOffset": "Position verticale", - "horizontalOffset": "Position horizontale", - "width": "Largeur", "lineLength": "Longueur des lignes", "minWords": "Mots min. par ligne", "maxWords": "Mots max. par ligne" diff --git a/src/i18n/locales/it/settings.json b/src/i18n/locales/it/settings.json index 37a585b2d..ff13f7495 100644 --- a/src/i18n/locales/it/settings.json +++ b/src/i18n/locales/it/settings.json @@ -299,19 +299,17 @@ "backgroundColor": "Colore dello sfondo", "backgroundOpacity": "Opacità", "position": "Posizione", - "positionTop": "Alto", - "positionMiddle": "Centro", - "positionBottom": "Basso", - "positionLeft": "Posizione a sinistra", - "positionCenter": "Posizione centrale", - "positionRight": "Posizione a destra", - "textAlign": "Allineamento testo", + "anchorBottom": "Basso", + "anchorTop": "Alto", + "anchorHintBottom": "I sottotitoli lunghi crescono verso l'alto: il bordo inferiore non si sposta.", + "anchorHintTop": "I sottotitoli lunghi crescono verso il basso: il bordo superiore non si sposta.", + "distanceFromBottom": "Distanza dal basso", + "distanceFromTop": "Distanza dall'alto", + "distanceFromLeft": "Distanza da sinistra", + "distanceFromRight": "Distanza da destra", "alignLeft": "Sinistra", "alignCenter": "Centro", "alignRight": "Destra", - "verticalOffset": "Posizione verticale", - "horizontalOffset": "Posizione orizzontale", - "width": "Larghezza", "lineLength": "Lunghezza riga", "minWords": "Parole min. per riga", "maxWords": "Parole max. per riga" diff --git a/src/i18n/locales/ja-JP/settings.json b/src/i18n/locales/ja-JP/settings.json index 02f27575e..30a9d0875 100644 --- a/src/i18n/locales/ja-JP/settings.json +++ b/src/i18n/locales/ja-JP/settings.json @@ -299,19 +299,17 @@ "backgroundColor": "背景色", "backgroundOpacity": "不透明度", "position": "位置", - "positionTop": "上", - "positionMiddle": "中央", - "positionBottom": "下", - "positionLeft": "左配置", - "positionCenter": "中央配置", - "positionRight": "右配置", - "textAlign": "文字揃え", + "anchorBottom": "下", + "anchorTop": "上", + "anchorHintBottom": "長い字幕は上に伸びます（下端は動きません）。", + "anchorHintTop": "長い字幕は下に伸びます（上端は動きません）。", + "distanceFromBottom": "下端からの距離", + "distanceFromTop": "上端からの距離", + "distanceFromLeft": "左端からの距離", + "distanceFromRight": "右端からの距離", "alignLeft": "左", "alignCenter": "中央", "alignRight": "右", - "verticalOffset": "垂直位置", - "horizontalOffset": "水平位置", - "width": "幅", "lineLength": "行の長さ", "minWords": "1 行の最小単語数", "maxWords": "1 行の最大単語数" diff --git a/src/i18n/locales/ko-KR/settings.json b/src/i18n/locales/ko-KR/settings.json index 69e4b35e1..676fe6959 100644 --- a/src/i18n/locales/ko-KR/settings.json +++ b/src/i18n/locales/ko-KR/settings.json @@ -299,19 +299,17 @@ "backgroundColor": "배경 색", "backgroundOpacity": "불투명도", "position": "위치", - "positionTop": "위", - "positionMiddle": "가운데", - "positionBottom": "아래", - "positionLeft": "왼쪽 배치", - "positionCenter": "가운데 배치", - "positionRight": "오른쪽 배치", - "textAlign": "텍스트 정렬", + "anchorBottom": "아래", + "anchorTop": "위", + "anchorHintBottom": "긴 자막은 위로 늘어납니다 — 아래쪽 가장자리는 그대로입니다.", + "anchorHintTop": "긴 자막은 아래로 늘어납니다 — 위쪽 가장자리는 그대로입니다.", + "distanceFromBottom": "아래에서의 거리", + "distanceFromTop": "위에서의 거리", + "distanceFromLeft": "왼쪽에서의 거리", + "distanceFromRight": "오른쪽에서의 거리", "alignLeft": "왼쪽", "alignCenter": "가운데", "alignRight": "오른쪽", - "verticalOffset": "세로 위치", - "horizontalOffset": "가로 위치", - "width": "너비", "lineLength": "줄 길이", "minWords": "줄당 최소 단어 수", "maxWords": "줄당 최대 단어 수" diff --git a/src/i18n/locales/pt-BR/settings.json b/src/i18n/locales/pt-BR/settings.json index acbd178b5..b6d4a78c4 100644 --- a/src/i18n/locales/pt-BR/settings.json +++ b/src/i18n/locales/pt-BR/settings.json @@ -299,19 +299,17 @@ "backgroundColor": "Cor do fundo", "backgroundOpacity": "Opacidade", "position": "Posição", - "positionTop": "Topo", - "positionMiddle": "Meio", - "positionBottom": "Base", - "positionLeft": "Posição à esquerda", - "positionCenter": "Posição central", - "positionRight": "Posição à direita", - "textAlign": "Alinhamento do texto", + "anchorBottom": "Base", + "anchorTop": "Topo", + "anchorHintBottom": "Legendas longas crescem para cima — a borda inferior não se move.", + "anchorHintTop": "Legendas longas crescem para baixo — a borda superior não se move.", + "distanceFromBottom": "Distância da base", + "distanceFromTop": "Distância do topo", + "distanceFromLeft": "Distância da esquerda", + "distanceFromRight": "Distância da direita", "alignLeft": "Esquerda", "alignCenter": "Centro", "alignRight": "Direita", - "verticalOffset": "Posição vertical", - "horizontalOffset": "Posição horizontal", - "width": "Largura", "lineLength": "Comprimento da linha", "minWords": "Mín. de palavras por linha", "maxWords": "Máx. de palavras por linha" diff --git a/src/i18n/locales/ru/settings.json b/src/i18n/locales/ru/settings.json index b070f0b95..d9c06b71c 100644 --- a/src/i18n/locales/ru/settings.json +++ b/src/i18n/locales/ru/settings.json @@ -299,19 +299,17 @@ "backgroundColor": "Цвет фона", "backgroundOpacity": "Непрозрачность", "position": "Положение", - "positionTop": "Сверху", - "positionMiddle": "По центру", - "positionBottom": "Снизу", - "positionLeft": "Положение слева", - "positionCenter": "Положение по центру", - "positionRight": "Положение справа", - "textAlign": "Выравнивание текста", + "anchorBottom": "Снизу", + "anchorTop": "Сверху", + "anchorHintBottom": "Длинные субтитры растут вверх — нижний край остаётся на месте.", + "anchorHintTop": "Длинные субтитры растут вниз — верхний край остаётся на месте.", + "distanceFromBottom": "Отступ снизу", + "distanceFromTop": "Отступ сверху", + "distanceFromLeft": "Отступ слева", + "distanceFromRight": "Отступ справа", "alignLeft": "Слева", "alignCenter": "По центру", "alignRight": "Справа", - "verticalOffset": "Положение по вертикали", - "horizontalOffset": "Положение по горизонтали", - "width": "Ширина", "lineLength": "Длина строки", "minWords": "Мин. слов в строке", "maxWords": "Макс. слов в строке" diff --git a/src/i18n/locales/tr/settings.json b/src/i18n/locales/tr/settings.json index c5f1791e1..c593e4db7 100644 --- a/src/i18n/locales/tr/settings.json +++ b/src/i18n/locales/tr/settings.json @@ -299,19 +299,17 @@ "backgroundColor": "Arka plan rengi", "backgroundOpacity": "Saydamlık", "position": "Konum", - "positionTop": "Üst", - "positionMiddle": "Orta", - "positionBottom": "Alt", - "positionLeft": "Sol konum", - "positionCenter": "Orta konum", - "positionRight": "Sağ konum", - "textAlign": "Metin hizalama", + "anchorBottom": "Alt", + "anchorTop": "Üst", + "anchorHintBottom": "Uzun altyazılar yukarı doğru büyür — alt kenar yerinde kalır.", + "anchorHintTop": "Uzun altyazılar aşağı doğru büyür — üst kenar yerinde kalır.", + "distanceFromBottom": "Alttan uzaklık", + "distanceFromTop": "Üstten uzaklık", + "distanceFromLeft": "Soldan uzaklık", + "distanceFromRight": "Sağdan uzaklık", "alignLeft": "Sol", "alignCenter": "Orta", "alignRight": "Sağ", - "verticalOffset": "Dikey konum", - "horizontalOffset": "Yatay konum", - "width": "Genişlik", "lineLength": "Satır uzunluğu", "minWords": "Satır başına en az kelime", "maxWords": "Satır başına en çok kelime" diff --git a/src/i18n/locales/vi/settings.json b/src/i18n/locales/vi/settings.json index b87203235..176f13eb6 100644 --- a/src/i18n/locales/vi/settings.json +++ b/src/i18n/locales/vi/settings.json @@ -299,19 +299,17 @@ "backgroundColor": "Màu nền", "backgroundOpacity": "Độ mờ", "position": "Vị trí", - "positionTop": "Trên", - "positionMiddle": "Giữa", - "positionBottom": "Dưới", - "positionLeft": "Vị trí trái", - "positionCenter": "Vị trí giữa", - "positionRight": "Vị trí phải", - "textAlign": "Căn chỉnh văn bản", + "anchorBottom": "Dưới", + "anchorTop": "Trên", + "anchorHintBottom": "Phụ đề dài sẽ cao dần lên trên — cạnh dưới không đổi.", + "anchorHintTop": "Phụ đề dài sẽ dài dần xuống dưới — cạnh trên không đổi.", + "distanceFromBottom": "Khoảng cách từ dưới", + "distanceFromTop": "Khoảng cách từ trên", + "distanceFromLeft": "Khoảng cách từ trái", + "distanceFromRight": "Khoảng cách từ phải", "alignLeft": "Trái", "alignCenter": "Giữa", "alignRight": "Phải", - "verticalOffset": "Vị trí dọc", - "horizontalOffset": "Vị trí ngang", - "width": "Chiều rộng", "lineLength": "Độ dài dòng", "minWords": "Số từ tối thiểu mỗi dòng", "maxWords": "Số từ tối đa mỗi dòng" diff --git a/src/i18n/locales/zh-CN/settings.json b/src/i18n/locales/zh-CN/settings.json index 7ab2ceef9..19f231703 100644 --- a/src/i18n/locales/zh-CN/settings.json +++ b/src/i18n/locales/zh-CN/settings.json @@ -299,19 +299,17 @@ "backgroundColor": "背景颜色", "backgroundOpacity": "不透明度", "position": "位置", - "positionTop": "顶部", - "positionMiddle": "中间", - "positionBottom": "底部", - "positionLeft": "左侧位置", - "positionCenter": "居中位置", - "positionRight": "右侧位置", - "textAlign": "文字对齐", + "anchorBottom": "底部", + "anchorTop": "顶部", + "anchorHintBottom": "较长的字幕向上延伸——底边保持不动。", + "anchorHintTop": "较长的字幕向下延伸——顶边保持不动。", + "distanceFromBottom": "距底部", + "distanceFromTop": "距顶部", + "distanceFromLeft": "距左侧", + "distanceFromRight": "距右侧", "alignLeft": "左对齐", "alignCenter": "居中", "alignRight": "右对齐", - "verticalOffset": "垂直位置", - "horizontalOffset": "水平位置", - "width": "宽度", "lineLength": "行长", "minWords": "每行最少词数", "maxWords": "每行最多词数" diff --git a/src/i18n/locales/zh-TW/settings.json b/src/i18n/locales/zh-TW/settings.json index 147d71233..e2bc412e9 100644 --- a/src/i18n/locales/zh-TW/settings.json +++ b/src/i18n/locales/zh-TW/settings.json @@ -300,19 +300,17 @@ "backgroundColor": "背景顏色", "backgroundOpacity": "不透明度", "position": "位置", - "positionTop": "上", - "positionMiddle": "中", - "positionBottom": "下", - "positionLeft": "靠左位置", - "positionCenter": "置中位置", - "positionRight": "靠右位置", - "textAlign": "文字對齊", + "anchorBottom": "下", + "anchorTop": "上", + "anchorHintBottom": "較長的字幕會向上延伸——下緣維持不動。", + "anchorHintTop": "較長的字幕會向下延伸——上緣維持不動。", + "distanceFromBottom": "距下緣", + "distanceFromTop": "距上緣", + "distanceFromLeft": "距左緣", + "distanceFromRight": "距右緣", "alignLeft": "靠左", "alignCenter": "置中", "alignRight": "靠右", - "verticalOffset": "垂直位置", - "horizontalOffset": "水平位置", - "width": "寬度", "lineLength": "行長", "minWords": "每行最少字數", "maxWords": "每行最多字數" diff --git a/src/lib/ai-edition/captions/captions.test.ts b/src/lib/ai-edition/captions/captions.test.ts index d6529489e..9179a3df4 100644 --- a/src/lib/ai-edition/captions/captions.test.ts +++ b/src/lib/ai-edition/captions/captions.test.ts @@ -1,18 +1,12 @@ import { describe, expect, it } from "vitest"; import type { AxcutDocument, AxcutTranscript } from "../schema"; import { captionCuesToTextRegions, deriveCaptionCues } from "./cues"; -import type { CaptionSettings, CaptionSettingsPatch } from "./settings"; import { - activeHorizontalPositionPreset, - activeVerticalPositionPreset, - CAPTION_BAND_HEIGHT_PCT, - CAPTION_POSITION_PRESET_EPSILON, captionBackgroundCss, - captionBandRect, - captionHorizontalPositionOffset, - captionInkHeightPct, - captionOffsetRange, + captionBoxRect, + captionSafeColumn, DEFAULT_CAPTION_SETTINGS, + defaultCaptionInsetY, getCaptionSettings, patchCaptionSettings, } from "./settings"; @@ -98,6 +92,18 @@ function doc(overrides: Partial = {}): AxcutDocument { } const ON = { ...DEFAULT_CAPTION_SETTINGS, enabled: true }; +const LANDSCAPE = 16 / 9; +const PORTRAIT = 9 / 16; + +/** Where the DRAWN block's edges land, given the box and the edge it is pinned to. + * The renderers put the block flush against the anchored edge of the box, so this + * is the same arithmetic the rasterizers do — expressed once, here, so the tests + * assert the thing the viewer sees rather than the box it lives in. */ +function drawnEdges(settings: typeof ON, aspect: number, blockHeightPct: number) { + const box = captionBoxRect(settings, aspect); + const top = box.verticalAlign === "bottom" ? box.y + box.height - blockHeightPct : box.y; + return { top, bottom: top + blockHeightPct }; +} describe("caption settings", () => { it("defaults to hidden so an existing project doesn't sprout captions on upgrade", () => { @@ -105,8 +111,8 @@ describe("caption settings", () => { }); it("round-trips a patch through the legacyEditor envelope", () => { - const next = patchCaptionSettings(doc(), { enabled: true, fontSize: 44, offsetY: -10 }); - expect(getCaptionSettings(next)).toMatchObject({ enabled: true, fontSize: 44, offsetY: -10 }); + const next = patchCaptionSettings(doc(), { enabled: true, fontSize: 44, insetY: 12 }); + expect(getCaptionSettings(next)).toMatchObject({ enabled: true, fontSize: 44, insetY: 12 }); }); it("keeps an explicit null language instead of falling back to the default", () => { @@ -122,209 +128,161 @@ describe("caption settings", () => { expect(getCaptionSettings(next)).toMatchObject({ minWordsPerLine: 3, maxWordsPerLine: 9 }); }); - it("leaves an untouched project exactly where it was", () => { - // The frame/screen-rect reinterpretation is the only intended visual change. The - // anchor arithmetic must not move on top of it, or every existing project shifts. - expect(captionBandRect({ ...ON, verticalPosition: "top" })).toMatchObject({ x: 10, y: 3 }); - expect(captionBandRect({ ...ON, verticalPosition: "middle" })).toMatchObject({ x: 10, y: 39 }); - expect(captionBandRect({ ...ON, verticalPosition: "bottom" })).toMatchObject({ x: 10, y: 75 }); + it("folds the opacity into the background colour, and reports 'transparent' when off", () => { + expect( + captionBackgroundCss({ ...ON, backgroundColor: "#10b981", backgroundOpacity: 0.5 }), + ).toBe("rgba(16, 185, 129, 0.5)"); + expect(captionBackgroundCss({ ...ON, backgroundEnabled: false })).toBe("transparent"); }); +}); - it("pushes the ink onto the frame edge, and no further", () => { - // The band is a 22% box whose text the renderers centre, so the box has to hang off - // the frame for the glyphs to reach the edge — asserting the BOX stays inside is what - // used to stop the caption a half-band short (#396). What must stay inside is the ink. - for (const verticalPosition of ["top", "middle", "bottom"] as const) { - const settings = { ...ON, verticalPosition }; - const range = captionOffsetRange(settings); - const half = captionInkHeightPct(settings) / 2; - - const low = captionBandRect({ ...settings, offsetY: range.y.min }); - expect(low.y + low.height / 2 - half).toBeCloseTo(0, 6); - expect(low.y).toBeLessThan(0); - - const high = captionBandRect({ ...settings, offsetY: range.y.max }); - expect(high.y + high.height / 2 + half).toBeCloseTo(100, 6); - expect(high.y + high.height).toBeGreaterThan(100); +describe("caption anchoring", () => { + // THE invariant of the redesign, asserted as a property rather than as numbers. + // The old model placed a fixed 22% band and let each renderer centre the ink in it, + // so the drawn block's edges moved with the font size, the background, the wrap — + // which is why widening the band shifted the caption vertically and why the offset + // had to be a signed number clamped against an estimate. + it("pins the anchored edge for every font size, background state and inset", () => { + for (const anchorV of ["bottom", "top"] as const) { + for (const insetY of [0, 0.5, 5, 12.5, 33.3, 50]) { + for (const fontSize of [12, 48, 120, 200]) { + for (const backgroundEnabled of [true, false]) { + const settings = { ...ON, anchorV, insetY, fontSize, backgroundEnabled }; + const box = captionBoxRect(settings, LANDSCAPE); + const pinned = anchorV === "bottom" ? box.y + box.height : box.y; + expect(pinned).toBeCloseTo(anchorV === "bottom" ? 100 - insetY : insetY, 6); + // And the box itself never leaves the frame, so there is no overhang to + // compensate for and no negative `y` for the schema to reject. + expect(box.y).toBeGreaterThanOrEqual(-1e-9); + expect(box.y + box.height).toBeLessThanOrEqual(100 + 1e-9); + } + } + } } }); - it("clamps an offset the anchor cannot reach instead of drawing off-frame", () => { - const pushed = captionBandRect({ ...ON, verticalPosition: "bottom", offsetY: 100 }); - const capped = captionBandRect({ - ...ON, - verticalPosition: "bottom", - offsetY: captionOffsetRange(ON).y.max, - }); - expect(pushed).toEqual(capped); - }); - - it("lets the band reach the left and right frame edges, but never past them", () => { - const range = captionOffsetRange(ON); - expect(captionBandRect({ ...ON, offsetX: range.x.min }).x).toBeCloseTo(0, 6); - expect(captionBandRect({ ...ON, offsetX: range.x.max }).x).toBeCloseTo(100 - ON.width, 6); - expect(captionBandRect({ ...ON, offsetX: 100 }).x).toBeCloseTo(100 - ON.width, 6); + it("holds the anchored edge still while the block grows — the complaint this fixes", () => { + // One line vs three, same settings: the anchored edge must not move. Under the + // old geometry the block was centred, so BOTH edges moved and the subtitle + // visibly drifted whenever its text wrapped. + const bottom = { ...ON, anchorV: "bottom" as const, insetY: 5 }; + expect(drawnEdges(bottom, LANDSCAPE, 6).bottom).toBeCloseTo( + drawnEdges(bottom, LANDSCAPE, 18).bottom, + 6, + ); + const top = { ...ON, anchorV: "top" as const, insetY: 5 }; + expect(drawnEdges(top, LANDSCAPE, 6).top).toBeCloseTo(drawnEdges(top, LANDSCAPE, 18).top, 6); }); - it("gives a full-width band no horizontal travel to offer", () => { - const full = { ...ON, width: 100 }; - const range = captionOffsetRange(full); - expect(range.x.min).toBeCloseTo(0, 6); - expect(range.x.max).toBeCloseTo(0, 6); - expect(captionBandRect({ ...full, offsetX: 40 }).x).toBeCloseTo(0, 6); + it("tells the compositor which edge to pin", () => { + expect(captionBoxRect({ ...ON, anchorV: "bottom" }, LANDSCAPE).verticalAlign).toBe("bottom"); + expect(captionBoxRect({ ...ON, anchorV: "top" }, LANDSCAPE).verticalAlign).toBe("top"); }); - it("shrinks the reach as the font grows, so big captions still fit", () => { - // The guaranteed-visible slice is font-derived: a 200px caption fills the whole band, - // leaving nothing to spill, while a small one can hang most of the band off-frame. - const small = captionOffsetRange({ ...ON, fontSize: 12 }).y.max; - const large = captionOffsetRange({ ...ON, fontSize: 200 }).y.max; - expect(small).toBeGreaterThan(large); - expect(large).toBeCloseTo(100 - CAPTION_BAND_HEIGHT_PCT - 75, 6); + it("keeps the vertical placement independent of everything on the other axis", () => { + // The old `width` slider moved the caption vertically, because a narrower band + // wrapped more, and more lines grew a centred block in both directions. + const base = { ...ON, anchorV: "bottom" as const, insetY: 8 }; + const pinned = (s: typeof base) => { + const b = captionBoxRect(s, LANDSCAPE); + return b.y + b.height; + }; + expect(pinned({ ...base, anchorH: "left", insetX: 0 })).toBeCloseTo(pinned(base), 6); + expect(pinned({ ...base, anchorH: "right", insetX: 25 })).toBeCloseTo(pinned(base), 6); }); +}); - it("re-clamps the offsets against the geometry a patch just created", () => { - // A patch can move the reachable span itself — `width`, `fontSize`, - // `backgroundEnabled` and `verticalPosition` all do. Clamping only on read - // would leave the stored number outside the span until someone read it, and - // the next patch would write that stale number straight back out. - const wide = patchCaptionSettings(doc(), { enabled: true, width: 20 }); - const pushed = patchCaptionSettings(wide, { - offsetX: captionOffsetRange(getCaptionSettings(wide)).x.max, - }); - const narrowed = patchCaptionSettings(pushed, { width: 100 }); +describe("caption horizontal anchoring", () => { + it("pins the named edge, and centres between the column when asked to", () => { + const column = captionSafeColumn(LANDSCAPE); - const stored = (narrowed.legacyEditor as { captions: CaptionSettings }).captions; - expect(stored.offsetX).toBeCloseTo(0, 6); - expect(stored.offsetX).toBeCloseTo(getCaptionSettings(narrowed).offsetX, 6); - }); + const left = captionBoxRect({ ...ON, anchorH: "left", insetX: 4 }, LANDSCAPE); + expect(left.x).toBeCloseTo(4, 6); - it("re-clamps when the anchor moves, not just when the width does", () => { - const low = patchCaptionSettings(doc(), { enabled: true, verticalPosition: "bottom" }); - const pushed = patchCaptionSettings(low, { - offsetY: captionOffsetRange(getCaptionSettings(low)).y.min, - }); - const flipped = patchCaptionSettings(pushed, { verticalPosition: "top" }); - - const stored = (flipped.legacyEditor as { captions: CaptionSettings }).captions; - const range = captionOffsetRange(getCaptionSettings(flipped)); - expect(stored.offsetY).toBeGreaterThanOrEqual(range.y.min - 1e-9); - expect(stored.offsetY).toBeLessThanOrEqual(range.y.max + 1e-9); - expect(stored.offsetY).toBeCloseTo(getCaptionSettings(flipped).offsetY, 6); - }); - - // `fontSize` and `backgroundEnabled` reach the range the long way round, through - // the height of the drawn block: the taller the ink, the less of the band is - // empty, and the empty part is all the band is allowed to hang off the frame. - it.each([ - { field: "fontSize", grow: { fontSize: 200 } as CaptionSettingsPatch }, - { field: "backgroundEnabled", grow: { backgroundEnabled: true } as CaptionSettingsPatch }, - ] as const)("re-clamps when $field narrows the reach", ({ grow }) => { - // Start where the reach is widest, so growing the ink has something to take. - const roomy = patchCaptionSettings(doc(), { - enabled: true, - verticalPosition: "bottom", - fontSize: 12, - backgroundEnabled: false, - }); - const pushed = patchCaptionSettings(roomy, { - offsetY: captionOffsetRange(getCaptionSettings(roomy)).y.max, - }); - const grown = patchCaptionSettings(pushed, grow); + const right = captionBoxRect({ ...ON, anchorH: "right", insetX: 4 }, LANDSCAPE); + expect(right.x + right.width).toBeCloseTo(96, 6); - const range = captionOffsetRange(getCaptionSettings(grown)); - // The reach really did narrow — otherwise this proves nothing. - expect(range.y.max).toBeLessThan(captionOffsetRange(getCaptionSettings(pushed)).y.max); - const stored = (grown.legacyEditor as { captions: CaptionSettings }).captions; - expect(stored.offsetY).toBeLessThanOrEqual(range.y.max + 1e-9); - expect(stored.offsetY).toBeCloseTo(getCaptionSettings(grown).offsetY, 6); + const centre = captionBoxRect({ ...ON, anchorH: "center" }, LANDSCAPE); + expect(centre.x).toBeCloseTo((100 - column.width) / 2, 6); + expect(centre.width).toBeCloseTo(column.width, 6); }); - it("normalises an offset left over from another anchor on read", () => { - // The stored value, the slider position and the drawn band stay the same number. - const parked = patchCaptionSettings(doc(), { enabled: true, offsetY: 45 }); - const read = getCaptionSettings(parked); - expect(read.offsetY).toBeCloseTo(captionOffsetRange(read).y.max, 6); + it("ignores insetX entirely when centred — there is no edge to measure from", () => { + const a = captionBoxRect({ ...ON, anchorH: "center", insetX: 0 }, LANDSCAPE); + const b = captionBoxRect({ ...ON, anchorH: "center", insetX: 25 }, LANDSCAPE); + expect(a).toEqual(b); }); - it("folds the opacity into the background colour, and reports 'transparent' when off", () => { - expect( - captionBackgroundCss({ ...ON, backgroundColor: "#10b981", backgroundOpacity: 0.5 }), - ).toBe("rgba(16, 185, 129, 0.5)"); - expect(captionBackgroundCss({ ...ON, backgroundEnabled: false })).toBe("transparent"); + it("narrows the box rather than pushing it off-frame", () => { + // A 90%-wide portrait column pushed 25% in would otherwise end at 115%. + const box = captionBoxRect({ ...ON, anchorH: "left", insetX: 25 }, PORTRAIT); + expect(box.x + box.width).toBeLessThanOrEqual(100 + 1e-9); + expect(box.width).toBeCloseTo(75, 6); }); }); -describe("caption position presets", () => { - it("reads a vertical preset as active only while there's no nudge off it", () => { - for (const verticalPosition of ["top", "middle", "bottom"] as const) { - const settings = { ...ON, verticalPosition, offsetY: 0 }; - expect(activeVerticalPositionPreset(settings)).toBe(verticalPosition); - // Any nudge at all — even one too small to see — means the band is no - // longer exactly at the preset, so nothing should read as "active". - expect(activeVerticalPositionPreset({ ...settings, offsetY: 5 })).toBeNull(); +describe("caption safe column", () => { + it("follows the BBC line-length table, and stays inside title-safe", () => { + expect(captionSafeColumn(LANDSCAPE)).toEqual({ x: 16, width: 68 }); + expect(captionSafeColumn(PORTRAIT)).toEqual({ x: 5, width: 90 }); + for (const aspect of [LANDSCAPE, 1, PORTRAIT]) { + const c = captionSafeColumn(aspect); + expect(c.x + c.width).toBeCloseTo(100 - c.x, 6); } }); - it("reads left/center/right off offsetX, and null off the preset grid", () => { - expect(activeHorizontalPositionPreset(ON)).toBe("center"); - const range = captionOffsetRange(ON); - expect(activeHorizontalPositionPreset({ ...ON, offsetX: range.x.min })).toBe("left"); - expect(activeHorizontalPositionPreset({ ...ON, offsetX: range.x.max })).toBe("right"); - expect(activeHorizontalPositionPreset({ ...ON, offsetX: range.x.min / 2 })).toBeNull(); + it("defaults a vertical export well clear of the platform chrome", () => { + // 5% on a 9:16 export puts the caption under the Reels/TikTok profile row. + expect(defaultCaptionInsetY(LANDSCAPE)).toBe(5); + expect(defaultCaptionInsetY(PORTRAIT)).toBeGreaterThan(10); }); +}); + +describe("migrating a pre-anchor document", () => { + // The rule is reproduce the PIXELS, not the fields: the old band was a fixed 22% + // box with the ink centred in it, so where the caption was drawn is recoverable, + // and the nearer edge becomes the anchor. A migrated project must not visibly move. + const legacy = (captions: Record) => + getCaptionSettings( + doc({ legacyEditor: { captions: { enabled: true, ...captions } } } as Partial), + LANDSCAPE, + ); - it("collapses to center when the band is full-width, since left/right have nowhere to go", () => { - expect(activeHorizontalPositionPreset({ ...ON, width: 100, offsetX: 0 })).toBe("center"); + it("keeps a default bottom caption at the bottom", () => { + const s = legacy({ verticalPosition: "bottom", offsetY: 0 }); + expect(s.anchorV).toBe("bottom"); + // The old band sat at y=75 and its ink — 48px × (2 lines × 1.4em + 0.2em of + // plate) = 13.33% of frame height — was centred in the 22% box, so the drawn + // block ended at 92.67%. The migrated inset must reproduce exactly that edge. + expect(s.insetY).toBeCloseTo(7.333, 2); }); - it("still picks left/right over center when a near-full-width band squeezes them inside the epsilon", () => { - // At width this close to 100, range.x.min/max themselves fall inside - // CAPTION_POSITION_PRESET_EPSILON of 0 — checking "is this near center?" - // first would wrongly claim an offset that is exactly at the true edge. - const squeezed = { ...ON, width: 100 - 1e-7 }; - const range = captionOffsetRange(squeezed); - expect(Math.abs(range.x.min)).toBeLessThan(CAPTION_POSITION_PRESET_EPSILON); - expect(activeHorizontalPositionPreset({ ...squeezed, offsetX: range.x.min })).toBe("left"); - expect(activeHorizontalPositionPreset({ ...squeezed, offsetX: range.x.max })).toBe("right"); - expect(activeHorizontalPositionPreset({ ...squeezed, offsetX: 0 })).toBe("center"); + it("flips the anchor for a caption that had been dragged to the top", () => { + const s = legacy({ verticalPosition: "bottom", offsetY: -79.333 }); + expect(s.anchorV).toBe("top"); + expect(s.insetY).toBeCloseTo(0, 1); }); - it("sets offsetX to the true frame edge for left/right, matching the reachable range", () => { - const range = captionOffsetRange(ON); - expect(captionHorizontalPositionOffset(ON, "left")).toBeCloseTo(range.x.min, 6); - expect(captionHorizontalPositionOffset(ON, "center")).toBe(0); - expect(captionHorizontalPositionOffset(ON, "right")).toBeCloseTo(range.x.max, 6); + it("keeps a top caption at the top", () => { + const s = legacy({ verticalPosition: "top", offsetY: 0 }); + expect(s.anchorV).toBe("top"); }); - it("reaches the true left and right frame edges through the left/right presets", () => { - const left = captionBandRect({ ...ON, offsetX: captionHorizontalPositionOffset(ON, "left") }); - expect(left.x).toBeCloseTo(0, 6); - const right = captionBandRect({ - ...ON, - offsetX: captionHorizontalPositionOffset(ON, "right"), - }); - expect(right.x + right.width).toBeCloseTo(100, 6); + it("maps the old band+textAlign pair onto the single horizontal anchor", () => { + expect(legacy({ width: 80, offsetX: 0, textAlign: "center" }).anchorH).toBe("center"); + expect(legacy({ width: 40, offsetX: -30, textAlign: "left" }).anchorH).toBe("left"); + expect(legacy({ width: 40, offsetX: 30, textAlign: "right" }).anchorH).toBe("right"); }); - it("de-activates the horizontal preset when width moves the band, without touching offsetX", () => { - // The real asymmetry against the vertical axis: `range.x` moves with - // `width` (`captionAnchor.x` depends on it), so a preset that was flush - // can stop being flush purely because the band got narrower or wider. - // `offsetY === 0` has no such dependency, so a vertical preset never does - // this — it's intended, not a regression. - const atWidth80 = { - ...ON, - width: 80, - offsetX: captionOffsetRange({ ...ON, width: 80 }).x.min, - }; - expect(activeHorizontalPositionPreset(atWidth80)).toBe("left"); - expect(captionBandRect(atWidth80).x).toBeCloseTo(0, 6); + it("lets a stored anchor win, so the migration runs once and then stays out of the way", () => { + const s = legacy({ verticalPosition: "top", offsetY: 0, anchorV: "bottom", insetY: 9 }); + expect(s.anchorV).toBe("bottom"); + expect(s.insetY).toBe(9); + }); - const narrowed = { ...atWidth80, width: 50 }; - expect(activeHorizontalPositionPreset(narrowed)).toBeNull(); - expect(narrowed.offsetX).toBe(atWidth80.offsetX); - expect(captionBandRect(narrowed).x).toBeCloseTo(15, 6); + it("gives a document with no caption settings the aspect-appropriate default", () => { + expect(getCaptionSettings(doc(), LANDSCAPE).insetY).toBe(5); + expect(getCaptionSettings(doc(), PORTRAIT).insetY).toBe(12.5); }); }); @@ -432,7 +390,7 @@ describe("deriveCaptionCues", () => { describe("captionCuesToTextRegions", () => { it("emits plain text regions with no annotationSource marker", () => { - const regions = captionCuesToTextRegions(deriveCaptionCues(doc(), ON, {}), ON); + const regions = captionCuesToTextRegions(deriveCaptionCues(doc(), ON, {}), ON, LANDSCAPE); expect(regions.length).toBeGreaterThan(0); for (const region of regions) { expect(region.type).toBe("text"); @@ -446,17 +404,35 @@ describe("captionCuesToTextRegions", () => { ...ON, color: "#fde047", fontSize: 40, - textAlign: "left" as const, + anchorH: "left" as const, backgroundEnabled: false, }; - const [region] = captionCuesToTextRegions(deriveCaptionCues(doc(), settings, {}), settings); + const [region] = captionCuesToTextRegions( + deriveCaptionCues(doc(), settings, {}), + settings, + LANDSCAPE, + ); expect(region.style).toMatchObject({ color: "#fde047", fontSize: 40, + // One horizontal control: the anchor IS the text alignment the rasterizer gets. textAlign: "left", backgroundColor: "transparent", }); }); + + it("tells the compositor which edge to pin, on every region", () => { + // Without this the rasterizers centre the block in the box, which is the whole + // bug: the caption would drift vertically every time its text wrapped. + const settings = { ...ON, anchorV: "top" as const }; + const regions = captionCuesToTextRegions( + deriveCaptionCues(doc(), settings, {}), + settings, + LANDSCAPE, + ); + expect(regions.length).toBeGreaterThan(0); + for (const region of regions) expect(region.verticalAlign).toBe("top"); + }); }); describe("caption translations", () => { diff --git a/src/lib/ai-edition/captions/cues.ts b/src/lib/ai-edition/captions/cues.ts index 64133e641..c3287ebcb 100644 --- a/src/lib/ai-edition/captions/cues.ts +++ b/src/lib/ai-edition/captions/cues.ts @@ -23,7 +23,12 @@ import { } from "@/lib/captioning/annotationsFromCaptions"; import type { CaptionSegment } from "@/lib/captioning/transcribe"; import type { AxcutClip, AxcutDocument, AxcutTranscript } from "../schema"; -import { type CaptionSettings, captionBackgroundCss, captionBandRect } from "./settings"; +import { + type CaptionAnchorV, + type CaptionSettings, + captionBackgroundCss, + captionBoxRect, +} from "./settings"; import { type CaptionTranslations, captionTranslationUnits } from "./translations"; /** One on-screen caption line, in whichever time base the producer documented. */ @@ -241,7 +246,14 @@ export function captionCueAt(cues: CaptionCue[], timeMs: number): CaptionCue | n * overhangs the frame edge — through `annotationRegionSchema`, which bounds position * to 0..100. Captions are never stored, so they never meet that schema. */ -export type CaptionTextRegion = AnnotationRegion & { space: "frame" }; +export type CaptionTextRegion = AnnotationRegion & { + space: "frame"; + /** Which edge of the drawn block the compositor pins to the region's box. Carried + * here rather than on `AnnotationTextStyle` for the same reason as `space`: an + * annotation must keep rendering centred, and widening the shared style would put + * the key in every stored annotation's payload. */ + verticalAlign: CaptionAnchorV; +}; /** * Cues as text annotation regions, so the export renderer draws captions through @@ -254,10 +266,15 @@ export type CaptionTextRegion = AnnotationRegion & { space: "frame" }; export function captionCuesToTextRegions( cues: CaptionCue[], settings: CaptionSettings, + aspectValue: number, ): CaptionTextRegion[] { - const rect = captionBandRect(settings); + const rect = captionBoxRect(settings, aspectValue); return cues.map((cue, index) => ({ space: "frame" as const, + // The edge the compositor pins the drawn block to inside `size`. Without it the + // rasterizers centre the block — which is what made a caption drift vertically + // every time its text wrapped to another line. + verticalAlign: rect.verticalAlign, id: cue.id, startMs: cue.startMs, endMs: cue.endMs, @@ -273,7 +290,10 @@ export function captionCuesToTextRegions( fontWeight: settings.fontWeight, fontStyle: "normal" as const, textDecoration: "none" as const, - textAlign: settings.textAlign, + // One horizontal control, not two: `anchorH` picks which edge of the block is + // pinned to the column, and the rasterizers' plate maths already snaps the + // plate onto that edge (`text_linux.rs` `plate_x`, and its two mirrors). + textAlign: settings.anchorH, textAnimation: "none" as const, }, zIndex: CAPTION_Z_INDEX_BASE + index, diff --git a/src/lib/ai-edition/captions/index.ts b/src/lib/ai-edition/captions/index.ts index c238654bc..ea5a58413 100644 --- a/src/lib/ai-edition/captions/index.ts +++ b/src/lib/ai-edition/captions/index.ts @@ -8,26 +8,20 @@ export { sourceSpanToTimelineSpans, } from "./cues"; export type { - CaptionBandRect, - CaptionHorizontalPosition, - CaptionOffsetRange, + CaptionAnchorH, + CaptionAnchorV, + CaptionBoxRect, CaptionSettings, CaptionSettingsPatch, - CaptionTextAlign, - CaptionVerticalPosition, } from "./settings"; export { - activeHorizontalPositionPreset, - activeVerticalPositionPreset, - CAPTION_BAND_HEIGHT_PCT, - CAPTION_EDGE_MARGIN_PCT, - CAPTION_POSITION_PRESET_EPSILON, + CAPTION_INSET_X_MAX, + CAPTION_INSET_Y_MAX, captionBackgroundCss, - captionBandRect, - captionHorizontalPositionOffset, - captionInkHeightPct, - captionOffsetRange, + captionBoxRect, + captionSafeColumn, DEFAULT_CAPTION_SETTINGS, + defaultCaptionInsetY, getCaptionSettings, patchCaptionSettings, } from "./settings"; diff --git a/src/lib/ai-edition/captions/settings.ts b/src/lib/ai-edition/captions/settings.ts index 469c67aaf..35682c9d9 100644 --- a/src/lib/ai-edition/captions/settings.ts +++ b/src/lib/ai-edition/captions/settings.ts @@ -12,18 +12,36 @@ import { clamp } from "@/utils/math"; import type { AxcutDocument } from "../schema"; -/** Vertical anchor of the caption band inside the frame. */ -export type CaptionVerticalPosition = "top" | "middle" | "bottom"; - -/** Horizontal alignment of the text inside the (always centred) caption band. */ -export type CaptionTextAlign = "left" | "center" | "right"; +/** + * Which frame edge the caption block is pinned to. The block grows AWAY from it: + * a bottom-anchored caption extends upward as its text wraps, so the anchored + * edge never moves. + * + * This is the whole redesign in one field. The old model placed a fixed-height + * band and let each renderer centre the ink inside it — and a centred block moves + * BOTH its edges when it grows, which is why widening the band used to shift the + * caption vertically and why no setting could hold it against an edge. + * + * `tts:displayAlign` (TTML/IMSC), `\an2` vs `\an8` (ASS), `line:0`/`line:-1` + * (WebVTT) are the same idea; bottom-anchored growth is the default in all of + * them. There is deliberately no "middle": it is the old pathology given a name + * (XSL 1.1 defines `display-align: center` as keeping both edge distances equal), + * and a bottom anchor with a large `insetY` reaches the same place while still + * growing upward. + */ +export type CaptionAnchorV = "bottom" | "top"; -/** Horizontal position preset for the caption band itself — a different axis of - * meaning from `CaptionTextAlign`, which aligns the text *inside* the band. - * Not a stored field: it is derived from `offsetX` (see - * `activeHorizontalPositionPreset`) and set by writing `offsetX` directly (see - * `captionHorizontalPositionOffset`). */ -export type CaptionHorizontalPosition = "left" | "center" | "right"; +/** + * Which edge of the caption block is pinned horizontally — and, for a wrapped + * caption, the ragged edge. + * + * One property, as in ASS, where the `\an` digit is the only horizontal control + * the format has. It replaces BOTH the old `textAlign` (which aligned text inside + * an invisible band) and the old `offsetX` (which moved that band): two controls + * that fought over one visual outcome, neither of which could be understood + * without seeing the band. + */ +export type CaptionAnchorH = "left" | "center" | "right"; export interface CaptionSettings { /** Master show/hide for the whole caption layer (preview AND export). */ @@ -47,19 +65,21 @@ export interface CaptionSettings { backgroundColor: string; /** 0–1. */ backgroundOpacity: number; - verticalPosition: CaptionVerticalPosition; - textAlign: CaptionTextAlign; - /** Fine vertical nudge, in % of OUTPUT FRAME height, applied on top of the anchor. - * Positive moves down. The reachable span depends on the anchor — see - * `captionOffsetRange`, which the inspector uses for its slider bounds so that - * every position on the slider is a position the band can actually take. */ - offsetY: number; - /** Fine horizontal nudge, in % of OUTPUT FRAME width, applied on top of the - * (centred) anchor. Positive moves toward the right edge of the exported frame — - * this is frame geometry, so it is never mirrored by an RTL interface locale. */ - offsetX: number; - /** Caption band width, in % of frame width. */ - width: number; + anchorV: CaptionAnchorV; + /** Distance from the frame edge named by `anchorV` to the near edge of what is + * actually DRAWN — the plate when the background is on, the glyph block when it + * is off — in % of frame height. Always ≥ 0: it is a margin from a named edge, + * which is how every subtitle format states position (ASS `MarginV`, WebVTT + * `line`, TTML `tts:origin`) and why nothing here is ever a signed number. + * 0 puts the caption flush against the frame edge; 50 puts that edge on the + * frame's midline. */ + insetY: number; + anchorH: CaptionAnchorH; + /** The same idea on the horizontal axis (ASS `MarginL` / `MarginR`): distance + * from the frame edge named by `anchorH` to the near edge of the drawn block. + * Ignored when `anchorH` is `"center"` — a centred block has no edge to measure + * from, so the inspector hides the control rather than offering a dead one. */ + insetX: number; /** Lower bound on words shown at once. */ minWordsPerLine: number; /** Upper bound on words shown at once. */ @@ -76,184 +96,153 @@ export const DEFAULT_CAPTION_SETTINGS: CaptionSettings = { backgroundEnabled: true, backgroundColor: "#000000", backgroundOpacity: 0.55, - verticalPosition: "bottom", - textAlign: "center", - offsetY: 0, - offsetX: 0, - width: 80, + anchorV: "bottom", + // Overridden per output aspect on first read (`defaultCaptionInsetY`); this is the + // landscape value, and the one a document keeps once anything has been written. + insetY: 5, + anchorH: "center", + insetX: 16, minWordsPerLine: 2, maxWordsPerLine: 7, }; -/** Band height as a % of frame height. Generous enough for two wrapped lines at - * the default size; the renderers clip to it, so it is deliberately not tight. */ -export const CAPTION_BAND_HEIGHT_PCT = 22; - -/** Margin between the band and the frame edge for the top/bottom anchors, in %. */ -export const CAPTION_EDGE_MARGIN_PCT = 3; - -/** Tolerance for "is this offset at a preset's clean value", in % of frame. - * Deliberately not `Number.EPSILON` (already used below as `sliderStep`'s - * divide-by-zero floor, and far too small to absorb real float noise) — - * matches the `toBeCloseTo(x, 6)` tolerance this file's own tests use. */ -export const CAPTION_POSITION_PRESET_EPSILON = 1e-6; - /** Reference frame height the px-valued settings are authored against, matching * `annotationScale.ts` — `fontSize` is "pixels at a 1080-high frame". */ const CAPTION_REFERENCE_FRAME_HEIGHT = 1080; -/** Line box as a multiple of the font size. Mirrors the rasterizers so the band - * maths and the drawn glyphs agree: `text_linux.rs` is `font_size * 1.4`, and the - * other two backends lay out through the same `text_plate` box model. */ -const CAPTION_LINE_HEIGHT_EM = 1.4; +/** Line box as a multiple of the font size, taken as an UPPER BOUND across the three + * rasterizers (cosmic-text is 1.4, DirectWrite and CoreText both lower). It only + * sizes the box's headroom, never the caption's position — see `captionBoxRect`. */ +const CAPTION_LINE_HEIGHT_EM = 1.5; /** Vertical padding the background plate adds above AND below the text block, * as a multiple of the font size — `text_plate.rs::PAD_Y_EM`. */ const CAPTION_PLATE_PAD_Y_EM = 0.1; -/** Lines the band is sized to hold. The band is a fixed 22% box and all three - * rasterizers centre the text inside it, so this is what decides how much of the - * box is guaranteed to carry ink — and therefore how far the box may hang off the - * frame before a caption would be clipped (see `captionOffsetRange`). */ -const CAPTION_BAND_CAPACITY_LINES = 2; - /** - * Height of the drawn caption block — the background plate when it is on, the text - * block alone when it is off — as a % of frame height, capped at the band it lives - * in. The band is deliberately taller than its content, so this is the slice of the - * band that actually carries pixels. + * Visual lines the box is sized to hold. + * + * HEADROOM, NOT A POSITION. Because the block is anchored to one edge of this box, + * getting the height wrong no longer moves the caption — it only changes how many + * lines can be drawn before the renderer clips. That is the entire point of the + * redesign: the old `CAPTION_BAND_CAPACITY_LINES = 2` *positioned* the ink (the band + * was fixed and the ink centred inside it), so being wrong about it moved the + * subtitle. + * + * Three is ample: `deriveCaptionCues` already groups the transcript into cues of + * `minWordsPerLine`..`maxWordsPerLine` words, so one cue is one logical line and only + * wraps when the column is narrow or the font large. At the default 48px this is 20% + * of frame height — slightly LESS than the 22% band it replaces, so the per-cue + * texture gets marginally smaller rather than larger. */ -export function captionInkHeightPct(settings: CaptionSettings): number { - const lines = CAPTION_BAND_CAPACITY_LINES * CAPTION_LINE_HEIGHT_EM; - const plate = settings.backgroundEnabled ? 2 * CAPTION_PLATE_PAD_Y_EM : 0; - const px = clamp(settings.fontSize, 12, 200) * (lines + plate); - return Math.min(CAPTION_BAND_HEIGHT_PCT, (px / CAPTION_REFERENCE_FRAME_HEIGHT) * 100); -} +const CAPTION_BOX_LINES = 3; + +/** Aspect ratio at or above which a frame counts as landscape for the safe column. */ +const CAPTION_LANDSCAPE_ASPECT = 1.5; /** - * How far the band may hang off the top/bottom of the frame, in % of frame height. + * The column captions are laid out in, as % of frame width. This IS the max-width — + * derived from the output aspect, never stored, never exposed as a control. + * + * It used to be a `width` slider, which was the most confusing control in the pane: + * the background plate hugs the TEXT, so moving it changed nothing visible until the + * text happened to be long enough to wrap. It only ever constrained wrapping, which + * is a question about how much text is on screen — and that is already answered, + * legibly, by `minWordsPerLine` / `maxWordsPerLine`. * - * This is the whole of the "the offset can't reach the edge" half of #396. The band - * is a 22%-tall box whose text every renderer centres, so a band stopped flush at the - * frame edge still leaves its glyphs half a band short of it. Letting the box spill by - * exactly its empty margin puts the ink on the edge while keeping every drawn pixel - * on-frame — and costs nothing in the rasterizers, which already clip to the box. + * The numbers are the BBC line-length table: 68% of a 16:9 frame (≈45 characters at + * 48px on 1080p, inside the Netflix 42 / BBC 37 band the editorial specs legislate), + * 90% for squarer and vertical frames. Centred, a 16:9 column runs 16%→84%, inside + * BBC's 12.5/87.5 title-safe box. */ -function captionBandOverhangPct(settings: CaptionSettings): number { - return Math.max(0, (CAPTION_BAND_HEIGHT_PCT - captionInkHeightPct(settings)) / 2); +export function captionSafeColumn(aspectValue: number): { x: number; width: number } { + return aspectValue >= CAPTION_LANDSCAPE_ASPECT ? { x: 16, width: 68 } : { x: 5, width: 90 }; } -/** The band's anchor position before the user's nudge, in % of the frame. */ -function captionAnchor(settings: CaptionSettings): { x: number; y: number } { - const width = clamp(settings.width, 20, 100); - const height = CAPTION_BAND_HEIGHT_PCT; - return { - x: (100 - width) / 2, - y: - settings.verticalPosition === "top" - ? CAPTION_EDGE_MARGIN_PCT - : settings.verticalPosition === "middle" - ? (100 - height) / 2 - : 100 - height - CAPTION_EDGE_MARGIN_PCT, - }; +/** Default distance from the anchored edge, in % of frame height, for a given output + * aspect. BBC puts the lowest line at a 5% inset; vertical formats need much more, + * because the bottom eighth of a 9:16 export is where TikTok, Reels and Shorts draw + * their own chrome over the video. */ +export function defaultCaptionInsetY(aspectValue: number): number { + return aspectValue >= CAPTION_LANDSCAPE_ASPECT ? 5 : 12.5; } -/** Inclusive min/max for each offset, in % of the frame. */ -export interface CaptionOffsetRange { - x: { min: number; max: number }; - y: { min: number; max: number }; -} +/** Upper bound for the vertical inset, in % of the frame. 50 puts the anchored edge + * on the midline — past that the anchor would point into the far half of the frame, + * which is what the opposite anchor is for. */ +export const CAPTION_INSET_Y_MAX = 50; -/** - * The offsets the current settings can actually honour. - * - * Both the reader's clamp and the inspector's sliders come from here, so the two can - * never disagree: every value the slider can produce moves the band, and no value it - * can produce is silently discarded. The old code hard-coded ±45 in both places and - * then clamped the *result*, which is why nearly half the bottom-anchored slider's - * travel did nothing at all. - */ -export function captionOffsetRange(settings: CaptionSettings): CaptionOffsetRange { - const width = clamp(settings.width, 20, 100); - const anchor = captionAnchor(settings); - const overhang = captionBandOverhangPct(settings); - return { - // Horizontally the band stays wholly on-frame: `textAlign` lets a line hug the - // band's own edge, so an overhang here would push text off the frame. - x: { min: -anchor.x, max: 100 - width - anchor.x }, - y: { - min: -overhang - anchor.y, - max: 100 - CAPTION_BAND_HEIGHT_PCT + overhang - anchor.y, - }, - }; -} +/** Upper bound for the horizontal inset. A finer adjustment on a narrower axis: the + * column is already inset from the frame edge, and pushing much past this eats the + * wrap width without moving the caption anywhere useful. */ +export const CAPTION_INSET_X_MAX = 25; -/** - * Which vertical preset, if any, the current settings match exactly. +/** Where the caption box sits, as percentages of the OUTPUT FRAME. * - * `verticalPosition` is always a real stored value, but a preset button should - * only read as "active" while the user hasn't nudged away from it — otherwise - * clicking a slider would leave a preset highlighted that no longer describes - * where the band actually is. `offsetY` is the nudge *from* the anchor, so - * "at the preset" is exactly "no nudge". - */ -export function activeVerticalPositionPreset( - settings: CaptionSettings, -): CaptionVerticalPosition | null { - return Math.abs(settings.offsetY) < CAPTION_POSITION_PRESET_EPSILON - ? settings.verticalPosition - : null; + * Not of the screen rect: captions are subtitles, so they belong to the frame the + * viewer sees and must hold still when padding resizes the footage underneath them. + * `cues.ts` stamps the regions it builds from this with `space: "frame"`, which is + * what tells the compositor to measure them against the frame. */ +export interface CaptionBoxRect { + x: number; + y: number; + width: number; + height: number; + /** Which edge of the drawn block the compositor pins to this box. Travels to the + * rasterizers as `verticalAlign`; absent there means "centre", which is the + * behaviour every annotation still gets. */ + verticalAlign: CaptionAnchorV; } /** - * Which horizontal position preset, if any, the current settings match exactly. + * The box a caption is laid out in, and the edge its drawn block is pinned to. + * + * The invariant this exists to guarantee, and the one the tests assert: + * + * > Bottom anchor: the drawn block's bottom edge is at `100 − insetY` % of frame + * > height. Top anchor: its top edge is at `insetY` %. For every font size, every + * > background state, every word count, every wrap outcome, every output resolution. * - * There is no stored `horizontalPosition` field — `offsetX` is already an - * absolute-feeling value centred on 0 with a range that reaches both frame - * edges (see `captionOffsetRange`), so "left"/"center"/"right" are just names - * for three points on that existing range. All three coincide at `offsetX===0` - * when the band is full-width (no travel) — and can also *nearly* coincide - * for a band merely close to full-width, where `range.x.min`/`max` shrink - * toward 0 as well. Picking the CLOSEST candidate (not the first one within - * epsilon) is what keeps that near-degenerate case from reporting "center" - * for an offset that is actually sitting exactly on `range.x.min`/`max`. + * Nothing in that sentence mentions line height, line count or the plate — and no + * estimate of the block's height participates in placing it. The height below is only + * how much room the block gets before it would be clipped, which is why being wrong + * about it is now cheap. */ -export function activeHorizontalPositionPreset( - settings: CaptionSettings, -): CaptionHorizontalPosition | null { - const range = captionOffsetRange(settings); - const { offsetX } = settings; - const candidates: ReadonlyArray<[CaptionHorizontalPosition, number]> = [ - ["center", 0], - ["left", range.x.min], - ["right", range.x.max], - ]; - let closest: CaptionHorizontalPosition | null = null; - let closestDistance = CAPTION_POSITION_PRESET_EPSILON; - for (const [preset, target] of candidates) { - const distance = Math.abs(offsetX - target); - if (distance < closestDistance) { - closest = preset; - closestDistance = distance; - } - } - return closest; -} +export function captionBoxRect(settings: CaptionSettings, aspectValue: number): CaptionBoxRect { + const column = captionSafeColumn(aspectValue); + const insetY = clamp(settings.insetY, 0, CAPTION_INSET_Y_MAX); + const insetX = clamp(settings.insetX, 0, CAPTION_INSET_X_MAX); + + // Room for `CAPTION_BOX_LINES` lines plus the plate's own padding, capped so the + // box itself never leaves the frame. + const lines = CAPTION_BOX_LINES * CAPTION_LINE_HEIGHT_EM; + const plate = settings.backgroundEnabled ? 2 * CAPTION_PLATE_PAD_Y_EM : 0; + const capacityPct = + ((clamp(settings.fontSize, 12, 200) * (lines + plate)) / CAPTION_REFERENCE_FRAME_HEIGHT) * 100; + const height = clamp(capacityPct, 10, 100 - insetY); + + // Left/right narrow the box rather than pushing it off-frame: the column is a MAX + // width, so moving the pinned edge inward simply leaves less room to wrap in — and + // that is visible in the guide overlay, unlike a range that silently shrinks. + const width = settings.anchorH === "center" ? column.width : Math.min(column.width, 100 - insetX); + const x = + settings.anchorH === "left" + ? insetX + : settings.anchorH === "right" + ? 100 - insetX - width + : (100 - width) / 2; -/** The `offsetX` that puts the band at a given horizontal preset, for a preset - * button's click handler to write. `left`/`right` reach the true frame edge — - * the same span `activeHorizontalPositionPreset` reads back against. */ -export function captionHorizontalPositionOffset( - settings: CaptionSettings, - preset: CaptionHorizontalPosition, -): number { - if (preset === "center") return 0; - const range = captionOffsetRange(settings); - return preset === "left" ? range.x.min : range.x.max; + return { + x, + y: settings.anchorV === "bottom" ? 100 - insetY - height : insetY, + width, + height, + verticalAlign: settings.anchorV, + }; } -const VERTICAL_POSITIONS: readonly CaptionVerticalPosition[] = ["top", "middle", "bottom"]; -const TEXT_ALIGNS: readonly CaptionTextAlign[] = ["left", "center", "right"]; +const ANCHORS_V: readonly CaptionAnchorV[] = ["bottom", "top"]; +const ANCHORS_H: readonly CaptionAnchorH[] = ["left", "center", "right"]; function isFiniteNumber(value: unknown): value is number { return typeof value === "number" && Number.isFinite(value); @@ -291,65 +280,175 @@ function storedCaptions(doc: AxcutDocument | null | undefined): Record, + fontSize: number, + backgroundEnabled: boolean, + fallbackInsetY: number, +): Pick | null { + const legacyVertical = raw.verticalPosition; + const hasLegacy = + typeof legacyVertical === "string" || + isFiniteNumber(raw.offsetY) || + isFiniteNumber(raw.offsetX) || + isFiniteNumber(raw.width); + if (!hasLegacy) return null; + + // The old band's anchor, before the user's nudge. + const width = clamp(isFiniteNumber(raw.width) ? raw.width : 80, 20, 100); + const anchorY = + legacyVertical === "top" + ? LEGACY_EDGE_MARGIN_PCT + : legacyVertical === "middle" + ? (100 - LEGACY_BAND_HEIGHT_PCT) / 2 + : 100 - LEGACY_BAND_HEIGHT_PCT - LEGACY_EDGE_MARGIN_PCT; + const anchorX = (100 - width) / 2; + + // The ink the old model actually drew: a block of this height, centred in the band. + const inkPx = + clamp(fontSize, 12, 200) * + (LEGACY_CAPACITY_LINES * LEGACY_LINE_HEIGHT_EM + (backgroundEnabled ? 0.2 : 0)); + const inkHeight = Math.min( + LEGACY_BAND_HEIGHT_PCT, + (inkPx / CAPTION_REFERENCE_FRAME_HEIGHT) * 100, + ); + const overhang = Math.max(0, (LEGACY_BAND_HEIGHT_PCT - inkHeight) / 2); + + const offsetY = clamp( + isFiniteNumber(raw.offsetY) ? raw.offsetY : 0, + -overhang - anchorY, + 100 - LEGACY_BAND_HEIGHT_PCT + overhang - anchorY, + ); + const bandY = anchorY + offsetY; + const inkTop = bandY + (LEGACY_BAND_HEIGHT_PCT - inkHeight) / 2; + const inkBottom = inkTop + inkHeight; + + // Pin the edge the caption was nearer to; ties go to the bottom, the default. + const distanceToTop = inkTop; + const distanceToBottom = 100 - inkBottom; + const anchorV: CaptionAnchorV = distanceToTop < distanceToBottom ? "top" : "bottom"; + const insetY = clamp( + anchorV === "top" ? distanceToTop : distanceToBottom, + 0, + CAPTION_INSET_Y_MAX, + ); + + // Horizontally the old model had two controls fighting: `offsetX` moved the band and + // `textAlign` moved the text inside it. What the viewer saw is the combination, so + // take the one anchor nearest to where the ink actually sat. + const offsetX = clamp( + isFiniteNumber(raw.offsetX) ? raw.offsetX : 0, + -anchorX, + 100 - width - anchorX, + ); + const bandX = anchorX + offsetX; + const legacyAlign = raw.textAlign; + const inkCentre = + legacyAlign === "left" ? bandX : legacyAlign === "right" ? bandX + width : bandX + width / 2; + const anchorH: CaptionAnchorH = + inkCentre < 100 / 3 ? "left" : inkCentre > (2 * 100) / 3 ? "right" : "center"; + + return { + anchorV, + insetY: Number.isFinite(insetY) ? insetY : fallbackInsetY, + anchorH, + insetX: 0, + }; +} + +/** + * Read the caption settings, migrating a pre-anchor document on the way through. + * + * `aspectValue` only decides the DEFAULT inset for a document that has never carried + * caption settings — a 5% inset is right for landscape and lands under the platform + * chrome on a 9:16 export. Once anything is written, the stored value wins. + */ +export function getCaptionSettings( + doc: AxcutDocument | null | undefined, + aspectValue = 16 / 9, +): CaptionSettings { const raw = storedCaptions(doc); const d = DEFAULT_CAPTION_SETTINGS; - if (!raw) return { ...d }; + const defaultInsetY = defaultCaptionInsetY(aspectValue); + if (!raw) return { ...d, insetY: defaultInsetY, insetX: captionSafeColumn(aspectValue).x }; const minWords = Math.round(readNumber(raw.minWordsPerLine, d.minWordsPerLine, 1, 12)); const maxWords = Math.round(readNumber(raw.maxWordsPerLine, d.maxWordsPerLine, 1, 12)); + const fontSize = readNumber(raw.fontSize, d.fontSize, 12, 200); + const backgroundEnabled = readBoolean(raw.backgroundEnabled, d.backgroundEnabled); + + // New fields win whenever they are present, so the migration runs once and is inert + // afterwards; it only speaks for a document that still carries the old ones. + const legacy = migrateLegacyPlacement(raw, fontSize, backgroundEnabled, defaultInsetY); + const placement = { + anchorV: readEnum(raw.anchorV, ANCHORS_V, legacy?.anchorV ?? d.anchorV), + insetY: readNumber(raw.insetY, legacy?.insetY ?? defaultInsetY, 0, CAPTION_INSET_Y_MAX), + anchorH: readEnum(raw.anchorH, ANCHORS_H, legacy?.anchorH ?? d.anchorH), + insetX: readNumber( + raw.insetX, + legacy?.insetX ?? captionSafeColumn(aspectValue).x, + 0, + CAPTION_INSET_X_MAX, + ), + }; - const settings: CaptionSettings = { + return { enabled: readBoolean(raw.enabled, d.enabled), // `null` is a meaningful value here ("show the original"), so an explicit // null must survive; only a missing/garbage entry falls back to the default. language: raw.language === null || typeof raw.language === "string" ? raw.language : d.language, - fontSize: readNumber(raw.fontSize, d.fontSize, 12, 200), + fontSize, fontFamily: readString(raw.fontFamily, d.fontFamily), fontWeight: readEnum(raw.fontWeight, ["normal", "bold"] as const, d.fontWeight), color: readString(raw.color, d.color), - backgroundEnabled: readBoolean(raw.backgroundEnabled, d.backgroundEnabled), + backgroundEnabled, backgroundColor: readString(raw.backgroundColor, d.backgroundColor), backgroundOpacity: readNumber(raw.backgroundOpacity, d.backgroundOpacity, 0, 1), - verticalPosition: readEnum(raw.verticalPosition, VERTICAL_POSITIONS, d.verticalPosition), - textAlign: readEnum(raw.textAlign, TEXT_ALIGNS, d.textAlign), - // Read wide here, then clamp to what the geometry allows below: the reachable - // span depends on the anchor, the width and the font size, which are only known - // once the rest of the object is built. - offsetY: readNumber(raw.offsetY, d.offsetY, -100, 100), - offsetX: readNumber(raw.offsetX, d.offsetX, -100, 100), - width: readNumber(raw.width, d.width, 20, 100), + ...placement, minWordsPerLine: Math.min(minWords, maxWords), maxWordsPerLine: Math.max(minWords, maxWords), }; - - // Normalising here rather than at the draw call keeps the stored value, the slider - // position and the drawn band the same number. A value that the current anchor - // cannot reach — a leftover from another anchor, or from the old ±45 domain — is - // pulled to the nearest reachable one instead of being clamped invisibly later. - const range = captionOffsetRange(settings); - settings.offsetY = clamp(settings.offsetY, range.y.min, range.y.max); - settings.offsetX = clamp(settings.offsetX, range.x.min, range.x.max); - return settings; } export type CaptionSettingsPatch = Partial; -/** Apply a patch and return the new document. Pure — no persistence. */ +/** + * Apply a patch and return the new document. Pure — no persistence. + * + * There is no re-clamping pass any more, and that absence is the point: an inset is a + * distance from a named edge, so it means the same thing whatever the font size, the + * background or the other axis does. The old offsets had to be re-clamped on every + * patch because their reachable span moved with four other fields — which is also why + * they could not be shown to a user as a plain number. + */ export function patchCaptionSettings( doc: AxcutDocument, patch: CaptionSettingsPatch, ): AxcutDocument { const next: CaptionSettings = { ...getCaptionSettings(doc), ...patch }; - // Re-clamp against the geometry the patch just produced, not the one it - // replaced: `width`, `fontSize`, `backgroundEnabled` and `verticalPosition` all - // move the reachable span, so a patch to any of them can strand an offset - // outside it. `getCaptionSettings` would pull it back on the next read, but - // until then the document would hold a number the band never draws — and the - // next patch would write that stale number back out. - const range = captionOffsetRange(next); - next.offsetY = clamp(next.offsetY, range.y.min, range.y.max); - next.offsetX = clamp(next.offsetX, range.x.min, range.x.max); + next.insetY = clamp(next.insetY, 0, CAPTION_INSET_Y_MAX); + next.insetX = clamp(next.insetX, 0, CAPTION_INSET_X_MAX); return { ...doc, legacyEditor: { @@ -359,39 +458,6 @@ export function patchCaptionSettings( }; } -/** Where the caption band sits, as percentages of the OUTPUT FRAME. - * - * Not of the screen rect: captions are subtitles, so they belong to the frame the - * viewer sees and must hold still when padding resizes the footage underneath them. - * `cues.ts` stamps the regions it builds from this with `space: "frame"`, which is - * what tells the compositor to measure them against the frame (see `CaptionTextRegion`). */ -export interface CaptionBandRect { - x: number; - y: number; - width: number; - height: number; -} - -/** - * Anchor preset plus the user's nudge, on both axes. - * - * `textAlign` aligns the text *inside* the band, which is how subtitles behave - * everywhere; `offsetX` moves the band itself, which is the only way to reach a - * corner. Offsets are clamped to `captionOffsetRange` — the same span the inspector - * hands its sliders, so nothing the user can dial in is quietly thrown away. - */ -export function captionBandRect(settings: CaptionSettings): CaptionBandRect { - const width = clamp(settings.width, 20, 100); - const anchor = captionAnchor(settings); - const range = captionOffsetRange(settings); - return { - x: anchor.x + clamp(settings.offsetX, range.x.min, range.x.max), - y: anchor.y + clamp(settings.offsetY, range.y.min, range.y.max), - width, - height: CAPTION_BAND_HEIGHT_PCT, - }; -} - /** `backgroundColor` + `backgroundOpacity` as one CSS/canvas colour, or * `"transparent"` when the plate is off. */ export function captionBackgroundCss(settings: CaptionSettings): string { diff --git a/src/lib/ai-edition/store/useCaptions.test.ts b/src/lib/ai-edition/store/useCaptions.test.ts index b0761c225..b4063d68c 100644 --- a/src/lib/ai-edition/store/useCaptions.test.ts +++ b/src/lib/ai-edition/store/useCaptions.test.ts @@ -109,7 +109,7 @@ describe("useCaptions drag snapshots", () => { it("does not record a snapshot of the project the user left", async () => { const { result, rerender } = renderHook(() => useCaptions()); - act(() => result.current.setLive({ width: 60 })); + act(() => result.current.setLive({ insetY: 20 })); act(() => { useProjectStore.setState({ projectId: "proj_b", document: docB }); @@ -135,7 +135,7 @@ describe("useCaptions drag snapshots", () => { it("does not hand a bare commit a base the edits since have already buried", async () => { const { result } = renderHook(() => useCaptions()); - act(() => result.current.setLive({ width: 60 })); + act(() => result.current.setLive({ insetY: 20 })); await act(async () => { await result.current.set({ fontSize: 30 }); diff --git a/src/native/sceneDescription.ts b/src/native/sceneDescription.ts index c9db97b35..38408631c 100644 --- a/src/native/sceneDescription.ts +++ b/src/native/sceneDescription.ts @@ -146,6 +146,11 @@ export interface SceneAnnotation { fontStyle: "normal" | "italic"; textDecoration: "none" | "underline"; textAlign: "left" | "center" | "right"; + /** Which edge of the drawn text block is pinned to the box. Omitted for + * annotations, which keep the historical centring — so their payload does not + * change shape at all. Captions send it because a centred block moves BOTH its + * edges as it grows, which made a subtitle drift every time its text wrapped. */ + verticalAlign?: "top" | "center" | "bottom"; animation: string | null; }; /** Present for `kind: "image"` — the authored `imageContent` (path or data URI). */ @@ -523,10 +528,19 @@ export function buildSceneDescription( // so the compositor measures it against the output frame while annotations stay on the screen // rect. Subtitles have to sit where the viewer's frame ends, not where the footage does, or // they slide inward the moment padding shrinks the screen rect (issue #396). - const captionSettings = getCaptionSettings(document); + // + // The output aspect is what decides the caption column and the default inset (a + // caption 5% off the bottom of a 16:9 export sits under the platform's own chrome + // on a 9:16 one). It comes off `pickOutputDims`, hoisted above the webcam block that + // used to own it so there is exactly ONE caller — preview and export cannot pick a + // different column from each other. + const outputDims = pickOutputDims(document, settings.aspectRatio); + const captionAspect = outputDims.height > 0 ? outputDims.width / outputDims.height : 16 / 9; + const captionSettings = getCaptionSettings(document, captionAspect); const captionRegions = captionCuesToTextRegions( deriveCaptionCues(document, captionSettings, getCaptionTranslations(document)), captionSettings, + captionAspect, ); const projectedAnnotations = projectRegionsToSource( [ @@ -567,7 +581,7 @@ export function buildSceneDescription( // fait sur la résolution de sortie (= taille du canvas rendu) avec les unités sources du // premier asset visible — la même convention que `pickOutputDims` + SCREEN_SOURCE_SIZE / // WEBCAM_SOURCE_SIZE dans PreviewCanvas — ce qui garde preview/export/natif alignés. - const outputDims = pickOutputDims(document, settings.aspectRatio); + // (`outputDims` est résolu plus haut, avec le bloc sous-titres qui en dépend aussi.) // ponytail: when the active camera has been probed (real webcam dims cached by // WebcamOverlay's loadedmetadata handler), use them so the box matches the actual // camera aspect. Without this the box defaults to a hardcoded 4:3 (960x720) and the @@ -805,6 +819,8 @@ export function buildSceneDescription( // Only captions carry a space; annotations must keep emitting the exact same keys // they always have, so the field is omitted rather than sent as null/undefined. const space = (region as { space?: "frame" }).space; + // Same treatment, same reason: only captions pin an edge. + const verticalAlign = (region as { verticalAlign?: "top" | "bottom" }).verticalAlign; const base = { id: region.id, startSec: region.startMs / 1000, @@ -839,6 +855,7 @@ export function buildSceneDescription( fontStyle: style.fontStyle, textDecoration: style.textDecoration, textAlign: style.textAlign, + ...(verticalAlign ? { verticalAlign } : {}), animation: style.textAnimation ?? null, }, }; From 0979b9ab997e6f033f9ddb8db72e3893feb3cf58 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sat, 22 Aug 2026 16:03:01 +0200 Subject: [PATCH 17/30] test(compositor): anchor on the line box, not on the last pixel of ink MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Linux anchor test measured the bottom-most inked row and called a 184→199 move a drift. It is not: "Hx" has no descenders and "replier" does, so the ink reaches further down inside an identically-placed line box. What this code pins is the line box — and therefore the plate, which is what the compositor draws and what the viewer sees. Anchoring the box rather than the last glyph pixel is the behaviour every text renderer has. So the assertion moves to `atlas.plate`, which is deterministic and is the actual contract. The ink is still checked, but only for the thing that is true of it: that it stays inside the plate carrying it, with the same `pad_y` tolerance `the_plate_hugs_the_text_instead_of_filling_the_box` already uses for that relationship — a glyph may overshoot its own line box slightly, which is precisely the assumption the first version got wrong. Found by CI: only text_windows.rs compiles on the machine this was written on. (cherry picked from commit b0617463d7c5323ef226550ec065e8210692664e) --- crates/compositor/src/text_linux.rs | 68 +++++++++++++++++++---------- 1 file changed, 45 insertions(+), 23 deletions(-) diff --git a/crates/compositor/src/text_linux.rs b/crates/compositor/src/text_linux.rs index d990c3dbe..825ea3eb9 100644 --- a/crates/compositor/src/text_linux.rs +++ b/crates/compositor/src/text_linux.rs @@ -601,50 +601,72 @@ mod tests { let (w, h) = (400usize, 200usize); let long = "un texte assez long pour devoir se replier sur plusieurs lignes"; - let one = |valign: &str, content: &str| { + // On mesure la PLAQUE, pas le dernier pixel d'encre. La plaque epouse la boite + // de lignes posee par cosmic-text : c'est exactement ce que ce code epingle et + // ce que le compositeur dessine. Le bas de l'ENCRE, lui, depend des jambages du + // contenu — "Hx" n'en a aucun, "replier" en a — donc il descend plus bas a + // ancrage identique. Ancrer la boite de lignes plutot que l'encre est le + // comportement typographique attendu partout, et c'est la premiere version de + // ce test qui avait tort : elle comparait 184 a 199 et appelait ca une derive. + let plate_of = |valign: &str, content: &str| { let mut s = spec(content, "center"); s.valign = valign.to_owned(); - let atlas = raster.build_atlas(&s).expect("atlas").pixels; - let rows = ink_rows(&atlas, w, 0, w); + let atlas = raster.build_atlas(&s).expect("atlas"); + let [_, py, _, ph] = atlas.plate; + let rows = ink_rows(&atlas.pixels, w, 0, w); assert!(!rows.is_empty(), "aucune encre pour {valign:?}"); - (rows[0], *rows.last().unwrap()) + (py, py + ph, rows[0], *rows.last().unwrap()) }; - let (_, short_bottom) = one("bottom", "Hx"); - let (_, long_bottom) = one("bottom", long); + let (short_top_edge, short_bottom, _, _) = plate_of("bottom", "Hx"); + let (long_top_edge, long_bottom, _, _) = plate_of("bottom", long); assert!( - (short_bottom as i32 - long_bottom as i32).abs() <= 1, + (short_bottom - long_bottom).abs() < 1.0, "ancrage bas : l'arete basse a bouge de {short_bottom} a {long_bottom} \ en passant d'une ligne a plusieurs" ); // Et le miroir, pour que « haut » ne soit pas juste « pas bas ». - let (short_top, _) = one("top", "Hx"); - let (long_top, _) = one("top", long); + let (short_top, _, _, _) = plate_of("top", "Hx"); + let (long_top, _, _, _) = plate_of("top", long); assert!( - (short_top as i32 - long_top as i32).abs() <= 1, + (short_top - long_top).abs() < 1.0, "ancrage haut : l'arete haute a bouge de {short_top} a {long_top}" ); - // Le texte long doit vraiment occuper plus de hauteur, sinon les deux - // assertions ci-dessus passeraient sur deux rendus identiques. - let (lt, lb) = one("bottom", long); - let (st, sb) = one("bottom", "Hx"); + // Le texte long doit vraiment se replier, sinon les deux assertions ci-dessus + // passeraient sur deux rendus identiques et ne prouveraient rien. assert!( - (lb - lt) > (sb - st), + (long_bottom - long_top_edge) > (short_bottom - short_top_edge) + 1.0, "le texte « long » ne s'est pas replie : le test ne prouve rien" ); - // Enfin, les trois ancrages doivent poser l'encre a trois endroits - // differents dans la boite — sinon `valign` n'est pas applique du tout. - let (top_t, _) = one("top", "Hx"); - let (ctr_t, _) = one("center", "Hx"); - let (bot_t, _) = one("bottom", "Hx"); + // Les trois ancrages doivent poser la plaque a trois endroits differents, + // sinon `valign` n'est pas applique du tout. + let (top_y, _, _, _) = plate_of("top", "Hx"); + let (ctr_y, _, _, _) = plate_of("center", "Hx"); + let (bot_y, _, _, _) = plate_of("bottom", "Hx"); assert!( - top_t < ctr_t && ctr_t < bot_t, - "les trois ancrages ne se distinguent pas : haut={top_t} centre={ctr_t} bas={bot_t}" + top_y < ctr_y && ctr_y < bot_y, + "les trois ancrages ne se distinguent pas : haut={top_y} centre={ctr_y} bas={bot_y}" ); - assert!(bot_t > h / 2, "l'ancrage bas laisse l'encre dans la moitie haute"); + + // Enfin, l'encre reste dans la plaque qui la porte, et la plaque dans la boite : + // c'est ce qui relie la boite mesuree ci-dessus a ce que le viewer voit. + // + // Tolerance `pad_y`, la meme que `the_plate_hugs_the_text_instead_of_filling_the_box` + // plus haut : l'encre est bornee par la boite de LIGNES, et un glyphe peut deborder + // legerement la sienne (jambages, accents) sans que rien ne soit casse. C'est + // exactement l'hypothese que la premiere version de ce test avait fausse. + for valign in ["top", "bottom"] { + let (py, pb, ink_top, ink_bottom) = plate_of(valign, long); + let (_, pad_y) = crate::text_plate::padding(40.0); + assert!( + (ink_top as f32) >= py - pad_y && (ink_bottom as f32) <= pb + pad_y, + "{valign} : l'encre ({ink_top}..{ink_bottom}) sort de la plaque ({py}..{pb})" + ); + assert!(pb <= (h as f32) + 0.01, "{valign} : la plaque sort de la boite"); + } } #[test] From 1e1c900424a87c8c931aa917d02169a3fe49af24 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sat, 22 Aug 2026 16:09:18 +0200 Subject: [PATCH 18/30] fix(captions): reserve the plate's margin on the anchored side, and migrate insetX MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings from CodeRabbit's review of #482, both real. **The plate lost its margin on the anchored side.** Pinning the TEXT block flush against the box edge left the plate laying all of its padding on the opposite side and none on the anchored one: at the default bottom anchor the background hugged the glyphs' baseline exactly while breathing twice as much above them. What should touch the box edge is the plate — that is what the viewer sees, and what the anchor invariant is stated in terms of. Reserving `pad_y` on the anchored side puts the plate's edge in the same place and makes its padding symmetric again. Conditioned on a plate actually being drawn, on all three backends. Windows was already inset but did it unconditionally, which would have placed its glyphs `pad_y` away from Linux and macOS whenever the background was off; it is now conditional too, so the three agree in both states. **The migration snapped left/right captions to the frame edge.** It returned `insetX: 0` for every document, so a migrated caption whose band sat at 5% moved to 0%. The vertical half already reconstructed the drawn edge exactly; the horizontal half now does the same from the values it had already computed — which is what "reproduce the pixels, not the fields" was supposed to mean. (cherry picked from commit 18f4d099d0687e5361ee103732764b1ec783f128) --- crates/compositor/src/text_linux.rs | 14 +++++++-- crates/compositor/src/text_macos.rs | 33 +++++++++++++++----- crates/compositor/src/text_windows.rs | 31 +++++++++++------- src/lib/ai-edition/captions/captions.test.ts | 13 ++++++++ src/lib/ai-edition/captions/settings.ts | 11 ++++++- 5 files changed, 81 insertions(+), 21 deletions(-) diff --git a/crates/compositor/src/text_linux.rs b/crates/compositor/src/text_linux.rs index 825ea3eb9..34bac45e1 100644 --- a/crates/compositor/src/text_linux.rs +++ b/crates/compositor/src/text_linux.rs @@ -209,12 +209,22 @@ impl TextRasterizer { // annotations. Les sous-titres epinglent une arete : c'est la seule facon // que l'arete ancree ne bouge pas quand le texte gagne une ligne, parce // qu'un bloc centre voit ses DEUX aretes se deplacer. + // + // `anchor_pad` reserve la marge de la plaque DU COTE ANCRE. Sans elle, coller + // le bloc de texte au bord laisse la plaque poser toute sa marge du cote + // oppose et zero du cote ancre : le fond epouse alors le bas des lettres au + // pixel pres tout en respirant deux fois trop au-dessus. Ce qui doit toucher + // le bord de la boite est la PLAQUE, pas les glyphes — c'est elle que le + // viewer voit. Sans plaque, il n'y a rien a reserver. + let has_plate = spec.background[3] > 0.0; + let anchor_pad = if has_plate { pad_y } else { 0.0 }; let slack_y = ((h as f32) - text_h).max(0.0); let y_offset = match spec.valign.as_str() { - "top" | "start" => 0.0, - "bottom" | "end" => slack_y, + "top" | "start" => anchor_pad, + "bottom" | "end" => slack_y - anchor_pad, _ => slack_y * 0.5, } + .clamp(0.0, slack_y) .round() as i32; // LA PLAQUE EPOUSE LE BLOC, PAS LA BOITE. Miroir de diff --git a/crates/compositor/src/text_macos.rs b/crates/compositor/src/text_macos.rs index 7e076f348..b7b7ab325 100644 --- a/crates/compositor/src/text_macos.rs +++ b/crates/compositor/src/text_macos.rs @@ -318,6 +318,9 @@ fn block_layout( text_h: CGFloat, align: u8, valign: &str, + /// Une plaque de fond est-elle dessinée ? Elle décide de la marge à réserver du + /// côté ancré — voir `anchor_pad` plus bas. + has_plate: bool, font_px: CGFloat, ) -> (CGRect, CGRect) { let (pad_x, pad_y) = plate_padding(font_px); @@ -332,12 +335,19 @@ fn block_layout( // épinglent une arête : un bloc centré voit ses DEUX arêtes bouger quand il // gagne une ligne, ce qui déplaçait le sous-titre. `top` est ici une distance // depuis le HAUT de la boîte, en coordonnées descendantes. + // `anchor_pad` réserve la marge de la plaque DU CÔTÉ ANCRÉ. Sans elle, coller le + // bloc de texte au bord laisse la plaque poser toute sa marge du côté opposé et + // zéro du côté ancré : le fond épouse le bas des lettres au pixel près tout en + // respirant deux fois trop au-dessus. Ce qui doit toucher le bord de la boîte est + // la PLAQUE, pas les glyphes. Sans plaque, il n'y a rien à réserver. + let anchor_pad = if has_plate { pad_y } else { 0.0 }; let slack_y = (box_h - text_h).max(0.0); let top = match valign { - "top" | "start" => 0.0, - "bottom" | "end" => slack_y, + "top" | "start" => anchor_pad, + "bottom" | "end" => slack_y - anchor_pad, _ => slack_y * 0.5, - }; + } + .clamp(0.0, slack_y); let frame_x = (box_w - avail_w) * 0.5; let frame = CGRect { origin: CGPoint { @@ -613,7 +623,16 @@ impl TextRasterizer { let text_h = measured.height.ceil().max(0.0); let (frame_rect, plate_rect) = - block_layout(box_w, box_h, text_w, text_h, alignment, &spec.valign, font_px); + block_layout( + box_w, + box_h, + text_w, + text_h, + alignment, + &spec.valign, + spec.background[3] > 0.0, + font_px, + ); // --- plaque de fond, sous le texte --- if spec.background[3] > 0.0 && plate_rect.size.width > 0.0 && plate_rect.size.height > 0.0 @@ -849,7 +868,7 @@ mod tests { /// Géométrie pure — pas de GPU, pas de CoreText. #[test] fn block_layout_centres_the_frame_and_sizes_the_plate() { - let (frame, plate) = block_layout(1536.0, 238.0, 500.0, 56.0, 2, "center", 48.0); + let (frame, plate) = block_layout(1536.0, 238.0, 500.0, 56.0, 2, "center", true, 48.0); // Cadre centré : autant de vide au-dessus qu'en dessous (repère CG, y vers le haut). let above = 238.0 - (frame.origin.y + frame.size.height); let below = frame.origin.y; @@ -864,7 +883,7 @@ mod tests { fn block_layout_never_lets_the_plate_leave_the_box() { for align in [0u8, 1, 2] { // Bloc plus large et plus haut que la boîte : la plaque doit se contenter d'elle. - let (_, plate) = block_layout(200.0, 60.0, 400.0, 200.0, align, "center", 48.0); + let (_, plate) = block_layout(200.0, 60.0, 400.0, 200.0, align, "center", true, 48.0); assert!(plate.origin.x >= 0.0, "align={align} : x={}", plate.origin.x); assert!(plate.origin.y >= 0.0, "align={align} : y={}", plate.origin.y); assert!(plate.origin.x + plate.size.width <= 200.0 + 0.01, "align={align}"); @@ -879,7 +898,7 @@ mod tests { fn block_layout_pins_the_anchored_edge_whatever_the_block_height() { let (box_w, box_h) = (1536.0, 238.0); let edges = |valign: &str, text_h: f64| { - let (frame, _) = block_layout(box_w, box_h, 500.0, text_h, 2, valign, 48.0); + let (frame, _) = block_layout(box_w, box_h, 500.0, text_h, 2, valign, true, 48.0); // (bas, haut) en distance depuis le bas de la boîte. (frame.origin.y, frame.origin.y + frame.size.height) }; diff --git a/crates/compositor/src/text_windows.rs b/crates/compositor/src/text_windows.rs index b9f6f9e10..e09f4cdc7 100644 --- a/crates/compositor/src/text_windows.rs +++ b/crates/compositor/src/text_windows.rs @@ -112,11 +112,13 @@ fn wide(s: &str) -> Vec { /// * horizontalement, le texte est dessiné à `pad_x` et commence donc à `pad_x + m.left` : /// la plaque part de `m.left`, l'inset de la boîte de mise en page et la marge de /// plaque se compensent exactement, quel que soit l'alignement ; -/// * verticalement, le texte est dessiné à `pad_y` dans une boîte de mise en page rentrée -/// de `2*pad_y`, donc son haut réel vaut `pad_y + m.top` et la plaque va de `m.top` à -/// `m.top + m.height + 2*pad_y`. Ancré en bas (`DWRITE_PARAGRAPH_ALIGNMENT_FAR`), ce -/// second terme tombe pile sur `box_h` : la marge basse tient tout juste au lieu d'être -/// rognée par le `.min()`. +/// * verticalement, la plaque n'est dessinée QUE si le fond est opaque, et dans ce cas le +/// texte est dessiné à `pad_y` dans une boîte de mise en page rentrée de `2*pad_y` +/// (`anchor_pad` vaut alors `pad_y`). Son haut réel vaut donc `pad_y + m.top` et la +/// plaque va de `m.top` à `m.top + m.height + 2*pad_y`. Ancré en bas +/// (`DWRITE_PARAGRAPH_ALIGNMENT_FAR`), ce second terme tombe pile sur `box_h` : la +/// marge basse tient tout juste au lieu d'être rognée par le `.min()`, et la plaque +/// respire autant en dessous qu'au-dessus du texte. /// /// Le bornage à la boîte est ce qui empêche la plaque d'être coupée net par le bord de /// la texture, où elle perdrait ses coins arrondis. @@ -232,11 +234,18 @@ impl TextRasterizer { let (pad_x, pad_y) = crate::text_plate::padding(font_px); let layout_w = crate::text_plate::layout_width(w as f32, font_px); // La boîte de mise en page est aussi rentrée VERTICALEMENT de la marge de plaque, - // et le texte se dessine à `pad_y`. Sans ça, un ancrage bas colle les glyphes au - // bord de la boîte et le `.min(h)` de la plaque, plus bas, rogne net sa marge - // basse. Le centrage est rigoureusement inchangé par cette paire (l'inset et le - // décalage s'annulent), donc les annotations ne bougent pas d'un pixel. - let layout_h = ((h as f32) - pad_y * 2.0).max(1.0); + // et le texte se dessine à `anchor_pad`. Sans ça, un ancrage bas colle les glyphes + // au bord de la boîte : la plaque pose alors toute sa marge du côté opposé et zéro + // du côté ancré, et le `.min(h)` plus bas rogne net sa marge basse. Ce qui doit + // toucher le bord de la boîte est la PLAQUE, pas les glyphes. + // + // Conditionné à la présence d'une plaque, comme sur les deux autres backends : + // sans fond il n'y a pas de marge à réserver, et réserver quand même décalerait + // le texte de `pad_y` par rapport à Linux et macOS. Le centrage est rigoureusement + // inchangé dans les deux cas (l'inset et le décalage s'annulent), donc les + // annotations ne bougent pas d'un pixel. + let anchor_pad = if spec.background[3] > 0.0 { pad_y } else { 0.0 }; + let layout_h = ((h as f32) - anchor_pad * 2.0).max(1.0); let layout = self .dwrite .CreateTextLayout(&text, &format, layout_w, layout_h)?; @@ -285,7 +294,7 @@ impl TextRasterizer { ); } rt.DrawTextLayout( - D2D_POINT_2F { x: pad_x, y: pad_y }, + D2D_POINT_2F { x: pad_x, y: anchor_pad }, &layout, &brush, D2D1_DRAW_TEXT_OPTIONS_NONE, diff --git a/src/lib/ai-edition/captions/captions.test.ts b/src/lib/ai-edition/captions/captions.test.ts index 9179a3df4..f6b0cde9c 100644 --- a/src/lib/ai-edition/captions/captions.test.ts +++ b/src/lib/ai-edition/captions/captions.test.ts @@ -274,6 +274,19 @@ describe("migrating a pre-anchor document", () => { expect(legacy({ width: 40, offsetX: 30, textAlign: "right" }).anchorH).toBe("right"); }); + it("reproduces the horizontal distance too, instead of snapping to the frame edge", () => { + // The old band's own left edge, not 0: `width: 40` centres its anchor at 30, so + // an offset of −25 put the band at 5% — and that is where the caption must stay. + const left = legacy({ width: 40, offsetX: -25, textAlign: "left" }); + expect(left.anchorH).toBe("left"); + expect(left.insetX).toBeCloseTo(5, 6); + + // Mirrored: the band ends at 95%, so the distance from the right edge is 5%. + const right = legacy({ width: 40, offsetX: 25, textAlign: "right" }); + expect(right.anchorH).toBe("right"); + expect(right.insetX).toBeCloseTo(5, 6); + }); + it("lets a stored anchor win, so the migration runs once and then stays out of the way", () => { const s = legacy({ verticalPosition: "top", offsetY: 0, anchorV: "bottom", insetY: 9 }); expect(s.anchorV).toBe("bottom"); diff --git a/src/lib/ai-edition/captions/settings.ts b/src/lib/ai-edition/captions/settings.ts index 35682c9d9..cc1898de5 100644 --- a/src/lib/ai-edition/captions/settings.ts +++ b/src/lib/ai-edition/captions/settings.ts @@ -368,12 +368,21 @@ function migrateLegacyPlacement( legacyAlign === "left" ? bandX : legacyAlign === "right" ? bandX + width : bandX + width / 2; const anchorH: CaptionAnchorH = inkCentre < 100 / 3 ? "left" : inkCentre > (2 * 100) / 3 ? "right" : "center"; + // And the distance from that edge to where the band actually sat — the same + // reproduce-the-pixels rule as the vertical half. Returning 0 here would have + // snapped every migrated left/right caption flush to the frame edge, which is a + // place the old band almost never was. + const insetX = clamp( + anchorH === "left" ? bandX : anchorH === "right" ? 100 - (bandX + width) : 0, + 0, + CAPTION_INSET_X_MAX, + ); return { anchorV, insetY: Number.isFinite(insetY) ? insetY : fallbackInsetY, anchorH, - insetX: 0, + insetX, }; } From 22e910988442421e6427d214481873bf8d95c7af Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sat, 22 Aug 2026 16:16:18 +0200 Subject: [PATCH 19/30] fix(compositor): a doc comment cannot sit on a function parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rustc rejects `///` on a parameter; it has to be a plain `//`. Only text_macos.rs compiles on macOS, so the machine this was written on could not see it — and rustfmt parses the file happily, so a local syntax gate would not have caught it either. CI did. (cherry picked from commit 2cf1b22789feb164a314a1b040c6a00aebb127a9) --- crates/compositor/src/text_macos.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/compositor/src/text_macos.rs b/crates/compositor/src/text_macos.rs index b7b7ab325..5979ab14c 100644 --- a/crates/compositor/src/text_macos.rs +++ b/crates/compositor/src/text_macos.rs @@ -318,8 +318,9 @@ fn block_layout( text_h: CGFloat, align: u8, valign: &str, - /// Une plaque de fond est-elle dessinée ? Elle décide de la marge à réserver du - /// côté ancré — voir `anchor_pad` plus bas. + // Une plaque de fond est-elle dessinée ? Elle décide de la marge à réserver du côté + // ancré — voir `anchor_pad` plus bas. (Commentaire ordinaire et pas `///` : rustc + // refuse un doc-comment sur un paramètre.) has_plate: bool, font_px: CGFloat, ) -> (CGRect, CGRect) { From 491939f97cfed9699f12b6bb48c94b63c8caa0da Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sat, 22 Aug 2026 16:25:43 +0200 Subject: [PATCH 20/30] fix(captions): let the first write know the output aspect too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `patchCaptionSettings` read the document without an aspect, so the write that MATERIALISES the defaults into a project that never had caption settings always wrote the landscape ones. A 9:16 export got `insetY: 5` frozen in, and the stored value then won for good — putting the caption under the platform's own chrome, which is the exact failure the aspect-derived default exists to prevent. `useCaptions` now resolves the aspect once (through `resolveAspectRatioValue`, the same resolver the preview and the scene description use, so all three agree on what a legacy "native" selection means) and hands it to the read and to every write: `set`, `setLive` and the language reset in `deleteTranslation`. Found by CodeRabbit on #482. (cherry picked from commit 096a551a4b74ed1ce5df3bea228ccd1d0f2dc579) --- src/lib/ai-edition/captions/captions.test.ts | 17 ++++++++++ src/lib/ai-edition/captions/settings.ts | 8 ++++- src/lib/ai-edition/store/useCaptions.ts | 34 +++++++++++++++----- 3 files changed, 50 insertions(+), 9 deletions(-) diff --git a/src/lib/ai-edition/captions/captions.test.ts b/src/lib/ai-edition/captions/captions.test.ts index f6b0cde9c..15ba27f9b 100644 --- a/src/lib/ai-edition/captions/captions.test.ts +++ b/src/lib/ai-edition/captions/captions.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import type { AxcutDocument, AxcutTranscript } from "../schema"; import { captionCuesToTextRegions, deriveCaptionCues } from "./cues"; +import type { CaptionSettings } from "./settings"; import { captionBackgroundCss, captionBoxRect, @@ -297,6 +298,22 @@ describe("migrating a pre-anchor document", () => { expect(getCaptionSettings(doc(), LANDSCAPE).insetY).toBe(5); expect(getCaptionSettings(doc(), PORTRAIT).insetY).toBe(12.5); }); + + it("freezes the PORTRAIT defaults on the first write to a vertical project", () => { + // The first patch is what materialises the defaults into the document, so it + // has to know the aspect too. Patching without it stored the landscape inset + // into a 9:16 project and the stored value then won for good — putting the + // caption under the platform's own chrome, the exact failure the + // aspect-derived default exists to prevent. + const first = patchCaptionSettings(doc(), { enabled: true }, PORTRAIT); + const stored = (first.legacyEditor as { captions: CaptionSettings }).captions; + expect(stored.insetY).toBe(12.5); + expect(stored.insetX).toBe(captionSafeColumn(PORTRAIT).x); + + // And it stays: a later patch reads what is stored rather than re-deriving. + const later = patchCaptionSettings(first, { fontSize: 60 }, PORTRAIT); + expect(getCaptionSettings(later, PORTRAIT).insetY).toBe(12.5); + }); }); describe("deriveCaptionCues", () => { diff --git a/src/lib/ai-edition/captions/settings.ts b/src/lib/ai-edition/captions/settings.ts index cc1898de5..1e776833f 100644 --- a/src/lib/ai-edition/captions/settings.ts +++ b/src/lib/ai-edition/captions/settings.ts @@ -454,8 +454,14 @@ export type CaptionSettingsPatch = Partial; export function patchCaptionSettings( doc: AxcutDocument, patch: CaptionSettingsPatch, + aspectValue = 16 / 9, ): AxcutDocument { - const next: CaptionSettings = { ...getCaptionSettings(doc), ...patch }; + // The aspect matters on the FIRST write to a document that has never carried + // caption settings: that write is what materialises the defaults, and a portrait + // export wants a much larger inset than a landscape one. Reading without it here + // would freeze the landscape default into a 9:16 project — the exact failure the + // aspect-derived default exists to prevent. + const next: CaptionSettings = { ...getCaptionSettings(doc, aspectValue), ...patch }; next.insetY = clamp(next.insetY, 0, CAPTION_INSET_Y_MAX); next.insetX = clamp(next.insetX, 0, CAPTION_INSET_X_MAX); return { diff --git a/src/lib/ai-edition/store/useCaptions.ts b/src/lib/ai-edition/store/useCaptions.ts index 36b13c95e..5238fa150 100644 --- a/src/lib/ai-edition/store/useCaptions.ts +++ b/src/lib/ai-edition/store/useCaptions.ts @@ -17,8 +17,10 @@ import { putCaptionTranslation, removeCaptionTranslation, } from "../captions"; +import { resolveAspectRatioValue } from "../document/outputFormat"; import type { AxcutDocument } from "../schema"; import { useProjectStore } from "./projectStore"; +import { useEditorSettings } from "./useEditorSettings"; export interface UseCaptionsResult { settings: CaptionSettings; @@ -48,7 +50,23 @@ export function useCaptions(): UseCaptionsResult { const setDocument = useProjectStore((s) => s.setDocument); const saveDocument = useProjectStore((s) => s.saveDocument); - const settings = useMemo(() => getCaptionSettings(document), [document]); + // The output aspect decides the caption column and the DEFAULT insets, so it has + // to reach both the read and every write: the first write to a document that has + // never carried caption settings is what freezes those defaults in, and a 9:16 + // export wants a much larger inset than a 16:9 one. `resolveAspectRatioValue` is + // the same resolver the preview and the scene description use — not + // `getAspectRatioValue`, which answers 16/9 for the legacy "native" selection and + // would disagree with what the compositor is handed. + const { settings: editorSettings } = useEditorSettings(); + const aspectValue = useMemo( + () => resolveAspectRatioValue(document, editorSettings.aspectRatio), + [document, editorSettings.aspectRatio], + ); + + const settings = useMemo( + () => getCaptionSettings(document, aspectValue), + [document, aspectValue], + ); const translations = useMemo(() => getCaptionTranslations(document), [document]); const cues = useMemo( () => deriveCaptionCues(document, settings, translations), @@ -65,14 +83,14 @@ export function useCaptions(): UseCaptionsResult { async (patch: CaptionSettingsPatch) => { const doc = useProjectStore.getState().document; if (!doc) return; - const next = patchCaptionSettings(doc, patch); + const next = patchCaptionSettings(doc, patch, aspectValue); // The optimistic write is not the edit — the save is. Only the one that can // fail records, and it names `doc` as what Ctrl+Z returns to because by then // the store already holds `next`. setDocument(next, { history: false }); await saveDocument(next, { history: true, historyBase: doc }); }, - [setDocument, saveDocument], + [setDocument, saveDocument, aspectValue], ); // See `useEditorSettings.setLive`: one undo step per slider drag, not one per @@ -95,12 +113,12 @@ export function useCaptions(): UseCaptionsResult { (patch: CaptionSettingsPatch) => { const doc = useProjectStore.getState().document; if (!doc) return; - const next = patchCaptionSettings(doc, patch); + const next = patchCaptionSettings(doc, patch, aspectValue); if (liveDocRef.current !== doc) liveBaseRef.current = doc; setDocument(next, { history: false }); liveDocRef.current = next; }, - [setDocument], + [setDocument, aspectValue], ); const commit = useCallback(async () => { @@ -133,13 +151,13 @@ export function useCaptions(): UseCaptionsResult { // language currently on screen is the one being deleted. const cleared = removeCaptionTranslation(doc, language); const next = - getCaptionSettings(cleared).language === language - ? patchCaptionSettings(cleared, { language: null }) + getCaptionSettings(cleared, aspectValue).language === language + ? patchCaptionSettings(cleared, { language: null }, aspectValue) : cleared; setDocument(next, { history: false }); await saveDocument(next, { history: true, historyBase: doc }); }, - [setDocument, saveDocument], + [setDocument, saveDocument, aspectValue], ); return { From 34f98eb5834b0c17ccb01e423e96c13a9f1ba690 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sat, 22 Aug 2026 17:33:32 +0200 Subject: [PATCH 21/30] docs(captions): describe the anchor model on the release branch too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The anchor redesign's doc rewrite rode along with the guide-overlay commit, which is add-then-revert within the PR and was therefore skipped here. This carries the surviving half: the settings table, and the sections that described the fixed band, the overhang and the preset machinery — none of which exists any more. Applied as one commit rather than cherry-picked because this branch's copy of the file never received #471's docs commit (docs are excluded from the cherry-pick lane), so its lineage differs from main's and the patch does not apply. --- .../transcription-and-captions.md | 90 ++++++++++++++----- 1 file changed, 66 insertions(+), 24 deletions(-) diff --git a/technical-documentation/architecture/transcription-and-captions.md b/technical-documentation/architecture/transcription-and-captions.md index df899cda3..245519ab5 100644 --- a/technical-documentation/architecture/transcription-and-captions.md +++ b/technical-documentation/architecture/transcription-and-captions.md @@ -519,7 +519,7 @@ unit falls back to the original words (`untranslatedUnits`, Caption appearance lives in `document.legacyEditor.captions`, accessed through `getCaptionSettings` / `patchCaptionSettings` -([`src/lib/ai-edition/captions/settings.ts:217,262`](../../src/lib/ai-edition/captions/settings.ts:217)). +([`src/lib/ai-edition/captions/settings.ts:387,445`](../../src/lib/ai-edition/captions/settings.ts:387)). | Field | Default | Notes | |---|---|---| @@ -528,11 +528,11 @@ through `getCaptionSettings` / `patchCaptionSettings` | `fontSize` | `48` | Pixels at a 1080-high frame; `annotationFontSizeFraction` turns that into a fraction of the box being drawn into (`src/lib/ai-edition/annotationScale.ts`), resolution-free. | | `fontFamily`, `fontWeight`, `color` | `Inter`, `bold`, `#ffffff` | Drawn from the same font families `src/index.css` already loads — anything else would render in the preview but fall back to a default in the export canvas. | | `backgroundEnabled`, `backgroundColor`, `backgroundOpacity` | `true`, `#000000`, `0.55` | When off, the text draws straight over the video with no plate. | -| `verticalPosition` | `bottom` | `top` / `middle` / `bottom`. `captionBandRect` anchors the band; `offsetY` nudges it from there. | -| `offsetY` | `0` | Fine nudge in % of **frame** height, on top of the anchor. Positive moves down. | -| `offsetX` | `0` | Fine nudge in % of **frame** width. Positive moves toward the right edge of the exported frame — frame geometry, so an RTL interface locale never mirrors it. | -| `width` | `80` | Band width in % of frame width. | -| `minWordsPerLine` / `maxWordsPerLine` | `2` / `7` | Line-group bounds; `groupTimedCaptionWordsIntoLines` packs inside the range, `[1, 12]` after clamp. | +| `anchorV` | `bottom` | `bottom` / `top` — which frame edge the drawn block is pinned to. It grows AWAY from that edge, so the edge never moves. | +| `insetY` | `5` (12.5 on a vertical export) | Distance from the edge named by `anchorV` to the near edge of what is DRAWN, in % of frame height. Always ≥ 0. | +| `anchorH` | `center` | `left` / `center` / `right` — which edge of the block is pinned horizontally, and the ragged edge when the text wraps. | +| `insetX` | the column's own margin | Distance from the edge named by `anchorH`. Ignored when `anchorH` is `center`, which has no edge to measure from. | +| `minWordsPerLine` / `maxWordsPerLine` | `2` / `7` | Line-group bounds; `groupTimedCaptionWordsIntoLines` packs inside the range, `[1, 12]` after clamp. Also the only control over how much text is on screen — there is no width slider. | #### Coordinate space @@ -558,26 +558,68 @@ The **font denominator follows the same box.** Flipping the rect without the denominator would hold a caption still while its glyphs kept shrinking with the padding slider, which is why both come off one `anchor` local in each backend. -#### Reach +#### Anchoring -`CAPTION_BAND_HEIGHT_PCT = 22` +**A caption is placed by pinning one edge of the drawn block, never by centring it +in a box.** `captionBoxRect` ([`src/lib/ai-edition/captions/settings.ts`](../../src/lib/ai-edition/captions/settings.ts)) -is generous enough for two wrapped lines at the default size — the -renderers clip to it, so it is deliberately not tight. - -The band is a **box**; the visible caption is a strip centred inside it. All three -rasterizers centre the text vertically, so a box stopped flush against the frame -edge leaves its glyphs half a band short of it. `captionOffsetRange` therefore -lets the box hang off the top or bottom by exactly its empty margin — the ink -reaches the edge, and nothing drawn leaves the frame. The size of that margin is -derived from `fontSize`, because the slice of the band that carries ink is: a -200px caption fills the whole band, gets no overhang, and stays whole. - -`captionOffsetRange` is also what the inspector's sliders take their bounds from, -so the reachable span and the slider span are the same span. Before #396 both -ends were hardcoded to ±45 while the result was clamped separately, which left -the bottom anchor honouring only −45…+3 — nearly half the slider moved the handle -and nothing else. +returns the box plus a `verticalAlign`, and the compositor puts the block flush +against that edge of it. The invariant, which the tests assert as a property: + +> Bottom anchor: the drawn block's bottom edge is at `100 − insetY` % of frame +> height. Top anchor: its top edge is at `insetY` %. For every font size, every +> background state, every word count, every wrap outcome, every output resolution. + +No estimate of the block's height participates in placing it. The box's height is +**headroom** — how many lines can be drawn before the renderer clips — so being +wrong about it costs a clipped fourth line, not a moved subtitle. + +That distinction is the whole of the redesign. The model this replaced put the +caption in a fixed 22 % box and let all three rasterizers centre the ink inside it. +A centred block moves BOTH its edges as it grows, so: + +- wrapping to another line shifted the caption vertically, and so did anything that + changed wrapping — which is how the *width* slider ended up moving the caption on + the *vertical* axis; +- the box had to be allowed to hang off the frame by its own empty margin for the + glyphs to reach the edge at all, and that margin was derived from `fontSize`; +- the offset therefore had to be signed and clamped against a reachable range that + moved with four other fields, which is why the inspector could show `-7.3 %` — + a number that corresponds to nothing in any subtitle format. + +All of that is deleted. An inset is a distance from a named edge, so it means the +same thing whatever else changes, and `patchCaptionSettings` has no re-clamping +pass any more. + +Bottom-anchored growth is not an invention here: it is the default in every +subtitle format. `tts:displayAlign="after"` (TTML/IMSC, which the BBC requires on +every region), `\an2` with `MarginV` measured from the bottom (ASS), `line:auto` +resolving to −1 and pushing the box upward (WebVTT), roll-up scrolling (CEA-708). +Deliberately absent: a "middle" anchor. XSL 1.1 defines `display-align: center` as +keeping both edge distances equal — which is precisely the pathology above — and a +bottom anchor with a large `insetY` reaches the same place while still growing +upward. + +#### The column, and why there is no width control + +`captionSafeColumn` derives the wrap width from the output aspect — 68 % of a 16:9 +frame, 90 % of a squarer or vertical one (the BBC line-length table; 68 % at 48 px +on 1080p is ≈45 characters, inside the Netflix 42 / BBC 37 band). It is never +stored and never exposed. + +It used to be a `width` slider, and that control could not be understood: the +background plate hugs the TEXT, not the box, so moving it changed nothing visible +until the text happened to be long enough to wrap. What it actually controlled is +how much text is on screen — a question `minWordsPerLine` / `maxWordsPerLine` +already answers in words rather than in percent. + +The horizontal axis has one control, `anchorH`, which reaches the rasterizers as +the existing `textAlign`. That is not a coincidence: their plate maths already +snaps the plate onto the box's left or right edge (`text_linux.rs`'s `plate_x`, +and its two mirrors), so the alignment *is* the pivot. The model before this had +two controls fighting over that one outcome — `offsetX` moved an invisible band and +`textAlign` moved the text inside it — and neither could be read without seeing the +band. The Captions pane itself ([`src/components/ai-edition/CaptionsPane.tsx`](../../src/components/ai-edition/CaptionsPane.tsx)) From b71311e4a34b20d4a2c6f7a49c7285b73f0944ff Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sat, 22 Aug 2026 17:43:29 +0200 Subject: [PATCH 22/30] fix(captions): default the insets to what actually sits well with the padding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1.5% from the anchored edge, 10% from the horizontal one. Picked by eye against the editor's default padding rather than from a broadcast spec: the footage sits inset inside the frame, so what reads as "just off the edge" is a much smaller number than the 5% BBC states for a full-bleed broadcast frame. Landscape only — those two values were eyeballed on a 16:9 export. Vertical keeps its 12.5%, which answers a different question: TikTok, Reels and Shorts draw their own chrome over the bottom eighth of a 9:16 video, so 1.5% there would put the caption behind a UI. Nobody has looked at that case, so it stays on the conservative value. (cherry picked from commit 7dcd9f0c1fa964bbc03528497e00a5ddc8ba49fb) --- src/lib/ai-edition/captions/captions.test.ts | 14 +++++--- src/lib/ai-edition/captions/index.ts | 1 + src/lib/ai-edition/captions/settings.ts | 38 ++++++++++++++------ 3 files changed, 38 insertions(+), 15 deletions(-) diff --git a/src/lib/ai-edition/captions/captions.test.ts b/src/lib/ai-edition/captions/captions.test.ts index 15ba27f9b..ce3d4a961 100644 --- a/src/lib/ai-edition/captions/captions.test.ts +++ b/src/lib/ai-edition/captions/captions.test.ts @@ -7,6 +7,7 @@ import { captionBoxRect, captionSafeColumn, DEFAULT_CAPTION_SETTINGS, + defaultCaptionInsetX, defaultCaptionInsetY, getCaptionSettings, patchCaptionSettings, @@ -233,8 +234,12 @@ describe("caption safe column", () => { }); it("defaults a vertical export well clear of the platform chrome", () => { - // 5% on a 9:16 export puts the caption under the Reels/TikTok profile row. - expect(defaultCaptionInsetY(LANDSCAPE)).toBe(5); + // The landscape values are eyeballed against the editor's default padding; the + // portrait one answers a different question — TikTok/Reels/Shorts draw their own + // chrome over the bottom eighth of a 9:16 export, so the same 1.5% would put the + // caption behind a UI. + expect(defaultCaptionInsetY(LANDSCAPE)).toBe(1.5); + expect(defaultCaptionInsetX(LANDSCAPE)).toBe(10); expect(defaultCaptionInsetY(PORTRAIT)).toBeGreaterThan(10); }); }); @@ -295,7 +300,8 @@ describe("migrating a pre-anchor document", () => { }); it("gives a document with no caption settings the aspect-appropriate default", () => { - expect(getCaptionSettings(doc(), LANDSCAPE).insetY).toBe(5); + expect(getCaptionSettings(doc(), LANDSCAPE).insetY).toBe(1.5); + expect(getCaptionSettings(doc(), LANDSCAPE).insetX).toBe(10); expect(getCaptionSettings(doc(), PORTRAIT).insetY).toBe(12.5); }); @@ -308,7 +314,7 @@ describe("migrating a pre-anchor document", () => { const first = patchCaptionSettings(doc(), { enabled: true }, PORTRAIT); const stored = (first.legacyEditor as { captions: CaptionSettings }).captions; expect(stored.insetY).toBe(12.5); - expect(stored.insetX).toBe(captionSafeColumn(PORTRAIT).x); + expect(stored.insetX).toBe(defaultCaptionInsetX(PORTRAIT)); // And it stays: a later patch reads what is stored rather than re-deriving. const later = patchCaptionSettings(first, { fontSize: 60 }, PORTRAIT); diff --git a/src/lib/ai-edition/captions/index.ts b/src/lib/ai-edition/captions/index.ts index ea5a58413..f827d62c0 100644 --- a/src/lib/ai-edition/captions/index.ts +++ b/src/lib/ai-edition/captions/index.ts @@ -21,6 +21,7 @@ export { captionBoxRect, captionSafeColumn, DEFAULT_CAPTION_SETTINGS, + defaultCaptionInsetX, defaultCaptionInsetY, getCaptionSettings, patchCaptionSettings, diff --git a/src/lib/ai-edition/captions/settings.ts b/src/lib/ai-edition/captions/settings.ts index 1e776833f..e50d2e9c1 100644 --- a/src/lib/ai-edition/captions/settings.ts +++ b/src/lib/ai-edition/captions/settings.ts @@ -97,11 +97,12 @@ export const DEFAULT_CAPTION_SETTINGS: CaptionSettings = { backgroundColor: "#000000", backgroundOpacity: 0.55, anchorV: "bottom", - // Overridden per output aspect on first read (`defaultCaptionInsetY`); this is the - // landscape value, and the one a document keeps once anything has been written. - insetY: 5, + // Overridden per output aspect on first read (`defaultCaptionInsetY` / + // `defaultCaptionInsetX`); these are the landscape values, and the ones a document + // keeps once anything has been written. + insetY: 1.5, anchorH: "center", - insetX: 16, + insetX: 10, minWordsPerLine: 2, maxWordsPerLine: 7, }; @@ -159,12 +160,27 @@ export function captionSafeColumn(aspectValue: number): { x: number; width: numb return aspectValue >= CAPTION_LANDSCAPE_ASPECT ? { x: 16, width: 68 } : { x: 5, width: 90 }; } -/** Default distance from the anchored edge, in % of frame height, for a given output - * aspect. BBC puts the lowest line at a 5% inset; vertical formats need much more, - * because the bottom eighth of a 9:16 export is where TikTok, Reels and Shorts draw - * their own chrome over the video. */ +/** Default distance from the anchored edge, in % of frame height. + * + * The landscape value is picked against the editor's DEFAULT PADDING rather than from + * a broadcast spec: the footage sits inset inside the frame, so a caption a hair off + * the frame edge lands where the eye expects it *relative to the picture*. It was + * chosen by looking at a 16:9 export. + * + * Vertical keeps a much larger inset, and for an unrelated reason: the bottom eighth + * of a 9:16 export is where TikTok, Reels and Shorts draw their own chrome over the + * video, so a caption sitting a hair off that edge is a caption behind a UI. Nobody + * has eyeballed this one — it is the conservative value, and the slider is right + * there if it proves wrong. */ export function defaultCaptionInsetY(aspectValue: number): number { - return aspectValue >= CAPTION_LANDSCAPE_ASPECT ? 5 : 12.5; + return aspectValue >= CAPTION_LANDSCAPE_ASPECT ? 1.5 : 12.5; +} + +/** Default distance for the horizontal anchor, in % of frame width. Same source as the + * landscape `insetY`: it matches the default padding, not the safe column's own margin, + * which is why it is stated here rather than borrowed from `captionSafeColumn`. */ +export function defaultCaptionInsetX(aspectValue: number): number { + return aspectValue >= CAPTION_LANDSCAPE_ASPECT ? 10 : 5; } /** Upper bound for the vertical inset, in % of the frame. 50 puts the anchored edge @@ -400,7 +416,7 @@ export function getCaptionSettings( const raw = storedCaptions(doc); const d = DEFAULT_CAPTION_SETTINGS; const defaultInsetY = defaultCaptionInsetY(aspectValue); - if (!raw) return { ...d, insetY: defaultInsetY, insetX: captionSafeColumn(aspectValue).x }; + if (!raw) return { ...d, insetY: defaultInsetY, insetX: defaultCaptionInsetX(aspectValue) }; const minWords = Math.round(readNumber(raw.minWordsPerLine, d.minWordsPerLine, 1, 12)); const maxWords = Math.round(readNumber(raw.maxWordsPerLine, d.maxWordsPerLine, 1, 12)); @@ -416,7 +432,7 @@ export function getCaptionSettings( anchorH: readEnum(raw.anchorH, ANCHORS_H, legacy?.anchorH ?? d.anchorH), insetX: readNumber( raw.insetX, - legacy?.insetX ?? captionSafeColumn(aspectValue).x, + legacy?.insetX ?? defaultCaptionInsetX(aspectValue), 0, CAPTION_INSET_X_MAX, ), From 9c6f624c2d8ab8d6da88611021ba1f7f39da0097 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 22 Aug 2026 15:51:32 +0000 Subject: [PATCH 23/30] chore(release): bump to 1.10.0-rc.3 [skip ci] --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index fb722503e..9ba76b6c4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "openscreen", - "version": "1.10.0-rc.2", + "version": "1.10.0-rc.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "openscreen", - "version": "1.10.0-rc.2", + "version": "1.10.0-rc.3", "dependencies": { "@fix-webm-duration/fix": "^1.0.1", "@langchain/anthropic": "^1.3.26", diff --git a/package.json b/package.json index 83b7210cd..baa1737d2 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "openscreen", "private": true, - "version": "1.10.0-rc.2", + "version": "1.10.0-rc.3", "description": "Record your screen and polish the demo", "homepage": "https://getopenscreen.com/", "license": "MIT", From c9cd061ddd5286718c89933017235594139fd2e4 Mon Sep 17 00:00:00 2001 From: NADY <3747231+naadydev@users.noreply.github.com> Date: Sun, 23 Aug 2026 10:54:52 +0200 Subject: [PATCH 24/30] fix: initialise CoreGraphics before building a window capture filter Selecting a window in the source picker aborted the ScreenCaptureKit helper before it produced a single frame: Assertion failed: (did_initialize), function CGS_REQUIRE_INIT, file CGInitialization.c, line 44 The helper is a plain command-line executable, so nothing in it ever connects to the window server. SCContentFilter(desktopIndependentWindow:) resolves which display a window sits on by calling into SkyLight (SLSGetDisplaysWithRect), and SkyLight asserts when CoreGraphics was never initialised in the process. Display capture is unaffected, because SCContentFilter(display:excludingWindows:) is handed an already-resolved display and never asks SkyLight to resolve a rect. That is why only the window branch of makeCaptureTarget crashed. Touching any CoreGraphics display API performs the initialisation, so a single CGMainDisplayID() at the top of main() is enough. CoreGraphics is already imported; this avoids pulling AppKit into the helper or standing up an NSApplication in a CLI process. --- .../ScreenCaptureRecorder.swift | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift b/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift index 75163dee7..2e88e5c41 100644 --- a/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift +++ b/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift @@ -802,6 +802,15 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { struct OpenScreenScreenCaptureKitHelper { static func main() async { do { + // This helper is a plain command-line executable, so nothing has connected it to + // the window server yet. `SCContentFilter(desktopIndependentWindow:)` reaches into + // SkyLight (`SLSGetDisplaysWithRect`) to find the display a window sits on, and + // SkyLight aborts with `CGS_REQUIRE_INIT` when CoreGraphics was never initialised + // in the process — so every window capture crashed before it produced a frame, + // while display capture (which never resolves a rect) worked fine. Touching any + // CoreGraphics display API first performs that initialisation. + _ = CGMainDisplayID() + guard CommandLine.arguments.count == 2 else { throw HelperError.invalidArguments } From f5b33f945c58138fb625e27e74c5bdd1c338aa11 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sun, 23 Aug 2026 13:25:52 +0200 Subject: [PATCH 25/30] fix(recording): use hardware H.264 on Windows by default, not just when asked MF_READWRITE_ENABLE_HARDWARE_TRANSFORMS defaults to FALSE, and the "default" sink-writer path (no preferSoftwareEncoder, no OPENSCREEN_WGC_ENABLE_DXGI_INPUT) never set it. So every plain recording ran on the software H.264 encoder regardless of what GPU the machine had -- the DXGI path was the only one that ever asked for hardware. On a fast CPU that's invisible; on the older machines in #460 (a 6th-gen i3, an i5-4590 with HD 4600) it's slow enough to blow the 50-60s stop-shutdown budget and lose the whole recording to a "Timed out waiting for native Windows capture to stop" failure. createSinkWriter now asks for hardware transforms whenever software is not forced, DXGI device manager or not. Verified against the real compiled helper: the default path went from videoEncoderRuntime "software" to "hardware" on this machine, with no other flags set. That uncovered a second, known issue the DXGI path had already fixed once: hardware MFTs default to constant bitrate, which spends the full configured budget doing nothing on a static screen. applyHardwareRateControl's VBR fix was gated on the DXGI path alone; it now runs whenever hardware transforms were requested, matching the wider condition above. Added videoEncoderRuntime ("hardware"/"software"/"unknown") to the encoder-selection event so a bug report can tell these two failure shapes apart going forward: a real hardware encoder stalling on a bad driver, versus every recording quietly running through software regardless of what hardware is on the machine. It introspects the sink writer's own resolved transform pipeline (IMFSinkWriterEx::GetTransformForStream) rather than trusting which path was configured, since MF is free to hand back software even when hardware was requested. Verified end to end on real hardware: compiled with MSVC/CMake, ran the actual helper through the full test matrix (default, software-encoder, DXGI, window, system-audio, microphone, audio-timeline, mic-selection) with no regressions. One accepted trade-off, confirmed back-to-back on this machine: hardware output ran roughly 5x larger than software for the same content even with VBR correctly engaged (8.7 Mbps vs 1.7 Mbps) -- a real rate-distortion difference between the two encoders, not a rate-control bug, and worth the CPU relief and stop-reliability it buys on weak machines. Co-Authored-By: Claude Sonnet 5 --- electron/ipc/handlers.ts | 7 + electron/native/wgc-capture/src/main.cpp | 9 +- .../native/wgc-capture/src/mf_encoder.cpp | 132 +++++++++++++++--- electron/native/wgc-capture/src/mf_encoder.h | 26 ++++ scripts/test-windows-wgc-helper.mjs | 28 ++++ src/lib/nativeWindowsRecording.ts | 7 + .../architecture/recording.md | 3 +- 7 files changed, 192 insertions(+), 20 deletions(-) diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index 8596dfafd..0f20dd564 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -1353,6 +1353,12 @@ function readNativeWindowsEncoderSelection(output: string) { // which is what `salvageNativeWindowsFragmentedCapture` asks. container?: string; preferSoftwareEncoder?: boolean; + // Whether BeginWriting() actually landed on a hardware H.264 MFT, as + // opposed to `video` above, which only says which configuration path + // was tried. "default" plus a software runtime means the machine never + // got hardware acceleration in the first place -- see + // kVideoEncoderRuntime* in mf_encoder.h. + videoEncoderRuntime?: string; }; } catch { return null; @@ -2531,6 +2537,7 @@ export function registerIpcHandlers( path: outputPath, helperPath, videoEncoderSelection: encoderSelection?.video ?? null, + videoEncoderRuntime: encoderSelection?.videoEncoderRuntime ?? null, webcamUnavailable, microphoneDefaulted, }; diff --git a/electron/native/wgc-capture/src/main.cpp b/electron/native/wgc-capture/src/main.cpp index 1479bc842..c64d79703 100644 --- a/electron/native/wgc-capture/src/main.cpp +++ b/electron/native/wgc-capture/src/main.cpp @@ -865,7 +865,14 @@ int main(int argc, char* argv[]) { << "\",\"container\":\"" << encoder.containerFormat() << "\",\"preferSoftwareEncoder\":" << (config.preferSoftwareEncoder ? "true" : "false") - << "}" << std::endl; + // What BeginWriting() actually landed on, not what the "video" + // field above asked for -- see kVideoEncoderRuntime* in + // mf_encoder.h. "default" plus "software" here means the machine + // never got a hardware encoder in the first place, which is a + // different bug report than "default" plus "hardware" stalling + // on stop. + << ",\"videoEncoderRuntime\":\"" << encoder.videoEncoderRuntime() + << "\"}" << std::endl; MFEncoder webcamEncoder; if (writeSeparateWebcam) { MFEncoderOptions webcamEncoderOptions = encoderOptions; diff --git a/electron/native/wgc-capture/src/mf_encoder.cpp b/electron/native/wgc-capture/src/mf_encoder.cpp index 4058ca249..c1e5c2bbe 100644 --- a/electron/native/wgc-capture/src/mf_encoder.cpp +++ b/electron/native/wgc-capture/src/mf_encoder.cpp @@ -140,7 +140,7 @@ enum class SinkWriterCreateStage { SoftwareEncoderRegistration, CreateAttributes, DisableHardwareTransforms, - ConfigureDxgiManager, + EnableHardwareTransforms, CreateFile, CreateFragmentedMediaSink, CreateSinkWriter, @@ -248,10 +248,30 @@ HRESULT createSinkWriter( failedStage = SinkWriterCreateStage::DisableHardwareTransforms; return hr; } - } else if (dxgiDeviceManager != nullptr) { - HRESULT hr = MFCreateAttributes(&attributes, 3); + } else { + // Ask for hardware transforms whenever software is not forced -- + // whether or not a DXGI device manager came with the request. + // MF_READWRITE_ENABLE_HARDWARE_TRANSFORMS defaults to FALSE, and + // leaving it unset (the old behaviour on the plain CPU-readback path) + // meant the sink writer never considered a hardware H.264 MFT even + // when one was registered and working: every "default" recording + // landed on the same software encoder forceSoftwareEncoder asks for + // explicitly, on any machine that had not separately opted into + // OPENSCREEN_WGC_ENABLE_DXGI_INPUT (getopenscreen/openscreen#460, + // confirmed by videoEncoderRuntime on real hardware: "default" read + // back "software" until the DXGI path was turned on, on a machine + // whose encoder is hardware-capable either way). + // + // A hardware MFT does not require the D3D manager to accept samples: + // without one it manages its own device and takes system-memory + // samples the same way the software encoder does, which is exactly + // the CPU-readback path this branch also serves. So the attribute is + // set unconditionally here; only the manager itself stays behind the + // null check, since supplying a manager the caller does not have would + // be undefined rather than merely declined. + HRESULT hr = MFCreateAttributes(&attributes, dxgiDeviceManager != nullptr ? 3 : 1); if (FAILED(hr)) { - std::cerr << "ERROR: MFCreateAttributes(DXGI sink writer) failed (hr=0x" + std::cerr << "ERROR: MFCreateAttributes(sink writer) failed (hr=0x" << std::hex << hr << std::dec << ")" << std::endl; failedStage = SinkWriterCreateStage::CreateAttributes; return hr; @@ -260,15 +280,17 @@ HRESULT createSinkWriter( if (FAILED(hr)) { std::cerr << "ERROR: Set MF_READWRITE_ENABLE_HARDWARE_TRANSFORMS(TRUE) failed (hr=0x" << std::hex << hr << std::dec << ")" << std::endl; - failedStage = SinkWriterCreateStage::ConfigureDxgiManager; + failedStage = SinkWriterCreateStage::EnableHardwareTransforms; return hr; } - hr = attributes->SetUnknown(MF_SINK_WRITER_D3D_MANAGER, dxgiDeviceManager); - if (FAILED(hr)) { - std::cerr << "ERROR: Set MF_SINK_WRITER_D3D_MANAGER failed (hr=0x" - << std::hex << hr << std::dec << ")" << std::endl; - failedStage = SinkWriterCreateStage::ConfigureDxgiManager; - return hr; + if (dxgiDeviceManager != nullptr) { + hr = attributes->SetUnknown(MF_SINK_WRITER_D3D_MANAGER, dxgiDeviceManager); + if (FAILED(hr)) { + std::cerr << "ERROR: Set MF_SINK_WRITER_D3D_MANAGER failed (hr=0x" + << std::hex << hr << std::dec << ")" << std::endl; + failedStage = SinkWriterCreateStage::EnableHardwareTransforms; + return hr; + } } } @@ -382,6 +404,68 @@ bool resolveStreamSinkIndex(IMFMediaSink* mediaSink, const GUID& majorType, DWOR return false; } +// Did the video stream's encoder MFT actually land on hardware? +// +// BeginWriting() succeeding says nothing about this: on the "default" path +// (see kVideoEncoderRuntime* in mf_encoder.h) no attribute asked for hardware +// transforms, so Media Foundation is free to hand the sink writer a software +// MFT even when a hardware one is registered and would have worked. The only +// way to know which one it actually picked is to ask the pipeline it built, +// after the fact -- IMFSinkWriterEx::GetTransformForStream walks the MFTs the +// sink writer inserted for a stream, and a hardware MFT instance is required +// to expose MFT_ENUM_HARDWARE_URL_Attribute on its own attribute store (not +// just on the IMFActivate MFTEnumEx returns), which is what distinguishes it +// from a software one at this point. +// +// Every failure path here returns "unknown" rather than guessing: this runs +// after the sink writer is already committed to, so it must never be able to +// fail configureSinkWriterAttempt, and a wrong hardware/software guess in a +// bug report would be worse than an admitted "could not tell." +const char* detectVideoEncoderRuntime(IMFSinkWriter* sinkWriter, DWORD videoStreamIndex) { + Microsoft::WRL::ComPtr sinkWriterEx; + if (FAILED(sinkWriter->QueryInterface(IID_PPV_ARGS(&sinkWriterEx)))) { + return kVideoEncoderRuntimeUnknown; + } + + for (DWORD mftIndex = 0;; mftIndex += 1) { + GUID category{}; + Microsoft::WRL::ComPtr transform; + const HRESULT hr = + sinkWriterEx->GetTransformForStream(videoStreamIndex, mftIndex, &category, &transform); + if (hr == MF_E_INVALIDINDEX) { + // Walked the whole pipeline (converters, the encoder, anything + // else the topology loader inserted) without finding an encoder + // node. Should not happen -- an H.264 stream has to have one -- + // but this is diagnostics code, not the recording path, so an + // unexpected shape is "unknown", not a crash. + return kVideoEncoderRuntimeUnknown; + } + if (FAILED(hr)) { + return kVideoEncoderRuntimeUnknown; + } + if (category != MFT_CATEGORY_VIDEO_ENCODER) { + // A colour converter or similar the sink writer inserted ahead of + // the encoder. Keep walking; the encoder is further down. + continue; + } + + Microsoft::WRL::ComPtr transformAttributes; + if (FAILED(transform->GetAttributes(&transformAttributes))) { + return kVideoEncoderRuntimeUnknown; + } + UINT32 hardwareUrlLength = 0; + const HRESULT hardwareUrlHr = + transformAttributes->GetStringLength(MFT_ENUM_HARDWARE_URL_Attribute, &hardwareUrlLength); + if (SUCCEEDED(hardwareUrlHr)) { + return kVideoEncoderRuntimeHardware; + } + if (hardwareUrlHr == MF_E_ATTRIBUTENOTFOUND) { + return kVideoEncoderRuntimeSoftware; + } + return kVideoEncoderRuntimeUnknown; + } +} + void logSinkWriterCreateFailure( HRESULT sinkWriterHr, const char* createCall, @@ -513,6 +597,10 @@ const char* MFEncoder::videoEncoderSelection() const { return videoEncoderSelection_; } +const char* MFEncoder::videoEncoderRuntime() const { + return videoEncoderRuntime_; +} + const char* MFEncoder::containerFormat() const { return containerFormat_; } @@ -600,6 +688,7 @@ bool MFEncoder::initialize( // encoder, never reaching the software encoder the knob is aimed at. useDxgiInput_ = options.useDxgiInput && !options.injectDefaultSinkWriterFailureOnce; videoEncoderSelection_ = kVideoEncoderSelectionDefault; + videoEncoderRuntime_ = kVideoEncoderRuntimeUnknown; if (!succeeded(MFStartup(MF_VERSION), "MFStartup")) { return false; @@ -689,6 +778,7 @@ bool MFEncoder::initialize( audioStreamIndex_ = 0; hasAudioStream_ = false; videoEncoderSelection_ = kVideoEncoderSelectionDefault; + videoEncoderRuntime_ = kVideoEncoderRuntimeUnknown; containerFormat_ = kContainerFormatMp4; }; @@ -780,7 +870,7 @@ bool MFEncoder::initialize( "SetInputMediaType")) { return false; } - if (useDxgiInput_) { + if (!forceSoftwareEncoder) { applyHardwareRateControl(std::max(1, bitrate)); } if (!succeeded(sinkWriter_->BeginWriting(), "BeginWriting")) { @@ -788,6 +878,7 @@ bool MFEncoder::initialize( } videoEncoderSelection_ = selection; + videoEncoderRuntime_ = detectVideoEncoderRuntime(sinkWriter_.Get(), videoStreamIndex_); containerFormat_ = fragmented ? kContainerFormatFragmentedMp4 : kContainerFormatMp4; return true; }; @@ -1204,12 +1295,17 @@ bool MFEncoder::initializeVideoProcessor() { } void MFEncoder::applyHardwareRateControl(int bitrate) { - // The D3D manager switches the sink writer onto a hardware MFT, and those - // default to constant bitrate: a static desktop then spends the full - // configured budget doing nothing, 16.9 Mbps measured against the 1.95 the - // software encoder the CPU path lands on produced for the same screen. Same - // budget, opposite reading of it. Ask for VBR so the GPU path spends what - // the picture costs, which is what users have been getting all along. + // Enabling MF_READWRITE_ENABLE_HARDWARE_TRANSFORMS can hand the sink + // writer a hardware MFT, and those default to constant bitrate: a static + // desktop then spends the full configured budget doing nothing, 16.9 Mbps + // measured against the 1.95 the software encoder produced for the same + // screen. Same budget, opposite reading of it. Ask for VBR so a hardware + // encoder spends what the picture costs, which is what the software + // encoder was already doing. Called whenever hardware transforms were + // requested, DXGI device manager or not (getopenscreen/openscreen#460) -- + // whether the sink writer actually landed on hardware is not knowable + // until after BeginWriting() (see MFEncoder::videoEncoderRuntime()), and + // this call is a no-op on a software MFT that ignores or lacks the knob. // // Best effort on purpose. An encoder that exposes neither knob still // produces a valid recording, and a bitrate we could not pin down is not diff --git a/electron/native/wgc-capture/src/mf_encoder.h b/electron/native/wgc-capture/src/mf_encoder.h index f8370874c..19fac7004 100644 --- a/electron/native/wgc-capture/src/mf_encoder.h +++ b/electron/native/wgc-capture/src/mf_encoder.h @@ -42,6 +42,28 @@ constexpr const char* kVideoEncoderSelectionDefault = "default"; constexpr const char* kVideoEncoderSelectionSoftwarePreferred = "software-preferred"; constexpr const char* kVideoEncoderSelectionSoftwareFallback = "software-fallback"; +// Whether BeginWriting() actually landed on a hardware-accelerated H.264 MFT. +// +// videoEncoderSelection() above says which *path* initialize() took -- +// whether the DXGI GPU pipeline was asked for, or software was forced -- but +// none of those labels says what Media Foundation itself picked, and that +// matters even now that createSinkWriter asks for hardware transforms on +// every path but the forced-software one: MF_READWRITE_ENABLE_HARDWARE_TRANSFORMS +// asks, it does not guarantee -- a machine with no hardware H.264 MFT +// registered, or one whose driver refuses it, still lands on software. That +// gap is exactly what this exists to close for a bug report: "default" alone +// cannot tell a real hardware encode apart from software Media Foundation +// picked anyway, which was the whole ambiguity behind a slow-CPU stop timeout +// (getopenscreen/openscreen#460) before this field existed. +constexpr const char* kVideoEncoderRuntimeHardware = "hardware"; +constexpr const char* kVideoEncoderRuntimeSoftware = "software"; +// Introspection itself failed (no IMFSinkWriterEx, no encoder node found in +// the resolved topology, GetAttributes refused). Reported as its own value +// rather than guessed into hardware or software, because a bug report that +// cannot tell "we checked and it's software" from "we couldn't check" would +// draw the wrong conclusion either way. +constexpr const char* kVideoEncoderRuntimeUnknown = "unknown"; + // Which MP4 flavour the recording was actually written in. The fragmented sink // writes a self-describing moof+mdat pair roughly every second, so a helper the // shutdown watchdog force-exits leaves a file that plays up to the last @@ -97,6 +119,9 @@ class MFEncoder { bool writeAudio(const BYTE* data, DWORD byteCount, int64_t timestampHns, int64_t durationHns); bool finalize(); const char* videoEncoderSelection() const; + // Best-effort, read only after initialize() returns true. See the + // kVideoEncoderRuntime* constants above for what each value means. + const char* videoEncoderRuntime() const; // Which container initialize() settled on, which is not necessarily the one // it asked for: the fragmented sink degrades to the plain one rather than // failing a recording. A bug report that cannot tell the two apart cannot @@ -202,5 +227,6 @@ class MFEncoder { bool finalized_ = false; bool useDxgiInput_ = false; const char* videoEncoderSelection_ = kVideoEncoderSelectionDefault; + const char* videoEncoderRuntime_ = kVideoEncoderRuntimeUnknown; const char* containerFormat_ = kContainerFormatMp4; }; diff --git a/scripts/test-windows-wgc-helper.mjs b/scripts/test-windows-wgc-helper.mjs index e1dc48148..1e513c721 100644 --- a/scripts/test-windows-wgc-helper.mjs +++ b/scripts/test-windows-wgc-helper.mjs @@ -554,6 +554,34 @@ if ( `WGC helper encoder selection was ${JSON.stringify(encoderSelection)}, expected ${expectedEncoderSelection} with preferSoftwareEncoder=${WITH_SOFTWARE_ENCODER}: ${result.stdout}`, ); } +// videoEncoderRuntime is separate from `video` above: it is what +// GetTransformForStream found in the sink writer's own resolved pipeline +// after BeginWriting(), not which configuration path was tried. "unknown" +// here on a run that otherwise passed means the introspection itself is +// broken (wrong COM call, wrong category, wrong attribute), not a real +// ambiguity -- a healthy sink writer always has exactly one encoder node. +if (!["hardware", "software", "unknown"].includes(encoderSelection.videoEncoderRuntime)) { + throw new Error( + `WGC helper reported an unrecognised videoEncoderRuntime: ${JSON.stringify(encoderSelection)}`, + ); +} +if (encoderSelection.videoEncoderRuntime === "unknown") { + throw new Error( + `WGC helper could not introspect its own sink writer for videoEncoderRuntime: ${JSON.stringify(encoderSelection)}`, + ); +} +// forceSoftwareEncoder disables hardware transforms explicitly +// (MF_READWRITE_ENABLE_HARDWARE_TRANSFORMS=FALSE), so this is deterministic +// regardless of what the test machine has registered -- unlike the "default" +// path, whose runtime legitimately depends on the machine. +if ( + (WITH_SOFTWARE_ENCODER || WITH_SOFTWARE_FALLBACK) && + encoderSelection.videoEncoderRuntime !== "software" +) { + throw new Error( + `WGC helper forced the software encoder but videoEncoderRuntime was ${encoderSelection.videoEncoderRuntime}, expected software: ${JSON.stringify(encoderSelection)}`, + ); +} // Every fallback path has to stay fragmented, not just the nominal one. The // helper degrades to the plain container rather than failing a recording, so // without this the fix could quietly stop applying and every other assertion diff --git a/src/lib/nativeWindowsRecording.ts b/src/lib/nativeWindowsRecording.ts index e9e8c0c50..5d5d92b31 100644 --- a/src/lib/nativeWindowsRecording.ts +++ b/src/lib/nativeWindowsRecording.ts @@ -47,6 +47,13 @@ export type NativeWindowsRecordingStartResult = { error?: string; /** Helper-reported encoder selection: "default", "software-preferred", or "software-fallback". */ videoEncoderSelection?: string | null; + /** + * Whether the helper actually landed on a hardware H.264 encoder MFT, as + * opposed to `videoEncoderSelection` above, which only says which + * configuration path was tried: "hardware", "software", or "unknown" when + * the helper could not introspect its own sink writer. + */ + videoEncoderRuntime?: string | null; /** * A camera was asked for and the helper could not open it, so this take is * screen and audio only. Still a success — the recording is worth keeping — diff --git a/technical-documentation/architecture/recording.md b/technical-documentation/architecture/recording.md index 1dc638375..bb6e143aa 100644 --- a/technical-documentation/architecture/recording.md +++ b/technical-documentation/architecture/recording.md @@ -81,8 +81,9 @@ Cursor samples are persisted as cursor telemetry rather than baked into editable - A window with odd client dimensions can produce black video: H.264 encoding requires even dimensions (`electron/native/wgc-capture/src/wgc_session.cpp:38`). - The Windows helper's frame lock (`electron/native/wgc-capture/src/main.cpp`) is still held across blocking, uninterruptible D3D11 work: the WGC callback's `CopyResource`, and whatever the video writer does with the frame. A driver that stalls inside either one still costs the recording. What no longer happens is a hang: stop detection runs on `CaptureControl::stopMutex`, which no frame thread ever touches, and a shutdown watchdog force-exits the helper when a step overruns its budget, naming the step it died in. Each step gets `OPENSCREEN_WGC_STEP_BUDGET_MS` (8s by default) and that is the bound which normally fires; the whole shutdown is capped by `OPENSCREEN_WGC_STOP_BUDGET_MS` (50s by default), which the encoder-finalize step alone is allowed to spend in full because a long software-encoder finalize legitimately takes seconds (issue #34). Picking the D3D adapter that actually drives the captured monitor instead of adapter 0 is still outstanding. -- The video writer has two ways to get a frame to the encoder. Which one it uses is a setting first and a per-machine outcome second: the GPU path has to be asked for, and is then kept only if the machine supports it. The GPU path (`videoInput: "dxgi-nv12"`) copies the frame across a keyed-mutex bridge to a second D3D11 device, converts BGRA to NV12 with the D3D11 video processor, and hands the hardware H.264 encoder a DXGI sample; it never touches system memory. The CPU path (`videoInput: "cpu-rgb32"`) is the original staging-texture `Map(D3D11_MAP_READ)` readback, and is what a `Map`/`Unmap` that never returns wedges (issue #252: Windows 10, WDDM 2.7, multi-adapter). The GPU path is OFF by default; `OPENSCREEN_WGC_ENABLE_DXGI_INPUT=1` turns it on. Once asked for, it degrades to the CPU path at every check made **during initialization** — no hardware encoder, no NV12 video-processor output, no shared keyed-mutex texture, no DXGI sample allocator — so a machine it does not fit records exactly as it did before it existed. That fallback ends when the first frame arrives: a `captureDxgiSample()` failure after encoding has started stops the recording, because the sink writer is configured for NV12 by then and there is no path left to take. That gap is why the default is off, and it is what cost the reporter in #336 their recording. It is skipped outright for `preferSoftwareEncoder` and for inline webcam PiP, both of which need the frame in system memory, The two paths land on different encoders, so the GPU one asks for VBR explicitly through `ICodecAPI`: hardware MFTs default to constant bitrate and would spend the full configured budget on a static screen (measured 16.9 Mbps against 1.95 for the same desktop). +- The video writer has two ways to get a frame to the encoder. Which one it uses is a setting first and a per-machine outcome second: the GPU path has to be asked for, and is then kept only if the machine supports it. The GPU path (`videoInput: "dxgi-nv12"`) copies the frame across a keyed-mutex bridge to a second D3D11 device, converts BGRA to NV12 with the D3D11 video processor, and hands the hardware H.264 encoder a DXGI sample; it never touches system memory. The CPU path (`videoInput: "cpu-rgb32"`) is the original staging-texture `Map(D3D11_MAP_READ)` readback, and is what a `Map`/`Unmap` that never returns wedges (issue #252: Windows 10, WDDM 2.7, multi-adapter). The GPU path is OFF by default; `OPENSCREEN_WGC_ENABLE_DXGI_INPUT=1` turns it on. Once asked for, it degrades to the CPU path at every check made **during initialization** — no hardware encoder, no NV12 video-processor output, no shared keyed-mutex texture, no DXGI sample allocator — so a machine it does not fit records exactly as it did before it existed. That fallback ends when the first frame arrives: a `captureDxgiSample()` failure after encoding has started stops the recording, because the sink writer is configured for NV12 by then and there is no path left to take. That gap is why the default is off, and it is what cost the reporter in #336 their recording. It is skipped outright for `preferSoftwareEncoder` and for inline webcam PiP, both of which need the frame in system memory. Whichever path lands on a hardware MFT asks for VBR explicitly through `ICodecAPI` (`MFEncoder::applyHardwareRateControl`, called whenever hardware transforms were requested — see the `videoEncoderRuntime` gap below): hardware MFTs default to constant bitrate and would spend the full configured budget on a static screen (measured 16.9 Mbps against 1.95 for the same desktop on the GPU path). VBR keeps that in check but does not make a hardware encoder's output match a software one byte-for-byte — measured back-to-back on the same idle desktop, this machine's hardware MFT still produced roughly 5x the bytes of the software encoder even with VBR correctly engaged (8.7 Mbps vs 1.7 Mbps), which looks like a genuine rate-distortion difference between the two encoder implementations rather than a rate-control bug. Accepted as the cost of the fix below rather than tuned further, since disk is cheap and a stuck recording losing the whole take is not. - Linux/Wayland can produce no usable frames on the `getDisplayMedia` fallback because Chromium initializes Vulkan against the Ozone Wayland backend. The PipeWire helper path is unaffected. - On Linux the compositor's source picker appears on every recording. That is deliberate — see "Why Linux sends no source identity" — but it is an interruption, and there is currently no way to reuse a previous choice without also making it impossible to change. - Holding a portal session across the countdown means the compositor's "screen is being shared" indicator is up before recording begins. That is honest — access really has been granted — but the user can click it to revoke, or close the window they picked. The helper's exit surfaces as a rejected `waitUntilSourceSelected`; the session is not yet subscribed to the portal's `Session::Closed` signal, so a revocation is reported as a failed start rather than a specific message. - `preferSoftwareEncoder` is read when recording starts. The recorder has no UI for setting it; Windows also accepts `OPENSCREEN_WGC_PREFER_SOFTWARE_ENCODER=true` in the helper request path. +- The `encoder-selection` event's `video` field ("default", "software-preferred", "software-fallback") names which *configuration path* `MFEncoder::initialize` took, not which encoder Media Foundation actually picked; `videoEncoderRuntime` ("hardware", "software", or "unknown") answers that, by walking the sink writer's own transform pipeline after `BeginWriting()` for a node that self-identifies as hardware (`MFT_ENUM_HARDWARE_URL_Attribute`). It exists because of what it found (getopenscreen/openscreen#460): `MF_READWRITE_ENABLE_HARDWARE_TRANSFORMS` defaults to FALSE, and until this was fixed, leaving it unset on the "default" (non-DXGI) path meant the sink writer only ever considered software MFTs there, even on a machine with a working hardware H.264 encoder — "default" recording was, in practice, software-encoded on every machine that had not separately opted into the DXGI path above. `createSinkWriter` now sets that attribute to TRUE whenever `forceSoftwareEncoder` is false, DXGI device manager or not, so a hardware encoder is used on the "default" path when one is registered and working — which is most of the point, since it is weak-CPU machines running the software encoder that blow the stop-timeout budget in the first place. `videoEncoderRuntime` stays in the event to catch the cases that don't fit that story: a machine with no real hardware encoder falling back to software regardless, or a hardware encoder that itself turns out to be the slow or unstable one. From 0c333afbd59a04e263dc6fc2df005d156b9408d5 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 23 Aug 2026 12:19:27 +0000 Subject: [PATCH 26/30] chore(release): bump to 1.10.0-rc.4 [skip ci] --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9ba76b6c4..5ef7f9e72 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "openscreen", - "version": "1.10.0-rc.3", + "version": "1.10.0-rc.4", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "openscreen", - "version": "1.10.0-rc.3", + "version": "1.10.0-rc.4", "dependencies": { "@fix-webm-duration/fix": "^1.0.1", "@langchain/anthropic": "^1.3.26", diff --git a/package.json b/package.json index baa1737d2..85e56d3db 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "openscreen", "private": true, - "version": "1.10.0-rc.3", + "version": "1.10.0-rc.4", "description": "Record your screen and polish the demo", "homepage": "https://getopenscreen.com/", "license": "MIT", From c283da8e57fb2defeba779762b182d13e32de65e Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sun, 23 Aug 2026 21:18:29 +0200 Subject: [PATCH 27/30] fix(app): reach Save Diagnostics from the menu and tray, not nowhere electron/ipc/handlers.ts and preload.ts fully implemented saveDiagnostic, and "Save Diagnostics" was localized into all 13 languages, but nothing in the app ever called it -- no button, no menu item, no keyboard shortcut. Found this while working out how to answer a #460 reporter's own question about where to find the diagnostic log: there was no working answer. Extracted the file-writing logic into an exported exportDiagnosticFile, shared by the existing IPC handler and three new entry points in main.ts: the tray's context menu (idle state), the Windows/Linux Help menu, and the macOS app menu. The tray one matters most for capture bugs like #460 -- it's reachable without opening any window, which is exactly the state a HUD is usually in right after a recording fails to stop. Reused "Save Diagnostics"'s existing translations (copied from the otherwise orphaned settings.support.saveDiagnostics key into common.json's actions) rather than inventing new strings across 13 locales. Verified: tsc --noEmit clean, biome clean, full suite (2161 tests) passes, i18n:check passes. Did not launch the dev Electron app -- native menu/tray changes aren't observable through the browser preview tooling, and a second instance risks the single-instance lock other active worktrees hold. Co-Authored-By: Claude Sonnet 5 --- electron/ipc/handlers.ts | 111 ++++++++++++++++------------- electron/main.ts | 49 ++++++++++++- src/i18n/locales/ar/common.json | 3 +- src/i18n/locales/en/common.json | 3 +- src/i18n/locales/es/common.json | 3 +- src/i18n/locales/fr/common.json | 3 +- src/i18n/locales/it/common.json | 3 +- src/i18n/locales/ja-JP/common.json | 3 +- src/i18n/locales/ko-KR/common.json | 3 +- src/i18n/locales/pt-BR/common.json | 3 +- src/i18n/locales/ru/common.json | 3 +- src/i18n/locales/tr/common.json | 3 +- src/i18n/locales/vi/common.json | 3 +- src/i18n/locales/zh-CN/common.json | 3 +- src/i18n/locales/zh-TW/common.json | 3 +- 15 files changed, 136 insertions(+), 63 deletions(-) diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index 0f20dd564..958ef6d96 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -1664,6 +1664,66 @@ async function resolveMediaLinksForVideo(videoPath: string): Promise<{ return { resolvedVia: "none" }; } +/** + * Writes the diagnostic bundle a bug report needs: app/OS facts, the native + * helpers' raw stdout/stderr (which is where `[stop-timing]` and + * `encoder-selection` land — see nativeWindowsCaptureStop.ts), and the main + * process's own recent console output. Shared by the renderer's IPC call and + * the menu/tray "Save Diagnostics" entry point in main.ts, which has no + * renderer-side `projectState`/`logs` to offer and does not need to. + */ +export async function exportDiagnosticFile(payload: { + error: string; + stack?: string; + projectState: unknown; + logs: string[]; +}) { + const { filePath, canceled } = await dialog.showSaveDialog({ + title: "Save Diagnostic File", + defaultPath: `openscreen-diagnostic-${Date.now()}.json`, + filters: [{ name: "JSON", extensions: ["json"] }], + }); + + if (canceled || !filePath) return { success: false, canceled: true }; + + const HELPER_OUTPUT_MAX_BYTES = 64 * 1024; + const tail = (s: string, max: number) => (s.length <= max ? s : s.slice(s.length - max)); + + const diagnostic = { + timestamp: new Date().toISOString(), + appVersion: app.getVersion(), + platform: process.platform, + arch: process.arch, + // The same fact the About box leads with, and for the same reason: it is what + // explains why a copy does or does not offer an update check. This file is the + // artifact users actually attach, so it must not be the one that omits it. + channel: getInstallChannel(), + osRelease: os.release(), + osVersion: os.version(), + totalMemoryMB: Math.round(os.totalmem() / 1024 / 1024), + nodeVersion: process.versions.node, + electronVersion: process.versions.electron, + chromeVersion: process.versions.chrome, + error: payload.error, + stack: payload.stack, + projectState: payload.projectState, + recentLogs: payload.logs, + helperOutput: { + windows: tail(nativeWindowsCaptureOutput, HELPER_OUTPUT_MAX_BYTES), + mac: tail(nativeMacCaptureOutput, HELPER_OUTPUT_MAX_BYTES), + }, + mainProcessLogs: mainLogBuffer.snapshot(), + }; + + try { + await fs.writeFile(filePath, JSON.stringify(diagnostic, null, 2), "utf-8"); + return { success: true, path: filePath }; + } catch (error) { + console.error("Failed to write diagnostic file:", error); + return { success: false, error: String(error) }; + } +} + export function registerIpcHandlers( createEditorWindow: () => void, createSourceSelectorWindow: () => BrowserWindow, @@ -4092,55 +4152,8 @@ export function registerIpcHandlers( ipcMain.handle( "save-diagnostic", - async ( - _, - payload: { error: string; stack?: string; projectState: unknown; logs: string[] }, - ) => { - const { filePath, canceled } = await dialog.showSaveDialog({ - title: "Save Diagnostic File", - defaultPath: `openscreen-diagnostic-${Date.now()}.json`, - filters: [{ name: "JSON", extensions: ["json"] }], - }); - - if (canceled || !filePath) return { success: false, canceled: true }; - - const HELPER_OUTPUT_MAX_BYTES = 64 * 1024; - const tail = (s: string, max: number) => (s.length <= max ? s : s.slice(s.length - max)); - - const diagnostic = { - timestamp: new Date().toISOString(), - appVersion: app.getVersion(), - platform: process.platform, - arch: process.arch, - // The same fact the About box leads with, and for the same reason: it is what - // explains why a copy does or does not offer an update check. This file is the - // artifact users actually attach, so it must not be the one that omits it. - channel: getInstallChannel(), - osRelease: os.release(), - osVersion: os.version(), - totalMemoryMB: Math.round(os.totalmem() / 1024 / 1024), - nodeVersion: process.versions.node, - electronVersion: process.versions.electron, - chromeVersion: process.versions.chrome, - error: payload.error, - stack: payload.stack, - projectState: payload.projectState, - recentLogs: payload.logs, - helperOutput: { - windows: tail(nativeWindowsCaptureOutput, HELPER_OUTPUT_MAX_BYTES), - mac: tail(nativeMacCaptureOutput, HELPER_OUTPUT_MAX_BYTES), - }, - mainProcessLogs: mainLogBuffer.snapshot(), - }; - - try { - await fs.writeFile(filePath, JSON.stringify(diagnostic, null, 2), "utf-8"); - return { success: true, path: filePath }; - } catch (error) { - console.error("Failed to write diagnostic file:", error); - return { success: false, error: String(error) }; - } - }, + async (_, payload: { error: string; stack?: string; projectState: unknown; logs: string[] }) => + exportDiagnosticFile(payload), ); // One instance each, not one per call. DocumentService serialises saves of a diff --git a/electron/main.ts b/electron/main.ts index 85eb063b8..6f2538587 100644 --- a/electron/main.ts +++ b/electron/main.ts @@ -41,7 +41,11 @@ import { } from "./globalShortcut"; import { mainT, setMainLocale } from "./i18n"; import { getInstallChannel, offersUpdateCheck, platformOwnsUpdates } from "./install-channel"; -import { getSelectedDesktopSource, registerIpcHandlers } from "./ipc/handlers"; +import { + exportDiagnosticFile, + getSelectedDesktopSource, + registerIpcHandlers, +} from "./ipc/handlers"; import { installMainProcessErrorGuards } from "./main-process-errors"; import { registerSttIpc, shutdownStt } from "./stt"; import { checkLatestRelease } from "./update-checker"; @@ -211,6 +215,11 @@ function setupApplicationMenu() { role: "about", label: mainT("common", "actions.about") || "About OpenScreen", }, + { type: "separator" as const }, + { + label: mainT("common", "actions.saveDiagnostics") || "Save Diagnostics", + click: runSaveDiagnostics, + }, // Omitted entirely — here, in the Help menu and in the tray — where a package // manager owns the update. See `canOfferUpdateCheck`. ...(canOfferUpdateCheck() @@ -369,6 +378,11 @@ function setupApplicationMenu() { label: mainT("common", "actions.about") || "About OpenScreen", click: runAboutDialog, }, + { type: "separator" as const }, + { + label: mainT("common", "actions.saveDiagnostics") || "Save Diagnostics", + click: runSaveDiagnostics, + }, ], }); } @@ -519,6 +533,31 @@ function runUpdateCheck() { }); } +/** + * Menu and tray entry point for exporting a diagnostic bundle. The backend + * (`exportDiagnosticFile`) and its "Save Diagnostics" label already existed — + * nothing in the app ever called it (getopenscreen/openscreen#460). Reveals + * the written file on success, the same confirmation the export flow's "Show + * in folder" gives, so there is no need for a second dialog on top of the + * native Save dialog the user already went through. + * + * No renderer `projectState`/`logs` to attach from here, unlike the in-app + * crash path this shares a payload shape with — the diagnostic value for a + * capture bug is almost entirely `helperOutput`/`mainProcessLogs`, which + * `exportDiagnosticFile` reads straight from the main process regardless. + */ +function runSaveDiagnostics() { + exportDiagnosticFile({ error: "Manual diagnostic export", projectState: null, logs: [] }) + .then((result) => { + if (result.success && result.path) { + shell.showItemInFolder(result.path); + } + }) + .catch((error) => { + console.error("[diagnostics] save failed", error); + }); +} + /** Mirrors the flag that already drives the tray icon. An update must never interrupt a take — * and on Windows it physically cannot, because the capture helpers spawn from inside the * install directory and NSIS cannot overwrite a running .exe. */ @@ -730,6 +769,14 @@ function updateTrayMenu(recording: boolean = false) { label: mainT("common", "actions.about") || "About OpenScreen", click: runAboutDialog, }, + // Right next to About, and reachable without opening any window: this is the + // one place in the app most likely to still be usable right after a recording + // failed to stop, which is exactly when the [stop-timing]/encoder-selection + // lines this exports are worth the most (getopenscreen/openscreen#460). + { + label: mainT("common", "actions.saveDiagnostics") || "Save Diagnostics", + click: runSaveDiagnostics, + }, { type: "separator" as const }, { label: mainT("common", "actions.quit") || "Quit", diff --git a/src/i18n/locales/ar/common.json b/src/i18n/locales/ar/common.json index ef0d6736f..b63cd0265 100644 --- a/src/i18n/locales/ar/common.json +++ b/src/i18n/locales/ar/common.json @@ -40,7 +40,8 @@ "services": "خدمات", "hide": "إخفاء OpenScreen", "hideOthers": "إخفاء الآخرين", - "unhide": "إظهار الكل" + "unhide": "إظهار الكل", + "saveDiagnostics": "حفظ التشخيصات" }, "updates": { "available": "يتوفر OpenScreen {{latestVersion}}. الإصدار المثبت هو {{currentVersion}}.", diff --git a/src/i18n/locales/en/common.json b/src/i18n/locales/en/common.json index 4f303ee62..6eac95830 100644 --- a/src/i18n/locales/en/common.json +++ b/src/i18n/locales/en/common.json @@ -40,7 +40,8 @@ "services": "Services", "hide": "Hide OpenScreen", "hideOthers": "Hide Others", - "unhide": "Show All" + "unhide": "Show All", + "saveDiagnostics": "Save Diagnostics" }, "updates": { "available": "OpenScreen {{latestVersion}} is available. You are using {{currentVersion}}.", diff --git a/src/i18n/locales/es/common.json b/src/i18n/locales/es/common.json index 3c92583a6..29a364265 100644 --- a/src/i18n/locales/es/common.json +++ b/src/i18n/locales/es/common.json @@ -40,7 +40,8 @@ "services": "Servicios", "hide": "Ocultar OpenScreen", "hideOthers": "Ocultar otros", - "unhide": "Mostrar todo" + "unhide": "Mostrar todo", + "saveDiagnostics": "Guardar diagnósticos" }, "updates": { "available": "OpenScreen {{latestVersion}} está disponible. Estás usando {{currentVersion}}.", diff --git a/src/i18n/locales/fr/common.json b/src/i18n/locales/fr/common.json index de72f3139..c74ef66b8 100644 --- a/src/i18n/locales/fr/common.json +++ b/src/i18n/locales/fr/common.json @@ -40,7 +40,8 @@ "services": "Services", "hide": "Masquer OpenScreen", "hideOthers": "Masquer les autres", - "unhide": "Tout afficher" + "unhide": "Tout afficher", + "saveDiagnostics": "Enregistrer les diagnostics" }, "updates": { "available": "OpenScreen {{latestVersion}} est disponible. Vous utilisez la version {{currentVersion}}.", diff --git a/src/i18n/locales/it/common.json b/src/i18n/locales/it/common.json index df18ace13..24f10fc61 100644 --- a/src/i18n/locales/it/common.json +++ b/src/i18n/locales/it/common.json @@ -40,7 +40,8 @@ "services": "Servizi", "hide": "Nascondi OpenScreen", "hideOthers": "Nascondi gli altri", - "unhide": "Mostra tutto" + "unhide": "Mostra tutto", + "saveDiagnostics": "Salva dati diagnostici" }, "updates": { "available": "OpenScreen {{latestVersion}} è disponibile. Stai usando la versione {{currentVersion}}.", diff --git a/src/i18n/locales/ja-JP/common.json b/src/i18n/locales/ja-JP/common.json index 88899975b..0d3ca001b 100644 --- a/src/i18n/locales/ja-JP/common.json +++ b/src/i18n/locales/ja-JP/common.json @@ -40,7 +40,8 @@ "services": "サービス", "hide": "OpenScreenを隠す", "hideOthers": "ほかを隠す", - "unhide": "すべて表示" + "unhide": "すべて表示", + "saveDiagnostics": "診断情報を保存" }, "updates": { "available": "OpenScreen {{latestVersion}} を利用できます。現在のバージョンは {{currentVersion}} です。", diff --git a/src/i18n/locales/ko-KR/common.json b/src/i18n/locales/ko-KR/common.json index 964fdd631..2ca4d25a5 100644 --- a/src/i18n/locales/ko-KR/common.json +++ b/src/i18n/locales/ko-KR/common.json @@ -40,7 +40,8 @@ "services": "서비스", "hide": "OpenScreen 숨기기", "hideOthers": "다른 항목 숨기기", - "unhide": "모두 보기" + "unhide": "모두 보기", + "saveDiagnostics": "Save Diagnostics" }, "updates": { "available": "OpenScreen {{latestVersion}} 버전을 사용할 수 있습니다. 현재 버전은 {{currentVersion}}입니다.", diff --git a/src/i18n/locales/pt-BR/common.json b/src/i18n/locales/pt-BR/common.json index af25d126d..d74e5b215 100644 --- a/src/i18n/locales/pt-BR/common.json +++ b/src/i18n/locales/pt-BR/common.json @@ -40,7 +40,8 @@ "services": "Serviços", "hide": "Ocultar OpenScreen", "hideOthers": "Ocultar Outros", - "unhide": "Mostrar Todos" + "unhide": "Mostrar Todos", + "saveDiagnostics": "Salvar Diagnósticos" }, "updates": { "available": "O OpenScreen {{latestVersion}} está disponível. Você está usando a versão {{currentVersion}}.", diff --git a/src/i18n/locales/ru/common.json b/src/i18n/locales/ru/common.json index d7ec15ae6..141e8b984 100644 --- a/src/i18n/locales/ru/common.json +++ b/src/i18n/locales/ru/common.json @@ -40,7 +40,8 @@ "services": "Сервисы", "hide": "Скрыть OpenScreen", "hideOthers": "Скрыть остальные", - "unhide": "Показать все" + "unhide": "Показать все", + "saveDiagnostics": "Сохранить диагностику" }, "updates": { "available": "Доступен OpenScreen {{latestVersion}}. Установлена версия {{currentVersion}}.", diff --git a/src/i18n/locales/tr/common.json b/src/i18n/locales/tr/common.json index e36c2dc33..923079b1e 100644 --- a/src/i18n/locales/tr/common.json +++ b/src/i18n/locales/tr/common.json @@ -40,7 +40,8 @@ "services": "Servisler", "hide": "OpenScreen’i Gizle", "hideOthers": "Diğerlerini Gizle", - "unhide": "Tümünü Göster" + "unhide": "Tümünü Göster", + "saveDiagnostics": "Teşhis Verilerini Kaydet" }, "updates": { "available": "OpenScreen {{latestVersion}} kullanılabilir. Mevcut sürümünüz {{currentVersion}}.", diff --git a/src/i18n/locales/vi/common.json b/src/i18n/locales/vi/common.json index 544cf3b89..089cf992a 100644 --- a/src/i18n/locales/vi/common.json +++ b/src/i18n/locales/vi/common.json @@ -40,7 +40,8 @@ "services": "Dịch vụ", "hide": "Ẩn OpenScreen", "hideOthers": "Ẩn ứng dụng khác", - "unhide": "Hiển thị tất cả" + "unhide": "Hiển thị tất cả", + "saveDiagnostics": "Lưu thông tin chẩn đoán" }, "updates": { "available": "Đã có OpenScreen {{latestVersion}}. Bạn đang dùng phiên bản {{currentVersion}}.", diff --git a/src/i18n/locales/zh-CN/common.json b/src/i18n/locales/zh-CN/common.json index 30b018cc1..0a7b2bd9b 100644 --- a/src/i18n/locales/zh-CN/common.json +++ b/src/i18n/locales/zh-CN/common.json @@ -40,7 +40,8 @@ "services": "服务", "hide": "隐藏 OpenScreen", "hideOthers": "隐藏其他", - "unhide": "显示全部" + "unhide": "显示全部", + "saveDiagnostics": "保存诊断信息" }, "updates": { "available": "OpenScreen {{latestVersion}} 已发布。当前版本为 {{currentVersion}}。", diff --git a/src/i18n/locales/zh-TW/common.json b/src/i18n/locales/zh-TW/common.json index 07d66259c..c54dc17ed 100644 --- a/src/i18n/locales/zh-TW/common.json +++ b/src/i18n/locales/zh-TW/common.json @@ -40,7 +40,8 @@ "services": "服務", "hide": "隱藏 OpenScreen", "hideOthers": "隱藏其他", - "unhide": "全部顯示" + "unhide": "全部顯示", + "saveDiagnostics": "儲存診斷資料" }, "updates": { "available": "OpenScreen {{latestVersion}} 已推出。目前版本為 {{currentVersion}}。", From e067355da1596b8ed99c1e9c1e4e917d9c88d9b4 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sun, 23 Aug 2026 23:17:23 +0200 Subject: [PATCH 28/30] fix(app): address CodeRabbit review on the Save Diagnostics PR Three findings, all confirmed against current code: - runSaveDiagnostics silently did nothing when exportDiagnosticFile resolved with success:false (a write failure after the user already picked a save location) -- it only handled the success and implicit-reject cases, so a real failure read as the menu action doing nothing. Now shows an error dialog with the underlying message as detail, cancellation still a no-op. - detectVideoEncoderRuntime's doc comment in mf_encoder.cpp still said the default path asks for no hardware-transform attribute at all, which was true when it was written but stopped being true once the default path started requesting MF_READWRITE_ENABLE_HARDWARE_TRANSFORMS. Updated to say what's actually true now: it's a request Media Foundation can still answer with software, which is why the runtime still has to be checked after the fact rather than assumed from the path. - ko-KR's actions.saveDiagnostics carried the English label because it was copied from settings.support.saveDiagnostics, which was itself never translated for Korean. Applied CodeRabbit's suggested translation. Verified: tsc --noEmit clean, biome clean, i18n:check passes, native helper rebuilds clean on MSVC, full suite (2161 tests) passes. Co-Authored-By: Claude Sonnet 5 --- electron/main.ts | 18 +++++++++++++++- .../native/wgc-capture/src/mf_encoder.cpp | 21 ++++++++++--------- src/i18n/locales/ko-KR/common.json | 2 +- 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/electron/main.ts b/electron/main.ts index 6f2538587..a85629bf3 100644 --- a/electron/main.ts +++ b/electron/main.ts @@ -549,7 +549,23 @@ function runUpdateCheck() { function runSaveDiagnostics() { exportDiagnosticFile({ error: "Manual diagnostic export", projectState: null, logs: [] }) .then((result) => { - if (result.success && result.path) { + if (result.canceled) return; + if (!result.success) { + // exportDiagnosticFile resolves rather than rejects on a write + // failure, so this is the branch that turns "user picked a save + // location and got silence" into a visible error instead of a + // menu action that looks like it did nothing. + showMessageBox({ + type: "error", + title: PRODUCT_NAME, + message: mainT("dialogs", "export.failed") || "Export Failed", + detail: result.error, + }).catch((error) => { + console.error("[diagnostics] failure dialog failed", error); + }); + return; + } + if (result.path) { shell.showItemInFolder(result.path); } }) diff --git a/electron/native/wgc-capture/src/mf_encoder.cpp b/electron/native/wgc-capture/src/mf_encoder.cpp index c1e5c2bbe..4130b1326 100644 --- a/electron/native/wgc-capture/src/mf_encoder.cpp +++ b/electron/native/wgc-capture/src/mf_encoder.cpp @@ -406,16 +406,17 @@ bool resolveStreamSinkIndex(IMFMediaSink* mediaSink, const GUID& majorType, DWOR // Did the video stream's encoder MFT actually land on hardware? // -// BeginWriting() succeeding says nothing about this: on the "default" path -// (see kVideoEncoderRuntime* in mf_encoder.h) no attribute asked for hardware -// transforms, so Media Foundation is free to hand the sink writer a software -// MFT even when a hardware one is registered and would have worked. The only -// way to know which one it actually picked is to ask the pipeline it built, -// after the fact -- IMFSinkWriterEx::GetTransformForStream walks the MFTs the -// sink writer inserted for a stream, and a hardware MFT instance is required -// to expose MFT_ENUM_HARDWARE_URL_Attribute on its own attribute store (not -// just on the IMFActivate MFTEnumEx returns), which is what distinguishes it -// from a software one at this point. +// BeginWriting() succeeding says nothing about this: even on the "default" +// path (see kVideoEncoderRuntime* in mf_encoder.h), which does now ask for +// MF_READWRITE_ENABLE_HARDWARE_TRANSFORMS, that is a request and not a +// guarantee -- Media Foundation is still free to hand the sink writer a +// software MFT when no hardware one is registered or the driver refuses it. +// The only way to know which one it actually picked is to ask the pipeline it +// built, after the fact -- IMFSinkWriterEx::GetTransformForStream walks the +// MFTs the sink writer inserted for a stream, and a hardware MFT instance is +// required to expose MFT_ENUM_HARDWARE_URL_Attribute on its own attribute +// store (not just on the IMFActivate MFTEnumEx returns), which is what +// distinguishes it from a software one at this point. // // Every failure path here returns "unknown" rather than guessing: this runs // after the sink writer is already committed to, so it must never be able to diff --git a/src/i18n/locales/ko-KR/common.json b/src/i18n/locales/ko-KR/common.json index 2ca4d25a5..98d025d9a 100644 --- a/src/i18n/locales/ko-KR/common.json +++ b/src/i18n/locales/ko-KR/common.json @@ -41,7 +41,7 @@ "hide": "OpenScreen 숨기기", "hideOthers": "다른 항목 숨기기", "unhide": "모두 보기", - "saveDiagnostics": "Save Diagnostics" + "saveDiagnostics": "진단 정보 저장" }, "updates": { "available": "OpenScreen {{latestVersion}} 버전을 사용할 수 있습니다. 현재 버전은 {{currentVersion}}입니다.", From bc792b9ce90aba90971df67d370b61af0ac10f0b Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Mon, 24 Aug 2026 10:34:42 +0200 Subject: [PATCH 29/30] fix(recording): stop burning the step budget on a join that cannot succeed Pinned down the exact mechanism behind #460 on Intel HD 520, confirmed by a reporter's Save Diagnostics file on rc.4: the WGC frame callback (main.cpp's session.setFrameCallback) takes the shared frame-state `mutex` and calls session.context()->CopyResource() while still holding it. On this hardware that CopyResource hangs inside the driver. writeVideoFrames() needs the same mutex for its own per-iteration wait -- including to notice stopRequested -- so once the callback wedges, the writer thread can never even check whether a stop was requested. That is why the watchdog reported encode_stage=idle: not idle, blocked on a lock a stuck GPU call holds forever. quiesceCapture() already detects this and gives up after its own 5s drain, returning wgcDrained=false. Nothing downstream listened: video-writer-join called stopVideoWriter() unconditionally, joining a thread that structurally could never return, and paid the full step budget (8s default) before the watchdog force-exited the process anyway -- the same outcome the fix below reaches, just ~8s later. When wgcDrained is false, detach the thread and terminate immediately rather than falling through to a join that cannot succeed. Deliberately does not continue into encoder.finalize(): that resets the D3D device/context state a still-blocked writer thread might resume touching the moment the lock frees. No data is lost either way -- the fragmented sink writes moof+mdat incrementally, so whatever was on disk before the wedge is on disk regardless of which path gets there. Added a new fault-injection point (OPENSCREEN_WGC_TEST_STALL_FRAME_CALLBACK_MS) to reproduce the exact failure shape and verify the fix rather than trust it compiles. Along the way, found that stalling the *first* frame trips an unrelated 10s startup timeout before ever reaching this code path -- the stall has to land on a later frame, matching what the real diagnostic showed (recording-started succeeded before the hang). Also found that moving the cursor alone does not reliably force a WGC frame on this machine (likely hardware cursor compositing bypassing the desktop bitmap); a moving window does. Measured: 5075ms to exit with the fix, versus what would have been ~13000ms (5s drain + 8s step budget) without it. Confirmed via the process exiting right at video-writer-join, with no encoder-finalize/wgc-session-close in the steps afterward. The pre-existing --stall-readback (#252) regression test is unaffected -- that scenario stalls the writer's own readback, not the frame callback, so it never touches this branch. This does not fix the underlying driver hang, which needs the actual failing hardware to diagnose further. It gets the user a faster, honest failure instead of a long one; the recording is still lost when the driver wedges. Verified: tsc --noEmit clean, biome clean, native helper rebuilds clean on MSVC, full suite (2161 tests) passes, both stall regression tests pass. Co-Authored-By: Claude Sonnet 5 --- electron/native/wgc-capture/src/main.cpp | 69 +++++++++++++- scripts/test-windows-wgc-helper.mjs | 113 ++++++++++++++++++++++- 2 files changed, 178 insertions(+), 4 deletions(-) diff --git a/electron/native/wgc-capture/src/main.cpp b/electron/native/wgc-capture/src/main.cpp index c64d79703..a9e21d45c 100644 --- a/electron/native/wgc-capture/src/main.cpp +++ b/electron/native/wgc-capture/src/main.cpp @@ -682,6 +682,16 @@ int main(int argc, char* argv[]) { // ordinary hardware, so the stop path can be regression-tested at all. const int testStallReadbackMs = std::max(0, readEnvInt("OPENSCREEN_WGC_TEST_STALL_READBACK_MS", 0)); + // Test-only: stall the WGC frame *callback* itself while it holds the + // same frame lock, rather than the writer's readback -- the shape + // getopenscreen/openscreen#460 actually reproduced on Intel HD 520 + // ("A WGC frame callback did not finish"). Distinct from + // testStallReadbackMs above because quiesceCapture()'s drain only ever + // sees the callback side: a stall placed in the writer instead leaves + // callbacksInFlight_ at zero and wgcDrained true, which cannot exercise + // the video-writer-join skip this stall exists to test. + const int testStallFrameCallbackMs = + std::max(0, readEnvInt("OPENSCREEN_WGC_TEST_STALL_FRAME_CALLBACK_MS", 0)); std::cout << "{\"event\":\"ready\",\"schemaVersion\":2}" << std::endl; @@ -931,6 +941,18 @@ int main(int argc, char* argv[]) { } } + // Gated on an already-arrived first frame: main() blocks up to 10s + // waiting for firstFrameWritten before it will even print + // recording-started, a startup budget this stall is meant to outlast + // (it needs to still be asleep when `stop` arrives, seconds later). + // Stalling the first frame trips that unrelated timeout instead of + // reaching the steady-state shutdown path this exists to test, and + // does not match the real report either -- getopenscreen/openscreen + // #460's diagnostic shows recording-started succeeding before the + // hang. + if (testStallFrameCallbackMs > 0 && firstFrameWritten.load()) { + std::this_thread::sleep_for(std::chrono::milliseconds(testStallFrameCallbackMs)); + } session.context()->CopyResource(latestFrameTexture.Get(), texture); latestFrameTimestampHns = timestampHns; if (!firstFrameWritten.exchange(true)) { @@ -1420,8 +1442,47 @@ int main(int argc, char* argv[]) { } logStopStep("audio-mixer"); beginStopStep("video-writer-join", stepBudgetMs); - stopVideoWriter(); - logStopStep("video-writer-join"); + if (wgcDrained) { + stopVideoWriter(); + logStopStep("video-writer-join"); + } else { + // wgc-quiesce already reported the frame callback stuck inside the + // driver (getopenscreen/openscreen#460 on Intel HD 520: a + // CopyResource that never returns), still holding the same + // frame-state `mutex` writeVideoFrames takes for its own + // per-iteration wait -- the one it also needs to notice + // stopRequested. Joining is not a step that can time out here, it is + // one that cannot ever succeed, and this is not the only step that + // assumed it would: encoder.finalize() below resets the very D3D + // device/context a still-blocked writer thread might resume touching + // the moment that lock frees, and quiesceCapture()/stop() already + // treat "leave everything alone and let process exit reclaim it" as + // the only safe response to exactly this state. So this ends the + // process here, on this thread, rather than pretending the rest of a + // clean shutdown is reachable -- which cost nothing extra before + // today: the same TerminateProcess happened anyway, just + // stepBudgetMs later, once this step's own watchdog gave up waiting + // on a join that could never return. detach() first, not because + // TerminateProcess needs it (it does not touch the C++ runtime, no + // std::thread destructor runs), but so nothing between here and the + // kill can trip over a still-joinable thread. + // + // The fragmented sink writes moof+mdat incrementally, roughly once a + // second, so this is not a new source of loss: whatever was already + // on disk before the callback wedged is on disk regardless of + // whether Finalize() ever runs, on this path or the slower one it + // replaces. + videoWriterThread.detach(); + std::cerr << "[stop-timing] step=video-writer-join elapsed_ms=" << stopElapsedMs() + << " phase=abandoned encode_stage=" << encoder.encodeStage() + << " audio_stage=" << encoder.audioStage() << " reason=frame-callback-stuck" + << std::endl; + std::cout << "{\"event\":\"stop-timeout\",\"schemaVersion\":2,\"step\":\"video-writer-join\"}" + << std::endl; + std::cout.flush(); + std::cerr.flush(); + TerminateProcess(GetCurrentProcess(), 3); + } if (usesDxgiInput) { std::cerr << "[frame-drops] gpu_bridge_contended=" << contendedFrames.load() << std::endl; } @@ -1430,7 +1491,9 @@ int main(int argc, char* argv[]) { // the encoder's GPU readback, and audioMixer->stop() joined the only other // thread that writes to it. MFEncoder's own writerMutex_ deliberately does // NOT cover copyFrameToBuffer, so finalizing before those joins would race - // the staging texture -- do not reorder these. + // the staging texture -- do not reorder these. Reaching this line at all + // means wgcDrained was true above: the branch that was not is a + // TerminateProcess call, not a fallthrough. beginStopStep("encoder-finalize", shutdownBudgetMs); const bool screenFinalized = encoder.finalize(); logStopStep("encoder-finalize"); diff --git a/scripts/test-windows-wgc-helper.mjs b/scripts/test-windows-wgc-helper.mjs index 1e513c721..ee40838e6 100644 --- a/scripts/test-windows-wgc-helper.mjs +++ b/scripts/test-windows-wgc-helper.mjs @@ -45,6 +45,20 @@ const WITH_STALLED_READBACK = process.env.OPENSCREEN_WGC_TEST_STALL_READBACK === "true" || process.argv.includes("--stall-readback"); const STALL_READBACK_MS = Number(process.env[STALL_READBACK_ENV] ?? 60_000); +const STALL_FRAME_CALLBACK_ENV = "OPENSCREEN_WGC_TEST_STALL_FRAME_CALLBACK_MS"; +/** + * Reproduces getopenscreen/openscreen#460 on ordinary hardware: stalls the WGC + * frame *callback* itself while it holds the frame lock, the shape that issue + * actually reproduced on Intel HD 520 ("A WGC frame callback did not finish"). + * Distinct from WITH_STALLED_READBACK above -- that stalls the writer's own + * readback, which quiesceCapture()'s drain cannot see (callbacksInFlight_ + * stays at zero), so it cannot exercise the video-writer-join skip this stall + * exists to test. + */ +const WITH_STALLED_FRAME_CALLBACK = + process.env.OPENSCREEN_WGC_TEST_STALL_FRAME_CALLBACK === "true" || + process.argv.includes("--stall-frame-callback"); +const STALL_FRAME_CALLBACK_MS = Number(process.env[STALL_FRAME_CALLBACK_ENV] ?? 60_000); const STOP_BUDGET_ENV = "OPENSCREEN_WGC_STOP_BUDGET_MS"; /** * The helper's global shutdown ceiling, pinned into its environment below so @@ -64,11 +78,15 @@ if (WITH_SOFTWARE_ENCODER && WITH_SOFTWARE_FALLBACK) { throw new Error("--software-encoder and --software-fallback are mutually exclusive"); } -function runHelper(config, { injectDefaultSinkWriterFailure = false, stallReadbackMs = 0 } = {}) { +function runHelper( + config, + { injectDefaultSinkWriterFailure = false, stallReadbackMs = 0, stallFrameCallbackMs = 0 } = {}, +) { return new Promise((resolve, reject) => { const env = { ...process.env }; delete env[INJECT_DEFAULT_SINK_WRITER_FAILURE_ENV]; delete env[STALL_READBACK_ENV]; + delete env[STALL_FRAME_CALLBACK_ENV]; env[STOP_BUDGET_ENV] = String(STOP_BUDGET_MS); if (injectDefaultSinkWriterFailure) { env[INJECT_DEFAULT_SINK_WRITER_FAILURE_ENV] = "1"; @@ -76,6 +94,9 @@ function runHelper(config, { injectDefaultSinkWriterFailure = false, stallReadba if (stallReadbackMs > 0) { env[STALL_READBACK_ENV] = String(stallReadbackMs); } + if (stallFrameCallbackMs > 0) { + env[STALL_FRAME_CALLBACK_ENV] = String(stallFrameCallbackMs); + } const child = spawn(HELPER_PATH, [JSON.stringify(config)], { env, stdio: ["pipe", "pipe", "pipe"], @@ -213,6 +234,48 @@ function startFixtureWindow() { }); } +/** + * Windows Graphics Capture delivers frames on compositor damage, not on a + * fixed clock -- on a genuinely idle desktop the frame pool can go a full + * test run without ever firing FrameArrived once. That is invisible to most + * of this harness, which just needs *a* frame eventually, but the + * stalled-frame-callback regression check needs one to land *inside* the + * DURATION_MS window specifically, so the stall this injects is actually the + * thing holding the frame lock when `stop` arrives. + * + * Moving the cursor alone does not reliably do this: most modern GPU/driver + * combinations composite the cursor on its own hardware overlay plane, so + * repositioning it never touches the desktop bitmap WGC captures (confirmed + * empirically here -- frames=0 with a cursor-only nudge running the whole + * test). A visible window changing position is not optional the way the + * cursor is; DWM has to redraw the area it moved across. Returns a stop + * function; always call it, paired failure or not, or the window and its + * PowerShell host outlive the test process. + */ +function startScreenActivity() { + const child = spawn( + "powershell", + [ + "-NoProfile", + "-Command", + "Add-Type -AssemblyName System.Windows.Forms; " + + "$f = New-Object System.Windows.Forms.Form; " + + "$f.StartPosition = 'Manual'; $f.Location = New-Object System.Drawing.Point(0,0); " + + "$f.Size = New-Object System.Drawing.Size(200,200); " + + "$f.TopMost = $true; $f.Show(); " + + "$x = 0; " + + "while ($true) { " + + "$f.Location = New-Object System.Drawing.Point($x, 0); " + + "$x = ($x + 20) % 200; " + + "[System.Windows.Forms.Application]::DoEvents(); " + + "Start-Sleep -Milliseconds 100; " + + "}", + ], + { stdio: ["ignore", "ignore", "ignore"], windowsHide: false }, + ); + return () => child.kill(); +} + function normalizeDeviceName(value) { return value .toLowerCase() @@ -413,16 +476,19 @@ const config = { }, }; +const stopScreenActivity = WITH_STALLED_FRAME_CALLBACK ? startScreenActivity() : null; let result; try { result = await runHelper(config, { injectDefaultSinkWriterFailure: WITH_SOFTWARE_FALLBACK, stallReadbackMs: WITH_STALLED_READBACK ? STALL_READBACK_MS : 0, + stallFrameCallbackMs: WITH_STALLED_FRAME_CALLBACK ? STALL_FRAME_CALLBACK_MS : 0, }); } finally { if (fixtureWindow) { fixtureWindow.child.kill(); } + stopScreenActivity?.(); } // The regression check for issue #252. With the frame lock deliberately wedged @@ -453,6 +519,51 @@ if (WITH_STALLED_READBACK) { process.exit(0); } +// The regression check for getopenscreen/openscreen#460: a frame callback +// wedged inside the driver, confirmed on real hardware via a Save Diagnostics +// report. Before the fix, video-writer-join burned its whole step budget +// joining a thread parked behind that same stuck callback -- this asserts +// both that the helper still exits promptly (not the ~13s that step's own +// budget alone would cost) and that it took the specific skip path rather +// than any other route to exiting. +if (WITH_STALLED_FRAME_CALLBACK) { + if (result.stopHung) { + throw new Error( + `Helper survived ${STOP_HANG_LIMIT_MS}ms past "stop" with a stalled frame callback. ` + + "Its shutdown watchdog did not fire (issue #460).", + ); + } + const steps = readStopTimingSteps(result.stderr); + if (!steps.includes("command-received")) { + throw new Error(`Helper never acknowledged "stop". Steps seen: ${steps.join(", ") || "none"}`); + } + if (!result.stderr.includes("reason=frame-callback-stuck")) { + throw new Error( + `Helper did not take the video-writer-join skip path. stderr:\n${result.stderr}`, + ); + } + // wgc-quiesce's own drain is a fixed 5000ms, so a healthy skip lands + // there plus the near-instant audio/microphone/webcam steps -- nowhere + // near the ~13s (5s drain + the 8s step budget) the join it replaces + // would have cost before this fix. + const STALLED_FRAME_CALLBACK_LATENCY_BUDGET_MS = 10_000; + if ( + result.stopLatencyMs !== null && + result.stopLatencyMs > STALLED_FRAME_CALLBACK_LATENCY_BUDGET_MS + ) { + throw new Error( + `Stop took ${result.stopLatencyMs}ms with a stalled frame callback, over the ` + + `${STALLED_FRAME_CALLBACK_LATENCY_BUDGET_MS}ms budget the video-writer-join skip should keep it under.`, + ); + } + console.log("WGC helper stalled-frame-callback stop check passed", { + stopLatencyMs: result.stopLatencyMs, + steps, + }); + fs.rmSync(outputPath, { force: true }); + process.exit(0); +} + assertStopWasClean(result); if (result.code !== 0) { From dbdadb7d27fe3287d994fb741d6f264a31066648 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 24 Aug 2026 10:33:39 +0000 Subject: [PATCH 30/30] chore(release): bump to 1.10.0 [skip ci] --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 5ef7f9e72..745a74e57 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "openscreen", - "version": "1.10.0-rc.4", + "version": "1.10.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "openscreen", - "version": "1.10.0-rc.4", + "version": "1.10.0", "dependencies": { "@fix-webm-duration/fix": "^1.0.1", "@langchain/anthropic": "^1.3.26", diff --git a/package.json b/package.json index 85e56d3db..056e4aecb 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "openscreen", "private": true, - "version": "1.10.0-rc.4", + "version": "1.10.0", "description": "Record your screen and polish the demo", "homepage": "https://getopenscreen.com/", "license": "MIT",