Skip to content

Commit

Permalink
[FIX] renderer: Fix grid rendering
Browse files Browse the repository at this point in the history
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 #5489

Task: 4448426
X-original-commit: 44979e2
Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
Signed-off-by: Rémi Rahir (rar) <[email protected]>
  • Loading branch information
rrahir committed Jan 16, 2025
1 parent e12286d commit c5970f7
Show file tree
Hide file tree
Showing 6 changed files with 229 additions and 34 deletions.
31 changes: 22 additions & 9 deletions src/helpers/internal_viewport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,10 @@ export class InternalViewport {

/**
*
* @param zone
* @returns Computes the absolute coordinate of a given zone inside the viewport
* Computes the visible coordinates & dimensions of a given zone inside the viewport
*
*/
getRect(zone: Zone): Rect | undefined {
getVisibleRect(zone: Zone): Rect | undefined {
const targetZone = intersection(zone, this);
if (targetZone) {
const x =
Expand All @@ -239,12 +239,25 @@ export class InternalViewport {
this.getters.getColRowOffset("ROW", targetZone.top, targetZone.bottom + 1),
this.viewportHeight
);
return {
x,
y,
width,
height,
};
return { x, y, width, height };
}
return undefined;
}

/**
*
* @returns Computes the absolute coordinates & dimensions of a given zone inside the viewport
*
*/
getFullRect(zone: Zone): Rect | undefined {
const targetZone = intersection(zone, this);
if (targetZone) {
const x = this.getters.getColRowOffset("COL", this.left, zone.left) + this.offsetCorrectionX;
const y = this.getters.getColRowOffset("ROW", this.top, zone.top) + this.offsetCorrectionY;
const width = this.getters.getColRowOffset("COL", zone.left, zone.right + 1);

const height = this.getters.getColRowOffset("ROW", zone.top, zone.bottom + 1);
return { x, y, width, height };
}
return undefined;
}
Expand Down
42 changes: 35 additions & 7 deletions src/plugins/ui_stateful/sheetview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ export class SheetViewPlugin extends UIPlugin {
"isPositionVisible",
"getColDimensionsInViewport",
"getRowDimensionsInViewport",
"getAllActiveViewportsZones",
"getRect",
] as const;

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

if (viewportRects.length === 0) {
return { x: 0, y: 0, width: 0, height: 0 };
}
const x = Math.min(...viewportRects.map((rect) => rect.x));
const y = Math.min(...viewportRects.map((rect) => rect.y));
const width = Math.max(...viewportRects.map((rect) => rect.x + rect.width)) - x;
const height = Math.max(...viewportRects.map((rect) => rect.y + rect.height)) - y;
return { x, y, width, height };
return this.recomposeRect(viewportRects);
}

/**
* Computes the actual size and position (:Rect) of the zone on the canvas
* regardless of the viewport dimensions.
*/
getRect(zone: Zone): Rect {
const sheetId = this.getters.getActiveSheetId();
const viewportRects = this.getSubViewports(sheetId)
.map((viewport) => viewport.getFullRect(zone))
.filter(isDefined);

if (viewportRects.length === 0) {
return { x: 0, y: 0, width: 0, height: 0 };
}
const rect = this.recomposeRect(viewportRects);
return { ...rect, x: rect.x + this.gridOffsetX, y: rect.y + this.gridOffsetY };
}

/**
Expand Down Expand Up @@ -588,11 +603,16 @@ export class SheetViewPlugin extends UIPlugin {
};
}

getAllActiveViewportsZones(): Zone[] {
const sheetId = this.getters.getActiveSheetId();
return this.getSubViewports(sheetId);
}

// ---------------------------------------------------------------------------
// Private
// ---------------------------------------------------------------------------

private ensureMainViewportExist(sheetId) {
private ensureMainViewportExist(sheetId: UID) {
if (!this.viewports[sheetId]) {
this.resetViewports(sheetId);
}
Expand Down Expand Up @@ -851,4 +871,12 @@ export class SheetViewPlugin extends UIPlugin {
const height = this.sheetViewHeight + this.gridOffsetY;
return { xRatio: offsetCorrectionX / width, yRatio: offsetCorrectionY / height };
}

private recomposeRect(viewportRects: Rect[]): Rect {
const x = Math.min(...viewportRects.map((rect) => rect.x));
const y = Math.min(...viewportRects.map((rect) => rect.y));
const width = Math.max(...viewportRects.map((rect) => rect.x + rect.width)) - x;
const height = Math.max(...viewportRects.map((rect) => rect.y + rect.height)) - y;
return { x, y, width, height };
}
}
44 changes: 31 additions & 13 deletions src/stores/grid_renderer_store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,23 @@ export class GridRenderer {
drawLayer(renderingContext: GridRenderingContext, layer: LayerName) {
switch (layer) {
case "Background":
const boxes = this.getGridBoxes();
this.drawBackground(renderingContext, boxes);
this.drawOverflowingCellBackground(renderingContext, boxes);
this.drawCellBackground(renderingContext, boxes);
this.drawBorders(renderingContext, boxes);
this.drawTexts(renderingContext, boxes);
this.drawIcon(renderingContext, boxes);
this.drawGlobalBackground(renderingContext);
for (const zone of this.getters.getAllActiveViewportsZones()) {
const { ctx } = renderingContext;
ctx.save();
ctx.beginPath();
const rect = this.getters.getVisibleRect(zone);
ctx.rect(rect.x, rect.y, rect.width, rect.height);
ctx.clip();
const boxes = this.getGridBoxes(zone);
this.drawBackground(renderingContext, boxes);
this.drawOverflowingCellBackground(renderingContext, boxes);
this.drawCellBackground(renderingContext, boxes);
this.drawBorders(renderingContext, boxes);
this.drawTexts(renderingContext, boxes);
this.drawIcon(renderingContext, boxes);
ctx.restore();
}
this.drawFrozenPanes(renderingContext);
break;
case "Headers":
Expand All @@ -106,13 +116,17 @@ export class GridRenderer {
}
}

private drawBackground(renderingContext: GridRenderingContext, boxes: Box[]) {
const { ctx, thinLineWidth } = renderingContext;
private drawGlobalBackground(renderingContext: GridRenderingContext) {
const { ctx } = renderingContext;
const { width, height } = this.getters.getSheetViewDimensionWithHeaders();

// white background
ctx.fillStyle = "#ffffff";
ctx.fillRect(0, 0, width + CANVAS_SHIFT, height + CANVAS_SHIFT);
}

private drawBackground(renderingContext: GridRenderingContext, boxes: Box[]) {
const { ctx, thinLineWidth } = renderingContext;

const areGridLinesVisible =
!this.getters.isDashboard() &&
Expand Down Expand Up @@ -581,7 +595,7 @@ export class GridRenderer {
const position = { sheetId, col, row };
const cell = this.getters.getEvaluatedCell(position);
const showFormula = this.getters.shouldShowFormulas();
const { x, y, width, height } = this.getters.getVisibleRect(zone);
const { x, y, width, height } = this.getters.getRect(zone);
const { verticalAlign } = this.getters.getCellStyle(position);

const box: Box = {
Expand Down Expand Up @@ -713,13 +727,17 @@ export class GridRenderer {
return box;
}

private getGridBoxes(): Box[] {
private getGridBoxes(zone: Zone): Box[] {
const boxes: Box[] = [];

const visibleCols = this.getters.getSheetViewVisibleCols();
const visibleCols = this.getters
.getSheetViewVisibleCols()
.filter((col) => col >= zone.left && col <= zone.right);
const left = visibleCols[0];
const right = visibleCols[visibleCols.length - 1];
const visibleRows = this.getters.getSheetViewVisibleRows();
const visibleRows = this.getters
.getSheetViewVisibleRows()
.filter((row) => row >= zone.top && row <= zone.bottom);
const top = visibleRows[0];
const bottom = visibleRows[visibleRows.length - 1];
const viewport = { left, right, top, bottom };
Expand Down
5 changes: 5 additions & 0 deletions tests/__snapshots__/renderer_store.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ exports[`renderer snapshot for a simple grid rendering 1`] = `
"context.save()",
"context.fillStyle="#ffffff";",
"context.fillRect(0, 0, 952.5, 974.5)",
"context.save()",
"context.beginPath()",
"context.rect(0, 0, 952, 974)",
"context.clip()",
"context.strokeStyle="#E2E3E3";",
"context.lineWidth=0.4;",
"context.strokeRect(0.04000000000000001, 0.04000000000000001, 95.92, 22.92)",
Expand Down Expand Up @@ -1301,6 +1305,7 @@ exports[`renderer snapshot for a simple grid rendering 1`] = `
"context.textAlign="right";",
"GET:font",
"context.fillText("1", 92, 7)",
"context.restore()",
"context.lineWidth=2.4000000000000004;",
"context.strokeStyle="#DADFE8";",
"context.beginPath()",
Expand Down
59 changes: 56 additions & 3 deletions tests/renderer_store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import {
addDataValidation,
copy,
deleteColumns,
freezeColumns,
freezeRows,
merge,
paste,
resizeColumns,
Expand All @@ -51,7 +53,14 @@ MockCanvasRenderingContext2D.prototype.measureText = function (text: string) {
jest.mock("../src/helpers/uuid", () => require("./__mocks__/uuid"));

function getBoxFromText(gridRenderer: GridRenderer, text: string): Box {
return (gridRenderer["getGridBoxes"]()! as Box[]).find(
const sheetId = gridRenderer["getters"].getActiveSheetId();
const zone = {
left: 0,
right: gridRenderer["getters"].getNumberCols(sheetId) - 1,
top: 0,
bottom: gridRenderer["getters"].getNumberRows(sheetId) - 1,
};
return (gridRenderer["getGridBoxes"](zone)! as Box[]).find(
(b) => (b.content?.textLines || []).join(" ") === text
)!;
}
Expand Down Expand Up @@ -2123,8 +2132,7 @@ describe("renderer", () => {
const { drawGridRenderer, gridRendererStore } = setRenderer(model);
let ctx = new MockGridRenderingContext(model, 1000, 1000, {});
drawGridRenderer(ctx);
//@ts-expect-error
const boxes = gridRendererStore.getGridBoxes();
const boxes = gridRendererStore["getGridBoxes"](toZone("A1:B2"));
const boxesText = boxes.map((box) => box.content?.textLines.join(""));
expect(boxesText).toEqual(["=MUNIT(2)", "", "", ""]);
});
Expand Down Expand Up @@ -2161,4 +2169,49 @@ describe("renderer", () => {
expect(renderedTexts).toContain("hello");
});
});

test("Each frozen pane is clipped in the grid", () => {
const { drawGridRenderer, model } = setRenderer();
setCellContent(model, "A1", "1");
freezeColumns(model, 2);
freezeRows(model, 1);
const spyFn = jest.fn();
let ctx = new MockGridRenderingContext(model, 1000, 1000, {
onFunctionCall: (key, args) => {
if (["rect", "clip"].includes(key)) {
spyFn(key, args);
}
},
});
drawGridRenderer(ctx);
expect(spyFn).toHaveBeenCalledTimes(8);
expect(spyFn).toHaveBeenNthCalledWith(1, "rect", [
0,
0,
DEFAULT_CELL_WIDTH * 2,
DEFAULT_CELL_HEIGHT,
]);
expect(spyFn).toHaveBeenNthCalledWith(2, "clip", []);
expect(spyFn).toHaveBeenNthCalledWith(3, "rect", [
DEFAULT_CELL_WIDTH * 2,
0,
760,
DEFAULT_CELL_HEIGHT,
]);
expect(spyFn).toHaveBeenNthCalledWith(4, "clip", []);
expect(spyFn).toHaveBeenNthCalledWith(5, "rect", [
0,
DEFAULT_CELL_HEIGHT,
DEFAULT_CELL_WIDTH * 2,
951,
]);
expect(spyFn).toHaveBeenNthCalledWith(6, "clip", []);
expect(spyFn).toHaveBeenNthCalledWith(7, "rect", [
DEFAULT_CELL_WIDTH * 2,
DEFAULT_CELL_HEIGHT,
760,
951,
]);
expect(spyFn).toHaveBeenNthCalledWith(8, "clip", []);
});
});
Loading

0 comments on commit c5970f7

Please sign in to comment.