Skip to content

Commit dfa8b79

Browse files
DR-UniverDR-Univer
committed
perf(sheets): skip text in subpixel dimensions
1 parent 9a52fcb commit dfa8b79

6 files changed

Lines changed: 148 additions & 0 deletions

File tree

‎packages/engine-render/src/components/sheets/extensions/__tests__/column-header-layout.spec.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,42 @@ describe('column header layout extension', () => {
119119
expect(ctx.fillText).toHaveBeenCalled();
120120
});
121121

122+
// TODO(@ai-review): Confirm four screen pixels remains the intended Excel-compatible boundary for column labels across supported zoom levels.
123+
it('omits column labels that cannot occupy four screen pixels', () => {
124+
const layout = new ColumnHeaderLayout();
125+
const ctx = createCtx();
126+
const compressedColumnCount = 228;
127+
const skeleton = {
128+
rowColumnSegment: { startRow: 0, endRow: 0, startColumn: 0, endColumn: compressedColumnCount },
129+
columnHeaderHeight: 20,
130+
rowHeightAccumulation: [20],
131+
columnWidthAccumulation: Array.from(
132+
{ length: compressedColumnCount + 1 },
133+
(_, column) => column < compressedColumnCount ? (column + 1) * 2 : compressedColumnCount * 2 + 20
134+
),
135+
columnTotalWidth: compressedColumnCount * 2 + 20,
136+
rowTotalHeight: 20,
137+
worksheet: {
138+
getSheetId: vi.fn(() => 'sheet-main'),
139+
},
140+
} as any;
141+
142+
layout.draw(ctx, { scaleX: 1, scaleY: 1 } as any, skeleton);
143+
144+
expect(ctx.fillText).toHaveBeenCalledOnce();
145+
expect(ctx.fillText).toHaveBeenCalledWith('HU', expect.any(Number), expect.any(Number));
146+
expect(ctx.stroke).toHaveBeenCalledTimes(compressedColumnCount + 2);
147+
148+
ctx.fillText.mockClear();
149+
layout.draw(ctx, { scaleX: 2, scaleY: 1 } as any, skeleton);
150+
expect(ctx.fillText).toHaveBeenCalledTimes(compressedColumnCount + 1);
151+
152+
ctx.fillText.mockClear();
153+
ctx.__mode = 'printing';
154+
layout.draw(ctx, { scaleX: 1, scaleY: 1 } as any, skeleton);
155+
expect(ctx.fillText).toHaveBeenCalledTimes(compressedColumnCount + 1);
156+
});
157+
122158
it('uses gapConfig default colors when drawing column gaps', () => {
123159
const layout = new ColumnHeaderLayout();
124160
const ctx = createCtx();

‎packages/engine-render/src/components/sheets/extensions/__tests__/font.spec.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,55 @@ describe('font extension', () => {
849849
expect(renderCellSpy).toHaveBeenCalledOnce();
850850
});
851851

852+
// TODO(@ai-review): Verify that four screen pixels is the right text legibility boundary across supported zoom levels.
853+
it('skips font work for rows that are too short to display text on screen', () => {
854+
const font = new Font() as any;
855+
const ctx = createCtx();
856+
const spreadsheetSkeleton = createSpreadsheetSkeleton();
857+
const fontMatrix = new ObjectMatrix<any>();
858+
const compressedRowCount = 228;
859+
for (let row = 0; row <= compressedRowCount; row++) {
860+
fontMatrix.setValue(row, 0, createFontCache());
861+
}
862+
spreadsheetSkeleton.stylesCache = { fontMatrix };
863+
spreadsheetSkeleton.rowHeightAccumulation = Array.from(
864+
{ length: compressedRowCount + 1 },
865+
(_, row) => row < compressedRowCount ? (row + 1) * 2 : compressedRowCount * 2 + 20
866+
);
867+
spreadsheetSkeleton.getRowCount = vi.fn(() => compressedRowCount + 1);
868+
spreadsheetSkeleton.columnTotalWidth = 120;
869+
spreadsheetSkeleton.rowTotalHeight = compressedRowCount * 2 + 20;
870+
const renderCellSpy = vi.spyOn(font, '_renderFontEachCell').mockReturnValue(true);
871+
872+
font.draw(ctx, { scaleX: 1, scaleY: 1 } as any, spreadsheetSkeleton, [], {
873+
viewRanges: [{ startRow: 0, endRow: compressedRowCount, startColumn: 0, endColumn: 0 }],
874+
checkOutOfViewBound: true,
875+
viewportKey: 'viewMain',
876+
} as any);
877+
878+
expect(renderCellSpy).toHaveBeenCalledOnce();
879+
expect(renderCellSpy).toHaveBeenCalledWith(expect.anything(), compressedRowCount, 0, fontMatrix, expect.anything());
880+
881+
renderCellSpy.mockClear();
882+
font.draw(ctx, { scaleX: 1, scaleY: 2 } as any, spreadsheetSkeleton, [], {
883+
viewRanges: [{ startRow: 0, endRow: compressedRowCount, startColumn: 0, endColumn: 0 }],
884+
checkOutOfViewBound: true,
885+
viewportKey: 'viewMain',
886+
} as any);
887+
888+
expect(renderCellSpy).toHaveBeenCalledTimes(compressedRowCount + 1);
889+
890+
renderCellSpy.mockClear();
891+
ctx.__mode = 'printing';
892+
font.draw(ctx, { scaleX: 1, scaleY: 1 } as any, spreadsheetSkeleton, [], {
893+
viewRanges: [{ startRow: 0, endRow: compressedRowCount, startColumn: 0, endColumn: 0 }],
894+
checkOutOfViewBound: true,
895+
viewportKey: 'viewMain',
896+
} as any);
897+
898+
expect(renderCellSpy).toHaveBeenCalledTimes(compressedRowCount + 1);
899+
});
900+
852901
it('skips merge lookup work when the sheet has no merged cells', () => {
853902
const font = new Font() as any;
854903
const ctx = createCtx();

‎packages/engine-render/src/components/sheets/extensions/__tests__/row-header-layout.spec.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,44 @@ describe('row header layout extension', () => {
119119
expect(ctx.fillText).toHaveBeenCalled();
120120
});
121121

122+
// TODO(@ai-review): Confirm dense row labels stay omitted without suppressing their borders or backgrounds.
123+
it('omits row labels that cannot occupy four screen pixels', () => {
124+
const layout = new RowHeaderLayout();
125+
const ctx = createCtx();
126+
const compressedRowCount = 228;
127+
const skeleton = {
128+
rowColumnSegment: { startRow: 0, endRow: compressedRowCount, startColumn: 0, endColumn: 0 },
129+
rowHeaderWidth: 30,
130+
rowHeightAccumulation: Array.from(
131+
{ length: compressedRowCount + 1 },
132+
(_, row) => row < compressedRowCount ? (row + 1) * 2 : compressedRowCount * 2 + 20
133+
),
134+
columnWidthAccumulation: [30],
135+
columnTotalWidth: 30,
136+
rowTotalHeight: compressedRowCount * 2 + 20,
137+
worksheet: {
138+
getSheetId: vi.fn(() => 'sheet-main'),
139+
},
140+
} as any;
141+
142+
layout.draw(ctx, { scaleX: 1, scaleY: 1 } as any, skeleton);
143+
144+
expect(ctx.fillText).toHaveBeenCalledOnce();
145+
expect(ctx.fillText).toHaveBeenCalledWith(`${compressedRowCount + 1}`, expect.any(Number), expect.any(Number));
146+
expect(ctx.stroke).toHaveBeenCalledTimes(compressedRowCount + 2);
147+
148+
ctx.fillText.mockClear();
149+
layout.draw(ctx, { scaleX: 1, scaleY: 2 } as any, skeleton);
150+
151+
expect(ctx.fillText).toHaveBeenCalledTimes(compressedRowCount + 1);
152+
153+
ctx.fillText.mockClear();
154+
ctx.__mode = 'printing';
155+
layout.draw(ctx, { scaleX: 1, scaleY: 1 } as any, skeleton);
156+
157+
expect(ctx.fillText).toHaveBeenCalledTimes(compressedRowCount + 1);
158+
});
159+
122160
it('uses gapConfig default colors when drawing row gaps', () => {
123161
const layout = new RowHeaderLayout();
124162
const ctx = createCtx();

‎packages/engine-render/src/components/sheets/extensions/column-header-layout.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { SheetColumnHeaderExtensionRegistry } from '../../extension';
2525
import { SheetExtension } from './sheet-extension';
2626

2727
const UNIQUE_KEY = 'DefaultColumnHeaderLayoutExtension';
28+
const MIN_TEXT_RENDER_WIDTH_IN_SCREEN_PX = 4;
2829

2930
export interface IColumnsHeaderCfgParam {
3031
headerStyle?: Partial<IHeaderStyleCfg>;
@@ -220,6 +221,13 @@ export class ColumnHeaderLayout extends SheetExtension {
220221
ctx.moveToByPrecision(cellBound.right, 0);
221222
ctx.lineToByPrecision(cellBound.right, cellBound.height);
222223
ctx.stroke();
224+
225+
// TODO(@ai-review): Verify sub-four-pixel column labels should remain omitted while borders and gap styling stay visible.
226+
if (ctx.__mode !== 'printing' && cellBound.width * Math.abs(parentScale.scaleX ?? 1) < MIN_TEXT_RENDER_WIDTH_IN_SCREEN_PX) {
227+
preColumnPosition = columnEndPosition;
228+
continue;
229+
}
230+
223231
// column header text
224232
const textX = (() => {
225233
switch (curColumnCfg.textAlign) {

‎packages/engine-render/src/components/sheets/extensions/font.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ import { calculateCellImageRect } from '../util';
5252
import { SheetExtension } from './sheet-extension';
5353

5454
const UNIQUE_KEY = 'DefaultFontExtension';
55+
const MIN_TEXT_RENDER_HEIGHT_IN_SCREEN_PX = 4;
5556

5657
const IMAGE_FALLBACK_SRC = 'data:image/svg+xml;base64,PHN2ZyBjbGFzcz0iaWNvbiIgdmlld0JveD0iMCAwIDEwMjQgMTAyNCIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIiB3aWR0aD0iMjAwIiBoZWlnaHQ9IjIwMCI+PHBhdGggZD0iTTMwNC4xMjggNDU2LjE5MmM0OC42NCAwIDg4LjA2NC0zOS40MjQgODguMDY0LTg4LjA2NHMtMzkuNDI0LTg4LjA2NC04OC4wNjQtODguMDY0LTg4LjA2NCAzOS40MjQtODguMDY0IDg4LjA2NCAzOS40MjQgODguMDY0IDg4LjA2NCA4OC4wNjR6bTAtMTE2LjIyNGMxNS4zNiAwIDI4LjE2IDEyLjI4OCAyOC4xNiAyOC4xNnMtMTIuMjg4IDI4LjE2LTI4LjE2IDI4LjE2LTI4LjE2LTEyLjI4OC0yOC4xNi0yOC4xNiAxMi4yODgtMjguMTYgMjguMTYtMjguMTZ6IiBmaWxsPSIjZTZlNmU2Ii8+PHBhdGggZD0iTTg4Ny4yOTYgMTU5Ljc0NEgxMzYuNzA0Qzk2Ljc2OCAxNTkuNzQ0IDY0IDE5MiA2NCAyMzIuNDQ4djU1OS4xMDRjMCAzOS45MzYgMzIuMjU2IDcyLjcwNCA3Mi43MDQgNzIuNzA0aDE5OC4xNDRMNTAwLjIyNCA2ODguNjRsLTM2LjM1Mi0yMjIuNzIgMTYyLjMwNC0xMzAuNTYtNjEuNDQgMTQzLjg3MiA5Mi42NzIgMjE0LjAxNi0xMDUuNDcyIDE3MS4wMDhoMzM1LjM2QzkyNy4yMzIgODY0LjI1NiA5NjAgODMyIDk2MCA3OTEuNTUyVjIzMi40NDhjMC0zOS45MzYtMzIuMjU2LTcyLjcwNC03Mi43MDQtNzIuNzA0em0tMTM4Ljc1MiA3MS42OHYuNTEySDg1Ny42YzE2LjM4NCAwIDMwLjIwOCAxMy4zMTIgMzAuMjA4IDMwLjIwOHYzOTkuODcyTDY3My4yOCA0MDguMDY0bDc1LjI2NC0xNzYuNjR6TTMwNC42NCA3OTIuMDY0SDE2NS44ODhjLTE2LjM4NCAwLTMwLjIwOC0xMy4zMTItMzAuMjA4LTMwLjIwOHYtOS43MjhsMTM4Ljc1Mi0xNjQuMzUyIDEwNC45NiAxMjQuNDE2LTc0Ljc1MiA3OS44NzJ6bTgxLjkyLTM1NS44NGwzNy4zNzYgMjI4Ljg2NC0uNTEyLjUxMi0xNDIuODQ4LTE2OS45ODRjLTMuMDcyLTMuNTg0LTkuMjE2LTMuNTg0LTEyLjI4OCAwTDEzNS42OCA2NTIuOFYyNjIuMTQ0YzAtMTYuMzg0IDEzLjMxMi0zMC4yMDggMzAuMjA4LTMwLjIwOGg0NzQuNjI0TDM4Ni41NiA0MzYuMjI0em01MDEuMjQ4IDMyNS42MzJjMCAxNi44OTYtMTMuMzEyIDMwLjIwOC0yOS42OTYgMzAuMjA4SDY4MC45Nmw1Ny4zNDQtOTMuMTg0LTg3LjU1Mi0yMDIuMjQgNy4xNjgtNy42OCAyMjkuODg4IDI3Mi44OTZ6IiBmaWxsPSIjZTZlNmU2Ii8+PC9zdmc+';
5758

@@ -229,6 +230,15 @@ export class Font extends SheetExtension {
229230

230231
const { startRow, endRow, startColumn, endColumn } = range;
231232
for (let row = startRow; row <= endRow; row++) {
233+
const rowStartPosition = rowHeightAccumulation[row - 1] ?? 0;
234+
const rowEndPosition = rowHeightAccumulation[row] ?? rowStartPosition;
235+
const rowGap = spreadsheetSkeleton.gapConfig?.rowGaps?.[row]?.size ?? 0;
236+
const rowHeight = Math.max(0, rowEndPosition - rowStartPosition - rowGap);
237+
// TODO(@ai-review): Confirm text in sub-four-pixel rows is never expected to overflow visibly outside its row.
238+
if (ctx.__mode !== 'printing' && rowHeight * Math.abs(parentScale.scaleY ?? 1) < MIN_TEXT_RENDER_HEIGHT_IN_SCREEN_PX) {
239+
continue;
240+
}
241+
232242
for (let col = startColumn; col <= endColumn; col++) {
233243
const fontCache = fontMatrix.getValue(row, col);
234244
if (!fontCache) {

‎packages/engine-render/src/components/sheets/extensions/row-header-layout.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import { SheetRowHeaderExtensionRegistry } from '../../extension';
2424
import { SheetExtension } from './sheet-extension';
2525

2626
const UNIQUE_KEY = 'DefaultRowHeaderLayoutExtension';
27+
const MIN_TEXT_RENDER_HEIGHT_IN_SCREEN_PX = 4;
2728

2829
export interface IRowsHeaderCfgParam {
2930
headerStyle?: Partial<IRowStyleCfg>;
@@ -224,6 +225,12 @@ export class RowHeaderLayout extends SheetExtension {
224225
ctx.lineToByPrecision(cellBound.right, cellBound.bottom);
225226
ctx.stroke();
226227

228+
// TODO(@ai-review): Verify omitting labels below this screen-space threshold preserves all required accessibility alternatives.
229+
if (ctx.__mode !== 'printing' && cellBound.height * Math.abs(parentScale.scaleY ?? 1) < MIN_TEXT_RENDER_HEIGHT_IN_SCREEN_PX) {
230+
preRowPosition = rowEndPosition;
231+
continue;
232+
}
233+
227234
// row header text
228235
const textX = (() => {
229236
switch (curRowCfg.textAlign) {

0 commit comments

Comments
 (0)