From 6b15bd96ed4dc13681e0e9a805abcc6a6b75eb91 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Wed, 19 Aug 2026 16:35:56 +0200 Subject: [PATCH 1/2] fix(compositor): anchor Linux annotations on the un-zoomed screen rect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rapport utilisateur : « les captions sont sensibles au zoom et rotation 3d sous linux, alors que ce n'est pas le cas pour les autres plateformes ». Les captions ne sont pas un calque a part -- `captionCuesToTextRegions` les projette en annotations texte -- donc c'est le placement des annotations qui derive, et seulement sur ce backend. `plan_frame` expose deux rects pour cette raison precise : `s_dst`, la boite ecran, que le zoom deplace et agrandit depuis l'issue #179, et `s_ann`, la meme boite AVANT le `remap_box` du zoom. Le contrat de `SceneAnnotation` veut le second (« deliberately NOT affected by the zoom crop »). 81839f01 l'avait pose et corrige les deux backends qui existaient alors ; il touche exactement `compositor_macos.rs` et `compositor_windows.rs`. Le port Linux a repris la formule d'avant le correctif et calculait ses rects, ainsi que sa taille de police, sur `s_dst` : sous un zoom 2.5 la bande deborde du cadre, et un preset de rotation 3D -- propriete d'une region de zoom -- la deplace lui aussi sans pour autant la poser sur le plan incline. Le natif etant desormais la seule source de pixels de l'apercu, la derive se voyait des l'edition, pas seulement a l'export. Plutot que de reparer la troisieme copie de la meme arithmetique, `FrameGeometry` expose ce dont un backend a besoin -- `annotation_dst` et `annotation_anchor_h_px` --, qui ne lisent que `s_ann` : le choix disparait du backend, ce qui est ce qui a rate ici. Deux tests. `the_annotation_rect_and_font_ignore_zoom_and_rotation` (`frame_geometry.rs`) tourne en CI sur les trois plateformes et epingle le contrat sur les accesseurs, avec ses deux garde-fous (le zoom doit bouger `s_dst`, le preset iso doit produire une rotation) sans lesquels les egalites seraient vraies pour la mauvaise raison. L'existant `the_annotation_anchor_ignores_the_zoom` prouvait que `plan_frame` CALCULE la bonne ancre, jamais qu'un backend s'en sert -- c'est exactement l'interstice ou ce bug a vecu. `compose_linux_annotation_ancree_hors_zoom` (`tests/compose_linux.rs`) le mesure sur GPU : une bande a plaque opaque rendue sans zoom, avec zoom, puis avec zoom + iso, chacune diffee contre son propre rendu sans annotation, boite englobante a 2 px pres. Opt-in comme tout ce fichier. Co-Authored-By: Claude --- crates/compositor/src/compositor_linux.rs | 27 +++--- crates/compositor/src/frame_geometry.rs | 108 ++++++++++++++++++++-- crates/compositor/tests/compose_linux.rs | 105 ++++++++++++++++++++- 3 files changed, 221 insertions(+), 19 deletions(-) diff --git a/crates/compositor/src/compositor_linux.rs b/crates/compositor/src/compositor_linux.rs index fc32ecaff..ea9849d0a 100644 --- a/crates/compositor/src/compositor_linux.rs +++ b/crates/compositor/src/compositor_linux.rs @@ -1285,10 +1285,20 @@ impl Compositor { }); // ANNOTATIONS -- calque le plus haut, place relativement au rect ecran - // `g.s_dst` (les coords x/y/w/h de l'annotation sont des fractions de ce - // rect, cf. `scene.rs`). Port de `compositor_macos::draw_annotations` : - // memes modes, memes replis, meme ordre. Seul le texte diverge, tinte - // cote shader (atlas R8) au lieu d'une couleur bakee dans la texture. + // (les coords x/y/w/h de l'annotation sont des fractions de ce rect, cf. + // `scene.rs`). Le rect est `g.s_ann`, l'ecran SANS ZOOM, et surtout pas + // `g.s_dst` : le contrat de `SceneAnnotation` dit « deliberately NOT + // affected by the zoom crop », donc annotations et sous-titres tiennent + // en place pendant que le contenu grossit dessous. `s_dst` a tenu ce role + // gratuitement tant que le zoom vivait dans la coupe source ; depuis + // l'issue #179 il vit dans la BOITE, et l'ancrer dessus fait zoomer les + // sous-titres avec l'ecran. Windows et macOS ont ete corriges alors, ce + // backend non -- d'ou le passage par `FrameGeometry::annotation_dst`, qui + // ne laisse plus le choix. Le natif peignant AUSSI l'apercu, la derive se + // voyait des l'edition, pas seulement a l'export. + // Port de `compositor_macos::draw_annotations` : memes modes, memes + // replis, meme ordre. Seul le texte diverge, tinte cote shader (atlas R8) + // au lieu d'une couleur bakee dans la texture. struct AnnDraw { _buf: wgpu::Buffer, /// Gardent l'atlas / la texture image en vie jusqu'au submit. `None` @@ -1324,12 +1334,7 @@ impl Compositor { if !visible(a) { continue; } - let dst = [ - g.s_dst[0] + a.x * g.s_dst[2], - g.s_dst[1] + a.y * g.s_dst[3], - a.w * g.s_dst[2], - a.h * g.s_dst[3], - ]; + let dst = g.annotation_dst(a.x, a.y, a.w, a.h); let quad_px = [dst[2] * rw, dst[3] * rh]; // Une boite degeneree ferait un atlas 0x0 et un draw invisible ; // macOS l'ecarte de la meme facon. @@ -1475,7 +1480,7 @@ impl Compositor { content: text.content.clone(), color, background, - font_size_px: text.font_size_rel * (g.s_dst[3] * rh), + font_size_px: text.font_size_rel * g.annotation_anchor_h_px(rh), font_family: text.font_family.clone(), bold: text.font_weight == "bold", italic: text.font_style == "italic", diff --git a/crates/compositor/src/frame_geometry.rs b/crates/compositor/src/frame_geometry.rs index 738a14f8e..d3ff79931 100644 --- a/crates/compositor/src/frame_geometry.rs +++ b/crates/compositor/src/frame_geometry.rs @@ -743,6 +743,37 @@ pub struct FrameGeometry { pub shape_fade: f32, } +impl FrameGeometry { + /// Rect de destination d'une annotation, en fractions de la sortie. + /// + /// `x`/`y`/`w`/`h` sont des fractions du rect ÉCRAN, et le rect en question est + /// `s_ann` — jamais `s_dst`. Les deux coïncident sans zoom, ce qui rend l'erreur + /// invisible sur la moitié des scènes ; sous zoom, `s_dst` grandit et emmène + /// annotations et sous-titres avec lui, alors que le contrat de `SceneAnnotation` + /// les veut « deliberately NOT affected by the zoom crop ». + /// + /// Cette méthode existe pour que le backend n'ait pas le choix : le bug est + /// reparu sur Linux après avoir été corrigé sur Windows et macOS (issue #179), + /// parce que chaque backend refaisait l'arithmétique dans son coin. + pub fn annotation_dst(&self, x: f32, y: f32, w: f32, h: f32) -> [f32; 4] { + [ + self.s_ann[0] + x * self.s_ann[2], + self.s_ann[1] + y * self.s_ann[3], + w * self.s_ann[2], + h * self.s_ann[3], + ] + } + + /// Hauteur en px du rect d'ancrage des annotations, pour `rh` px de sortie. + /// + /// `font_size_rel` est une fraction de cette hauteur (cf. `annotationScale.ts`) : + /// la prendre sur `s_dst` ferait grossir le texte avec le zoom, exactement comme + /// `annotation_dst` le déplacerait. + pub fn annotation_anchor_h_px(&self, rh: f32) -> f32 { + self.s_ann[3] * rh + } +} + /// Où va chaque calque, pour une frame — sans toucher au GPU. /// /// C'est la première moitié de `compose_frame`, mot pour mot : 353 lignes qui ne @@ -1291,10 +1322,11 @@ mod tests { } } - /// La même scène, avec une région de zoom active à `t = 1.5 s`. - fn zoomed_golden_scene() -> Scene { - Scene::from_json( - r##"{ + /// Le JSON de la scène zoomée, brut : `tilted_golden_scene` n'en change QUE la + /// rotation, et le faire par substitution garantit que les deux scènes ne diffèrent + /// pas ailleurs sans qu'on s'en aperçoive. + fn zoomed_golden_scene_json() -> &'static str { + r##"{ "clips":[{"screenPath":"/s.mp4","webcamPath":"/w.mp4","sourceStartSec":0,"sourceEndSec":10,"webcamOffsetSec":0,"hasAudio":true}], "layout":{"preset":"picture-in-picture","webcamSize":0.44,"webcamShape":"circle","webcamMirror":false, "webcamPosition":{"cx":0.8577,"cy":0.8159},"webcamReactiveZoom":false}, @@ -1304,9 +1336,12 @@ mod tests { "cursor":{"show":true,"size":7.76,"smoothing":0,"motionBlur":0.35,"clickBounce":1,"clipToBounds":false,"theme":"default"}, "cropByClip":[{"x":0,"y":0,"width":0.61,"height":0.61}], "output":{"width":1170,"height":658,"fps":60} - }"##, - ) - .expect("zoomed golden scene") + }"## + } + + /// La même scène, avec une région de zoom active à `t = 1.5 s`. + fn zoomed_golden_scene() -> Scene { + Scene::from_json(zoomed_golden_scene_json()).expect("zoomed golden scene") } /// L'ancre des annotations ne bouge PAS avec le zoom, alors que la boîte écran, si. @@ -1339,6 +1374,65 @@ mod tests { assert_eq!(a.s_ann, a.s_dst, "sans zoom, ancre et boîte écran coïncident"); } + /// La même scène, zoomée ET inclinée par un préset de rotation 3D. + fn tilted_golden_scene() -> Scene { + Scene::from_json( + &zoomed_golden_scene_json().replace(r#""rotation":"none""#, r#""rotation":"iso""#), + ) + .expect("tilted golden scene") + } + + /// Le rect et la taille de police d'une annotation ne bougent ni sous le zoom ni sous + /// une rotation 3D. + /// + /// `the_annotation_anchor_ignores_the_zoom` prouve que `plan_frame` **calcule** la + /// bonne ancre ; il ne dit rien de ce que le backend en fait. Linux, lui, refaisait + /// l'arithmétique contre `s_dst` — donc sous-titres qui grossissent et dérivent à + /// l'export, sur la seule plateforme où personne ne l'avait vu. Ce test porte sur les + /// accesseurs que les backends appellent maintenant, pas sur le champ brut. + /// + /// La rotation compte autant que le zoom : un préset iso/left/right est une propriété + /// de région de zoom, donc l'incliner amenait aussi la boîte — et les sous-titres + /// partaient avec elle, sans pour autant suivre le plan incliné. Les deux symptômes, + /// une seule cause. + #[test] + fn the_annotation_rect_and_font_ignore_zoom_and_rotation() { + let cfg = crate::config::all().pop().expect("au moins une config"); + let rh = 658.0; + // Un rect d'annotation quelconque, décentré : au centre, un rect qui suivrait le + // zoom garderait le même centre et la moitié de l'erreur passerait inaperçue. + let (x, y, w, h) = (0.04, 0.78, 0.92, 0.22); + + let plain = plan_frame(&golden_input(&golden_scene(), &cfg)); + let zoomed = plan_frame(&golden_input(&zoomed_golden_scene(), &cfg)); + let tilted = plan_frame(&golden_input(&tilted_golden_scene(), &cfg)); + + // Le garde-fou : sans lui, un `plan_frame` qui cesserait d'appliquer le zoom + // rendrait les assertions suivantes vraies pour la mauvaise raison. + assert_ne!( + plain.s_dst, zoomed.s_dst, + "le zoom doit agir sur la boîte écran — sinon ce test ne prouve rien" + ); + assert!( + !crate::regions::is_identity_rotation(tilted.zoom_rotation), + "le préset iso doit produire une rotation — sinon ce test ne prouve rien" + ); + + let expected = plain.annotation_dst(x, y, w, h); + for (name, g) in [("zoom", &zoomed), ("rotation 3D", &tilted)] { + let got = g.annotation_dst(x, y, w, h); + assert_eq!( + got, expected, + "le rect de l'annotation a suivi le {name} : {got:?} au lieu de {expected:?}" + ); + assert_eq!( + g.annotation_anchor_h_px(rh), + plain.annotation_anchor_h_px(rh), + "la taille de police a suivi le {name}" + ); + } + } + /// **Le golden iso-render.** /// /// Les deux backends ne peuvent pas tourner sur la même machine, donc « iso avec diff --git a/crates/compositor/tests/compose_linux.rs b/crates/compositor/tests/compose_linux.rs index 9aa60d928..aa058a026 100644 --- a/crates/compositor/tests/compose_linux.rs +++ b/crates/compositor/tests/compose_linux.rs @@ -1107,8 +1107,15 @@ fn compose_linux_sans_camera_ne_dessine_pas_de_vignette() { /// Scene de base des tests d'annotation : fond uni, pas d'effets, une liste /// d'annotations injectee telle quelle. fn annotation_scene(annotations: &str) -> String { + annotation_scene_with_zoom("", annotations) +} + +/// La meme, avec une liste de regions de zoom injectee telle quelle. Le zoom est +/// ce qui separe le rect d'ancrage des annotations (`s_ann`) de la boite ecran +/// (`s_dst`) : sans lui les deux coincident et rien ne peut le mesurer. +fn annotation_scene_with_zoom(zoom_regions: &str, annotations: &str) -> String { format!( - r##"{{"clips":[],"layout":{{"preset":"no-webcam","webcamSize":1,"webcamShape":"rectangle","webcamMirror":false,"webcamPosition":null,"webcamReactiveZoom":false}},"effects":{{"padding":0.1,"blur":false,"shadow":0,"roundnessFrac":0,"motionBlur":0}},"background":{{"kind":"color","color":"#00ff00"}},"zoomRegions":[],"annotations":[{annotations}],"cursor":{{"show":false,"size":1,"smoothing":0,"motionBlur":0,"clickBounce":0,"clipToBounds":false,"theme":"default"}},"cropByClip":[],"output":{{"width":1920,"height":1080,"fps":30}}}}"## + r##"{{"clips":[],"layout":{{"preset":"no-webcam","webcamSize":1,"webcamShape":"rectangle","webcamMirror":false,"webcamPosition":null,"webcamReactiveZoom":false}},"effects":{{"padding":0.1,"blur":false,"shadow":0,"roundnessFrac":0,"motionBlur":0}},"background":{{"kind":"color","color":"#00ff00"}},"zoomRegions":[{zoom_regions}],"annotations":[{annotations}],"cursor":{{"show":false,"size":1,"smoothing":0,"motionBlur":0,"clickBounce":0,"clipToBounds":false,"theme":"default"}},"cropByClip":[],"output":{{"width":1920,"height":1080,"fps":30}}}}"## ) } @@ -1468,3 +1475,99 @@ fn compose_linux_animation_texte() { contre {full_right} a la fin" ); } + +/// Une caption ne bouge ni ne grossit quand le contenu zoome ou s'incline. +/// +/// C'est le contrat de `SceneAnnotation` — l'overlay de la preview est FRERE de +/// l'element qui porte la transform, donc les sous-titres tiennent en place +/// pendant que la video zoome dessous — et c'est celui que ce backend violait : +/// il ancrait les annotations sur `s_dst`, la boite ecran, au lieu de `s_ann`. +/// Windows et macOS avaient ete corriges a l'issue #179, Linux non, et personne +/// ne l'a vu parce que la preview (web) reste juste sur les trois plateformes : +/// seul l'export divergeait. +/// +/// La bande porte un fond OPAQUE : son empreinte est alors le rect entier, pas +/// la silhouette des glyphes, donc la boite englobante mesure directement la +/// geometrie de placement et ne depend pas de ce que la video montre dessous. +/// Chaque rendu est diffe contre son propre rendu sans annotation, sinon le +/// changement du calque video sous le zoom dominerait la mesure. +/// +/// La rotation compte autant que le zoom : un preset iso/left/right est une +/// propriete de region de zoom, donc l'incliner amenait la boite avec lui — et +/// les sous-titres partaient avec elle sans pour autant suivre le plan incline. +#[test] +fn compose_linux_annotation_ancree_hors_zoom() { + if std::env::var("OPENSCREEN_LINUX_COMPOSE").is_err() || !Path::new(FIXTURE).is_file() { + eprintln!("compose_linux annotation hors zoom: opt-in. Skip."); + return; + } + let gpu = Gpu::create(false).expect("Gpu::create"); + let comp = Compositor::new_sized(&gpu, W, H).expect("Compositor::new_sized"); + let mut screen = Decoder::open(FIXTURE, &gpu).expect("Decoder::open"); + + let mut render = |zoom: &str, annotations: &str| -> Vec { + let parsed = + Scene::from_json(&annotation_scene_with_zoom(zoom, annotations)).expect("scene json"); + comp.set_live_params(openscreen_compositor::compositor::live_params_from_scene(&parsed)); + comp.set_scene(Some(parsed)); + comp.set_timeline_time(Some(3.0)); + unsafe { + let sf = screen.seek_to(1.0).expect("seek"); + // Comme `compose_linux_ecran_tilte` : frame 90 tombe au coeur de la + // region [0, 6], rampe d'entree finie, zoom et rotation a pleine force. + comp.compose_frame(sf, sf, 90.0, &Cfg::c8()).expect("compose_frame"); + comp.readback_direct().expect("readback").2 + } + }; + + let zoom = |rotation: &str| { + format!( + r##"{{"id":"z1","clipIndex":0,"startSec":0,"endSec":6,"scale":2.5,"focusX":0.5,"focusY":0.3,"focusMode":"manual","rotation":{rotation}}}"## + ) + }; + // Bande basse pleine largeur, comme une vraie caption : hors du centre, donc + // un ancrage qui suivrait le zoom la deplacerait ET la ferait deborder. + const BAND: &str = r##"{"id":"c1","kind":"text","x":0.04,"y":0.74,"w":0.92,"h":0.22,"startSec":0,"endSec":10,"zIndex":1,"text":{"content":"Sous-titre","color":"#ffffff","backgroundColor":"#e0245e","fontSizeRel":0.09,"fontFamily":"","fontWeight":"bold","fontStyle":"normal","textDecoration":"none","textAlign":"center"}}"##; + + let (z, tilt) = (zoom("null"), zoom(r#""iso""#)); + let bare_plain = render("", ""); + let with_plain = render("", BAND); + let bare_zoom = render(&z, ""); + let with_zoom = render(&z, BAND); + let bare_tilt = render(&tilt, ""); + let with_tilt = render(&tilt, BAND); + + let band_bbox = |bare: &[u8], with: &[u8], label: &str| { + let px = changed_pixels(bare, with); + assert!(px.len() > 500, "bande absente ({} px changes) — {label}", px.len()); + bbox(&px, W) + }; + let plain = band_bbox(&bare_plain, &with_plain, "sans zoom"); + let zoomed = band_bbox(&bare_zoom, &with_zoom, "avec zoom"); + let tilted = band_bbox(&bare_tilt, &with_tilt, "avec rotation 3D"); + + // Garde-fou : le zoom doit vraiment agir sur l'image, sinon les egalites + // ci-dessous seraient vraies pour la mauvaise raison. + let moved = changed_pixels(&bare_plain, &bare_zoom); + assert!( + moved.len() > 10_000, + "le zoom n'a change que {} px — la scene zoomee n'est pas appliquee", + moved.len() + ); + + // 2 px de tolerance pour l'antialiasing du bord de la plaque ; l'erreur que ce + // test attrape se compte en dizaines de pixels (la bande deborde du cadre). + for (name, got) in [("zoom", zoomed), ("rotation 3D", tilted)] { + let d = [ + got.0.abs_diff(plain.0), + got.1.abs_diff(plain.1), + got.2.abs_diff(plain.2), + got.3.abs_diff(plain.3), + ]; + assert!( + d.iter().all(|&v| v <= 2), + "la bande a suivi le {name} : {got:?} contre {plain:?} sans — \ + les annotations sont ancrees sur `s_dst` au lieu de `s_ann`" + ); + } +} From a31452b815dcdbc777b819cc9d5356fb29b06895 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Wed, 19 Aug 2026 18:02:01 +0200 Subject: [PATCH 2/2] refactor(compositor): share the annotation rect across the three backends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #398 found the fix correct but its two supporting claims not. `annotation_dst`'s doc said it existed « pour que le backend n'ait pas le choix », yet only Linux was converted: Metal and D3D still open-coded the same four lines and the same `font_size_rel * rect[3] * rh`, and both took the anchor as a parameter named `screen_dst` — pointing at the wrong field, one token away from re-shipping #179 on two more platforms. The arithmetic now lives once, in `annotation_dst_in`. It is a free function and not a method because Windows destructures `FrameGeometry` on arrival and has no `&self` left to offer; the method forwards to it for Linux, which does hold the geometry. Both `draw_annotations` parameters are renamed `s_ann`, so the name states the contract instead of contradicting it. What this does NOT do, and the doc now says so: remove the rect CHOICE from the Metal and D3D call sites. Only rendering pixels can catch that, which is the opt-in Linux test. The unit test grew the two assertions that make it more than a tautology over `s_ann`: that `annotation_dst_in` agrees with the method (the path Metal and D3D take is now covered on all three CI jobs), and that the same function fed `s_dst` returns a DIFFERENT rect — without which a constant implementation would satisfy everything above it. Comment corrections, all of them things the review caught as false: - `compose_linux.rs` claimed the drift was invisible in the preview and hit only the export. The native compositor is the preview's sole pixel source (`AnnotationOverlay.tsx` paints selection chrome, not pixels), so it was visible while editing — and the claim contradicted the comment this same branch added to `compositor_linux.rs`. - « son empreinte est le rect entier » was wrong: the plate comes from `glyphs.plate`, hugged to the laid-out lines. The measurement is still sound, the stated reason was not. - « frame 90 tombe au coeur de la region » was wrong twice: `FPS` is a hard 60, so frame 90 is 1.5 s, and `set_timeline_time(3.0)` is what actually drives sampling. Fixed here and in `compose_linux_ecran_tilte`, which is where the error was copied from. - The 2 px tolerance note undersold the error by an order of magnitude: the band drops ~450 px and leaves the frame, so « bande absente » is what fires. That assert now names `s_dst` instead of leaving the reader to guess why a band went missing. One `seek_to` for the six renders instead of six identical ones, as `compose_linux_ecran_tilte` already does — same AVFrame, so the decoder cannot contribute a difference that reads as a displacement. Not compiled locally: the crate's build.rs needs vendored ffmpeg headers absent from this machine. The three CI Rust jobs are the verification. Co-Authored-By: Claude --- crates/compositor/src/compositor_macos.rs | 20 +++--- crates/compositor/src/compositor_windows.rs | 25 +++++--- crates/compositor/src/frame_geometry.rs | 69 +++++++++++++++------ crates/compositor/tests/compose_linux.rs | 52 +++++++++++----- 4 files changed, 112 insertions(+), 54 deletions(-) diff --git a/crates/compositor/src/compositor_macos.rs b/crates/compositor/src/compositor_macos.rs index 4ea09a42c..352fcb57a 100644 --- a/crates/compositor/src/compositor_macos.rs +++ b/crates/compositor/src/compositor_macos.rs @@ -976,14 +976,19 @@ impl Compositor { } - /// Annotations : calque le plus haut, ancré sur `screen_dst` — le conteneur que reçoit - /// l'overlay web. Port de `compositor_windows::draw_annotations`. + /// Annotations : calque le plus haut, ancré sur `s_ann` — le rect écran SANS ZOOM, le + /// conteneur que reçoit l'overlay web. Port de `compositor_windows::draw_annotations`. + /// + /// Le paramètre s'appelle `s_ann` et pas `screen_dst` parce que c'est le seul rect + /// correct : lui passer `s_dst` fait dériver et grossir les sous-titres sous un zoom + /// (issue #179, puis #397 sur Linux). L'arithmétique elle-même vit dans + /// `frame_geometry::annotation_dst_in`, partagée par les trois backends. unsafe fn draw_annotations( &self, cmd: &metal::CommandBufferRef, scene: Option<&Scene>, t: f32, - screen_dst: [f32; 4], + s_ann: [f32; 4], ) -> Result<()> { let Some(scene) = scene else { return Ok(()) }; if scene.annotations.is_empty() { @@ -1017,12 +1022,7 @@ impl Compositor { if !visible(a) { continue; } - let dst = [ - screen_dst[0] + a.x * screen_dst[2], - screen_dst[1] + a.y * screen_dst[3], - a.w * screen_dst[2], - a.h * screen_dst[3], - ]; + let dst = crate::frame_geometry::annotation_dst_in(s_ann, a.x, a.y, a.w, a.h); let quad_px = [dst[2] * rw, dst[3] * rh]; if quad_px[0] <= 0.0 || quad_px[1] <= 0.0 { continue; @@ -1137,7 +1137,7 @@ impl Compositor { color: parse_hex(&text.color).unwrap_or([1.0, 1.0, 1.0, 1.0]), background: parse_hex(&text.background_color) .unwrap_or([0.0, 0.0, 0.0, 0.0]), - font_size_px: text.font_size_rel * (screen_dst[3] * rh), + font_size_px: text.font_size_rel * (s_ann[3] * rh), font_family: text.font_family.clone(), bold: text.font_weight == "bold", italic: text.font_style == "italic", diff --git a/crates/compositor/src/compositor_windows.rs b/crates/compositor/src/compositor_windows.rs index b4f19b9be..95d40af48 100644 --- a/crates/compositor/src/compositor_windows.rs +++ b/crates/compositor/src/compositor_windows.rs @@ -1578,12 +1578,18 @@ impl Compositor { Ok(()) } - /// Dessine les annotations visibles à `t`. `screen_dst` = rect écran en fractions de sortie. + /// Dessine les annotations visibles à `t`. `s_ann` = rect écran SANS ZOOM, en fractions de + /// sortie. + /// + /// Le paramètre s'appelle `s_ann` et pas `screen_dst` parce que c'est le seul rect correct : + /// lui passer `s_dst` fait dériver et grossir les sous-titres sous un zoom (issue #179, puis + /// #397 sur Linux). L'arithmétique elle-même vit dans `frame_geometry::annotation_dst_in`, + /// partagée par les trois backends. /// /// Seule la « figure » (flèche) est rendue à ce stade ; texte, image et flou suivront. Les /// types non gérés sont ignorés silencieusement plutôt que dessinés de travers : mieux vaut /// l'absence connue qu'un placeholder qui ferait croire à un bug de style. - unsafe fn draw_annotations(&self, scene: Option<&Scene>, t: f32, screen_dst: [f32; 4]) { + unsafe fn draw_annotations(&self, scene: Option<&Scene>, t: f32, s_ann: [f32; 4]) { let Some(scene) = scene else { return }; if scene.annotations.is_empty() { return; @@ -1610,12 +1616,13 @@ impl Compositor { if !visible(annotation) { continue; } - let dst = [ - screen_dst[0] + annotation.x * screen_dst[2], - screen_dst[1] + annotation.y * screen_dst[3], - annotation.w * screen_dst[2], - annotation.h * screen_dst[3], - ]; + let dst = crate::frame_geometry::annotation_dst_in( + s_ann, + annotation.x, + annotation.y, + annotation.w, + annotation.h, + ); let quad_px = [dst[2] * self.rw(), dst[3] * self.rh()]; if quad_px[0] <= 0.0 || quad_px[1] <= 0.0 { continue; @@ -1750,7 +1757,7 @@ impl Compositor { // `font_size_rel` est une fraction de la HAUTEUR DU RECT ÉCRAN (cf. le contrat // et `annotationScale.ts`) : on la ramène en pixels de sortie ici, avec le même // produit que la preview applique contre sa propre boîte. - let screen_h_px = screen_dst[3] * self.rh(); + let screen_h_px = s_ann[3] * self.rh(); let spec = crate::text::TextSpec { content: text.content.clone(), color: parse_hex(&text.color).unwrap_or([1.0, 1.0, 1.0, 1.0]), diff --git a/crates/compositor/src/frame_geometry.rs b/crates/compositor/src/frame_geometry.rs index d3ff79931..902ba1d8f 100644 --- a/crates/compositor/src/frame_geometry.rs +++ b/crates/compositor/src/frame_geometry.rs @@ -743,25 +743,32 @@ pub struct FrameGeometry { pub shape_fade: f32, } +/// Rect de destination d'une annotation dans un rect d'ancrage, en fractions de la sortie. +/// +/// `anchor` est TOUJOURS `s_ann`, le rect écran sans le zoom — jamais `s_dst`. Les deux +/// coïncident sans zoom, ce qui rend l'erreur invisible sur la moitié des scènes ; sous +/// zoom, `s_dst` grandit et emmène annotations et sous-titres avec lui, alors que le +/// contrat de `SceneAnnotation` les veut « deliberately NOT affected by the zoom crop ». +/// +/// Version libre plutôt que méthode : Windows déstructure `FrameGeometry` dès l'entrée de +/// `compose_frame`, donc il n'a plus de `&self` à offrir quand il dessine les annotations. +/// Les trois backends partagent malgré tout CETTE arithmétique-ci — le bug est reparu sur +/// Linux après avoir été corrigé sur Windows et macOS (issue #179) parce que chacun en +/// gardait sa copie. +/// +/// Attention : le choix du rect passé en `anchor` reste, lui, au call site des backends +/// Metal et D3D (leur `draw_annotations` prend le rect en paramètre). Seul Linux part +/// directement de `FrameGeometry`. Passer `s_dst` ici reste donc possible sur deux +/// backends sur trois — d'où le nom du paramètre côté appelants, et les tests. +pub fn annotation_dst_in(anchor: [f32; 4], x: f32, y: f32, w: f32, h: f32) -> [f32; 4] { + [anchor[0] + x * anchor[2], anchor[1] + y * anchor[3], w * anchor[2], h * anchor[3]] +} + impl FrameGeometry { - /// Rect de destination d'une annotation, en fractions de la sortie. - /// - /// `x`/`y`/`w`/`h` sont des fractions du rect ÉCRAN, et le rect en question est - /// `s_ann` — jamais `s_dst`. Les deux coïncident sans zoom, ce qui rend l'erreur - /// invisible sur la moitié des scènes ; sous zoom, `s_dst` grandit et emmène - /// annotations et sous-titres avec lui, alors que le contrat de `SceneAnnotation` - /// les veut « deliberately NOT affected by the zoom crop ». - /// - /// Cette méthode existe pour que le backend n'ait pas le choix : le bug est - /// reparu sur Linux après avoir été corrigé sur Windows et macOS (issue #179), - /// parce que chaque backend refaisait l'arithmétique dans son coin. + /// `annotation_dst_in` appliqué à `s_ann`, pour les backends qui tiennent la géométrie + /// entière — c'est-à-dire ceux qui n'ont aucune raison de choisir un rect. pub fn annotation_dst(&self, x: f32, y: f32, w: f32, h: f32) -> [f32; 4] { - [ - self.s_ann[0] + x * self.s_ann[2], - self.s_ann[1] + y * self.s_ann[3], - w * self.s_ann[2], - h * self.s_ann[3], - ] + annotation_dst_in(self.s_ann, x, y, w, h) } /// Hauteur en px du rect d'ancrage des annotations, pour `rh` px de sortie. @@ -1387,9 +1394,14 @@ mod tests { /// /// `the_annotation_anchor_ignores_the_zoom` prouve que `plan_frame` **calcule** la /// bonne ancre ; il ne dit rien de ce que le backend en fait. Linux, lui, refaisait - /// l'arithmétique contre `s_dst` — donc sous-titres qui grossissent et dérivent à - /// l'export, sur la seule plateforme où personne ne l'avait vu. Ce test porte sur les - /// accesseurs que les backends appellent maintenant, pas sur le champ brut. + /// l'arithmétique contre `s_dst` — donc sous-titres qui grossissent et dérivent, sur + /// la seule plateforme qui n'avait pas été corrigée. Ce test porte sur les fonctions + /// que les backends appellent maintenant, pas sur le champ brut : la méthode côté + /// Linux ET `annotation_dst_in`, par où passent Metal et D3D. + /// + /// Ce qu'il ne couvre toujours PAS : le choix du rect au call site de Metal et D3D, + /// qui prennent leur ancre en paramètre. Ce niveau-là n'est vérifiable qu'en rendant + /// des pixels — c'est `compose_linux_annotation_ancree_hors_zoom`, opt-in. /// /// La rotation compte autant que le zoom : un préset iso/left/right est une propriété /// de région de zoom, donc l'incliner amenait aussi la boîte — et les sous-titres @@ -1430,6 +1442,23 @@ mod tests { plain.annotation_anchor_h_px(rh), "la taille de police a suivi le {name}" ); + // Metal et D3D n'appellent pas la méthode : ils passent leur rect d'ancrage à + // `annotation_dst_in`. Les deux chemins doivent rendre le MÊME rect, sinon le + // « corrigé sur une plateforme seulement » recommence par le bas. + assert_eq!( + annotation_dst_in(g.s_ann, x, y, w, h), + got, + "le chemin des backends Metal/D3D diverge de la méthode sous le {name}" + ); + // Et le garde-fou qui donne un sens aux deux précédents : nourrie avec `s_dst`, + // la même fonction rend un rect DIFFÉRENT. Sans ça, un `annotation_dst_in` + // devenu constant satisferait tout ce qui précède. + assert_ne!( + annotation_dst_in(g.s_dst, x, y, w, h), + expected, + "sous le {name}, ancrer sur `s_dst` devrait déplacer le rect — \ + si les deux coïncident, ce test ne prouve plus rien" + ); } } diff --git a/crates/compositor/tests/compose_linux.rs b/crates/compositor/tests/compose_linux.rs index aa058a026..62361572d 100644 --- a/crates/compositor/tests/compose_linux.rs +++ b/crates/compositor/tests/compose_linux.rs @@ -171,8 +171,9 @@ fn compose_linux_ecran_tilte() { cfg.shadow = false; let (w, h, upright, tilted) = unsafe { let sf = dec.seek_to(1.0).expect("Decoder::seek_to"); - // `frame` = 90 -> source_t = 3 s, au coeur de la region [0, 6] : la rampe - // d'entree est finie, la rotation est a pleine force. + // `frame` = 90 -> source_t = 90 / 60 = 1,5 s (`FPS` vaut 60 en dur dans + // `frame_geometry`, ce n'est pas le `fps` de la scene), dans la region [0, 6] et + // au-dela de la rampe d'entree : la rotation est a pleine force. let render = |json: String| { let scene = Scene::from_json(&json).expect("scene json"); // Le padding transite par les live_params, pas la scene brute. @@ -1482,13 +1483,17 @@ fn compose_linux_animation_texte() { /// l'element qui porte la transform, donc les sous-titres tiennent en place /// pendant que la video zoome dessous — et c'est celui que ce backend violait : /// il ancrait les annotations sur `s_dst`, la boite ecran, au lieu de `s_ann`. -/// Windows et macOS avaient ete corriges a l'issue #179, Linux non, et personne -/// ne l'a vu parce que la preview (web) reste juste sur les trois plateformes : -/// seul l'export divergeait. +/// Windows et macOS avaient ete corriges a l'issue #179, Linux non. Le natif +/// etant la SEULE source de pixels de l'apercu (le DOM ne peint que la poignee +/// de selection, cf. `AnnotationOverlay.tsx`), la derive se voyait des l'edition +/// sur cette plateforme, pas seulement a l'export. /// -/// La bande porte un fond OPAQUE : son empreinte est alors le rect entier, pas -/// la silhouette des glyphes, donc la boite englobante mesure directement la -/// geometrie de placement et ne depend pas de ce que la video montre dessous. +/// La bande porte un fond OPAQUE : son empreinte est un RECT PLEIN et non la +/// silhouette des glyphes, donc la boite englobante mesure une geometrie de +/// placement et pas ce que la video montre dessous. Ce rect est la plaque du +/// rasteriseur (`glyphs.plate`), serree sur les lignes posees — pas la boite +/// 0.92x0.22 de l'annotation : peu importe ici, la plaque est une fonction +/// deterministe de `dst` et de `font_size_px`, tous deux ancres sur `s_ann`. /// Chaque rendu est diffe contre son propre rendu sans annotation, sinon le /// changement du calque video sous le zoom dominerait la mesure. /// @@ -1505,16 +1510,22 @@ fn compose_linux_annotation_ancree_hors_zoom() { let comp = Compositor::new_sized(&gpu, W, H).expect("Compositor::new_sized"); let mut screen = Decoder::open(FIXTURE, &gpu).expect("Decoder::open"); - let mut render = |zoom: &str, annotations: &str| -> Vec { + // UN SEUL `seek_to`, comme `compose_linux_ecran_tilte` : les six rendus partagent + // la meme AVFrame, donc le decodeur ne peut pas introduire une difference qu'on + // prendrait pour un deplacement de la bande. + let sf = unsafe { screen.seek_to(1.0).expect("seek") }; + + let render = |zoom: &str, annotations: &str| -> Vec { let parsed = Scene::from_json(&annotation_scene_with_zoom(zoom, annotations)).expect("scene json"); comp.set_live_params(openscreen_compositor::compositor::live_params_from_scene(&parsed)); comp.set_scene(Some(parsed)); + // C'est CET override qui fixe le temps echantillonne, pas le 3e argument de + // `compose_frame` : `plan_frame` lit `timeline_t_override.unwrap_or(frame / FPS)`, + // et `FPS` y vaut 60 en dur (pas le `fps` de la scene). t = 3 s tombe au coeur + // de la region [0, 6], rampe d'entree finie, zoom et rotation a pleine force. comp.set_timeline_time(Some(3.0)); unsafe { - let sf = screen.seek_to(1.0).expect("seek"); - // Comme `compose_linux_ecran_tilte` : frame 90 tombe au coeur de la - // region [0, 6], rampe d'entree finie, zoom et rotation a pleine force. comp.compose_frame(sf, sf, 90.0, &Cfg::c8()).expect("compose_frame"); comp.readback_direct().expect("readback").2 } @@ -1539,7 +1550,15 @@ fn compose_linux_annotation_ancree_hors_zoom() { let band_bbox = |bare: &[u8], with: &[u8], label: &str| { let px = changed_pixels(bare, with); - assert!(px.len() > 500, "bande absente ({} px changes) — {label}", px.len()); + // C'est CE garde-fou qui saute en premier quand le bug est present : ancree sur + // `s_dst`, la bande part a y ~= 840 px dans un cadre de 540 et sort du champ, donc + // elle ne change plus aucun pixel au lieu de se decaler de quelques-uns. + assert!( + px.len() > 500, + "bande absente ({} px changes) — {label} : sortie du cadre, donc ancree sur \ + `s_dst` au lieu de `s_ann`", + px.len() + ); bbox(&px, W) }; let plain = band_bbox(&bare_plain, &with_plain, "sans zoom"); @@ -1555,8 +1574,11 @@ fn compose_linux_annotation_ancree_hors_zoom() { moved.len() ); - // 2 px de tolerance pour l'antialiasing du bord de la plaque ; l'erreur que ce - // test attrape se compte en dizaines de pixels (la bande deborde du cadre). + // 2 px de tolerance pour l'antialiasing du bord de la plaque. L'erreur visee est + // d'un tout autre ordre : sous zoom 2.5 la bande ancree sur `s_dst` descend de + // ~450 px et grossit d'autant, donc en pratique elle quitte le cadre et c'est le + // garde-fou « bande absente » ci-dessus qui tranche. Cette boucle attrape le reste : + // un ancrage qui deriverait sans sortir du champ. for (name, got) in [("zoom", zoomed), ("rotation 3D", tilted)] { let d = [ got.0.abs_diff(plain.0),