Skip to content

Commit 361330e

Browse files
committed
[FIX] renderer: Fix grid rendering
When we implemented the frozen panes, we accidently destroyed the distinction between the actual size of a cell/merge and its visual dimension in the viewport. i.e. the dimension of the part that we be rendered. However, the rest of the rendering process was relying on the box size to compute several things (button , text positions for instance) This means that all computations that relied on the actual size of a cell/merge are now false. A good example is the merge. Create a merge with some text inside of it and make sure it spans over several columns. The text is aligned at the center of the merge. Now scroll a bit and the text will stay centered in the VISIBLE part of the merge but not the merge itself. On another hand, we did not account for some text formatting, specifically text aligned on the right can overflow outside of their pane. This revision fixes the two issues: - Reintroduce a getter `getRect` to properly compute the positiontioning of text/icons of a box - Change the rendering process of the grid by splitting the rendering one pane at a time. closes #5493 Task: 4448426 X-original-commit: eccd9fa Signed-off-by: Lucas Lefèvre (lul) <[email protected]> Signed-off-by: Rémi Rahir (rar) <[email protected]>
1 parent 4f96c47 commit 361330e

File tree

6 files changed

+231
-36
lines changed

6 files changed

+231
-36
lines changed

Diff for: src/helpers/internal_viewport.ts

+22-9
Original file line numberDiff line numberDiff line change
@@ -226,10 +226,10 @@ export class InternalViewport {
226226

227227
/**
228228
*
229-
* @param zone
230-
* @returns Computes the absolute coordinate of a given zone inside the viewport
229+
* Computes the visible coordinates & dimensions of a given zone inside the viewport
230+
*
231231
*/
232-
getRect(zone: Zone): Rect | undefined {
232+
getVisibleRect(zone: Zone): Rect | undefined {
233233
const targetZone = intersection(zone, this);
234234
if (targetZone) {
235235
const x =
@@ -246,12 +246,25 @@ export class InternalViewport {
246246
this.getters.getColRowOffset("ROW", targetZone.top, targetZone.bottom + 1),
247247
this.viewportHeight
248248
);
249-
return {
250-
x,
251-
y,
252-
width,
253-
height,
254-
};
249+
return { x, y, width, height };
250+
}
251+
return undefined;
252+
}
253+
254+
/**
255+
*
256+
* @returns Computes the absolute coordinates & dimensions of a given zone inside the viewport
257+
*
258+
*/
259+
getFullRect(zone: Zone): Rect | undefined {
260+
const targetZone = intersection(zone, this);
261+
if (targetZone) {
262+
const x = this.getters.getColRowOffset("COL", this.left, zone.left) + this.offsetCorrectionX;
263+
const y = this.getters.getColRowOffset("ROW", this.top, zone.top) + this.offsetCorrectionY;
264+
const width = this.getters.getColRowOffset("COL", zone.left, zone.right + 1);
265+
266+
const height = this.getters.getColRowOffset("ROW", zone.top, zone.bottom + 1);
267+
return { x, y, width, height };
255268
}
256269
return undefined;
257270
}

Diff for: src/plugins/ui_stateful/sheetview.ts

+35-7
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@ export class SheetViewPlugin extends UIPlugin {
104104
"isPositionVisible",
105105
"getColDimensionsInViewport",
106106
"getRowDimensionsInViewport",
107+
"getAllActiveViewportsZones",
108+
"getRect",
107109
] as const;
108110

109111
readonly viewports: Record<UID, SheetViewports | undefined> = {};
@@ -530,17 +532,30 @@ export class SheetViewPlugin extends UIPlugin {
530532
getVisibleRectWithoutHeaders(zone: Zone): Rect {
531533
const sheetId = this.getters.getActiveSheetId();
532534
const viewportRects = this.getSubViewports(sheetId)
533-
.map((viewport) => viewport.getRect(zone))
535+
.map((viewport) => viewport.getVisibleRect(zone))
534536
.filter(isDefined);
535537

536538
if (viewportRects.length === 0) {
537539
return { x: 0, y: 0, width: 0, height: 0 };
538540
}
539-
const x = Math.min(...viewportRects.map((rect) => rect.x));
540-
const y = Math.min(...viewportRects.map((rect) => rect.y));
541-
const width = Math.max(...viewportRects.map((rect) => rect.x + rect.width)) - x;
542-
const height = Math.max(...viewportRects.map((rect) => rect.y + rect.height)) - y;
543-
return { x, y, width, height };
541+
return this.recomposeRect(viewportRects);
542+
}
543+
544+
/**
545+
* Computes the actual size and position (:Rect) of the zone on the canvas
546+
* regardless of the viewport dimensions.
547+
*/
548+
getRect(zone: Zone): Rect {
549+
const sheetId = this.getters.getActiveSheetId();
550+
const viewportRects = this.getSubViewports(sheetId)
551+
.map((viewport) => viewport.getFullRect(zone))
552+
.filter(isDefined);
553+
554+
if (viewportRects.length === 0) {
555+
return { x: 0, y: 0, width: 0, height: 0 };
556+
}
557+
const rect = this.recomposeRect(viewportRects);
558+
return { ...rect, x: rect.x + this.gridOffsetX, y: rect.y + this.gridOffsetY };
544559
}
545560

546561
/**
@@ -588,11 +603,16 @@ export class SheetViewPlugin extends UIPlugin {
588603
};
589604
}
590605

606+
getAllActiveViewportsZones(): Zone[] {
607+
const sheetId = this.getters.getActiveSheetId();
608+
return this.getSubViewports(sheetId);
609+
}
610+
591611
// ---------------------------------------------------------------------------
592612
// Private
593613
// ---------------------------------------------------------------------------
594614

595-
private ensureMainViewportExist(sheetId) {
615+
private ensureMainViewportExist(sheetId: UID) {
596616
if (!this.viewports[sheetId]) {
597617
this.resetViewports(sheetId);
598618
}
@@ -863,4 +883,12 @@ export class SheetViewPlugin extends UIPlugin {
863883
const height = this.sheetViewHeight + this.gridOffsetY;
864884
return { xRatio: offsetCorrectionX / width, yRatio: offsetCorrectionY / height };
865885
}
886+
887+
private recomposeRect(viewportRects: Rect[]): Rect {
888+
const x = Math.min(...viewportRects.map((rect) => rect.x));
889+
const y = Math.min(...viewportRects.map((rect) => rect.y));
890+
const width = Math.max(...viewportRects.map((rect) => rect.x + rect.width)) - x;
891+
const height = Math.max(...viewportRects.map((rect) => rect.y + rect.height)) - y;
892+
return { x, y, width, height };
893+
}
866894
}

Diff for: src/stores/grid_renderer_store.ts

+31-13
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,23 @@ export class GridRenderer {
9292
drawLayer(renderingContext: GridRenderingContext, layer: LayerName) {
9393
switch (layer) {
9494
case "Background":
95-
const boxes = this.getGridBoxes();
96-
this.drawBackground(renderingContext, boxes);
97-
this.drawOverflowingCellBackground(renderingContext, boxes);
98-
this.drawCellBackground(renderingContext, boxes);
99-
this.drawBorders(renderingContext, boxes);
100-
this.drawTexts(renderingContext, boxes);
101-
this.drawIcon(renderingContext, boxes);
95+
this.drawGlobalBackground(renderingContext);
96+
for (const zone of this.getters.getAllActiveViewportsZones()) {
97+
const { ctx } = renderingContext;
98+
ctx.save();
99+
ctx.beginPath();
100+
const rect = this.getters.getVisibleRect(zone);
101+
ctx.rect(rect.x, rect.y, rect.width, rect.height);
102+
ctx.clip();
103+
const boxes = this.getGridBoxes(zone);
104+
this.drawBackground(renderingContext, boxes);
105+
this.drawOverflowingCellBackground(renderingContext, boxes);
106+
this.drawCellBackground(renderingContext, boxes);
107+
this.drawBorders(renderingContext, boxes);
108+
this.drawTexts(renderingContext, boxes);
109+
this.drawIcon(renderingContext, boxes);
110+
ctx.restore();
111+
}
102112
this.drawFrozenPanes(renderingContext);
103113
break;
104114
case "Headers":
@@ -110,13 +120,17 @@ export class GridRenderer {
110120
}
111121
}
112122

113-
private drawBackground(renderingContext: GridRenderingContext, boxes: Box[]) {
114-
const { ctx, thinLineWidth } = renderingContext;
123+
private drawGlobalBackground(renderingContext: GridRenderingContext) {
124+
const { ctx } = renderingContext;
115125
const { width, height } = this.getters.getSheetViewDimensionWithHeaders();
116126

117127
// white background
118128
ctx.fillStyle = "#ffffff";
119129
ctx.fillRect(0, 0, width + CANVAS_SHIFT, height + CANVAS_SHIFT);
130+
}
131+
132+
private drawBackground(renderingContext: GridRenderingContext, boxes: Box[]) {
133+
const { ctx, thinLineWidth } = renderingContext;
120134

121135
const areGridLinesVisible =
122136
!this.getters.isDashboard() &&
@@ -591,7 +605,7 @@ export class GridRenderer {
591605
const position = { sheetId, col, row };
592606
const cell = this.getters.getEvaluatedCell(position);
593607
const showFormula = this.getters.shouldShowFormulas();
594-
const { x, y, width, height } = this.getters.getVisibleRect(zone);
608+
const { x, y, width, height } = this.getters.getRect(zone);
595609
const { verticalAlign } = this.getters.getCellStyle(position);
596610
let style = this.getters.getCellComputedStyle(position);
597611
if (this.fingerprints.isEnabled) {
@@ -732,13 +746,17 @@ export class GridRenderer {
732746
return box;
733747
}
734748

735-
private getGridBoxes(): Box[] {
749+
private getGridBoxes(zone: Zone): Box[] {
736750
const boxes: Box[] = [];
737751

738-
const visibleCols = this.getters.getSheetViewVisibleCols();
752+
const visibleCols = this.getters
753+
.getSheetViewVisibleCols()
754+
.filter((col) => col >= zone.left && col <= zone.right);
739755
const left = visibleCols[0];
740756
const right = visibleCols[visibleCols.length - 1];
741-
const visibleRows = this.getters.getSheetViewVisibleRows();
757+
const visibleRows = this.getters
758+
.getSheetViewVisibleRows()
759+
.filter((row) => row >= zone.top && row <= zone.bottom);
742760
const top = visibleRows[0];
743761
const bottom = visibleRows[visibleRows.length - 1];
744762
const viewport = { left, right, top, bottom };

Diff for: tests/__snapshots__/renderer_store.test.ts.snap

+5
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ exports[`renderer snapshot for a simple grid rendering 1`] = `
55
"context.save()",
66
"context.fillStyle="#ffffff";",
77
"context.fillRect(0, 0, 952.5, 974.5)",
8+
"context.save()",
9+
"context.beginPath()",
10+
"context.rect(0, 0, 952, 974)",
11+
"context.clip()",
812
"context.strokeStyle="#E2E3E3";",
913
"context.lineWidth=0.4;",
1014
"context.strokeRect(0.04000000000000001, 0.04000000000000001, 95.92, 22.92)",
@@ -1301,6 +1305,7 @@ exports[`renderer snapshot for a simple grid rendering 1`] = `
13011305
"context.textAlign="right";",
13021306
"GET:font",
13031307
"context.fillText("1", 92, 7)",
1308+
"context.restore()",
13041309
"context.lineWidth=2.4000000000000004;",
13051310
"context.strokeStyle="#DADFE8";",
13061311
"context.beginPath()",

Diff for: tests/renderer_store.test.ts

+58-5
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ import {
3232
addDataValidation,
3333
copy,
3434
deleteColumns,
35+
freezeColumns,
36+
freezeRows,
3537
merge,
3638
paste,
3739
resizeColumns,
@@ -53,7 +55,14 @@ MockCanvasRenderingContext2D.prototype.measureText = function (text: string) {
5355
jest.mock("../src/helpers/uuid", () => require("./__mocks__/uuid"));
5456

5557
function getBoxFromText(gridRenderer: GridRenderer, text: string): Box {
56-
return (gridRenderer["getGridBoxes"]()! as Box[]).find(
58+
const sheetId = gridRenderer["getters"].getActiveSheetId();
59+
const zone = {
60+
left: 0,
61+
right: gridRenderer["getters"].getNumberCols(sheetId) - 1,
62+
top: 0,
63+
bottom: gridRenderer["getters"].getNumberRows(sheetId) - 1,
64+
};
65+
return (gridRenderer["getGridBoxes"](zone)! as Box[]).find(
5766
(b) => (b.content?.textLines || []).join(" ") === text
5867
)!;
5968
}
@@ -2180,8 +2189,7 @@ describe("renderer", () => {
21802189
const { drawGridRenderer, gridRendererStore } = setRenderer(model);
21812190
let ctx = new MockGridRenderingContext(model, 1000, 1000, {});
21822191
drawGridRenderer(ctx);
2183-
//@ts-expect-error
2184-
const boxes = gridRendererStore.getGridBoxes();
2192+
const boxes = gridRendererStore["getGridBoxes"](toZone("A1:B2"));
21852193
const boxesText = boxes.map((box) => box.content?.textLines.join(""));
21862194
expect(boxesText).toEqual(["=MUNIT(2)", "", "", ""]);
21872195
});
@@ -2228,13 +2236,58 @@ describe("renderer", () => {
22282236
let ctx = new MockGridRenderingContext(model, 1000, 1000, {});
22292237
drawGridRenderer(ctx);
22302238

2231-
let box = gridRendererStore["getGridBoxes"]().filter((box) => box.content)[0];
2239+
let box = gridRendererStore["getGridBoxes"](toZone("A1:A100")).filter((box) => box.content)[0];
22322240
const expectedSpaces = 20 - 2 * MIN_CELL_TEXT_MARGIN;
22332241
expect(box.content?.textLines).toEqual(["1".padStart(expectedSpaces)]);
22342242

22352243
setFormat(model, "A1", "0*c");
22362244
drawGridRenderer(ctx);
2237-
box = gridRendererStore["getGridBoxes"]().filter((box) => box.content)[0];
2245+
box = gridRendererStore["getGridBoxes"](toZone("A1:A100")).filter((box) => box.content)[0];
22382246
expect(box.content?.textLines).toEqual(["1".padEnd(expectedSpaces, "c")]);
22392247
});
2248+
2249+
test("Each frozen pane is clipped in the grid", () => {
2250+
const { drawGridRenderer, model } = setRenderer();
2251+
setCellContent(model, "A1", "1");
2252+
freezeColumns(model, 2);
2253+
freezeRows(model, 1);
2254+
const spyFn = jest.fn();
2255+
let ctx = new MockGridRenderingContext(model, 1000, 1000, {
2256+
onFunctionCall: (key, args) => {
2257+
if (["rect", "clip"].includes(key)) {
2258+
spyFn(key, args);
2259+
}
2260+
},
2261+
});
2262+
drawGridRenderer(ctx);
2263+
expect(spyFn).toHaveBeenCalledTimes(8);
2264+
expect(spyFn).toHaveBeenNthCalledWith(1, "rect", [
2265+
0,
2266+
0,
2267+
DEFAULT_CELL_WIDTH * 2,
2268+
DEFAULT_CELL_HEIGHT,
2269+
]);
2270+
expect(spyFn).toHaveBeenNthCalledWith(2, "clip", []);
2271+
expect(spyFn).toHaveBeenNthCalledWith(3, "rect", [
2272+
DEFAULT_CELL_WIDTH * 2,
2273+
0,
2274+
760,
2275+
DEFAULT_CELL_HEIGHT,
2276+
]);
2277+
expect(spyFn).toHaveBeenNthCalledWith(4, "clip", []);
2278+
expect(spyFn).toHaveBeenNthCalledWith(5, "rect", [
2279+
0,
2280+
DEFAULT_CELL_HEIGHT,
2281+
DEFAULT_CELL_WIDTH * 2,
2282+
951,
2283+
]);
2284+
expect(spyFn).toHaveBeenNthCalledWith(6, "clip", []);
2285+
expect(spyFn).toHaveBeenNthCalledWith(7, "rect", [
2286+
DEFAULT_CELL_WIDTH * 2,
2287+
DEFAULT_CELL_HEIGHT,
2288+
760,
2289+
951,
2290+
]);
2291+
expect(spyFn).toHaveBeenNthCalledWith(8, "clip", []);
2292+
});
22402293
});

0 commit comments

Comments
 (0)