diff --git a/crates/compositor/src/compositor_macos.rs b/crates/compositor/src/compositor_macos.rs index 822d1d417..a6d168c9a 100644 --- a/crates/compositor/src/compositor_macos.rs +++ b/crates/compositor/src/compositor_macos.rs @@ -19,10 +19,13 @@ //! Metal n'a pas d'équivalent de `ID3D11DeviceContext::Map` sur une ressource //! `Private`. Les cibles de rendu (`rt`, `nv12_y`, `nv12_uv`) sont donc en //! `StorageMode::Private`, et chaque passe se termine par un `MTLBlitCommandEncoder` -//! vers un miroir `Shared` (`rt_read`, `nv12_read_y`, `nv12_read_uv`) sur lequel -//! `getBytes` est légal. Le `waitUntilCompleted` qui suit est ce qui rend +//! vers un miroir lisible par le CPU (`rt_read`, `nv12_read_y`, `nv12_read_uv`) sur +//! lequel `getBytes` est légal. Le `waitUntilCompleted` qui suit est ce qui rend //! `readback_direct` synchrone, comme son homologue Windows : sans lui, la preview //! lirait le contenu de la frame précédente (ou du noir au premier tour). +//! +//! Ce miroir est `Shared` sur un GPU Apple et `Managed` sur un Mac Intel, où `Shared` +//! n'existe pas pour les textures : cf. `cpu_storage` et `copy_for_cpu`. use crate::config::Cfg; use crate::d3d::Gpu; @@ -219,14 +222,13 @@ const SEGMENTATION_HZ: u32 = 30; /// Cible RGBA + miroir de lecture pour extraire la frame webcam à la résolution du modèle. /// /// Deux textures, pas une : `rt` est `Private` parce que c'est une cible de rendu, et -/// `get_bytes` n'est légal que sur du `Shared`. C'est exactement le couple -/// `nv12_y`/`nv12_read_y` du chemin d'encodage, en RGBA et à 256x144 — cf. l'en-tête du -/// module. `Managed` n'a pas sa place ici : rien dans ce fichier n'en utilise, et c'est le -/// seul mode de stockage qui exigerait un `synchronizeResource` avant la lecture. +/// `get_bytes` n'est légal que sur une texture lisible par le CPU (`cpu_storage`). C'est +/// exactement le couple `nv12_y`/`nv12_read_y` du chemin d'encodage, en RGBA et à 256x144 +/// (cf. l'en-tête du module). struct SegCapture { /// Cible de la passe de capture. `Private` : écrite par le GPU, jamais lue par le CPU. rt: metal::Texture, - /// Miroir `Shared` de `rt`, rempli par blit dans le même command buffer. + /// Miroir CPU de `rt` (`cpu_storage`), rempli par blit dans le même command buffer. read: metal::Texture, width: u32, height: u32, @@ -294,13 +296,13 @@ pub struct Compositor { /// Render target principal RGBA8. Cible de `compose_frame`. `Private` : c'est une /// cible de rendu pure, jamais lue par le CPU (c'est `rt_read` qui l'est). rt: metal::Texture, - /// Miroir `Shared` de `rt`, rempli par blit à la fin de `compose_frame` — la seule + /// Miroir CPU de `rt` (`cpu_storage`), rempli par blit dans `readback_direct`, la seule /// façon d'atteindre `getBytes` depuis une cible `Private`. rt_read: metal::Texture, /// NV12 interne : plan Y `R8Unorm`, plan UV `RG8Unorm` (demi-résolution). nv12_y: metal::Texture, nv12_uv: metal::Texture, - /// Miroirs `Shared` des deux plans, pour `read_nv12_scaled`. + /// Miroirs CPU des deux plans, pour `read_nv12_scaled`. nv12_read_y: metal::Texture, nv12_read_uv: metal::Texture, @@ -418,6 +420,56 @@ fn make_texture( device.new_texture(&desc) } +/// Mode de stockage d'une texture que le CPU écrit (`replace_region`) ou lit (`get_bytes`). +/// +/// Apple ne garantit `Shared` aux TEXTURES que sur ses propres GPU. Sur un Mac Intel (UHD +/// 630 intégré comme Radeon Pro discret), le mode par défaut d'une texture est `Managed`, +/// le seul que ces GPU tiennent pour une texture lue par le CPU. Le RT relu par la preview +/// y était quand même créé `Shared` : preview vide, ou figée sur sa première image, pendant que l'export, +/// qui rend dans les `CVPixelBuffer` de l'encodeur sans jamais relire le RT, sortait juste +/// (#1238). +/// +/// `Managed` garde une copie CPU et une copie GPU : un `replace_region` met la seconde à +/// jour tout seul, mais une écriture GPU n'atteint la première qu'après un +/// `synchronizeResource`, et c'est ce que fait `copy_for_cpu`. +pub(crate) fn cpu_storage(device: &metal::DeviceRef) -> metal::MTLStorageMode { + cpu_storage_for(device.supports_family(metal::MTLGPUFamily::Apple1)) +} + +fn cpu_storage_for(apple_gpu: bool) -> metal::MTLStorageMode { + if apple_gpu { + metal::MTLStorageMode::Shared + } else { + metal::MTLStorageMode::Managed + } +} + +/// Blit de `src` (`Private`) vers son miroir CPU `dst`, `w`x`h` depuis l'origine. Un miroir +/// `Managed` (cf. `cpu_storage`) est synchronisé dans le même encodeur : sans cela, +/// `get_bytes` y lirait une copie CPU que le blit n'a pas touchée. +fn copy_for_cpu( + blit: &metal::BlitCommandEncoderRef, + src: &metal::TextureRef, + dst: &metal::TextureRef, + w: u32, + h: u32, +) { + blit.copy_from_texture( + src, + 0, + 0, + metal::MTLOrigin { x: 0, y: 0, z: 0 }, + metal::MTLSize { width: w as u64, height: h as u64, depth: 1 }, + dst, + 0, + 0, + metal::MTLOrigin { x: 0, y: 0, z: 0 }, + ); + if dst.storage_mode() == metal::MTLStorageMode::Managed { + blit.synchronize_resource(dst); + } +} + /// Tout ce qui a la taille du rendu, et rien d'autre : ce que `new_sized` alloue et que /// `resized` réalloue. Cf. `compositor_windows::Targets`. struct Targets { @@ -451,7 +503,7 @@ fn make_targets(device: &metal::Device, rw: u32, rh: u32) -> Targets { metal::MTLPixelFormat::RGBA8Unorm, rw, rh, - metal::MTLStorageMode::Shared, + cpu_storage(device), metal::MTLTextureUsage::ShaderRead, ); let nv12_y = make_texture( @@ -479,7 +531,7 @@ fn make_targets(device: &metal::Device, rw: u32, rh: u32) -> Targets { metal::MTLPixelFormat::R8Unorm, rw, rh, - metal::MTLStorageMode::Shared, + cpu_storage(device), metal::MTLTextureUsage::ShaderRead, ); let nv12_read_uv = make_texture( @@ -487,7 +539,7 @@ fn make_targets(device: &metal::Device, rw: u32, rh: u32) -> Targets { metal::MTLPixelFormat::RG8Unorm, rw / 2, rh / 2, - metal::MTLStorageMode::Shared, + cpu_storage(device), metal::MTLTextureUsage::ShaderRead, ); let accum = make_texture( @@ -649,7 +701,7 @@ impl Compositor { /// Comme `new`, mais avec une taille de rendu explicite. Câble le moteur Metal : /// - `CVMetalTextureCache` (zero-copy CVPixelBuffer → MTLTexture), - /// - render targets (RT RGBA, RT NV12 Y/UV, miroirs `Shared`), + /// - render targets (RT RGBA, RT NV12 Y/UV, miroirs CPU), /// - compilation MSL (`shaders.metal` → `MTLLibrary`), /// - pipeline states (principal + passes fullscreen). pub fn new_sized(gpu: &Gpu, w: u32, h: u32) -> Result { @@ -1227,7 +1279,7 @@ impl Compositor { metal::MTLPixelFormat::RGBA8Unorm, w, h, - metal::MTLStorageMode::Shared, + cpu_storage(&self.gpu.device), metal::MTLTextureUsage::ShaderRead, ); tex.replace_region( @@ -1256,7 +1308,7 @@ impl Compositor { metal::MTLPixelFormat::R16Float, sdf.width, sdf.height, - metal::MTLStorageMode::Shared, + cpu_storage(&self.gpu.device), metal::MTLTextureUsage::ShaderRead, ); tex.replace_region( @@ -1958,8 +2010,8 @@ impl Compositor { /// /// Trois étapes, la forme prescrite par l'en-tête du module et déjà tenue par /// `render_nv12` + `read_nv12_scaled` : rendu dans une cible `Private`, blit vers un - /// miroir `Shared`, `get_bytes`. Pas de `Managed`, donc pas de `synchronizeResource` — - /// c'est le seul mode de stockage qui l'exigerait, et rien dans ce fichier n'en utilise. + /// miroir CPU (`copy_for_cpu`, qui synchronise le miroir `Managed` d'un Mac Intel), + /// `get_bytes`. /// /// Le buffer `out` est réutilisé d'un appel à l'autre : il est dimensionné au RGBA lu /// puis compacté sur place en RGB, ce qui laisse sa capacité au maximum des deux et ne @@ -1995,7 +2047,7 @@ impl Compositor { metal::MTLPixelFormat::RGBA8Unorm, width, height, - metal::MTLStorageMode::Shared, + cpu_storage(&self.gpu.device), metal::MTLTextureUsage::ShaderRead, ), width, @@ -2038,17 +2090,7 @@ impl Compositor { enc.end_encoding(); } let blit = cmd_buf.new_blit_command_encoder(); - blit.copy_from_texture( - &cap.rt, - 0, - 0, - metal::MTLOrigin { x: 0, y: 0, z: 0 }, - metal::MTLSize { width: width as u64, height: height as u64, depth: 1 }, - &cap.read, - 0, - 0, - metal::MTLOrigin { x: 0, y: 0, z: 0 }, - ); + copy_for_cpu(blit, &cap.rt, &cap.read, width, height); blit.end_encoding(); cmd_buf.commit(); cmd_buf.wait_until_completed(); @@ -2079,8 +2121,9 @@ impl Compositor { /// Publie le masque de segmentation du sujet webcam (R8, `width`x`height`, 0 = fond). /// - /// La texture est `Shared` et réécrite en place par `replace_region` ; elle n'est - /// recréée que si la résolution du modèle change, ce qui n'arrive pas en régime établi. + /// La texture est écrivable par le CPU (`cpu_storage`) et réécrite en place par + /// `replace_region` ; elle n'est recréée que si la résolution du modèle change, ce qui + /// n'arrive pas en régime établi. /// /// Réécrire une texture que le GPU pourrait encore lire serait une course — ici il ne /// le peut pas : les trois chemins de frame macOS drainent la file avant de rendre la @@ -2107,7 +2150,7 @@ impl Compositor { metal::MTLPixelFormat::R8Unorm, width, height, - metal::MTLStorageMode::Shared, + cpu_storage(&self.gpu.device), metal::MTLTextureUsage::ShaderRead, ), width, @@ -2443,7 +2486,7 @@ impl Compositor { } /// Compose la frame : fond, ombre écran, écran, ombre caméra, caméra — puis miroir - /// `Shared` pour la lecture CPU. + /// CPU pour la lecture (`readback_direct`). /// /// La géométrie vient de `frame_geometry::plan_frame`, la MÊME fonction que le moteur /// D3D11 appelle. Ce qui reste ici n'est donc que l'émission des draws ; c'est aussi @@ -3050,24 +3093,10 @@ impl Compositor { Ok(()) } - /// Copie `rt` (`Private`) vers `rt_read` (`Shared`) dans le command buffer donné. + /// Copie `rt` (`Private`) vers son miroir CPU `rt_read` dans le command buffer donné. fn mirror_rt(&self, cmd_buf: &metal::CommandBufferRef) { let blit = cmd_buf.new_blit_command_encoder(); - blit.copy_from_texture( - &self.rt, - 0, - 0, - metal::MTLOrigin { x: 0, y: 0, z: 0 }, - metal::MTLSize { - width: self.render_w as u64, - height: self.render_h as u64, - depth: 1, - }, - &self.rt_read, - 0, - 0, - metal::MTLOrigin { x: 0, y: 0, z: 0 }, - ); + copy_for_cpu(blit, &self.rt, &self.rt_read, self.render_w, self.render_h); blit.end_encoding(); } @@ -3143,7 +3172,7 @@ impl Compositor { /// Convertit le RT RGBA → `nv12_y` (R8) et `nv12_uv` (RG8) via deux passes /// fullscreen (`ps_y` puis `ps_uv` sur `vs_fs`), puis recopie vers les miroirs - /// `Shared` que `read_nv12_scaled` lit. Miroir Metal de + /// CPU que `read_nv12_scaled` lit. Miroir Metal de /// `compositor_windows::render_nv12` — même conversion BT.709 limited. pub unsafe fn render_nv12(&self) -> Result<()> { let cmd_buf = self.gpu.context.new_command_buffer(); @@ -3179,21 +3208,7 @@ impl Compositor { self.render_h / 2, ), ] { - blit.copy_from_texture( - src, - 0, - 0, - metal::MTLOrigin { x: 0, y: 0, z: 0 }, - metal::MTLSize { - width: w as u64, - height: h as u64, - depth: 1, - }, - dst, - 0, - 0, - metal::MTLOrigin { x: 0, y: 0, z: 0 }, - ); + copy_for_cpu(blit, src, dst, w, h); } blit.end_encoding(); @@ -3203,7 +3218,7 @@ impl Compositor { /// Lit le RT RGBA vers un `Vec` CPU (preview live). Renvoie `(w, h, RGBA8)`. pub unsafe fn readback_direct(&self) -> Result<(u32, u32, Vec)> { - // Le miroir `Shared` se fait ICI plutôt qu'à chaque composition : seul ce chemin le + // Le miroir CPU se fait ICI plutôt qu'à chaque composition : seul ce chemin le // lit, et il n'est emprunté que par la preview. let cmd_buf = self.gpu.context.new_command_buffer(); self.mirror_rt(cmd_buf); @@ -3350,7 +3365,7 @@ mod tests { metal::MTLPixelFormat::R8Unorm, w, h, - metal::MTLStorageMode::Shared, + cpu_storage(device), metal::MTLTextureUsage::ShaderRead, ); let mut plane = vec![0u8; (w * h) as usize]; @@ -3367,7 +3382,7 @@ mod tests { metal::MTLPixelFormat::RG8Unorm, uw, uh, - metal::MTLStorageMode::Shared, + cpu_storage(device), metal::MTLTextureUsage::ShaderRead, ); let chroma = vec![UV_NEUTRAL; (uw * uh * 2) as usize]; @@ -3448,6 +3463,70 @@ mod tests { assert_eq!(px(&out, 3 * w / 4, h / 2), right); } + #[test] + fn only_an_apple_gpu_gets_shared_textures() { + assert_eq!(cpu_storage_for(true), metal::MTLStorageMode::Shared); + assert_eq!(cpu_storage_for(false), metal::MTLStorageMode::Managed); + } + + /// Le chemin d'un Mac Intel (#1238), joué sur le GPU qu'on a : un GPU Apple accepte aussi + /// les textures `Managed`. Deux couleurs à la suite, parce que le défaut rapporté est une + /// preview vide puis figée : une seule lecture ne verrait pas la seconde moitié. + #[test] + fn a_managed_mirror_reads_back_each_frame_the_gpu_drew() { + let Ok(gpu) = crate::d3d::Gpu::create(false) else { + eprintln!("pas de device Metal — test sauté"); + return; + }; + let (w, h) = (64u32, 32u32); + let rt = make_texture( + &gpu.device, + metal::MTLPixelFormat::RGBA8Unorm, + w, + h, + metal::MTLStorageMode::Private, + metal::MTLTextureUsage::RenderTarget | metal::MTLTextureUsage::ShaderRead, + ); + let read = make_texture( + &gpu.device, + metal::MTLPixelFormat::RGBA8Unorm, + w, + h, + metal::MTLStorageMode::Managed, + metal::MTLTextureUsage::ShaderRead, + ); + + for (color, expected) in [ + (metal::MTLClearColor::new(1.0, 0.0, 0.0, 1.0), [255u8, 0, 0, 255]), + (metal::MTLClearColor::new(0.0, 0.0, 1.0, 1.0), [0u8, 0, 255, 255]), + ] { + let cmd = gpu.context.new_command_buffer(); + let pass = metal::RenderPassDescriptor::new(); + let ca = pass.color_attachments().object_at(0).expect("color attachment 0"); + ca.set_texture(Some(&rt)); + ca.set_load_action(metal::MTLLoadAction::Clear); + ca.set_clear_color(color); + ca.set_store_action(metal::MTLStoreAction::Store); + cmd.new_render_command_encoder(pass).end_encoding(); + let blit = cmd.new_blit_command_encoder(); + copy_for_cpu(blit, &rt, &read, w, h); + blit.end_encoding(); + cmd.commit(); + cmd.wait_until_completed(); + + let mut data = vec![0u8; (w * h * 4) as usize]; + read.get_bytes( + data.as_mut_ptr() as *mut std::ffi::c_void, + (w * 4) as u64, + region(w, h), + 0, + ); + let last = data.len() - 4; + assert_eq!(data[..4], expected, "premier pixel"); + assert_eq!(data[last..], expected, "dernier pixel"); + } + } + #[test] fn a_capture_of_zero_size_is_refused_rather_than_rendered() { let Ok(gpu) = crate::d3d::Gpu::create(false) else { diff --git a/crates/compositor/src/frame_geometry.rs b/crates/compositor/src/frame_geometry.rs index be60b69c7..2988aabc2 100644 --- a/crates/compositor/src/frame_geometry.rs +++ b/crates/compositor/src/frame_geometry.rs @@ -3058,9 +3058,11 @@ pub fn plan_frame(input: &FrameGeometryInput) -> FrameGeometry { // clip, elle tombe dans le trou d'un trim ou dans le clip d'avant. Les régions y valent // ce qu'elles valent hors du clip (Full Camera coupée par un trim : 0), et le flou de // mouvement traînait la caméra plein cadre jusqu'à son coin PiP sur la frame de la coupe. + // Comparé en f32, comme les bornes des régions : la première frame d'un clip tombe pile + // sur son début, et un début inexact en f32 (11.388) relu en f64 passait sous la borne. let source_t_prev = clock.source_at(clock.at(source_t) - 1.0 / FPS); let source_t_prev = match scene.and_then(|s| s.clips.get(s.active_clip_index)) { - Some(clip) if f64::from(source_t) >= clip.source_start_sec => { + Some(clip) if source_t >= clip.source_start_sec as f32 => { source_t_prev.max(clip.source_start_sec as f32) } _ => source_t_prev, @@ -4849,6 +4851,41 @@ mod tests { assert_eq!(g.w_dst_prev, g.w_dst, "pas de traînée depuis le trou du trim"); } + /// La même coupe, mais la première frame du clip tombe PILE sur son début, comme à l'export + /// (`walk_composited_timeline` pose `target_source_time as f32`), et ce début n'est pas exact + /// en f32 : `11.388f32` relu en f64 vaut 11.38799953…, sous le début du clip. La borne de la + /// frame précédente ne s'appliquait donc pas, alors que la phase Full Camera, comparée en + /// f32, voyait déjà la caméra plein cadre : frame 600 traînée vers le coin PiP à l'export et + /// dans `getFrames` (passe e2e macOS de v2.1.0-rc.3). + #[test] + fn the_first_frame_of_a_clip_starting_inside_full_camera_has_no_trail() { + let cfg = crate::config::all().pop().expect("au moins une config"); + let scene = Scene::from_json( + r##"{ + "clips":[{"screenPath":"/s.mp4","webcamPath":"/w.mp4","sourceStartSec":0,"sourceEndSec":10.001,"webcamOffsetSec":0,"hasAudio":true}, + {"screenPath":"/s.mp4","webcamPath":"/w.mp4","sourceStartSec":11.388,"sourceEndSec":27.12,"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}, + "effects":{"padding":0.51,"blur":false,"shadow":0.35,"roundnessFrac":0.0255,"motionBlur":0.2}, + "background":{"kind":"color","color":"#1e1e2e"}, + "zoomRegions":[], + "cameraFullscreenRegions":[{"clipIndex":0,"startSec":8.006,"endSec":10.001,"continuesAfter":true}, + {"clipIndex":1,"startSec":11.388,"endSec":14.061,"continuesBefore":true}], + "cursor":{"show":true,"size":7.76,"smoothing":0,"motionBlur":0.35,"clickBounce":1,"clipToBounds":false,"theme":"default"}, + "output":{"width":1080,"height":1920,"fps":60} + }"##, + ) + .expect("scène coupée") + .for_clip_window(1, 11.388, 27.12); + assert!(f64::from(11.388f32) < 11.388, "le cas suppose un début inexact en f32"); + let mut input = golden_input(&scene, &cfg); + input.timeline_t_override = Some(11.388f64 as f32); + let g = plan_frame(&input); + + assert_eq!(g.w_dst, [0.0, 0.0, 1.0, 1.0], "la caméra reste plein cadre à travers le trim"); + assert_eq!(g.w_dst_prev, g.w_dst, "pas de traînée depuis le trou du trim"); + } + /// La caméra en orbite, en focus auto : celle qui bouge avec le pointeur. Sans piste, elle /// rend comme l'orbite manuelle posée au centre. const ORBIT: &str = r#""orbit","focusMode":"auto""#; diff --git a/crates/compositor/src/live.rs b/crates/compositor/src/live.rs index 6024645ed..2165d1d35 100644 --- a/crates/compositor/src/live.rs +++ b/crates/compositor/src/live.rs @@ -57,6 +57,12 @@ fn webcam_seek_time(screen_source_time_sec: f64, webcam_offset_sec: f64) -> f64 (screen_source_time_sec - webcam_offset_sec).max(0.0) } +/// The playback accumulator that puts the clock at `t` on a frame just positioned there. The +/// clock is the frame's pts plus `acc`, and a held frame's pts can be seconds before `t`. +fn anchor_acc(t: f64, pts: f64) -> f64 { + (t - pts).max(0.0) +} + /// Décodeurs déjà ouverts ET positionnés au bon playhead pour un clip à venir — le résultat /// d'un préchargement en tâche de fond (voir `open_and_seek_clip`/`maybe_start_prefetch` /// dans `render_thread`). Appliquer ceci à un `Player` (`apply_prefetched`) ne fait plus @@ -391,6 +397,8 @@ impl Player { self.idx = (source_time_sec * self.sdec.fps()).round().max(0.0) as u32; self.has_current_frame = true; self.use_current_on_next_step = true; + // Not composed yet: a `recompose` before the next frame must not reuse the old time. + self.composed_time_sec.set(self.sdec.cur_time_sec()); Ok(true) } @@ -424,6 +432,8 @@ impl Player { self.idx = incoming.idx; self.has_current_frame = true; self.use_current_on_next_step = true; + // Not composed yet: a `recompose` before the next frame must not reuse the old clip's time. + self.composed_time_sec.set(self.sdec.cur_time_sec()); outgoing } @@ -578,50 +588,13 @@ impl Player { return Ok(false); } - let target_webcam_t = (self.sdec.cur_time_sec() - self.webcam_offset_sec).max(0.0); let wf = if !self.webcam_decoder_is_real { // Pas de caméra : le remplaçant n'est pas décodé (cf. `open_webcam_or_stand_in`). sf } else if use_current { self.wdec.cur_frame() } else { - let cur = self.wdec.cur_frame(); - if cur.is_null() { - // Jamais décodée (nouvelle ouverture) : on saute directement au temps synchronisé. - self.wdec.seek_to(target_webcam_t)? - } else { - // Rattrape la webcam vers `target_webcam_t` par pts réel, jamais au-delà — - // même sémantique de hold que l'écran ci-dessus (et que `advance_decoder_to`) : - // adopter une frame webcam dont le pts dépasse `target_webcam_t` l'afficherait - // en avance sur son heure. Le garde-fou ne joue que contre un cas pathologique. - // - // BUG corrigé : à son EOF la webcam était reseekée à 0 alors que - // `target_webcam_t` continue de croître avec le temps écran. Au tick - // suivant, le rattrapage repartait donc de 0 et réavalait le fichier - // entier vers une cible toujours aussi lointaine — 1000 frames par tick - // (le plafond du garde-fou), en boucle, pour l'éternité. Un décodage - // permanent à fond, pour afficher une webcam qui n'a plus rien à montrer. - // Une fois l'EOF traité comme un hold, la décision webcam est EXACTEMENT - // celle de l'écran à l'export : `frame_step` couvre les quatre cas sans - // rien de spécifique, et ses tests couvrent donc aussi ce chemin. - let mut wf = cur; - let mut guard = 0u32; - loop { - match frame_step(self.wdec.peek_next_time_sec()?, 0.0, target_webcam_t) { - FrameStep::Commit => wf = self.wdec.commit_peek()?, - FrameStep::CommitAndStop => { - wf = self.wdec.commit_peek()?; - break; - } - FrameStep::Hold => break, - } - guard += 1; - if guard > 1000 { - break; - } - } - wf - } + self.advance_webcam_to(self.sdec.cur_time_sec())? }; if wf.is_null() { self.has_current_frame = false; @@ -629,12 +602,71 @@ impl Player { } self.has_current_frame = true; - self.sync_time(comp); + if use_current { + // A frame just positioned by a seek or a clip switch can be a held one, seconds + // before the playhead: compose it at the clock, which the caller anchored there. + self.sync_time_at(comp, target_source_time.max(self.sdec.cur_time_sec())); + } else { + self.sync_time(comp); + } comp.compose_frame(sf, wf, self.idx as f32, cfg)?; self.idx = self.idx.wrapping_add(1); Ok(true) } + /// Avance la vraie caméra jusqu'au temps écran `screen_source_time_sec` (décalage compris) + /// et rend sa frame courante. + unsafe fn advance_webcam_to(&mut self, screen_source_time_sec: f64) -> Result<*mut crate::ffi::AVFrame> { + let target_webcam_t = webcam_seek_time(screen_source_time_sec, self.webcam_offset_sec); + let cur = self.wdec.cur_frame(); + if cur.is_null() { + // Jamais décodée (nouvelle ouverture) : on saute directement au temps synchronisé. + return self.wdec.seek_to(target_webcam_t); + } + // Rattrape la webcam vers `target_webcam_t` par pts réel, jamais au-delà — + // même sémantique de hold que l'écran dans `step` (et que `advance_decoder_to`) : + // adopter une frame webcam dont le pts dépasse `target_webcam_t` l'afficherait + // en avance sur son heure. Le garde-fou ne joue que contre un cas pathologique. + // + // BUG corrigé : à son EOF la webcam était reseekée à 0 alors que + // `target_webcam_t` continue de croître avec le temps écran. Au tick + // suivant, le rattrapage repartait donc de 0 et réavalait le fichier + // entier vers une cible toujours aussi lointaine — 1000 frames par tick + // (le plafond du garde-fou), en boucle, pour l'éternité. Un décodage + // permanent à fond, pour afficher une webcam qui n'a plus rien à montrer. + // Une fois l'EOF traité comme un hold, la décision webcam est EXACTEMENT + // celle de l'écran à l'export : `frame_step` couvre les quatre cas sans + // rien de spécifique, et ses tests couvrent donc aussi ce chemin. + let mut wf = cur; + let mut guard = 0u32; + loop { + match frame_step(self.wdec.peek_next_time_sec()?, 0.0, target_webcam_t) { + FrameStep::Commit => wf = self.wdec.commit_peek()?, + FrameStep::CommitAndStop => { + wf = self.wdec.commit_peek()?; + break; + } + FrameStep::Hold => break, + } + guard += 1; + if guard > 1000 { + break; + } + } + Ok(wf) + } + + /// `recompose_at` for a held screen frame during playback: the camera is advanced to + /// `clock_sec` first. Otherwise it stood still through the hold and decoded the whole + /// stretch at once on the next screen frame, a 50–90 ms stall after every long hold. + pub unsafe fn hold_at(&mut self, comp: &Compositor, cfg: &Cfg, clock_sec: f64) -> Result { + // A frame placed by a seek or a clip switch is composed as is by the next `step`. + if self.has_current_frame && self.webcam_decoder_is_real && !self.use_current_on_next_step { + self.advance_webcam_to(clock_sec)?; + } + self.recompose_at(comp, cfg, clock_sec) + } + /// Positionne `comp` sur le temps source RÉEL (pts) de la frame écran courante, pour que le /// curseur ET les zoom/full-camera regions du clip actif restent exacts quelle /// que soit la cadence réelle de l'enregistrement — BUG corrigé : tout dérivait auparavant @@ -654,8 +686,20 @@ impl Player { } /// Recompose la frame courante (déjà décodée) — rafraîchit après un changement de param. + /// At the time it was last composed at, not at its pts: a held frame placed by a seek on + /// Linux, or held when playback paused, is shown at the playhead, and a settle pass or a + /// param change sent the cursor and the published position back to the pts. pub unsafe fn recompose(&self, comp: &Compositor, cfg: &Cfg) -> Result { - self.recompose_at(comp, cfg, self.sdec.cur_time_sec()) + self.recompose_at(comp, cfg, self.composed_time_sec.get()) + } + + /// Anchors the playhead at `t` on a frame positioned but not composed yet (a re-anchor or + /// a clip change from the app): a `recompose` composes it there, and the returned + /// accumulator puts the playback clock there (`anchor_acc`). + pub unsafe fn anchor_at(&self, t: f64) -> f64 { + let pts = self.sdec.cur_time_sec(); + self.composed_time_sec.set(t.max(pts)); + anchor_acc(t, pts) } /// Recomposes the current frame with the cursor, zooms and programme time at `clock_sec` @@ -699,7 +743,9 @@ impl Player { } self.has_current_frame = true; self.use_current_on_next_step = false; - self.sync_time(comp); + // At `target_sec`, not at the held frame's pts: the cursor and zooms show where the + // playhead is, and the published position matches the app's. + self.sync_time_at(comp, target_sec.max(self.sdec.cur_time_sec())); // "idx" ne sert plus qu'au fallback fixture (jamais lu si une scène est posée) — dérivé // du temps réel pour rester cohérent si jamais consulté. self.idx = (target_sec * self.sdec.fps()).round().max(0.0) as u32; @@ -1402,6 +1448,9 @@ unsafe fn maybe_start_prefetch( /// bascule est instantanée (juste un échange de champs, `Player::apply_prefetched`) ; sinon /// on retombe sur l'ouverture synchrone habituelle (`Player::set_active_clip`) — correct dans /// tous les cas, juste plus lent quand le préchargement n'a pas eu le temps de finir. +/// +/// Returns the source time the next clip starts at when the switch happened, for the caller to +/// anchor its clock there (`anchor_acc`): the clip can start inside a still stretch. #[allow(clippy::too_many_arguments)] unsafe fn advance_to_next_scene_clip( player: &mut Player, @@ -1415,9 +1464,9 @@ unsafe fn advance_to_next_scene_clip( raw_cursor: &mut Option, loaded_cursor_path: &mut String, last_smoothing: &mut f32, -) { +) -> Option { let Some(next_index) = next_clip_index(scene, *active_clip_index) else { - return; + return None; }; let next_clip = &scene.clips[next_index]; @@ -1489,8 +1538,12 @@ unsafe fn advance_to_next_scene_clip( None => comp.clear_cursor(), } *last_smoothing = -1.0; + Some(next_clip.source_start_sec) + } + Err(e) => { + eprintln!("[live] auto-advance clip: {e:#}"); + None } - Err(e) => eprintln!("[live] auto-advance clip: {e:#}"), } } @@ -1650,6 +1703,9 @@ unsafe fn render_thread( }; match switch_result { Ok(()) => { + // The app's re-anchor names the playhead: the clock restarts there, and a + // paused recompose (the `clip_changed` branch below) composes there. + acc = player.anchor_at(request.source_time_sec); // Condition sur `repositioned`, pas sur `same_media` : un repositionnement // qui a échoué est retombé sur l'ouverture complète, donc des décodeurs // ONT été fermés et le cache doit être vidé malgré des médias identiques. @@ -1845,7 +1901,9 @@ unsafe fn render_thread( // affiché jusqu'à ce qu'une autre action provoque un compose. (settle_until, last_settle) = open_settle_window(now); } - acc = 0.0; // resynchronise l'accumulateur de lecture libre après un seek + // Resynchronise l'accumulateur de lecture libre après un seek : l'horloge repart du + // temps demandé, pas du pts de la frame tenue (`anchor_acc`). + acc = anchor_acc(target, player.screen_time_sec()); } else if shared.playing.load(Ordering::Relaxed) { // BUG corrigé : la lecture libre décodait auparavant exactement 1 frame RÉELLE par // tranche de 1/60s de temps réel écoulé, quelle que soit la densité effective de @@ -1912,7 +1970,7 @@ unsafe fn render_thread( break; } if player.screen_time_sec() >= clip.source_end_sec { - advance_to_next_scene_clip( + if let Some(start) = advance_to_next_scene_clip( &mut player, &comp, scene, @@ -1924,7 +1982,9 @@ unsafe fn render_thread( &mut raw_cursor, &mut loaded_cursor_path, &mut last_smoothing, - ); + ) { + acc = anchor_acc(start, player.screen_time_sec()); + } } } } @@ -1938,9 +1998,10 @@ unsafe fn render_thread( // finit alors par boucler tout seul sur l'EOF réel (temps qui recule // brutalement). On détecte ce recul et on corrige immédiatement en enchaînant // sur le clip suivant, plutôt que de rester bloqué sur le 1er clip. + let mut crossed_anchor = None; if let (Some(scene), Some(t_before)) = (&full_scene, screen_time_before_step) { if player.screen_time_sec() < t_before { - advance_to_next_scene_clip( + crossed_anchor = advance_to_next_scene_clip( &mut player, &comp, scene, @@ -1952,9 +2013,13 @@ unsafe fn render_thread( &mut raw_cursor, &mut loaded_cursor_path, &mut last_smoothing, - ); + ) + .map(|start| anchor_acc(start, player.screen_time_sec())); } } + if let Some(anchored) = crossed_anchor { + acc = anchored; + } if !committed { // Rien n'est dû pour l'instant (hold) : `acc` reste tel quel — il continue // de s'accumuler aux ticks suivants jusqu'à ce qu'une vraie frame arrive. @@ -1962,7 +2027,8 @@ unsafe fn render_thread( } stepped = true; let after = player.screen_time_sec(); - acc = consume_acc(acc, before, after); + // A switch just anchored the clock on the new clip: nothing of the old one to consume. + acc = crossed_anchor.unwrap_or_else(|| consume_acc(acc, before, after)); n += 1; if n >= max_steps { // Rattrapage plafonné : le contenu dû est plus dense que ce qu'on peut décoder @@ -1980,7 +2046,7 @@ unsafe fn render_thread( let clock_sec = hold_clock_sec(player.screen_time_sec(), acc, clip_end_sec); if should_recompose_hold(now, last_play_compose, clock_sec, player.composed_time_sec()) { - if let Ok(true) = player.recompose_at(&comp, &cfg, clock_sec) { + if let Ok(true) = player.hold_at(&comp, &cfg, clock_sec) { stepped = true; } } @@ -2392,6 +2458,13 @@ mod tests { use super::*; + #[test] + fn the_clock_restarts_at_the_requested_time_on_a_held_frame() { + // Before: the clock restarted at the held frame's pts, 0.9 s behind the playhead. + assert_eq!(hold_clock_sec(0.1, anchor_acc(1.0, 0.1), Some(5.0)), 1.0); + assert_eq!(anchor_acc(1.0, 1.02), 0.0, "never behind the frame's own pts"); + } + fn multiclip_scene() -> Scene { Scene::from_json(r##"{ "clips": [ diff --git a/crates/compositor/src/shared_frames.rs b/crates/compositor/src/shared_frames.rs index a6b4f8415..cee094618 100644 --- a/crates/compositor/src/shared_frames.rs +++ b/crates/compositor/src/shared_frames.rs @@ -16,9 +16,13 @@ use crate::frame_geometry::FootageQuad; use crate::live::FramePosition; use std::time::{Duration, Instant}; -/// Cases de l'anneau : une prête, une en transit vers le renderer (tenue jusqu'à ce que -/// Chromium la relâche), une en écriture, et une de marge pour la latence de la libération. -pub const RING_SLOTS: u32 = 4; +/// Cases de l'anneau : une prête, une en écriture, et celles que Chromium tient encore. Une case +/// relâchée reste tenue 40 ms de plus côté Electron (`SHARED_SLOT_RELEASE_DELAY_MS`), le temps +/// que le GPU ait copié la frame dans le canvas : à 60 images/s, trois à quatre cases sont alors +/// tenues en même temps. Avec quatre, le rendu attendait une case libre et sautait des frames ; +/// huit laissent de la marge jusqu'à ~150 images/s, pour 8 × (largeur × hauteur × 4) octets de +/// VRAM (66 Mo en 1080p). +pub const RING_SLOTS: u32 = 8; /// Une case tenue plus longtemps que ça a perdu sa libération (renderer rechargé, import /// échoué en route) : Chromium relâche les autres dans la milliseconde. Même ordre que le @@ -294,8 +298,9 @@ mod tests { book.publish(frame(u64::from(slot) + 1, slot)); book.take(u64::from(slot), now); } - book.publish(frame(9, 3)); - assert_eq!(book.claim(RING_SLOTS), Some(3)); + let last = RING_SLOTS - 1; + book.publish(frame(u64::from(RING_SLOTS) + 1, last)); + assert_eq!(book.claim(RING_SLOTS), Some(last)); // Retirée : la livrer maintenant, pendant qu'on la réécrit, donnerait une image déchirée. assert_eq!(book.take(0, now), None); } diff --git a/crates/compositor/src/text_macos.rs b/crates/compositor/src/text_macos.rs index 0cfd49af5..219b4ef01 100644 --- a/crates/compositor/src/text_macos.rs +++ b/crates/compositor/src/text_macos.rs @@ -571,7 +571,7 @@ impl TextRasterizer { desc.set_width(w as u64); desc.set_height(h as u64); desc.set_usage(metal::MTLTextureUsage::ShaderRead); - desc.set_storage_mode(metal::MTLStorageMode::Shared); + desc.set_storage_mode(crate::compositor_macos::cpu_storage(&gpu.device)); let texture = gpu.device.new_texture(&desc); texture.replace_region( diff --git a/crates/compositor/tests/held_frame_clock.rs b/crates/compositor/tests/held_frame_clock.rs index 106656300..c5dad9c9e 100644 --- a/crates/compositor/tests/held_frame_clock.rs +++ b/crates/compositor/tests/held_frame_clock.rs @@ -3,6 +3,9 @@ //! ```sh //! OPENSCREEN_LIVE_SOURCE=/path/source.mp4 cargo test -p openscreen-compositor --test held_frame_clock -- --nocapture //! ``` +//! +//! The camera test also needs `OPENSCREEN_LIVE_WEBCAM`, a camera file with a frame between 1 s and +//! 1.25 s (any constant-frame-rate recording). The source itself does not count as a camera. #![cfg(any(windows, target_os = "macos"))] @@ -52,6 +55,7 @@ fn a_held_frame_follows_the_playback_clock() { assert!(player.present_frame(&comp, &cfg, 1.0).expect("seek"), "paused seek"); let held = player.screen_time_sec(); let at_pts = player.programme_time().expect("programme clock"); + assert_eq!(player.composed_time_sec(), 1.0f64.max(held), "a seek reports the playhead"); comp.set_programme_time(None); assert!(player.recompose_at(&comp, &cfg, held + 0.25).expect("recompose_at")); @@ -63,9 +67,59 @@ fn a_held_frame_follows_the_playback_clock() { "programme time {advanced} instead of {at_pts} + 0.25 s" ); + // Paused there, a param change or a settle pass recomposes: the frame keeps the time it + // was shown at. At the pts, the cursor and the published position went back. comp.set_programme_time(None); assert!(player.recompose(&comp, &cfg).expect("recompose")); - assert_eq!(comp.programme_time(), Some(at_pts), "recompose stays at the pts"); - assert_eq!(player.composed_time_sec(), held); + assert_eq!(comp.programme_time(), Some(advanced), "recompose keeps the time it was composed at"); + assert_eq!(player.composed_time_sec(), held + 0.25); + + // A re-anchor positions the decoders without composing: the next recompose is at the + // anchored playhead, never at the time the frame before it was composed at. + assert!(player.seek_active(0.5).expect("seek_active"), "re-anchor"); + player.anchor_at(0.5); + assert!(player.recompose(&comp, &cfg).expect("recompose")); + assert_eq!(player.composed_time_sec(), 0.5f64.max(player.screen_time_sec())); + } +} + +#[test] +fn a_held_frame_advances_the_camera_to_the_clock() { + let (Ok(source), Ok(camera)) = + (std::env::var("OPENSCREEN_LIVE_SOURCE"), std::env::var("OPENSCREEN_LIVE_WEBCAM")) + else { + println!("SKIP: set OPENSCREEN_LIVE_SOURCE and OPENSCREEN_LIVE_WEBCAM (file header)."); + return; + }; + let gpu = Gpu::create(false).expect("device"); + let mut cfg = config::all().pop().expect("at least one config"); + cfg.zoom = false; + cfg.layout_anim = false; + let comp = Compositor::new_sized(&gpu, 480, 270).expect("compositor"); + let scene = Scene::from_json(&scene_json(&source)).expect("valid scene"); + comp.set_live_params(live_params_from_scene(&scene)); + comp.set_scene(Some(scene.clone())); + comp.clear_cursor(); + + unsafe { + let mut player = Player::open(&source, &camera, &gpu).expect("open the source"); + assert!(player.webcam_decoder_is_real(), "the camera file must open"); + player.set_programme_clock(Some(&scene), 0); + assert!(player.present_frame(&comp, &cfg, 1.0).expect("seek"), "paused seek"); + let held = player.screen_time_sec(); + let camera_before = player.webcam_time_sec(); + + let clock = held + 0.25; + assert!(player.hold_at(&comp, &cfg, clock).expect("hold_at")); + assert_eq!(player.screen_time_sec(), held, "the screen frame must stay the held one"); + let camera_after = player.webcam_time_sec(); + assert!( + camera_after > camera_before, + "the camera stood still at {camera_before:.3} s through the hold" + ); + assert!( + camera_after <= clock + 1e-6, + "camera at {camera_after:.3} s, ahead of the clock {clock:.3} s" + ); } } diff --git a/electron/ai-edition/chat-service.toolloop.test.ts b/electron/ai-edition/chat-service.toolloop.test.ts index d140cf468..11719179d 100644 --- a/electron/ai-edition/chat-service.toolloop.test.ts +++ b/electron/ai-edition/chat-service.toolloop.test.ts @@ -200,6 +200,47 @@ describe("runChat tool loop", () => { expect(undo.prompt).toBe("cut the silence"); }); + it("reports the id it stored the user's message under, which a rewind accepts", async () => { + // The panel shows the message before the turn answers, under a local id the session does + // not know. Without this id it could only rewind after reloading the conversation. + const session = createSession("proj_rewind_sent"); + const result = await runChat( + "proj_rewind_sent", + session.id, + "cut the silence", + stubConfig(), + fixtureDocument(), + ); + const { selectSession } = await import("./chat-service"); + const stored = selectSession("proj_rewind_sent", session.id)?.messages[0]; + expect(result.userMessageId).toBe(stored?.id); + expect(result.userMessageCheckpointId).toBe(stored?.id); + + const rewound = rewindToMessage("proj_rewind_sent", session.id, result.userMessageId ?? ""); + expect(rewound.success).toBe(true); + }); + + it("reports the stored user message id when the model answers nothing", async () => { + invokeMock.mockImplementationOnce(async (args) => ({ + text: "", + document: args.document, + mutated: false, + })); + const session = createSession("proj_empty_reply"); + const result = await runChat( + "proj_empty_reply", + session.id, + "hi", + stubConfig(), + fixtureDocument(), + ); + const { selectSession } = await import("./chat-service"); + expect(result.success).toBe(false); + expect(result.userMessageId).toBe( + selectSession("proj_empty_reply", session.id)?.messages[0].id, + ); + }); + it("rewindToMessage refuses when the target has no checkpoint", async () => { const { createSession, selectSession, runChat } = await import("./chat-service"); const session = createSession("proj_rewind_no_cp"); diff --git a/electron/ai-edition/chat-service.ts b/electron/ai-edition/chat-service.ts index ab57b273b..e056f04b1 100644 --- a/electron/ai-edition/chat-service.ts +++ b/electron/ai-edition/chat-service.ts @@ -484,6 +484,9 @@ export async function runChat( return { success: false, error: result.reason ?? "Empty response from model.", + // The message is stored all the same, and can be rewound to retry it. + userMessageId: userMessage.id, + userMessageCheckpointId: userMessage.checkpointId ?? undefined, }; } @@ -508,6 +511,7 @@ export async function runChat( document: result.mutated && editsAllowed ? result.document : undefined, toolCalls: appliedToolCalls.length ? appliedToolCalls : undefined, userMessageCheckpointId: userMessage.checkpointId ?? undefined, + userMessageId: userMessage.id, }; } diff --git a/electron/native-bridge/services/compositorViewService.test.ts b/electron/native-bridge/services/compositorViewService.test.ts index 40987fe62..36e4048c3 100644 --- a/electron/native-bridge/services/compositorViewService.test.ts +++ b/electron/native-bridge/services/compositorViewService.test.ts @@ -10,6 +10,8 @@ import { CompositorViewService, ffmpegSharedBinCandidates, resolveSceneAssetPaths, + SHARED_SLOT_RELEASE_DELAY_MS, + sharedSlotHoldMs, } from "./compositorViewService"; describe("native GIF cancellation capability", () => { @@ -112,7 +114,13 @@ describe("CompositorViewService frames handed over as shared GPU textures", () = footageProjective: false, }; - function setup(options: { addon?: Partial; gpuCompositing?: boolean } = {}) { + function setup( + options: { + addon?: Partial; + gpuCompositing?: boolean; + displayHz?: number | null; + } = {}, + ) { const addon = { createView: vi.fn(() => 7), setSharedFrames: vi.fn(() => true), @@ -135,6 +143,7 @@ describe("CompositorViewService frames handed over as shared GPU textures", () = addon: addon as unknown as CompositorViewAddon, sharedTexture, gpuCompositing: () => options.gpuCompositing ?? true, + displayHz: () => options.displayHz ?? 60, }); const id = service.createView({ x: 0, y: 0, width: 4, height: 2 }); return { @@ -185,13 +194,74 @@ describe("CompositorViewService frames handed over as shared GPU textures", () = expect(addon.releaseSharedFrame).not.toHaveBeenCalled(); }); - it("hands the slot back once Chromium has let go of the texture everywhere", async () => { - const { addon, id, releaseEverywhere, service } = setup(); - await service.readFrame(id, 0, target); + it("hands the slot back a while after Chromium has let go of the texture everywhere", async () => { + vi.useFakeTimers(); + try { + const { addon, id, releaseEverywhere, service } = setup(); + await service.readFrame(id, 0, target); - releaseEverywhere(); + releaseEverywhere(); + // Not at once: the GPU copy into the canvas may still be queued behind the release. + vi.advanceTimersByTime(SHARED_SLOT_RELEASE_DELAY_MS - 1); + expect(addon.releaseSharedFrame).not.toHaveBeenCalled(); - expect(addon.releaseSharedFrame).toHaveBeenCalledWith(id, 1, 3); + vi.advanceTimersByTime(1); + expect(addon.releaseSharedFrame).toHaveBeenCalledWith(id, 1, 3); + } finally { + vi.useRealTimers(); + } + }); + + it("holds the slot two frames of a slow display, never less than the floor", async () => { + expect(sharedSlotHoldMs(60)).toBe(SHARED_SLOT_RELEASE_DELAY_MS); + expect(sharedSlotHoldMs(144)).toBe(SHARED_SLOT_RELEASE_DELAY_MS); + expect(sharedSlotHoldMs(30)).toBe(67); + expect(sharedSlotHoldMs(null)).toBe(SHARED_SLOT_RELEASE_DELAY_MS); + expect(sharedSlotHoldMs(0)).toBe(SHARED_SLOT_RELEASE_DELAY_MS); + + vi.useFakeTimers(); + try { + const { addon, id, releaseEverywhere, service } = setup({ displayHz: 30 }); + await service.readFrame(id, 0, target); + releaseEverywhere(); + vi.advanceTimersByTime(66); + expect(addon.releaseSharedFrame).not.toHaveBeenCalled(); + vi.advanceTimersByTime(1); + expect(addon.releaseSharedFrame).toHaveBeenCalledWith(id, 1, 3); + } finally { + vi.useRealTimers(); + } + }); + + it("holds every frame in flight on its own clock, so none of them loses its slot", async () => { + vi.useFakeTimers(); + try { + const frames = [ + { ...sharedFrame, gen: 3, slot: 1 }, + { ...sharedFrame, gen: 4, slot: 2 }, + ]; + const { addon, id, service, sharedTexture } = setup({ + addon: { readSharedFrame: vi.fn(() => frames.shift() ?? null) }, + }); + const released: Array<() => void> = []; + sharedTexture.importSharedTexture.mockImplementation((options) => { + released.push(options.allReferencesReleased as () => void); + return { release: vi.fn() } as unknown as Electron.SharedTextureImported; + }); + await service.readFrame(id, 0, target); + await service.readFrame(id, 3, target); + + released[0](); + vi.advanceTimersByTime(20); + released[1](); + vi.advanceTimersByTime(SHARED_SLOT_RELEASE_DELAY_MS - 20); + expect(addon.releaseSharedFrame).toHaveBeenCalledTimes(1); + expect(addon.releaseSharedFrame).toHaveBeenLastCalledWith(id, 1, 3); + vi.advanceTimersByTime(20); + expect(addon.releaseSharedFrame).toHaveBeenLastCalledWith(id, 2, 4); + } finally { + vi.useRealTimers(); + } }); it("goes back to read-back when a delivery fails", async () => { diff --git a/electron/native-bridge/services/compositorViewService.ts b/electron/native-bridge/services/compositorViewService.ts index 34f4004d8..8bb4e888a 100644 --- a/electron/native-bridge/services/compositorViewService.ts +++ b/electron/native-bridge/services/compositorViewService.ts @@ -2,7 +2,14 @@ import fs from "node:fs"; import { createRequire } from "node:module"; import path from "node:path"; import { fileURLToPath } from "node:url"; -import { app, sharedTexture, type WebFrameMain } from "electron"; +import { + app, + BrowserWindow, + screen, + sharedTexture, + type WebFrameMain, + webContents, +} from "electron"; import type { ShaderEffectCheck } from "../../../src/lib/ai-edition/shaderEffects"; import { type CursorKind, @@ -231,6 +238,37 @@ export interface CompositorViewServiceOptions { /** Whether Chromium composites on the GPU, where a shared texture is imported. Injectable * for tests; defaults to `app.getGPUFeatureStatus()`. */ gpuCompositing?: () => boolean; + /** Refresh rate of the display the frame's window is on, `null` when unknown. Injectable for + * tests; defaults to Electron's `screen`. */ + displayHz?: (target: WebFrameMain) => number | null; +} + +/** + * How long a shared frame's ring slot stays out of the render thread's reach after Chromium has + * let go of it. Chromium lets go when the renderer drops its references, which is as soon as it + * has issued the draw into the canvas, not once the GPU has run it: the copy runs when the frame + * carrying it is drawn, up to a vsync later. A slot written again before that copy put a later + * frame on the canvas, then the one in between, so the picture and the cursor stepped back a frame + * (tens of times in 18 s at 60 Hz, a few at 280 Hz). 40 ms took the steps back to none at both + * rates. On a slower display the copy can come later still: see `sharedSlotHoldMs`. + */ +export const SHARED_SLOT_RELEASE_DELAY_MS = 40; + +/** How long to hold a released slot for a window on a `hz` display: two of its frames, never less + * than `SHARED_SLOT_RELEASE_DELAY_MS` (67 ms at 30 Hz, 40 ms from 50 Hz up). */ +export function sharedSlotHoldMs(hz: number | null): number { + if (!hz || !Number.isFinite(hz) || hz <= 0) return SHARED_SLOT_RELEASE_DELAY_MS; + return Math.max(SHARED_SLOT_RELEASE_DELAY_MS, Math.ceil(2000 / hz)); +} + +function defaultDisplayHz(target: WebFrameMain): number | null { + try { + const contents = webContents.fromFrame(target); + const win = contents && BrowserWindow.fromWebContents(contents); + return win ? screen.getDisplayMatching(win.getBounds()).displayFrequency : null; + } catch { + return null; + } } /** The two calls a shared preview frame needs from Electron's `sharedTexture` module. */ @@ -735,6 +773,7 @@ export class CompositorViewService { sourceTimeSec: frame.sourceTimeSec, }; const api = this.sharedTextureApi(); + const holdMs = sharedSlotHoldMs((this.options.displayHz ?? defaultDisplayHz)(target)); let imported: Electron.SharedTextureImported | undefined; try { if (!api) { @@ -746,8 +785,11 @@ export class CompositorViewService { codedSize: { width: frame.width, height: frame.height }, handle: { ntHandle: frame.handle }, }, - // Chromium is done with the texture in every process: the slot may be written again. - allReferencesReleased: () => addon.releaseSharedFrame?.(id, frame.slot, frame.gen), + // Chromium is done with the texture in every process: after a grace period, the slot + // may be written again (SHARED_SLOT_RELEASE_DELAY_MS says why it waits). + allReferencesReleased: () => { + setTimeout(() => addon.releaseSharedFrame?.(id, frame.slot, frame.gen), holdMs); + }, }); await api.sendSharedTexture({ frame: target, importedSharedTexture: imported }, meta); return { ...meta, shared: true }; diff --git a/nix/package.nix b/nix/package.nix index f744ecf21..fd305bcf2 100644 --- a/nix/package.nix +++ b/nix/package.nix @@ -52,7 +52,7 @@ buildNpmPackage { ); }; - npmDepsHash = "sha256-qSDxX9ReTPNfFPdLIwr3vl3RiQjj2QR2hScAwperOWE="; + npmDepsHash = "sha256-ZwmB2QPWLweVfkDOkjEX6ydsOMq5XsojJE4l95DUvXo="; env.ELECTRON_SKIP_BINARY_DOWNLOAD = "1"; diff --git a/package-lock.json b/package-lock.json index a6bbff51f..a7f2b59d9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "openscreen", - "version": "2.0.0", + "version": "2.1.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "openscreen", - "version": "2.0.0", + "version": "2.1.0", "license": "MIT", "dependencies": { "@fix-webm-duration/fix": "^1.0.1", diff --git a/package.json b/package.json index 2f6da283a..7ef069e46 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "openscreen", "private": true, - "version": "2.0.0", + "version": "2.1.0", "description": "Record your screen and polish the demo", "homepage": "https://getopenscreen.com/", "license": "MIT", diff --git a/src/components/ai-edition/LeftPanel.tsx b/src/components/ai-edition/LeftPanel.tsx index 84c5227b7..144d0b4c7 100644 --- a/src/components/ai-edition/LeftPanel.tsx +++ b/src/components/ai-edition/LeftPanel.tsx @@ -26,6 +26,7 @@ import { } from "../../../electron/ai-edition/provider-registry"; import { ChatWelcome } from "./ChatWelcome"; import { canSendChat } from "./chatAvailability"; +import { adoptStoredUserMessage } from "./chatTranscript"; import { ChatHistoryModal } from "./Modals"; import styles from "./NewEditorShell.module.css"; import { useChatBudget } from "./useChatBudget"; @@ -634,6 +635,19 @@ export function ChatStripPanel() { const { result, applyDocument } = await runAgentTurn((documentSnapshot) => nativeBridgeClient.aiEdition.chatRun(projectId, sessionId, text, documentSnapshot), ); + setMessages((prev) => adoptStoredUserMessage(prev, optimisticUserId, result)); + // A rewind confirmation opened during the turn names the message too. Stored without a + // checkpoint, the message has nothing to rewind to and the confirmation closes. + const storedId = result.userMessageId; + if (storedId) { + setRewindFor((open) => + open?.messageId !== optimisticUserId + ? open + : result.userMessageCheckpointId + ? { ...open, messageId: storedId } + : null, + ); + } const assistant = result.assistantMessage; if (result.success && assistant) { if (result.document) { diff --git a/src/components/ai-edition/NewEditorShell.tsx b/src/components/ai-edition/NewEditorShell.tsx index 60a209b6f..13e5c1c47 100644 --- a/src/components/ai-edition/NewEditorShell.tsx +++ b/src/components/ai-edition/NewEditorShell.tsx @@ -1203,22 +1203,17 @@ export function NewEditorShell() { return; } - // An effect region goes with its definition (`RegionSnapshot`), and with the span of the - // whole pill: one cut by a clip boundary is several fragments. + // The pill on screen, not the fragment clicked: a region cut by a clip boundary is several. + const { wholePillOf } = await import("@/lib/ai-edition/timeline/timelineMap"); + + // An effect region goes with its definition (`RegionSnapshot`). if (sel.kind === "shaderEffect") { - const { coalesceRegionsForRuler } = await import("@/lib/ai-edition/timeline/timelineMap"); - const pill = coalesceRegionsForRuler(tl.shaderEffectRanges).find((p) => - p.ids.includes(sel.id), - ); - const definition = pill && tl.shaderEffects.find((e) => e.id === pill.member.effectId); - if (!pill || !definition) return; + const region = wholePillOf(tl.shaderEffectRanges, sel.id); + const definition = region && tl.shaderEffects.find((e) => e.id === region.effectId); + if (!region || !definition) return; copyRegion({ kind: "shaderEffect", - region: { - ...pill.member, - startMs: Math.round(pill.start * 1000), - endMs: Math.round(pill.end * 1000), - } as unknown as Record, + region: region as unknown as Record, definition, }); setCopiedClipId(null); @@ -1233,7 +1228,10 @@ export function NewEditorShell() { : sel.kind === "speed" ? tl.speedRegions : tl.cameraFullscreenRegions; - const region = (source as Array<{ id: string }>).find((r) => r.id === sel.id); + const region = wholePillOf( + source as Array<{ id: string; startMs: number; endMs: number }>, + sel.id, + ); if (!region) return; copyRegion({ kind: sel.kind, region: region as unknown as Record }); // One clipboard wins at a time: a copied pill retires the copied clip. diff --git a/src/components/ai-edition/chatTranscript.test.ts b/src/components/ai-edition/chatTranscript.test.ts new file mode 100644 index 000000000..6e8b19249 --- /dev/null +++ b/src/components/ai-edition/chatTranscript.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from "vitest"; +import { adoptStoredUserMessage } from "./chatTranscript"; + +const local = { + id: "local_1_u", + role: "user", + content: "cut the silence", + checkpointId: "local_1_u", +}; +const earlier = { id: "m0", role: "assistant", content: "hi", checkpointId: null }; + +describe("adoptStoredUserMessage", () => { + it("gives the message just sent the id the session stored it under, so it can be rewound", () => { + const next = adoptStoredUserMessage([earlier, local], "local_1_u", { + userMessageId: "stored-1", + userMessageCheckpointId: "stored-1", + }); + expect(next[1]).toMatchObject({ + id: "stored-1", + checkpointId: "stored-1", + content: "cut the silence", + }); + expect(next[0]).toBe(earlier); + }); + + it("hides the rewind of a message stored without a checkpoint", () => { + // A text-only turn (no document open) records no snapshot: there is nothing to rewind to. + const next = adoptStoredUserMessage([local], "local_1_u", { userMessageId: "stored-1" }); + expect(next[0]).toMatchObject({ id: "stored-1", checkpointId: null }); + }); + + it("leaves the transcript as it is when the turn never stored the message", () => { + const messages = [earlier, local]; + expect(adoptStoredUserMessage(messages, "local_1_u", {})).toBe(messages); + }); +}); diff --git a/src/components/ai-edition/chatTranscript.ts b/src/components/ai-edition/chatTranscript.ts new file mode 100644 index 000000000..2d70460c1 --- /dev/null +++ b/src/components/ai-edition/chatTranscript.ts @@ -0,0 +1,21 @@ +import type { AiEditionChatResult } from "@/native/contracts"; + +/** + * The panel shows a sent message before the main process has stored it, under a `local_…` id + * the session does not know. Once the turn answers with the stored id, the message takes it, + * with its real checkpoint: a rewind names the message by that id, and with the local one it + * failed with "Message not found in this session" until the conversation was reloaded. + */ +export function adoptStoredUserMessage( + messages: M[], + localId: string, + result: Pick, +): M[] { + const storedId = result.userMessageId; + if (!storedId) return messages; + return messages.map((message) => + message.id === localId + ? { ...message, id: storedId, checkpointId: result.userMessageCheckpointId ?? null } + : message, + ); +} diff --git a/src/lib/ai-edition/timeline/timelineMap.test.ts b/src/lib/ai-edition/timeline/timelineMap.test.ts index 11aa30c50..dbaebf9db 100644 --- a/src/lib/ai-edition/timeline/timelineMap.test.ts +++ b/src/lib/ai-edition/timeline/timelineMap.test.ts @@ -18,6 +18,7 @@ import { resolveNativePosition, resolvePillIds, resolveRegionIds, + wholePillOf, } from "./timelineMap"; function clip(overrides: Partial & Pick): AxcutClip { @@ -343,6 +344,40 @@ describe("projectRegionsToSource", () => { const out = projectRegionsToSource([partial], [c], [c], () => "x"); expect(out).toEqual([{ ...partial, startMs: 3000, endMs: 5000, clipIndex: 0 }]); }); + + // A piece at a cut is gated in f32 by the compositor (`continues_before` → `t < start`, + // `continues_after` → `t > end`), and a clip's first and last frames land on the clip's own + // bounds. A bound rounded to the ms past them left that frame out of the Full Camera piece: + // camera back in its PiP corner, motion blur trailing it there (issue #1300, cuts of the + // v2.1.0-rc.5 passes). The cuts that rounded the other way were clean: the controls. + const pieceAcross = (trimStartSec: number, trimEndSec: number) => { + const c = clip({ + id: "c1", + assetId: "a", + sourceStartSec: 0, + sourceEndSec: 60, + timelineEndSec: 60, + }); + const anchored = { ...region("r", 0, 60), clipId: "c1", sourceStartSec: 0, sourceEndSec: 60 }; + const segments = resolvePlaybackSegments([c], [trim("a", trimStartSec, trimEndSec)]); + return projectRegionsToSource([anchored], segments, [c], () => "r2"); + }; + + it.each([ + 10.292957, 18.361809, 46.34986635, 6.284394, + ])("keeps the first frame after a cut at %s in the Full Camera piece", (cut) => { + const [, after] = pieceAcross(cut - 2, cut); + expect(after.continuesBefore).toBe(true); + expect(Math.fround(after.startMs / 1000)).toBe(Math.fround(cut)); + }); + + it.each([ + 6.284394, 14.340314, 10.292957, + ])("keeps the last frame before a cut at %s in the Full Camera piece", (cut) => { + const [before] = pieceAcross(cut, cut + 2); + expect(before.continuesAfter).toBe(true); + expect(Math.fround(before.endMs / 1000)).toBe(Math.fround(cut)); + }); }); describe("resolveNativePosition", () => { @@ -787,6 +822,18 @@ describe("pills wired to the universal rules", () => { expect(pills[0].ids).toEqual(["a", "b"]); }); + it("wholePillOf gives the pill a fragment belongs to, over its whole span", () => { + // One zoom cut by the clip boundary at 25.557 s: two fragments, one pill. + const regions = [ + { id: "a", startMs: 20000, endMs: 25557, depth: 3 }, + { id: "b", startMs: 25557, endMs: 30000, depth: 3 }, + { id: "c", startMs: 30000, endMs: 31000, depth: 5 }, + ]; + expect(wholePillOf(regions, "b")).toEqual({ id: "a", startMs: 20000, endMs: 30000, depth: 3 }); + expect(wholePillOf(regions, "c")).toEqual(regions[2]); + expect(wholePillOf(regions, "gone")).toBeNull(); + }); + it("keeps adjacent regions with different properties as separate pills", () => { const pills = coalesceRegionsForRuler([ { id: "a", startMs: 2000, endMs: 5000, speed: 3 }, diff --git a/src/lib/ai-edition/timeline/timelineMap.ts b/src/lib/ai-edition/timeline/timelineMap.ts index 451987ce9..15c9907ba 100644 --- a/src/lib/ai-edition/timeline/timelineMap.ts +++ b/src/lib/ai-edition/timeline/timelineMap.ts @@ -304,6 +304,24 @@ export function coalesceRegionsForRuler( + regions: T[], + id: string, +): T | null { + const pill = coalesceRegionsForRuler(regions).find((p) => p.ids.includes(id)); + if (!pill) return null; + return { + ...pill.member, + startMs: Math.round(pill.start * 1000), + endMs: Math.round(pill.end * 1000), + }; +} + /** * The regions that render as the SAME pill as `id`. Recomputed from the merge rule * rather than stored: nothing records which regions belong together, because equal @@ -671,8 +689,11 @@ export function projectRegionsToSource< out.push({ ...region, id: emitted === 0 ? region.id : makeId(), - startMs: Math.round(srcStartSec * 1000), - endMs: Math.round(srcEndSec * 1000), + // A bound at a cut is not rounded to the ms: it must sit exactly on the segment's + // bound, where native puts the clip's first or last frame. Rounded past it, that + // frame fell out of the piece and Full Camera trailed to its PiP corner (#1300). + startMs: flags.continuesBefore ? srcStartSec * 1000 : Math.round(srcStartSec * 1000), + endMs: flags.continuesAfter ? srcEndSec * 1000 : Math.round(srcEndSec * 1000), clipIndex, // Omitted rather than sent as `false`: every payload without a trim under a // modifier stays byte-for-byte what it was. diff --git a/src/native/contracts.ts b/src/native/contracts.ts index c837b216e..521cc993e 100644 --- a/src/native/contracts.ts +++ b/src/native/contracts.ts @@ -407,6 +407,9 @@ export interface AiEditionChatResult { error?: string; /** Document checkpoint id recorded for the user message that triggered this turn (axcut parity). */ userMessageCheckpointId?: string; + /** Id the session stored the user's message under, once it did. The panel shows the message + * before that under a local id, which a rewind cannot name. */ + userMessageId?: string; } export interface AiEditionChatRewindResult {