fix(engine-render): prevent viewport scroll trails (#7601)

Author: DR-Univer

packages/engine-render/src/__tests__/engine-scene-viewport.spec.ts

@@ -333,6 +333,53 @@ describe('engine scene viewport extra', () => {
         engine.dispose();
     });
 
+    it('keeps shared viewport boundary pixels out of incremental scroll copies', () => {
+        const { engine, scene, viewport } = createFixture();
+        viewport.setViewportSize({ left: 46, top: 20, width: 260, height: 160 });
+        const rowHeaderViewport = new Viewport('viewRowBottom', scene, {
+            left: 0,
+            top: 20,
+            width: 47,
+            height: 160,
+            active: true,
+            allowCache: true,
+        });
+        new Viewport('viewColumnRight', scene, {
+            left: 46,
+            top: 0,
+            width: 260,
+            height: 21,
+            active: true,
+            allowCache: true,
+        });
+
+        viewport.updateScrollVal({ scrollX: 0, scrollY: 35, viewportScrollX: 0, viewportScrollY: 35 });
+        rowHeaderViewport.updateScrollVal({ scrollX: 0, scrollY: 35, viewportScrollX: 0, viewportScrollY: 35 });
+        scene.render();
+
+        const ctx = engine.getCanvas().getContext();
+        const drawImageSpy = vi.spyOn(ctx, 'drawImage');
+        const clearRectSpy = vi.spyOn(ctx, 'clearRect');
+
+        viewport.updateScrollVal({ scrollX: 0, scrollY: 0, viewportScrollX: 0, viewportScrollY: 0 });
+        rowHeaderViewport.updateScrollVal({ scrollX: 0, scrollY: 0, viewportScrollX: 0, viewportScrollY: 0 });
+        scene.makeDirtyForScrolling();
+        scene.render();
+
+        const engineScrollCopies = drawImageSpy.mock.calls.filter(([source]) => source === ctx.canvas);
+        const mainCopy = engineScrollCopies.find(([, sourceX, sourceY, sourceWidth]) =>
+            sourceX === 47 && sourceY === 21 && Number(sourceWidth) > 200
+        );
+        expect(mainCopy).toBeDefined();
+        expect(mainCopy?.[5]).toBe(47);
+        expect(Number(mainCopy?.[6])).toBeGreaterThan(Number(mainCopy?.[2]));
+        const copiedOffsetY = Number(mainCopy?.[6]) - Number(mainCopy?.[2]);
+        expect(clearRectSpy.mock.calls).toContainEqual([47, 21, expect.any(Number), copiedOffsetY]);
+
+        scene.dispose();
+        engine.dispose();
+    });
+
     it('uses a full render after an after-render observer mutates the engine canvas', () => {
         const { engine, scene, viewport } = createFixture();
         const layer = scene.getLayer(1);

packages/engine-render/src/components/sheets/watermark/__tests__/watermark-layer.spec.ts

@@ -14,7 +14,8 @@
  * limitations under the License.
  */
 
-import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { Layer } from '../../../../layer';
 import { IWatermarkTypeEnum } from '../type';
 import { WatermarkLayer } from '../watermark-layer';
 
@@ -36,6 +37,9 @@ function createCtx(id = 'main') {
     return {
         getId: vi.fn(() => id),
         save: vi.fn(),
+        beginPath: vi.fn(),
+        rect: vi.fn(),
+        clip: vi.fn(),
         restore: vi.fn(),
     } as any;
 }
@@ -45,6 +49,10 @@ describe('WatermarkLayer', () => {
         renderWatermarkMock.mockClear();
     });
 
+    afterEach(() => {
+        vi.restoreAllMocks();
+    });
+
     it('renders configured text watermark with the latest user info', () => {
         const layer = new WatermarkLayer(createScene() as never);
         const config = {
@@ -137,4 +145,21 @@ describe('WatermarkLayer', () => {
 
         expect(renderWatermarkMock).not.toHaveBeenCalled();
     });
+
+    it('clips incremental watermark rendering to dirty bounds', () => {
+        const layer = new WatermarkLayer(createScene() as never);
+        const ctx = createCtx();
+        const options = {
+            dirtyBounds: [{ left: 10, top: 20, right: 30, bottom: 50 }],
+            preserveCache: true,
+            viewportInfos: new Map(),
+        };
+        const baseRenderSpy = vi.spyOn(Layer.prototype, 'render').mockReturnThis();
+
+        layer.render(ctx, false, options);
+
+        expect(baseRenderSpy).toHaveBeenCalledWith(ctx, false, options);
+        expect(ctx.rect).toHaveBeenCalledWith(10, 20, 20, 30);
+        expect(ctx.clip).toHaveBeenCalledOnce();
+    });
 });

packages/engine-render/src/components/sheets/watermark/watermark-layer.ts

@@ -16,6 +16,7 @@
 
 import type { IUser, Nullable } from '@univerjs/core';
 import type { UniverRenderingContext } from '../../../context';
+import type { ILayerRenderOptions } from '../../../layer';
 import type { IWatermarkConfigWithType } from './type';
 import { Layer } from '../../../layer';
 import { IWatermarkTypeEnum } from './type';
@@ -26,11 +27,20 @@ export class WatermarkLayer extends Layer {
     private _image: Nullable<HTMLImageElement>;
     private _user: Nullable<IUser>;
 
-    override render(ctx?: UniverRenderingContext, isMaxLayer = false) {
-        super.render(ctx, isMaxLayer);
+    override render(ctx?: UniverRenderingContext, isMaxLayer = false, options: ILayerRenderOptions = {}) {
+        super.render(ctx, isMaxLayer, options);
         const mainCtx = ctx || this.scene.getEngine()?.getCanvas().getContext();
         if (mainCtx && mainCtx.getId()) {
+            mainCtx.save();
+            if (options.dirtyBounds) {
+                mainCtx.beginPath();
+                for (const bound of options.dirtyBounds) {
+                    mainCtx.rect(bound.left, bound.top, bound.right - bound.left, bound.bottom - bound.top);
+                }
+                mainCtx.clip();
+            }
             this._renderWatermark(mainCtx);
+            mainCtx.restore();
         }
         return this;
     }

packages/engine-render/src/scene.ts

@@ -42,6 +42,7 @@ const SCROLLBAR_SEEK_SETTLE_MS = 64;
 const SCROLLBAR_SEEK_EXPENSIVE_RENDER_MIN_MS = 32;
 const SCROLLBAR_SEEK_EXPENSIVE_RENDER_FRAME_INTERVALS = 2;
 const SCROLLBAR_SEEK_RENDER_COST_SAMPLE_WEIGHT = 0.25;
+const VIEWPORT_SHARED_EDGE_TOLERANCE = 1;
 
 export interface ISceneInputControlOptions {
     enableDown: boolean;
@@ -108,6 +109,50 @@ function createScrollbarBounds(viewport: Viewport, contentBounds: IBoundRectNoAn
     return scrollbarBounds;
 }
 
+// Viewports are rendered in order, so a later viewport owns any shared boundary pixel.
+// Excluding that pixel from earlier canvas copies prevents fixed headers from leaking into scrollable content.
+function trimSharedViewportEdges(bounds: IBoundRectNoAngle, followingViewportBounds: IBoundRectNoAngle[]) {
+    const result = { ...bounds };
+
+    for (const other of followingViewportBounds) {
+        const overlapWidth = Math.min(result.right, other.right) - Math.max(result.left, other.left);
+        const overlapHeight = Math.min(result.bottom, other.bottom) - Math.max(result.top, other.top);
+        if (overlapWidth <= 0 || overlapHeight <= 0) {
+            continue;
+        }
+
+        const width = result.right - result.left;
+        const height = result.bottom - result.top;
+        const coversWidth = overlapWidth >= width - VIEWPORT_SHARED_EDGE_TOLERANCE;
+        const coversHeight = overlapHeight >= height - VIEWPORT_SHARED_EDGE_TOLERANCE;
+
+        if (coversHeight && other.left <= result.left && other.right > result.left) {
+            result.left = Math.min(other.right, result.right);
+        } else if (coversHeight && other.left < result.right && other.right >= result.right) {
+            result.right = Math.max(other.left, result.left);
+        }
+
+        if (coversWidth && other.top <= result.top && other.bottom > result.top) {
+            result.top = Math.min(other.bottom, result.bottom);
+        } else if (coversWidth && other.top < result.bottom && other.bottom >= result.bottom) {
+            result.bottom = Math.max(other.top, result.top);
+        }
+    }
+
+    return result;
+}
+
+function isInvalidScrollBounds(bounds: IBoundRectNoAngle, offsetX: number, offsetY: number) {
+    const width = bounds.right - bounds.left;
+    const height = bounds.bottom - bounds.top;
+    return !Number.isFinite(offsetX) ||
+        !Number.isFinite(offsetY) ||
+        width <= 0 ||
+        height <= 0 ||
+        Math.abs(offsetX) >= width ||
+        Math.abs(offsetY) >= height;
+}
+
 function createViewportScrollRenderState(viewport: Viewport, scaleX: number, scaleY: number): IViewportScrollRenderState {
     const viewportInfo = viewport.calcViewportInfo();
     const { diffX = 0, diffY = 0, viewPortPosition } = viewportInfo;
@@ -128,24 +173,13 @@ function createViewportScrollRenderState(viewport: Viewport, scaleX: number, sca
         right: viewPortPosition.right - (scrollBar?.enableVertical ? scrollBar.totalSize : 0),
         bottom: viewPortPosition.bottom - (scrollBar?.enableHorizontal ? scrollBar.totalSize : 0),
     };
-    const width = bounds.right - bounds.left;
-    const height = bounds.bottom - bounds.top;
-    const isInvalidScroll = !Number.isFinite(offsetX) ||
-        !Number.isFinite(offsetY) ||
-        width <= 0 ||
-        height <= 0 ||
-        Math.abs(offsetX) >= width ||
-        Math.abs(offsetY) >= height;
-    if (isInvalidScroll) {
+    if (isInvalidScrollBounds(bounds, offsetX, offsetY)) {
         return { canPreserveEngine: false, dirtyBounds: [], viewportInfo };
     }
 
     return {
         canPreserveEngine: true,
-        dirtyBounds: [
-            ...createExposedScrollBounds(bounds, offsetX, offsetY),
-            ...createScrollbarBounds(viewport, bounds, viewPortPosition),
-        ],
+        dirtyBounds: createScrollbarBounds(viewport, bounds, viewPortPosition),
         scrollRenderInfo: { bounds, offsetX, offsetY },
         viewportInfo,
     };
@@ -880,21 +914,31 @@ export class Scene extends Disposable {
         const { scaleX, scaleY } = this.getAncestorScale();
         let canPreserveEngine = true;
 
-        for (const viewport of this._viewports) {
-            if (!viewport.shouldIntoRender()) {
-                continue;
-            }
+        const viewportStates = this._viewports
+            .filter((viewport) => viewport.shouldIntoRender())
+            .map((viewport) => createViewportScrollRenderState(viewport, scaleX, scaleY));
 
-            const viewportState = createViewportScrollRenderState(viewport, scaleX, scaleY);
+        for (const [index, viewportState] of viewportStates.entries()) {
             const { viewportInfo, scrollRenderInfo } = viewportState;
-            viewportInfos.set(viewport.viewportKey, viewportInfo);
+            viewportInfos.set(viewportInfo.viewportKey, viewportInfo);
             if (!viewportState.canPreserveEngine) {
                 canPreserveEngine = false;
                 continue;
             }
             dirtyBounds.push(...viewportState.dirtyBounds);
             if (scrollRenderInfo) {
-                scrollRenderInfos.push(scrollRenderInfo);
+                const followingViewportBounds = viewportStates
+                    .slice(index + 1)
+                    .map((state) => state.viewportInfo.viewPortPosition)
+                    .filter((bounds): bounds is IBoundRectNoAngle => bounds != null);
+                const bounds = trimSharedViewportEdges(scrollRenderInfo.bounds, followingViewportBounds);
+                const { offsetX, offsetY } = scrollRenderInfo;
+                if (isInvalidScrollBounds(bounds, offsetX, offsetY)) {
+                    canPreserveEngine = false;
+                    continue;
+                }
+                dirtyBounds.push(...createExposedScrollBounds(bounds, offsetX, offsetY));
+                scrollRenderInfos.push({ bounds, offsetX, offsetY });
             }
         }
 

packages/sheets-ui/src/services/selection/__tests__/selection-layer.spec.ts

@@ -0,0 +1,43 @@
+/**
+ * Copyright 2023-present DreamNum Co., Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import type { ILayerRenderOptions, UniverRenderingContext } from '@univerjs/engine-render';
+import { Layer } from '@univerjs/engine-render';
+import { describe, expect, it, vi } from 'vitest';
+import { SelectionLayer } from '../selection-layer';
+
+describe('SelectionLayer', () => {
+    it('forwards incremental render options to the base layer', () => {
+        const next = vi.fn();
+        const scene = {
+            getEngine: () => ({
+                renderFrameTimeMetric$: { next },
+            }),
+        };
+        const ctx = {} as UniverRenderingContext;
+        const options: ILayerRenderOptions = {
+            dirtyBounds: [{ left: 0, top: 0, right: 100, bottom: 24 }],
+            preserveCache: true,
+            viewportInfos: new Map(),
+        };
+        const render = vi.spyOn(Layer.prototype, 'render').mockReturnThis();
+        const layer = new SelectionLayer(scene as never);
+
+        expect(layer.render(ctx, false, options)).toBe(layer);
+        expect(render).toHaveBeenCalledWith(ctx, false, options);
+        expect(next).toHaveBeenCalledWith(['selectionLayer', expect.any(Number)]);
+    });
+});

packages/sheets-ui/src/services/selection/selection-layer.ts

@@ -14,14 +14,14 @@
  * limitations under the License.
  */
 
-import type { Engine, UniverRenderingContext } from '@univerjs/engine-render';
+import type { Engine, ILayerRenderOptions, UniverRenderingContext } from '@univerjs/engine-render';
 import { Tools } from '@univerjs/core';
 import { Layer } from '@univerjs/engine-render';
 
 export class SelectionLayer extends Layer {
-    override render(ctx?: UniverRenderingContext, isMaxLayer = false) {
+    override render(ctx?: UniverRenderingContext, isMaxLayer = false, options: ILayerRenderOptions = {}) {
         const startTime = Tools.now();
-        super.render(ctx, isMaxLayer);
+        super.render(ctx, isMaxLayer, options);
         this._afterRender(startTime);
         return this;
     }