Skip to content

Commit

Permalink
fix: remove unecessary dom re-calc in grid render (#1632)
Browse files Browse the repository at this point in the history
While looking at some perf snapshots I noticed calls to
updateCanvasScale on every grid render had a ~0.25-0.5ms perf cost
(including table ticks). Mostly due to touching the dom with the set
width/set height calls triggering a recalculate style call and the
getScale call. While insignificant, it is also unnecessary unless size
actually changes. Making it conditional improves perf on most
re-renders. I used parseFloat in case grid somehow ends up with
fractional pixel if set outside of our control (which shouldn't happen,
but you never know with plugins).

Before:

![image](https://github.com/deephaven/web-client-ui/assets/1576283/2d24c2b7-cc32-409d-aa7c-4020a263cbee)

After:

![image](https://github.com/deephaven/web-client-ui/assets/1576283/b2fe80ce-354a-47d1-a788-6abb071c462d)
updateCanvasScale is now gone from the perf profile entirely on a normal
grid render.

Tested re-sizeing panels, and window with table both shown and hidden
still resizes grid correctly.
  • Loading branch information
dsmmcken authored Nov 9, 2023
1 parent 0a6965a commit ce7cc3e
Showing 1 changed file with 10 additions and 1 deletion.
11 changes: 10 additions & 1 deletion packages/grid/src/Grid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -824,14 +824,23 @@ class Grid extends PureComponent<GridProps, GridState> {
if (!canvasContext) throw new Error('canvasContext not set');
if (!canvasWrapper.current) throw new Error('canvasWrapper not set');

const scale = Grid.getScale(canvasContext);
// the parent wrapper has 100% width/height, and is used for determining size
// we don't want to stretch the canvas to 100%, to avoid fractional pixels.
// A wrapper element must be used for sizing, and canvas size must be
// set manually to a floored value in css and a scaled value in width/height
const rect = canvasWrapper.current.getBoundingClientRect();
const width = Math.floor(rect.width);
const height = Math.floor(rect.height);

// avoid triggering a dom re-calc if size hasn't changed
if (
parseFloat(canvas.style.width) === width &&
parseFloat(canvas.style.height) === height
) {
return;
}

const scale = Grid.getScale(canvasContext);
canvas.style.width = `${width}px`;
canvas.style.height = `${height}px`;
canvas.width = width * scale;
Expand Down

0 comments on commit ce7cc3e

Please sign in to comment.