fix: sheet getRowVisible lags after filter (#3396)

dream-num · Sep 12, 2024 · cb2cf7e · cb2cf7e
1 parent 85b9d90
commit cb2cf7e
Show file tree

Hide file tree

Showing 20 changed files with 224 additions and 211 deletions.
diff --git a/packages/core/src/sheets/row-manager.ts b/packages/core/src/sheets/row-manager.ts
@@ -15,9 +15,9 @@
  */
 
 import { getArrayLength, type IObjectArrayPrimitiveType } from '../shared/object-matrix';
-import type { Nullable } from '../shared/types';
 import { BooleanNumber } from '../types/enum';
 import { type IRange, type IRowData, type IWorksheetData, RANGE_TYPE } from './typedef';
+import type { Nullable } from '../shared/types';
 import type { SheetViewModel } from './view-model';
 
 /**
@@ -76,7 +76,7 @@ export class RowManager {
     /**
      * Get row data of given row
      * @param rowPos row index
-     * @returns
+     * @returns {Nullable<Partial<IRowData>>} rowData
      */
     getRow(rowPos: number): Nullable<Partial<IRowData>> {
         return this._rowData[rowPos];

diff --git a/packages/core/src/sheets/view-model.ts b/packages/core/src/sheets/view-model.ts
@@ -14,9 +14,9 @@
  * limitations under the License.
  */
 
-import type { IDisposable } from '../common/di';
-
 import { Disposable, toDisposable } from '../shared/lifecycle';
+
+import type { IDisposable } from '../common/di';
 import type { Nullable } from '../shared/types';
 import type { ICellData, ICellDataForSheetInterceptor } from './typedef';
 

diff --git a/packages/core/src/sheets/worksheet.ts b/packages/core/src/sheets/worksheet.ts
@@ -14,17 +14,17 @@
  * limitations under the License.
  */
 
-import type { IObjectMatrixPrimitiveType, Nullable } from '../shared';
 import { ObjectMatrix, Rectangle, Tools } from '../shared';
 import { createRowColIter } from '../shared/row-col-iter';
 import { type BooleanNumber, CellValueType } from '../types/enum';
 import { ColumnManager } from './column-manager';
 import { Range } from './range';
 import { RowManager } from './row-manager';
 import { mergeWorksheetSnapshotWithDefault } from './sheet-snapshot-utils';
+import { SheetViewModel } from './view-model';
+import type { IObjectMatrixPrimitiveType, Nullable } from '../shared';
 import type { Styles } from './styles';
 import type { ICellData, ICellDataForSheetInterceptor, IFreeze, IRange, IWorksheetData } from './typedef';
-import { SheetViewModel } from './view-model';
 
 /**
  * The model of a Worksheet.
@@ -463,19 +463,25 @@ export class Worksheet {
     }
 
     /**
-     * Get if the row in visible. It may be affected by features like filter and view.
+     * Row is filtered out, that means this row is invisible.
+     * @param row
+     * @returns {boolean} is row hidden by filter
+     */
+    isRowFiltered(row: number): boolean {
+        return this._viewModel.getRowFiltered(row);
+    }
+
+    /**
+     * Get if the row is visible. It may be affected by features like filter and view.
      * @param row the row index
-     * @returns if the row in visible to the user
+     * @returns {boolean} if the row in visible to the user
      */
     getRowVisible(row: number): boolean {
-        const filtered = this._viewModel.getRowFiltered(row);
-        if (filtered) return false;
-
-        return this.getRowRawVisible(row);
+        return !this.isRowFiltered(row) && this.getRowRawVisible(row);
     }
 
     /**
-     * Get if the row does not have `hidden` property.
+     * Get if the row does not have `hidden` property. This value won't affected by features like filter and view.
      * @param row the row index
      * @returns if the row does not have `hidden` property
      */
@@ -504,7 +510,7 @@ export class Worksheet {
     }
 
     /**
-     * Get all visible rows in the sheet.
+     * Get all visible rows in the sheet.(not include filter & view, like getRawVisibleRows)
      * @returns Visible rows range list
      */
     getVisibleRows(): IRange[] {
@@ -513,7 +519,7 @@ export class Worksheet {
     }
 
     /**
-     * Get all visible columns in the sheet.
+     * Get all visible columns in the sheet.(not include filter & view)
      * @returns Visible columns range list
      */
     getVisibleCols(): IRange[] {

diff --git a/packages/engine-render/src/basics/tools.ts b/packages/engine-render/src/basics/tools.ts
@@ -830,7 +830,8 @@ export function expandRangeIfIntersects(mainRanges: IRange[], ranges: IRange[])
             }
         }
     }
-    return [...mainRanges, ...intersects];
+    // return [...mainRanges, ...intersects];
+    return mainRanges.concat(intersects); // concat is slightly faster than spread
 }
 
 export function clampRanges(range: IRange) {

diff --git a/packages/engine-render/src/components/sheets/extensions/border.ts b/packages/engine-render/src/components/sheets/extensions/border.ts
@@ -67,7 +67,7 @@ export class Border extends SheetExtension {
                 return true;
             }
 
-            const cellInfo = this.getCellIndex(
+            const cellInfo = this.getCellByIndex(
                 rowIndex,
                 columnIndex,
                 rowHeightAccumulation,

diff --git a/packages/engine-render/src/components/sheets/extensions/custom.ts b/packages/engine-render/src/components/sheets/extensions/custom.ts
@@ -44,12 +44,15 @@ export class Custom extends SheetExtension {
         const subUnitId = worksheet.getSheetId();
 
         Range.foreach(rowColumnSegment, (row, col) => {
+            if (!worksheet.getRowVisible(row) || !worksheet.getColVisible(col)) {
+                return;
+            }
             let cellData = worksheet.getCell(row, col);
             if (!cellData?.customRender) {
                 return;
             }
 
-            let primaryWithCoord = this.getCellIndex(row, col, rowHeightAccumulation, columnWidthAccumulation, dataMergeCache);
+            let primaryWithCoord = this.getCellByIndex(row, col, rowHeightAccumulation, columnWidthAccumulation, dataMergeCache);
 
             const { mergeInfo } = primaryWithCoord;
             if (!this.isRenderDiffRangesByRow(mergeInfo.startRow, mergeInfo.endRow, diffRanges)) {
@@ -75,7 +78,7 @@ export class Custom extends SheetExtension {
                     return;
                 }
 
-                primaryWithCoord = this.getCellIndex(mainCell.row, mainCell.col, rowHeightAccumulation, columnWidthAccumulation, dataMergeCache);
+                primaryWithCoord = this.getCellByIndex(mainCell.row, mainCell.col, rowHeightAccumulation, columnWidthAccumulation, dataMergeCache);
             }
 
             const renderInfo = {
@@ -89,11 +92,6 @@ export class Custom extends SheetExtension {
                 unitId: worksheet.unitId,
             };
 
-                // current cell is hidden
-            if (!worksheet.getColVisible(col) || !worksheet.getRowVisible(row)) {
-                return;
-            }
-
             const customRender = cellData.customRender.sort(sortRules);
 
             ctx.save();

diff --git a/packages/engine-render/src/components/sheets/extensions/font.ts b/packages/engine-render/src/components/sheets/extensions/font.ts
@@ -60,10 +60,14 @@ export class Font extends SheetExtension {
         diffRanges: IRange[],
         moreBoundsInfo: IDrawInfo
     ) {
-        const { stylesCache, dataMergeCache, overflowCache, worksheet, rowHeightAccumulation, columnTotalWidth, columnWidthAccumulation, rowTotalHeight } = spreadsheetSkeleton;
+        const { stylesCache, dataMergeCache, overflowCache, worksheet } = spreadsheetSkeleton;
         const { font: fontList } = stylesCache;
-        if (!worksheet || !fontList) return;
+        if (!spreadsheetSkeleton || !worksheet || !fontList) {
+            return;
+        }
 
+        const { rowHeightAccumulation, columnTotalWidth, columnWidthAccumulation, rowTotalHeight } =
+            spreadsheetSkeleton;
         if (
             !rowHeightAccumulation ||
             !columnWidthAccumulation ||
@@ -100,7 +104,7 @@ export class Font extends SheetExtension {
                         return true;
                     }
                 }
-                const cellInfo = this.getCellIndex(
+                const cellInfo = this.getCellByIndex(
                     rowIndex,
                     columnIndex,
                     rowHeightAccumulation,
@@ -296,6 +300,7 @@ export class Font extends SheetExtension {
         const cellHeight = endY - startY;
         const cellWidth = endX - startX;
 
+        // WRAP means next line
         if (wrapStrategy === WrapStrategy.WRAP && vertexAngle === 0) {
             documentSkeleton.getViewModel().getDataModel().updateDocumentDataPageSize(cellWidth);
             documentSkeleton.calculate();
@@ -306,7 +311,9 @@ export class Font extends SheetExtension {
         // Use fix https://github.com/dream-num/univer/issues/927, Set the actual width of the content to the page width of the document,
         // so that the divide will be aligned when the skeleton is calculated.
         const overflowRectangle = overflowCache.getValue(row, column);
-        if (!(wrapStrategy === WrapStrategy.WRAP && !overflowRectangle && vertexAngle === 0)) {
+
+        const isOverflow = !(wrapStrategy === WrapStrategy.WRAP && !overflowRectangle && vertexAngle === 0);
+        if (isOverflow) {
             const contentSize = getDocsSkeletonPageSize(documentSkeleton);
 
             const documentStyle = documentSkeleton.getViewModel().getDataModel().getSnapshot().documentStyle;

diff --git a/packages/engine-render/src/components/sheets/extensions/marker.ts b/packages/engine-render/src/components/sheets/extensions/marker.ts
@@ -50,8 +50,12 @@ export class Marker extends SheetExtension {
 
         // eslint-disable-next-line max-lines-per-function
         Range.foreach(rowColumnSegment, (row, col) => {
+            if (!worksheet.getRowVisible(row) || !worksheet.getColVisible(col)) {
+                return;
+            }
+
             let cellData = worksheet.getCell(row, col);
-            const cellInfo = this.getCellIndex(
+            const cellInfo = this.getCellByIndex(
                 row,
                 col,
                 skeleton.rowHeightAccumulation,
@@ -90,11 +94,6 @@ export class Marker extends SheetExtension {
                 mergeCellRendered.add(rangeStr);
             }
 
-            // current cell is hidden
-            if (!worksheet.getColVisible(col) || !worksheet.getRowVisible(row)) {
-                return;
-            }
-
             if (!cellData) {
                 return;
             }

diff --git a/packages/engine-render/src/components/sheets/extensions/sheet-extension.ts b/packages/engine-render/src/components/sheets/extensions/sheet-extension.ts
@@ -40,9 +40,9 @@ export class SheetExtension extends ComponentExtension<SpreadsheetSkeleton, SHEE
      * @param rowHeightAccumulation
      * @param columnWidthAccumulation
      * @param dataMergeCache
-     * @returns ISelectionCellWithMergeInfo
+     * @returns {ISelectionCellWithMergeInfo} cell Position & mergeInfo
      */
-    getCellIndex(
+    getCellByIndex(
         rowIndex: number,
         columnIndex: number,
         rowHeightAccumulation: number[],