Skip to content

Commit

Permalink
fix: merged cell bg not render
Browse files Browse the repository at this point in the history 
fix: improve bg ext performance

fix: topleft of merged cell may be null
  • Loading branch information
@lumixraku
lumixraku committed Oct 10, 2024
1 parent f5e97b1 commit 16bf55f
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 55 deletions.
6 changes: 3 additions & 3 deletions packages/core/src/sheets/workbook.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,18 @@
* limitations under the License.
*/

import { BehaviorSubject, Subject } from 'rxjs';
import type { Observable } from 'rxjs';
import type { Nullable } from '../shared';

import type { IRangeType, IWorkbookData, IWorksheetData } from './typedef';
import { BehaviorSubject, Subject } from 'rxjs';
import { UnitModel, UniverInstanceType } from '../common/unit';
import { ILogService } from '../services/log/log.service';
import { Tools } from '../shared';
import { BooleanNumber } from '../types/enum';
import { getEmptySnapshot } from './empty-snapshot';
import { Styles } from './styles';
import { Worksheet } from './worksheet';
import type { Nullable } from '../shared';
import type { IRangeType, IWorkbookData, IWorksheetData } from './typedef';

export function getWorksheetUID(workbook: Workbook, worksheet: Worksheet): string {
return `${workbook.getUnitId()}|${worksheet.getSheetId()}`;
Expand Down
9 changes: 5 additions & 4 deletions packages/engine-render/src/components/extension.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@
* limitations under the License.
*/

import { Registry } from '@univerjs/core';
import type { IDocumentRenderConfig, IRange, IScale, Nullable } from '@univerjs/core';

import { getScale } from '../basics/tools';
import type { BaseObject } from '../base-object';
import type { Vector2 } from '../basics/vector2';

import type { IBoundRectNoAngle, Vector2 } from '../basics/vector2';
import type { UniverRenderingContext } from '../context';
import { Registry } from '@univerjs/core';
import { getScale } from '../basics/tools';

export interface IExtensionConfig {
originTranslate?: Vector2; // docs
Expand All @@ -35,6 +35,7 @@ export interface IDrawInfo {
viewRanges: IRange[];
viewportKey: string;
checkOutOfViewBound?: boolean;
viewBound?: IBoundRectNoAngle;
}
export class ComponentExtension<T, U, V> {
uKey: string = '';
Expand Down
68 changes: 40 additions & 28 deletions packages/engine-render/src/components/sheets/extensions/background.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ interface IRenderBGContext {
scaleY: number;
viewRanges: IRange[];
diffRanges: IRange[];
cellInfo: ISelectionCellWithMergeInfo;
}

export class Background extends SheetExtension {
Expand All @@ -56,6 +57,7 @@ export class Background extends SheetExtension {
return (this.parent as Spreadsheet)?.isPrinting ? this.PRINTING_Z_INDEX : this.Z_INDEX;
}

// eslint-disable-next-line max-lines-per-function
override draw(
ctx: UniverRenderingContext,
_parentScale: IScale,
Expand Down Expand Up @@ -93,12 +95,43 @@ export class Background extends SheetExtension {

renderBGContext.backgroundPaths = backgroundPaths;
ctx.beginPath();
// bgColorMatrix.forValue(renderBGByCell);

const mergeRanges: IRange[] = [];
viewRanges.forEach((range) => {
Range.foreach(range, (row, col) => {
const cellInfo = spreadsheetSkeleton.getCellByIndexWithNoHeader(row, col);
if (!cellInfo) return;
if (cellInfo.isMerged || cellInfo.isMergedMainCell) {
// to check if mergeRanges has this merge range already. if not, push it.
const f = mergeRanges.filter((r: IRange) => {
return r.startRow === cellInfo.mergeInfo.startRow && r.startColumn === cellInfo.mergeInfo.startColumn;
});
if (f.length === 0) {
mergeRanges.push(cellInfo.mergeInfo);
return;
}
}
const bgConfig = bgColorMatrix.getValue(row, col);
if (bgConfig) {
renderBGContext.cellInfo = cellInfo;
this.renderBGByCell(renderBGContext, row, col);
}
});
});
mergeRanges.forEach((range) => {
// draw once in each merge range.
// DO NOT use get topleft cell of bgColorMatrix, may be null(if you jump to bottom of merged cell)
// As we only need to draw merged cell once, so add a flag handled to break the loop.
let handled = false;
Range.foreach(range, (row, col) => {
if (handled) return;
const bgConfig = bgColorMatrix.getValue(row, col);
if (bgConfig) {
const cellInfo = spreadsheetSkeleton.getCellByIndexWithNoHeader(row, col);
if (!cellInfo) return;
renderBGContext.cellInfo = cellInfo;
this.renderBGByCell(renderBGContext, row, col);
handled = true;
}
});
});
Expand All @@ -111,40 +144,19 @@ export class Background extends SheetExtension {
}

renderBGByCell(bgContext: IRenderBGContext, row: number, col: number) {
const { spreadsheetSkeleton, backgroundPositions, backgroundPaths, scaleX, scaleY, viewRanges, diffRanges } = bgContext;
// if (!checkOutOfViewBound && !inViewRanges(viewRanges, row, col)) {
// return true;
// }

const cellInfo = backgroundPositions?.getValue(row, col);
if (cellInfo == null) {
return true;
}
const { spreadsheetSkeleton, backgroundPaths, scaleX, scaleY, viewRanges, diffRanges, cellInfo } = bgContext;

let { startY, endY, startX, endX } = cellInfo;
const { isMerged, isMergedMainCell, mergeInfo } = cellInfo;
const renderRange = diffRanges && diffRanges.length > 0 ? diffRanges : viewRanges;

// isMerged isMergedMainCell are mutually exclusive. isMerged true then isMergedMainCell false.
if (isMerged) {
startY = mergeInfo.startY;
endY = mergeInfo.endY;
startX = mergeInfo.startX;
endX = mergeInfo.endX;
}
// isMergedMainCell has draw all other merged cells, no need draw again.
// For merged cells, and the current cell is the top-left cell in the merged region.
if (isMergedMainCell) {
startY = mergeInfo.startY;
endY = mergeInfo.endY;
startX = mergeInfo.startX;
endX = mergeInfo.endX;
}

// in merge range , but not top-left cell.
// if (isMerged) return true;

// const combineWithMergeRanges = mergeTo;
//expandRangeIfIntersects([...mergeTo], [mergeInfo]);
startY = mergeInfo.startY;
endY = mergeInfo.endY;
startX = mergeInfo.startX;
endX = mergeInfo.endX;

// If curr cell is not in the viewrange (viewport + merged cells), exit early.
if ((!isMerged && !isMergedMainCell) && !inViewRanges(renderRange!, row, col)) {
Expand Down
32 changes: 29 additions & 3 deletions packages/engine-render/src/components/sheets/extensions/font.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
/* eslint-disable max-lines-per-function */
/* eslint-disable complexity */

import type { ICellDataForSheetInterceptor, IRange, IScale, Nullable, ObjectMatrix } from '@univerjs/core';
import type { ICellDataForSheetInterceptor, IRange, IScale, ISelectionCellWithMergeInfo, Nullable, ObjectMatrix } from '@univerjs/core';
import type { UniverRenderingContext } from '../../../context';
import type { Documents } from '../../docs/document';
import type { IDrawInfo } from '../../extension';
Expand Down Expand Up @@ -51,6 +51,7 @@ interface IRenderFontContext {
endY: number;
startX: number;
endX: number;
cellInfo: ISelectionCellWithMergeInfo;
}

export class Font extends SheetExtension {
Expand Down Expand Up @@ -103,11 +104,36 @@ export class Font extends SheetExtension {
// fontMatrix.forValue((row: number, col: number, fontsConfig: IFontCacheItem) => {
// this.renderFontByCellMatrix(renderFontContext, row, col, fontsConfig);
// });

const mergeRanges: IRange[] = [];
viewRanges.forEach((range) => {
range.startColumn -= EXPAND_SIZE_FOR_RENDER_OVERFLOW;
range.endColumn += EXPAND_SIZE_FOR_RENDER_OVERFLOW;
range = clampRange(range);
Range.foreach(range, (row, col) => {
const cellInfo = spreadsheetSkeleton.getCellByIndexWithNoHeader(row, col);
if (!cellInfo) return;

// put all merged cell to another pass to handle.
if (cellInfo.isMerged || cellInfo.isMergedMainCell) {
// to check if mergeRanges has this merge range already. if not, push it.
const f = mergeRanges.filter((r: IRange) => {
return r.startRow === cellInfo.mergeInfo.startRow && r.startColumn === cellInfo.mergeInfo.startColumn;
});
if (f.length === 0) {
mergeRanges.push(cellInfo.mergeInfo);
return;
}
}

renderFontContext.cellInfo = cellInfo;
this.renderFontEachCell(renderFontContext, row, col, fontMatrix);
});
});
mergeRanges.forEach((range) => {
Range.foreach(range, (row, col) => {
const cellInfo = spreadsheetSkeleton.getCellByIndexWithNoHeader(row, col);
renderFontContext.cellInfo = cellInfo;
this.renderFontEachCell(renderFontContext, row, col, fontMatrix);
});
});
Expand Down Expand Up @@ -204,11 +230,11 @@ export class Font extends SheetExtension {
}

renderFontEachCell(renderFontContext: IRenderFontContext, row: number, col: number, fontMatrix: ObjectMatrix<IFontCacheItem>) {
const { ctx, viewRanges, diffRanges, spreadsheetSkeleton } = renderFontContext;
const { ctx, viewRanges, diffRanges, spreadsheetSkeleton, cellInfo } = renderFontContext;

//#region merged cell
// const calcHeader = false;
const cellInfo = spreadsheetSkeleton.getCellByIndexWithNoHeader(row, col);
// const cellInfo = spreadsheetSkeleton.getCellByIndexWithNoHeader(row, col);
let { startY, endY, startX, endX } = cellInfo;
const { isMerged, isMergedMainCell, mergeInfo } = cellInfo;

Expand Down
6 changes: 6 additions & 0 deletions packages/engine-render/src/components/sheets/sheet-skeleton.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1062,6 +1062,12 @@ export class SpreadsheetSkeleton extends Skeleton {
};
}

/**
* New merge info, but position without header.
* @param row
* @param column
* @returns {ISelectionCellWithMergeInfo} cellInfo with merge info
*/
getCellByIndexWithNoHeader(row: number, column: number): ISelectionCellWithMergeInfo {
const { rowHeightAccumulation, columnWidthAccumulation } = this;

Expand Down
30 changes: 16 additions & 14 deletions packages/engine-render/src/components/sheets/spreadsheet.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,30 +14,31 @@
* limitations under the License.
*/

import { BooleanNumber, sortRules, Tools } from '@univerjs/core';
import type { IRange, ISelectionCellWithMergeInfo, Nullable, ObjectMatrix } from '@univerjs/core';

import { FIX_ONE_PIXEL_BLUR_OFFSET, RENDER_CLASS_TYPE } from '../../basics/const';

// import { clearLineByBorderType } from '../../basics/draw';
import { getCellPositionByIndex, getColor } from '../../basics/tools';
import { Documents } from '../docs/document';
import { SpreadsheetExtensionRegistry } from '../extension';
import { SHEET_EXTENSION_PREFIX } from './extensions/sheet-extension';
import { type IPaintForRefresh, type IPaintForScrolling, SHEET_VIEWPORT_KEY } from './interfaces';
// import type { BorderCacheItem } from './interfaces';
import { SheetComponent } from './sheet-component';
import type { IBoundRectNoAngle, IViewportInfo, Vector2 } from '../../basics/vector2';

import type { Canvas } from '../../canvas';

import type { UniverRenderingContext2D } from '../../context';
import type { Engine } from '../../engine';
import type { Scene } from '../../scene';
import type { SceneViewer } from '../../scene-viewer';

import type { IDrawInfo } from '../extension';
import type { Background } from './extensions/background';
import type { Border } from './extensions/border';
import type { Font } from './extensions/font';
import type { SpreadsheetSkeleton } from './sheet-skeleton';
import { BooleanNumber, sortRules, Tools } from '@univerjs/core';
import { FIX_ONE_PIXEL_BLUR_OFFSET, RENDER_CLASS_TYPE } from '../../basics/const';
// import { clearLineByBorderType } from '../../basics/draw';
import { getCellPositionByIndex, getColor } from '../../basics/tools';

import { Documents } from '../docs/document';
import { SpreadsheetExtensionRegistry } from '../extension';
import { SHEET_EXTENSION_PREFIX } from './extensions/sheet-extension';
import { type IPaintForRefresh, type IPaintForScrolling, SHEET_VIEWPORT_KEY } from './interfaces';
// import type { BorderCacheItem } from './interfaces';
import { SheetComponent } from './sheet-component';

const OBJECT_KEY = '__SHEET_EXTENSION_FONT_DOCUMENT_INSTANCE__';

Expand Down Expand Up @@ -136,7 +137,8 @@ export class Spreadsheet extends SheetComponent {
viewRanges,
checkOutOfViewBound: true,
viewportKey: viewportInfo.viewportKey,
});
viewBound: viewportInfo.cacheBound,
} as IDrawInfo);
this.addRenderFrameTimeMetricToScene(timeKey, Tools.now() - st, scene);
}
}
Expand Down
6 changes: 3 additions & 3 deletions packages/sheets/src/commands/commands/move-range.command.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
*/

import type { IAccessor, ICellData, ICommand, IMutationInfo, IRange, Nullable } from '@univerjs/core';
import type { IMoveRangeMutationParams } from '../mutations/move-range.mutation';

import type { ISetSelectionsOperationParams } from '../operations/selection.operation';
import {
cellToRange,
CommandType,
Expand All @@ -29,11 +32,8 @@ import {
sequenceExecute,
Tools,
} from '@univerjs/core';

import { SheetInterceptorService } from '../../services/sheet-interceptor/sheet-interceptor.service';
import type { IMoveRangeMutationParams } from '../mutations/move-range.mutation';
import { MoveRangeMutation } from '../mutations/move-range.mutation';
import type { ISetSelectionsOperationParams } from '../operations/selection.operation';
import { SetSelectionsOperation } from '../operations/selection.operation';
import { alignToMergedCellsBorders, getPrimaryForRange } from './utils/selection-utils';
import { getSheetCommandTarget } from './utils/target-util';
Expand Down

0 comments on commit 16bf55f

Please sign in to comment.