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.

Task: 4448426
  • Loading branch information
rrahir committed Jan 7, 2025
1 parent 9d0e335 commit 1695ab3
Show file tree
Hide file tree
Showing 6 changed files with 210 additions and 20 deletions.
28 changes: 21 additions & 7 deletions src/helpers/internal_viewport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ export class InternalViewport {
* @param zone
* @returns Computes the absolute coordinate of a given zone inside the viewport
*/
getRect(zone: Zone): Rect | undefined {
getVisibleRect(zone: Zone): Rect | undefined {
const targetZone = intersection(zone, this.zone);
if (targetZone) {
const x =
Expand All @@ -240,12 +240,26 @@ export class InternalViewport {
this.getters.getColRowOffset("ROW", targetZone.top, targetZone.bottom + 1),
this.viewportHeight
);
return {
x,
y,
width,
height,
};
return { x, y, width, height };
} else {
return undefined;
}
}

getFullRect(zone: Zone): Rect | undefined {
const targetZone = intersection(zone, this.zone);
if (targetZone) {
const x =
this.getters.getColRowOffset("COL", this.zone.left, zone.left) + this.offsetCorrectionX;
const y =
this.getters.getColRowOffset("ROW", this.zone.top, zone.top) + this.offsetCorrectionY;
const width =
this.getters.getColRowOffset("COL", this.boundaries.left, zone.right + 1) -
this.getters.getColRowOffset("COL", this.boundaries.left, zone.left);
const height =
this.getters.getColRowOffset("ROW", this.boundaries.top, zone.bottom + 1) -
this.getters.getColRowOffset("ROW", this.boundaries.top, zone.top);
return { x, y, width, height };
} else {
return undefined;
}
Expand Down
31 changes: 22 additions & 9 deletions src/plugins/ui/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,21 @@ export class RendererPlugin extends UIPlugin {
drawGrid(renderingContext: GridRenderingContext, layer: LAYERS) {
switch (layer) {
case LAYERS.Background:
this.boxes = this.getGridBoxes();
this.drawBackground(renderingContext);
this.drawCellBackground(renderingContext);
this.drawBorders(renderingContext);
this.drawTexts(renderingContext);
this.drawIcon(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();
this.boxes = this.getGridBoxes(zone);
this.drawCellBackground(renderingContext);
this.drawBorders(renderingContext);
this.drawTexts(renderingContext);
this.drawIcon(renderingContext);
ctx.restore();
}
this.drawFrozenPanes(renderingContext);
break;
case LAYERS.Headers:
Expand Down Expand Up @@ -532,7 +541,7 @@ export class RendererPlugin extends UIPlugin {
const row: HeaderIndex = zone.top;
const cell = this.getters.getCell(sheetId, col, row);
const showFormula = this.getters.shouldShowFormulas();
const { x, y, width, height } = this.getters.getVisibleRect(zone);
const { x, y, width, height } = this.getters.getRect(zone);

const box: Box = {
x,
Expand Down Expand Up @@ -659,13 +668,17 @@ export class RendererPlugin extends UIPlugin {
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
36 changes: 34 additions & 2 deletions src/plugins/ui/sheetview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ export class SheetViewPlugin extends UIPlugin {
"getSheetViewVisibleCols",
"getSheetViewVisibleRows",
"getFrozenSheetViewRatio",
"getAllActiveViewportsZones",
"getRect",
] as const;

readonly viewports: Record<UID, SheetViewports | undefined> = {};
Expand Down Expand Up @@ -457,7 +459,32 @@ export class SheetViewPlugin extends UIPlugin {
getVisibleRect(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: x + this.gridOffsetX,
y: y + this.gridOffsetY,
width,
height,
};
}

/**
* 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) {
Expand Down Expand Up @@ -488,11 +515,16 @@ export class SheetViewPlugin extends UIPlugin {
return { x, y };
}

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
5 changes: 5 additions & 0 deletions tests/plugins/__snapshots__/renderer.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,18 @@ Array [
"context.moveTo(0, 989)",
"context.lineTo(952, 989)",
"context.stroke()",
"context.save()",
"context.beginPath()",
"context.rect(0, 0, 952, 974)",
"context.clip()",
"context.lineWidth=0.12;",
"context.strokeStyle=\\"#111\\";",
"context.textBaseline=\\"top\\";",
"context.font=\\"400 13px 'Roboto', arial\\";",
"context.fillStyle=\\"#000\\";",
"context.textAlign=\\"right\\";",
"context.fillText(\\"1\\", 92, 6)",
"context.restore()",
"context.lineWidth=2.4000000000000004;",
"context.strokeStyle=\\"#DADFE8\\";",
"context.beginPath()",
Expand Down
48 changes: 48 additions & 0 deletions tests/plugins/renderer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import {
copy,
createFilter,
deleteColumns,
freezeColumns,
freezeRows,
merge,
paste,
resizeColumns,
Expand Down Expand Up @@ -1362,4 +1364,50 @@ describe("renderer", () => {
SELECTION_BORDER_COLOR, // selection drawGrid
]);
});

test("Each frozen pane is clipped in the grid", () => {
const model = new Model();

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);
}
},
});
model.drawGrid(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", []);
});
});
82 changes: 80 additions & 2 deletions tests/plugins/sheetview.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -899,18 +899,96 @@ describe("Viewport of Simple sheet", () => {
});
});

test("getVisibleRect with freezed panes returns the actual visible part of a zone", () => {
test("getVisibleRect with frozen panes returns the actual visible part of a zone", () => {
freezeColumns(model, 1);
freezeRows(model, 1);
const width = 4.5 * DEFAULT_CELL_WIDTH;
const height = 5.5 * DEFAULT_CELL_HEIGHT;
model.dispatch("RESIZE_SHEETVIEW", { gridOffsetX: 0, gridOffsetY: 0, width, height });
expect(model.getters.getVisibleRect(model.getters.getActiveMainViewport())).toEqual({
const zone = model.getters.getActiveMainViewport();
expect(model.getters.getVisibleRect(zone)).toEqual({
x: DEFAULT_CELL_WIDTH,
y: DEFAULT_CELL_HEIGHT,
width: 3.5 * DEFAULT_CELL_WIDTH,
height: 4.5 * DEFAULT_CELL_HEIGHT,
});
setViewportOffset(model, DEFAULT_CELL_WIDTH, DEFAULT_CELL_HEIGHT);
expect(model.getters.getVisibleRect(zone)).toEqual({
x: DEFAULT_CELL_WIDTH,
y: DEFAULT_CELL_HEIGHT,
width: 3 * DEFAULT_CELL_WIDTH,
height: 4 * DEFAULT_CELL_HEIGHT,
});
});

test("getVisibleRect takes the scroll into account", () => {
merge(model, "A1:B2");
const zone = toZone("A1:B2");
expect(model.getters.getVisibleRect(zone)).toEqual({
x: 0,
y: 0,
width: DEFAULT_CELL_WIDTH * 2,
height: DEFAULT_CELL_HEIGHT * 2,
});
setViewportOffset(model, DEFAULT_CELL_WIDTH, DEFAULT_CELL_HEIGHT);
expect(model.getters.getVisibleRect(zone)).toEqual({
x: 0,
y: 0,
width: DEFAULT_CELL_WIDTH,
height: DEFAULT_CELL_HEIGHT,
});
});

test("getRect returns the full zone dimensions regardless of the viewport size", () => {
const width = 4.5 * DEFAULT_CELL_WIDTH;
const height = 5.5 * DEFAULT_CELL_HEIGHT;
model.dispatch("RESIZE_SHEETVIEW", { gridOffsetX: 0, gridOffsetY: 0, width, height });
expect(model.getters.getRect(model.getters.getActiveMainViewport())).toEqual({
x: 0,
y: 0,
width: 5 * DEFAULT_CELL_WIDTH,
height: 6 * DEFAULT_CELL_HEIGHT,
});
});

test("getRect with frozen panes returns the full part of a zone", () => {
freezeColumns(model, 1);
freezeRows(model, 1);
const width = 4.5 * DEFAULT_CELL_WIDTH;
const height = 5.5 * DEFAULT_CELL_HEIGHT;
model.dispatch("RESIZE_SHEETVIEW", { gridOffsetX: 0, gridOffsetY: 0, width, height });
const zone = model.getters.getActiveMainViewport();
expect(model.getters.getRect(zone)).toEqual({
x: DEFAULT_CELL_WIDTH,
y: DEFAULT_CELL_HEIGHT,
width: 4 * DEFAULT_CELL_WIDTH,
height: 5 * DEFAULT_CELL_HEIGHT,
});
setViewportOffset(model, DEFAULT_CELL_WIDTH, DEFAULT_CELL_HEIGHT);
expect(model.getters.getRect(zone)).toEqual({
x: 0,
y: 0,
width: 4 * DEFAULT_CELL_WIDTH,
height: 5 * DEFAULT_CELL_HEIGHT,
});
});

test("getRect takes the scroll into account", () => {
merge(model, "A1:B2");
const zone = toZone("A1:B2");
expect(model.getters.getRect(zone)).toEqual({
x: 0,
y: 0,
width: DEFAULT_CELL_WIDTH * 2,
height: DEFAULT_CELL_HEIGHT * 2,
});
setViewportOffset(model, DEFAULT_CELL_WIDTH, DEFAULT_CELL_HEIGHT);
expect(model.getters.getRect(zone)).toEqual({
x: -DEFAULT_CELL_WIDTH,
y: -DEFAULT_CELL_HEIGHT,
width: DEFAULT_CELL_WIDTH * 2,
height: DEFAULT_CELL_HEIGHT * 2,
});
});

test("Loading a model with initial revisions in sheet that is deleted doesn't crash", () => {
Expand Down

0 comments on commit 1695ab3

Please sign in to comment.