Skip to content

Commit e54c0ac

Browse files
authored
fix: DH-21658: Fix grid rendering hidden columns (#2626)
Two performance optimizations: - grid renderer should not iterate over hidden columns - metrics should not recalculate the width of a hidden column
1 parent ae96ce4 commit e54c0ac

File tree

3 files changed

+51
-19
lines changed

3 files changed

+51
-19
lines changed

packages/grid/src/GridMetricCalculator.ts

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ export class GridMetricCalculator {
262262
const treePaddingY = 0; // We don't support trees on columns (at least not yet)
263263

264264
const visibleRowHeights = this.getVisibleRowHeights(state);
265-
const visibleColumnWidths = this.getVisibleColumnWidths(
265+
const [visibleColumnWidths, columnsForRender] = this.getVisibleColumnWidths(
266266
state,
267267
firstColumn,
268268
treePaddingX
@@ -612,10 +612,13 @@ export class GridMetricCalculator {
612612
scrollableViewportWidth,
613613
scrollableViewportHeight,
614614

615-
// Array of visible rows/columns, by grid index
615+
// Array of visible (i.e., in the viewport) rows/columns, by VisibleIndex
616616
visibleRows,
617617
visibleColumns,
618618

619+
// Array of visible, non-hidden columns, by VisibleIndex
620+
columnsForRender,
621+
619622
// Map of the height/width of columns in the viewport (excluding floating columns)
620623
visibleRowHeights,
621624
visibleColumnWidths,
@@ -1060,18 +1063,19 @@ export class GridMetricCalculator {
10601063
/**
10611064
* Retrieve a map of the width of all the visible columns (non-floating)
10621065
* @param state The grid metric state
1063-
* @returns The widths of all the visible columns
1066+
* @returns The widths of all the visible columns and an array of visible, non-hidden columns
10641067
*/
10651068
getVisibleColumnWidths(
10661069
state: GridMetricState,
10671070
firstColumn: VisibleIndex = this.getFirstColumn(state),
10681071
treePaddingX: number = this.calculateTreePaddingX(state)
1069-
): SizeMap {
1072+
): [SizeMap, VisibleIndex[]] {
10701073
const { left, leftOffset, width, model } = state;
10711074

10721075
let x = 0;
10731076
let column = left;
10741077
const columnWidths = new Map();
1078+
const columnsForRender: VisibleIndex[] = [];
10751079
const { columnCount, floatingRightColumnCount } = model;
10761080
while (
10771081
x < width + leftOffset &&
@@ -1084,11 +1088,14 @@ export class GridMetricCalculator {
10841088
treePaddingX
10851089
);
10861090
columnWidths.set(column, columnWidth);
1091+
if (columnWidth > 0) {
1092+
columnsForRender.push(column);
1093+
}
10871094
x += columnWidth;
10881095
column += 1;
10891096
}
10901097

1091-
return columnWidths;
1098+
return [columnWidths, columnsForRender];
10921099
}
10931100

10941101
/**
@@ -1706,6 +1713,13 @@ export class GridMetricCalculator {
17061713
return columnWidth;
17071714
}
17081715

1716+
const cachedValue = this.calculatedColumnWidths.get(modelColumn);
1717+
1718+
// Performance optimization: if the column is hidden and has a cached value, avoid recalculating until the column is shown again
1719+
if (cachedValue != null && this.isColumnHidden(modelColumn)) {
1720+
return cachedValue;
1721+
}
1722+
17091723
const headerWidth = this.calculateColumnHeaderWidth(
17101724
modelColumn,
17111725
state,
@@ -1716,7 +1730,6 @@ export class GridMetricCalculator {
17161730
state,
17171731
maxColumnWidth
17181732
);
1719-
const cachedValue = this.calculatedColumnWidths.get(modelColumn);
17201733
let columnWidth = Math.ceil(Math.max(headerWidth, dataWidth));
17211734
columnWidth = Math.max(minColumnWidth, columnWidth);
17221735
columnWidth = Math.min(maxColumnWidth, columnWidth);
@@ -1737,6 +1750,21 @@ export class GridMetricCalculator {
17371750
return columnWidth;
17381751
}
17391752

1753+
/**
1754+
* Checks if a column is hidden either by a user setting the width to 0 or by initial width being 0
1755+
* @param modelIndex the model index of the column to check
1756+
* @returns true if the column is hidden, false otherwise
1757+
*/
1758+
isColumnHidden(modelIndex: ModelIndex): boolean {
1759+
const userSetWidth = this.userColumnWidths.get(modelIndex);
1760+
// The column is hidden if the user set the width to 0 or if the initial width is 0 and the user hasn't set a width
1761+
return (
1762+
userSetWidth === 0 ||
1763+
(userSetWidth === undefined &&
1764+
this.initialColumnWidths.get(modelIndex) === 0)
1765+
);
1766+
}
1767+
17401768
/**
17411769
* Calculate the width of the specified column's header
17421770
* @param modelColumn ModelIndex of the column to get the header width for

packages/grid/src/GridMetrics.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,14 @@ export type GridMetrics = {
129129
scrollableViewportWidth: number;
130130
scrollableViewportHeight: number;
131131

132-
// Array of visible rows/columns, by grid index
132+
// Array of visible (i.e., in the viewport) rows/columns, by VisibleIndex
133133
visibleRows: readonly VisibleIndex[];
134134
visibleColumns: readonly VisibleIndex[];
135135

136+
// Renderer performance optimization - array of visible, non-hidden columns, by VisibleIndex
137+
// Use visibleColumns if hidden columns should be included (e.g. rendering column headers)
138+
columnsForRender: readonly VisibleIndex[];
139+
136140
// Map of the height/width of visible rows/columns
137141
visibleRowHeights: SizeMap;
138142
visibleColumnWidths: SizeMap;

packages/grid/src/GridRenderer.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ export class GridRenderer {
259259
floatingBottomRowCount,
260260
floatingRows,
261261
rowCount,
262-
visibleColumns,
262+
columnsForRender,
263263
allRowYs,
264264
allRowHeights,
265265
} = metrics;
@@ -284,7 +284,7 @@ export class GridRenderer {
284284
this.drawGridLinesForItems(
285285
context,
286286
state,
287-
visibleColumns,
287+
columnsForRender,
288288
floatingRows,
289289
theme.floatingGridColumnColor,
290290
theme.floatingGridRowColor
@@ -293,7 +293,7 @@ export class GridRenderer {
293293
this.drawCellBackgroundsForItems(
294294
context,
295295
state,
296-
visibleColumns,
296+
columnsForRender,
297297
floatingRows
298298
);
299299

@@ -323,8 +323,8 @@ export class GridRenderer {
323323
}
324324

325325
// Draw the cell content...
326-
for (let c = 0; c < visibleColumns.length; c += 1) {
327-
const column = visibleColumns[c];
326+
for (let c = 0; c < columnsForRender.length; c += 1) {
327+
const column = columnsForRender[c];
328328
for (let r = 0; r < floatingRows.length; r += 1) {
329329
const row = floatingRows[r];
330330
this.drawCellContent(context, state, column, row);
@@ -850,12 +850,12 @@ export class GridRenderer {
850850
state: GridRenderState
851851
): void {
852852
const { metrics, theme } = state;
853-
const { visibleColumns, visibleRows } = metrics;
853+
const { columnsForRender, visibleRows } = metrics;
854854

855855
this.drawGridLinesForItems(
856856
context,
857857
state,
858-
visibleColumns,
858+
columnsForRender,
859859
visibleRows,
860860
theme.gridColumnColor,
861861
theme.gridRowColor
@@ -927,11 +927,11 @@ export class GridRenderer {
927927
state: GridRenderState
928928
): void {
929929
const { metrics } = state;
930-
const { visibleColumns, visibleRows } = metrics;
930+
const { columnsForRender, visibleRows } = metrics;
931931
this.drawCellBackgroundsForItems(
932932
context,
933933
state,
934-
visibleColumns,
934+
columnsForRender,
935935
visibleRows
936936
);
937937
}
@@ -1004,10 +1004,10 @@ export class GridRenderer {
10041004
state: GridRenderState
10051005
): void {
10061006
const { metrics } = state;
1007-
const { visibleColumns } = metrics;
1007+
const { columnsForRender } = metrics;
10081008

1009-
for (let i = 0; i < visibleColumns.length; i += 1) {
1010-
const column = visibleColumns[i];
1009+
for (let i = 0; i < columnsForRender.length; i += 1) {
1010+
const column = columnsForRender[i];
10111011
this.drawColumnCellContents(context, state, column);
10121012
}
10131013
}

0 commit comments

Comments
 (0)