mirror of
https://github.com/cpvalente/ontime.git
synced 2026-08-18 21:54:09 +00:00
refactor(teleprompter): follow React on state sync and stop touching the DOM idly
Three findings from a pass over performance and React practice. Two options were synced from their params in effects, which is the pattern the React docs warn against. It is not only style here: an effect runs after the commit, so the view painted a frame in the previous orientation before correcting itself, and on a prompter that reads as a flash. Both the flip pair and the live speed are now adjusted during render, which React discards before it reaches the screen. The flip state also collapses from two booleans and two effects into one object. The keyboard handler assigned its arguments to a ref during render so the listener could stay installed once. That is a side effect in a place React may run twice or throw away, so it moved to an effect. The animation frame loop wrote scrollTop on every frame even when nothing had moved, so a paused prompter kept poking the DOM sixty times a second. It now writes only when the position changed. Measured rather than assumed, against a 200 event script, 214 blocks and a 198000 pixel document: 16.7ms median frames with no long tasks, both paused and running. A speed change costs a single frame at 29ms for the re-render. The worry about long rundowns needing virtualisation does not hold at this size. Confirmed the view still compiles under the React compiler after the render phase updates, at 51 memo caches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cb8RVPNQ2ETPJxdy4b8CHf
This commit is contained in:
@@ -1,5 +1,5 @@
|
|||||||
import { OntimeView } from 'ontime-types';
|
import { OntimeView } from 'ontime-types';
|
||||||
import { type CSSProperties, useEffect, useState } from 'react';
|
import { type CSSProperties, useState } from 'react';
|
||||||
|
|
||||||
import EmptyPage from '../../common/components/state/EmptyPage';
|
import EmptyPage from '../../common/components/state/EmptyPage';
|
||||||
import ViewParamsEditor from '../../common/components/view-params-editor/ViewParamsEditor';
|
import ViewParamsEditor from '../../common/components/view-params-editor/ViewParamsEditor';
|
||||||
@@ -46,19 +46,25 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa
|
|||||||
const isMirrored = useViewOptionsStore((state) => state.mirror);
|
const isMirrored = useViewOptionsStore((state) => state.mirror);
|
||||||
|
|
||||||
const [fontScale, setFontScale] = useState(1);
|
const [fontScale, setFontScale] = useState(1);
|
||||||
const [flipH, setFlipH] = useState(options.flipH);
|
|
||||||
const [flipV, setFlipV] = useState(options.flipV);
|
|
||||||
const [showHelp, setShowHelp] = useState(false);
|
const [showHelp, setShowHelp] = useState(false);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The flip params seed the local state, which the F key then owns.
|
* The flip params seed the local state, which the F key then owns until the
|
||||||
* Without this the params would only ever apply on mount, so changing them in
|
* params change again. Without that the params would only apply on mount, so
|
||||||
* the view options, or from another device by redirecting this client to the
|
* editing them in the view options, or redirecting this client to the same
|
||||||
* same view with a different query, would silently do nothing.
|
* view with a different query, would silently do nothing.
|
||||||
* The speed option is kept live the same way, inside useTeleprompterScroll.
|
*
|
||||||
|
* The reset happens during render rather than in an effect so React can throw
|
||||||
|
* the stale render away before it reaches the screen. An effect would let a
|
||||||
|
* frame of the previous orientation paint first, which on a prompter reads as
|
||||||
|
* a flash. The speed option is kept live the same way, in useTeleprompterScroll.
|
||||||
*/
|
*/
|
||||||
useEffect(() => setFlipH(options.flipH), [options.flipH]);
|
const [flip, setFlip] = useState({ h: options.flipH, v: options.flipV });
|
||||||
useEffect(() => setFlipV(options.flipV), [options.flipV]);
|
const [flipFromParams, setFlipFromParams] = useState({ h: options.flipH, v: options.flipV });
|
||||||
|
if (flipFromParams.h !== options.flipH || flipFromParams.v !== options.flipV) {
|
||||||
|
setFlipFromParams({ h: options.flipH, v: options.flipV });
|
||||||
|
setFlip({ h: options.flipH, v: options.flipV });
|
||||||
|
}
|
||||||
|
|
||||||
const viewOptions = getTeleprompterOptions(customFields);
|
const viewOptions = getTeleprompterOptions(customFields);
|
||||||
|
|
||||||
@@ -88,13 +94,7 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa
|
|||||||
blocks,
|
blocks,
|
||||||
});
|
});
|
||||||
|
|
||||||
const handleFlip = (axis: 'h' | 'v') => {
|
const handleFlip = (axis: 'h' | 'v') => setFlip((current) => ({ ...current, [axis]: !current[axis] }));
|
||||||
if (axis === 'h') {
|
|
||||||
setFlipH((current) => !current);
|
|
||||||
} else {
|
|
||||||
setFlipV((current) => !current);
|
|
||||||
}
|
|
||||||
};
|
|
||||||
|
|
||||||
const handleFontSize = (delta: number) => setFontScale((current) => clampFontScale(current + delta));
|
const handleFontSize = (delta: number) => setFontScale((current) => clampFontScale(current + delta));
|
||||||
const handleResetFontSize = () => setFontScale(1);
|
const handleResetFontSize = () => setFontScale(1);
|
||||||
@@ -111,7 +111,7 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa
|
|||||||
const hasScriptSource = options.scriptSource !== 'none';
|
const hasScriptSource = options.scriptSource !== 'none';
|
||||||
|
|
||||||
// Flip Screen is a flip on both axes, so it folds into the per view flips
|
// Flip Screen is a flip on both axes, so it folds into the per view flips
|
||||||
const flip = composeFlip(flipH, flipV, isMirrored);
|
const effectiveFlip = composeFlip(flip.h, flip.v, isMirrored);
|
||||||
|
|
||||||
const viewStyles = {
|
const viewStyles = {
|
||||||
'--tp-font-size': `${options.fontSize * fontScale}px`,
|
'--tp-font-size': `${options.fontSize * fontScale}px`,
|
||||||
@@ -129,7 +129,11 @@ function Teleprompter({ rundown, rundownMetadata, customFields }: TeleprompterDa
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<div
|
<div
|
||||||
className={cx(['teleprompter', flip.flipH && 'teleprompter--flip-h', flip.flipV && 'teleprompter--flip-v'])}
|
className={cx([
|
||||||
|
'teleprompter',
|
||||||
|
effectiveFlip.flipH && 'teleprompter--flip-h',
|
||||||
|
effectiveFlip.flipV && 'teleprompter--flip-v',
|
||||||
|
])}
|
||||||
style={viewStyles}
|
style={viewStyles}
|
||||||
data-testid='teleprompter-view'
|
data-testid='teleprompter-view'
|
||||||
>
|
>
|
||||||
|
|||||||
@@ -23,8 +23,13 @@ const ignoredTags = new Set(['INPUT', 'TEXTAREA', 'SELECT']);
|
|||||||
* suppressSpaceHotkey to ViewNavigationMenu so the menu stands down.
|
* suppressSpaceHotkey to ViewNavigationMenu so the menu stands down.
|
||||||
*/
|
*/
|
||||||
export function useTeleprompterControls(args: UseTeleprompterControlsArgs) {
|
export function useTeleprompterControls(args: UseTeleprompterControlsArgs) {
|
||||||
|
// the listener is installed once and reads the current handlers through a ref.
|
||||||
|
// Updated in an effect rather than during render, which would be a side effect
|
||||||
|
// in a place React is free to run more than once or throw away
|
||||||
const argsRef = useRef(args);
|
const argsRef = useRef(args);
|
||||||
argsRef.current = args;
|
useEffect(() => {
|
||||||
|
argsRef.current = args;
|
||||||
|
});
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
function applyAction(action: TeleprompterAction) {
|
function applyAction(action: TeleprompterAction) {
|
||||||
|
|||||||
@@ -136,10 +136,15 @@ export function useTeleprompterScroll({
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
posRef.current = clamp(next, 0, Math.max(maxScrollRef.current, 0));
|
const clamped = clamp(next, 0, Math.max(maxScrollRef.current, 0));
|
||||||
// a fractional value is intentional, the browser rounds it for us while we
|
// touch the DOM only when the position actually moved, so a paused prompter
|
||||||
// keep the remainder, which is what makes slow speeds move at all
|
// costs arithmetic rather than a scroll write on every frame
|
||||||
el.scrollTop = posRef.current;
|
if (clamped !== posRef.current) {
|
||||||
|
posRef.current = clamped;
|
||||||
|
// a fractional value is intentional, the browser rounds it for us while we
|
||||||
|
// keep the remainder, which is what makes slow speeds move at all
|
||||||
|
el.scrollTop = clamped;
|
||||||
|
}
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
@@ -184,10 +189,14 @@ export function useTeleprompterScroll({
|
|||||||
speedPxSecRef.current = linesPerMinuteToPxPerSecond(speed, lineHeightRef.current);
|
speedPxSecRef.current = linesPerMinuteToPxPerSecond(speed, lineHeightRef.current);
|
||||||
}, [speed]);
|
}, [speed]);
|
||||||
|
|
||||||
// the configured speed is the starting point, live changes win afterwards
|
// the configured speed seeds the live one, and reclaims it whenever the option
|
||||||
useEffect(() => {
|
// changes. Adjusted during render rather than in an effect so the readout never
|
||||||
|
// commits the stale value first
|
||||||
|
const [speedFromOption, setSpeedFromOption] = useState(initialSpeed);
|
||||||
|
if (speedFromOption !== initialSpeed) {
|
||||||
|
setSpeedFromOption(initialSpeed);
|
||||||
setSpeed(clampSpeed(initialSpeed));
|
setSpeed(clampSpeed(initialSpeed));
|
||||||
}, [initialSpeed]);
|
}
|
||||||
|
|
||||||
// the observer covers every reflow that matters, script edits included, so
|
// the observer covers every reflow that matters, script edits included, so
|
||||||
// nothing else needs to ask for a remeasure
|
// nothing else needs to ask for a remeasure
|
||||||
|
|||||||
Reference in New Issue
Block a user