#1370 Enhance gridline performance

[langonginc]

Fix #1370

[Copilot]

Pull request overview

This PR addresses #1370 by improving gridline rendering performance in the SVG canvas, primarily by decoupling gridline updates from SvgWrapper re-renders during viewport dragging/zooming.

Changes:

• Switched gridline updates from SvgWrapper state updates to an imperative ref API (GridLinesRef.updateGrid) to avoid re-rendering SvgWrapper on every drag frame.
• Updated GridLines to use forwardRef + useImperativeHandle and accept x/y/zoom props instead of a gridLineOffset object.
• Adjusted gridline generation logic (including step sizing at higher zoom levels) and added guards against negative array sizes.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/components/svg-wrapper.tsx	Uses an imperative GridLinesRef to update gridlines during viewport transforms without updating local state.
src/components/grid-lines.tsx	Refactors GridLines to support imperative updates and new prop shape; tweaks grid density logic and array bounds.

Comments suppressed due to low confidence (1)

src/components/svg-wrapper.tsx:147

• updateViewportTransform is memoized with [gridLines] but doesn’t actually reference gridLines in the callback body. Dropping it from the dependency array would avoid unnecessary callback churn (and re-running the useLayoutEffect that depends on this callback) when the preference toggles.

    const updateViewportTransform = React.useCallback(
        (min: { x: number; y: number }, zoom: number) => {
            if (viewportRef.current) {
                const scale = 100 / zoom;
                const x = -min.x * scale;
                const y = -min.y * scale;

                viewportRef.current.setAttribute('transform', `translate(${x}, ${y}) scale(${scale})`);

                if (gridLinesRef.current) {
                    gridLinesRef.current.updateGrid(min.x, min.y, zoom);
                }
            }
        },
        [gridLines]
    );

[Copilot]

useEffect always calls setViewState on mount/prop changes and updateGrid always sets a new object, which forces extra re-renders even when values didn’t change (and this component is expensive to render). Consider guarding both updates with a functional setViewState(prev => prev.x===nextX && prev.y===nextY && prev.zoom===nextZoom ? prev : {…}) so React can bail out when unchanged.

Suggested change

	setViewState({ x: propsX, y: propsY, zoom: propsZoom });
	}, [propsX, propsY, propsZoom]);
	useImperativeHandle(ref, () => ({
	updateGrid: (newX, newY, newZoom) => {
	setViewState({ x: newX, y: newY, zoom: newZoom });
	setViewState(prev =>
	prev.x === propsX && prev.y === propsY && prev.zoom === propsZoom
	? prev
	: { x: propsX, y: propsY, zoom: propsZoom }
	);
	}, [propsX, propsY, propsZoom]);
	useImperativeHandle(ref, () => ({
	updateGrid: (newX, newY, newZoom) => {
	setViewState(prev =>
	prev.x === newX && prev.y === newY && prev.zoom === newZoom
	? prev
	: { x: newX, y: newY, zoom: newZoom }
	);

[Copilot]

The nested ternary used to compute step is hard to read/modify, and introduces additional zoom thresholds. Refactoring this into a small helper with if/else (and ideally named constants/comments for the thresholds) would reduce the chance of mistakes when tuning grid performance later.