perf(transport): parité de fluidité entre scrub barre et scrub timeline - #208
Merged
Conversation
…rogression Les deux chemins n'ont rien à voir, et la sonde les confondait — ou plutôt, elle ne voyait que le premier : `setUiProbeScrubbing` n'était appelé que depuis `V4Timeline.startScrub`. Un scrub via la barre de progression atterrissait donc dans les buckets `repos` ou `preview`, ce qui pollue silencieusement toute mesure où l'utilisateur touche la barre — y compris le `repos@N` qui a servi à juger l'accumulation au fil des bascules de clip. Les états deviennent `scrub-tl` et `scrub-bar` (et leurs variantes `+preview`). Les mélanger reviendrait à moyenner deux populations différentes, exactement le biais que cette sonde existe pour éviter — c'est déjà arrivé une fois avec le franchissement de clip, où l'écart avait été attribué à un changement de code qui n'y était pour rien. Ce commit ne change AUCUN comportement : il ne fait qu'étiqueter, pour que le correctif d'alignement qui suit soit jugeable sur un avant/après plutôt que sur une impression.
`onChange` d'un `<input type="range">` se déclenche à la cadence du POINTEUR — 125 à 1000 Hz selon la souris — et appelait `onSeek` à chacun. Or `onSeek` est `handleSeek` (NewEditorShell), qui écrit au store ET repose un `seekTarget` neuf dans l'état de la RACINE de l'éditeur : chaque appel re-rend tout l'arbre (timeline, waveforms, preview, inspecteur) et fait poser `<video>.currentTime`, un vrai seek média. La timeline fait exactement les mêmes appels, mais coalescés en rAF — ~60 par seconde au lieu de jusqu'à 1000. C'est cette seule différence de cadence qui séparait les deux chemins, et elle explique le constat d'usage : « ça rame plus avec la barre qu'avec la timeline ». Ce commit ne fait que rétablir la parité : même travail, cadence d'écran, plus un commit de la dernière position au relâchement (sans lui, le mouvement entre le dernier rAF et le `pointerup` serait perdu et la tête s'arrêterait un cran avant le doigt). `pointercancel` et `blur` ferment aussi le drag — un pointeur capturé puis interrompu ne produit pas de `pointerup`. Délibérément PAS « ne poser le seekTarget qu'au relâchement », bien que ce soit tentant : ce serait un comportement DIFFÉRENT de la timeline, où le `<video>` suit pendant le drag. La divergence entre deux chemins censés faire la même chose est précisément ce qui a produit les bugs de cette zone. Vérifié : tsc, 69/69 tests src/components/ai-edition. Trois tests ajoutés, dont la nature diffère et mérite d'être dite — un seul échoue sans ce correctif (la coalescence, vérifié en retirant le patch) ; les deux autres passent avant comme après et servent d'équivalence : ils garantissent que le commit final au relâchement et le saut clavier immédiat ne sont pas cassés au passage. NON vérifié : le gain ressenti. La sonde étiquette désormais `scrub-bar` séparément de `scrub-tl` (commit précédent), donc l'avant/après est mesurable — il reste à le mesurer.
…M pendant un drag Dernière différence de parité identifiable entre les deux chemins de scrub. Le remplissage (`scrubProgress`) et le curseur (`scrubThumb`) n'étaient positionnés que par React, depuis `progress` dérivé du store — donc en retard d'un commit à chaque mouvement. La tête de lecture de la timeline, elle, est écrite directement dans le DOM (`playheadElRef.style.left`) et colle au pointeur. Même patron appliqué ici, pour la même raison : la position que l'utilisateur VOIT ne doit pas attendre un rendu. React continue de les positionner hors drag, et au rendu suivant pendant le drag avec une valeur au pire vieille d'une frame puisque le rAF du commit précédent écrit au store à 60 Hz. Les deux écritures convergent donc au lieu de se contredire — c'est la condition qui manquait quand un throttle mal placé avait fait diverger position React et position DOM plus tôt dans cette série. Mesuré avec la sonde, pondéré par le nombre d'échantillons, avant ce commit : `scrub-tl` ≈ 6–8 % de frames > 25 ms contre `scrub-bar` ≈ 12,6 %. L'écart ressenti était donc réel et mesurable ; ce commit s'attaque à ce qu'il en reste après la coalescence. Un piège rencontré en écrivant ce correctif, corrigé avant commit : le bloc utilisait `inputMax` dans les dépendances d'un `useCallback` déclaré AVANT lui — la liste de deps étant évaluée au rendu, c'était une ReferenceError par TDZ à chaque montage. Le bloc est désormais placé après les déclarations qu'il consomme. Vérifié : tsc, 70/70 tests src/components/ai-edition. Un test ajouté, et il échoue bien sans ce correctif (vérifié en retirant le patch) : il constate que le visuel bouge AVANT tout vidage de rAF, donc sans passer par React. NON vérifié : le gain mesuré de ce commit précis. À faire avec `scrub-bar` sur la sonde.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contexte
Suite de la série « fluidité du scrub », après #206 et #207 mergées. Le point de départ : scruber via la barre de progression du playback semble moins fluide que scruber via la timeline, alors que les deux font conceptuellement la même chose (poser une position). Cette PR referme l'écart en rétablissant la parité entre les deux chemins.
Ce que fait chaque commit
b2dbd890— la sonde de fluidité distingue désormaisscrub-tl(timeline) descrub-bar(barre). Neutre en comportement ; c'est l'instrument qui rend l'écart mesurable au lieu de « ressenti ».6eff6b97— coalescence du drag de la barre en rAF. L'onChanged'un<input type="range">se déclenche à la cadence du pointeur (jusqu'à 1000 Hz) et appelaitonSeekà chaque fois — donc jusqu'à 1000 re-rendus de tout l'éditeur + autant de<video>.currentTimepar seconde. La timeline, elle, coalesce déjà en rAF (~60 Hz). On rétablit exactement cette parité, plus unonSeekfinal au relâchement (pointerup/pointercancel/blur) pour ne pas perdre le dernier mouvement.a29458d3— écriture DIRECTE du remplissage et du curseur dans le DOM pendant un drag, commeplayheadElRefdans la timeline. Le visuel ne passe plus par un commit React : latence nulle.Mesures (sonde intégrée)
scrub-tlscrub-bar(cette PR)scrub-bar(avant la série)Honnêteté sur les chiffres — la comparaison
scrub-tlvsscrub-barci-dessus est biaisée : les deux bras n'ont pas été entrelacés, et entre les deux la ligne de base au repos avait dérivé (reposde 0 % à 22 % de frames > 25 ms sur la session).scrub-bara donc été mesuré dans un régime plus dégradé quescrub-tl. Le fait propre et interne à la session : pendant tout lescrub-bar, le p50 est resté à 16,7 ms (60 fps) alors qu'au repos il avait dérivé à 25,0 ms — scruber est devenu plus sain que ne rien faire. Le gain du commit6eff6b97est net au ressenti et cohérent avec la mesure ; le gain marginal dea29458d3par-dessus n'est pas isolé proprement (pas d'A/B contrôlé).Hors périmètre
La dérive au repos (
repos0 % → 22 %, p50 se verrouillant à 25,0 ms, monotone avec le nombre de bascules de clip,2 <video>stables) est le vrai signal restant. Elle survit à cette PR comme aux précédentes et sent la fuite par bascule de clip côté transport/<video>, pas côté React. Elle fera l'objet d'une investigation séparée.Vérification
tsc --noEmitpropre.src/components/ai-edition, dont 4 nouveaux surTransportBar(un seek/frame, position finale au relâchement, saut immédiat hors drag, et le visuel qui bouge avant tout rAF — vérifié qu'il échoue sans le correctif).