Skip to content

Commit

Permalink
fix(sheets-drawing-ui): sheet-drawing memory leak (#4067)
Browse files Browse the repository at this point in the history
  • Loading branch information
@weird94
weird94 authored Nov 14, 2024
1 parent 7550f1c commit 4e64ebb
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 28 deletions.
18 changes: 18 additions & 0 deletions mockdata/src/sheets/demo/default-workbook-data-demo.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24136,6 +24136,24 @@ export const DEFAULT_WORKBOOK_DATA_DEMO: IWorkbookData = {
'UPBpL-A3RvKoOqPdd8VIq',
],
},
'dv-test': {
data: {
'UPBpL-test': {
unitId: 'workbook-01',
subUnitId: 'dv-test',
drawingId: 'UPBpL-test',
drawingType: 8,
componentKey: 'ImageDemo',
sheetTransform: {
from: { column: 2, columnOffset: 8, row: 9, rowOffset: 9 },
to: { column: 4, columnOffset: 62, row: 23, rowOffset: 0 },
},
transform: { flipY: false, flipX: false, angle: 0, skewX: 0, skewY: 0, left: 200, top: 200, width: 200, height: 200 },
data: { aa: '128' },
},
},
order: ['UPBpL-test'],
},
}),
},
{
Expand Down
30 changes: 15 additions & 15 deletions packages/sheets-drawing-ui/src/controllers/sheet-drawing-update.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,26 @@
* limitations under the License.
*/

import type { IRange, Nullable, Workbook } from '@univerjs/core';
import type { IImageData, IImageIoServiceParam } from '@univerjs/drawing';
import type { IRenderContext, IRenderModule } from '@univerjs/engine-render';
import type { WorkbookSelections } from '@univerjs/sheets';
import type { ISheetDrawing, ISheetDrawingPosition } from '@univerjs/sheets-drawing';
import type { IInsertDrawingCommandParams, ISetDrawingCommandParams } from '../commands/commands/interfaces';
import type { ISetDrawingArrangeCommandParams } from '../commands/commands/set-drawing-arrange.command';
import { Disposable, FOCUSING_COMMON_DRAWINGS, ICommandService, IContextService, Inject, LocaleService } from '@univerjs/core';
import { MessageType } from '@univerjs/design';
import { DRAWING_IMAGE_ALLOW_IMAGE_LIST, DRAWING_IMAGE_ALLOW_SIZE, DRAWING_IMAGE_COUNT_LIMIT, DRAWING_IMAGE_HEIGHT_LIMIT, DRAWING_IMAGE_WIDTH_LIMIT, DrawingTypeEnum, getImageSize, IDrawingManagerService, IImageIoService, ImageUploadStatusType } from '@univerjs/drawing';
import { SheetsSelectionsService } from '@univerjs/sheets';
import { ISheetDrawingService } from '@univerjs/sheets-drawing';
import { attachRangeWithCoord, ISheetSelectionRenderService, SheetSkeletonManagerService } from '@univerjs/sheets-ui';
import { ILocalFileService, IMessageService } from '@univerjs/ui';
import type { IRange, Nullable, Workbook } from '@univerjs/core';
import type { IImageData, IImageIoServiceParam } from '@univerjs/drawing';
import type { IRenderContext, IRenderModule } from '@univerjs/engine-render';
import type { WorkbookSelections } from '@univerjs/sheets';
import type { ISheetDrawing, ISheetDrawingPosition } from '@univerjs/sheets-drawing';
import { drawingPositionToTransform, transformToDrawingPosition } from '../basics/transform-position';
import { GroupSheetDrawingCommand } from '../commands/commands/group-sheet-drawing.command';
import { InsertSheetDrawingCommand } from '../commands/commands/insert-sheet-drawing.command';
import { SetDrawingArrangeCommand } from '../commands/commands/set-drawing-arrange.command';
import { SetSheetDrawingCommand } from '../commands/commands/set-sheet-drawing.command';
import { UngroupSheetDrawingCommand } from '../commands/commands/ungroup-sheet-drawing.command';
import type { IInsertDrawingCommandParams, ISetDrawingCommandParams } from '../commands/commands/interfaces';
import type { ISetDrawingArrangeCommandParams } from '../commands/commands/set-drawing-arrange.command';

export class SheetDrawingUpdateController extends Disposable implements IRenderModule {
private readonly _workbookSelections: WorkbookSelections;
Expand Down Expand Up @@ -241,7 +241,7 @@ export class SheetDrawingUpdateController extends Disposable implements IRenderM
}

private _updateOrderListener() {
this._drawingManagerService.featurePluginOrderUpdate$.subscribe((params) => {
this.disposeWithMe(this._drawingManagerService.featurePluginOrderUpdate$.subscribe((params) => {
const { unitId, subUnitId, drawingIds, arrangeType } = params;

this._commandService.executeCommand(SetDrawingArrangeCommand.id, {
Expand All @@ -250,11 +250,11 @@ export class SheetDrawingUpdateController extends Disposable implements IRenderM
drawingIds,
arrangeType,
} as ISetDrawingArrangeCommandParams);
});
}));
}

private _updateImageListener() {
this._drawingManagerService.featurePluginUpdate$.subscribe((params) => {
this.disposeWithMe(this._drawingManagerService.featurePluginUpdate$.subscribe((params) => {
const drawings: Partial<ISheetDrawing>[] = [];

if (params.length === 0) {
Expand Down Expand Up @@ -300,19 +300,19 @@ export class SheetDrawingUpdateController extends Disposable implements IRenderM
drawings,
} as ISetDrawingCommandParams);
}
});
}));
}

private _groupDrawingListener() {
this._drawingManagerService.featurePluginGroupUpdate$.subscribe((params) => {
this.disposeWithMe(this._drawingManagerService.featurePluginGroupUpdate$.subscribe((params) => {
this._commandService.executeCommand(GroupSheetDrawingCommand.id, params);
const { unitId, subUnitId, drawingId } = params[0].parent;
this._drawingManagerService.focusDrawing([{ unitId, subUnitId, drawingId }]);
});
}));

this._drawingManagerService.featurePluginUngroupUpdate$.subscribe((params) => {
this.disposeWithMe(this._drawingManagerService.featurePluginUngroupUpdate$.subscribe((params) => {
this._commandService.executeCommand(UngroupSheetDrawingCommand.id, params);
});
}));
}

private _focusDrawingListener() {
Expand Down
17 changes: 7 additions & 10 deletions packages/sheets-drawing-ui/src/services/canvas-float-dom-manager.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -430,10 +430,6 @@ export class SheetCanvasFloatDomManagerService extends Disposable {
}

private _scrollUpdateListener() {
const workbook = this._univerInstanceService.getCurrentUnitForType<Workbook>(UniverInstanceType.UNIVER_SHEET)!;
if (!workbook) {
return;
}
const updateSheet = (unitId: string, subUnitId: string) => {
const renderObject = this._getSceneAndTransformerByDrawingSearch(unitId);
const map = this._ensureMap(unitId, subUnitId);
Expand All @@ -453,21 +449,22 @@ export class SheetCanvasFloatDomManagerService extends Disposable {
};

this.disposeWithMe(
workbook.activeSheet$.pipe(
this._univerInstanceService.getCurrentTypeOfUnit$<Workbook>(UniverInstanceType.UNIVER_SHEET).pipe(
filter((workbook) => !!workbook),
switchMap((workbook) => workbook.activeSheet$),
filter((sheet) => !!sheet),
map((sheet) => {
const render = this._renderManagerService.getRenderById(sheet.getUnitId());
return render ? { render, unitId: workbook.getUnitId(), subUnitId: sheet.getSheetId() } : null;
return render ? { render, unitId: sheet.getUnitId(), subUnitId: sheet.getSheetId() } : null;
}),
filter((render) => !!render),
switchMap((render) =>
fromEventSubject(render.render.scene.getViewport(VIEWPORT_KEY.VIEW_MAIN)!.onScrollAfter$)
.pipe(map(() => ({ unitId: render.unitId, subUnitId: render.subUnitId })))
)
)
.subscribe(({ unitId, subUnitId }) => {
updateSheet(unitId, subUnitId);
})
).subscribe(({ unitId, subUnitId }) => {
updateSheet(unitId, subUnitId);
})
);

this.disposeWithMe(this._commandService.onCommandExecuted((commandInfo) => {
Expand Down
13 changes: 10 additions & 3 deletions packages/ui/src/views/components/dom/FloatDom.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ const FloatDomSingle = memo((props: { layer: IFloatDom; id: string }) => {
const domRef = useRef<HTMLDivElement>(null);
const innerDomRef = useRef<HTMLDivElement>(null);
const transformRef = useRef<string>(`transform: rotate(${position?.rotate}deg) translate(${position?.startX}px, ${position?.startY}px)`);
const topRef = useRef<number>(position?.startY ?? 0);
const leftRef = useRef<number>(position?.startX ?? 0);
const innerStyle = useRef<React.CSSProperties>({

});
Expand All @@ -50,9 +52,13 @@ const FloatDomSingle = memo((props: { layer: IFloatDom; id: string }) => {

useEffect(() => {
const subscription = layer.position$.subscribe((position) => {
transformRef.current = `rotate(${position.rotate}deg) translate(${position.startX}px, ${position.startY}px)`;
transformRef.current = `rotate(${position.rotate}deg)`;
topRef.current = position.startY;
leftRef.current = position.startX;
if (domRef.current) {
domRef.current.style.transform = transformRef.current;
domRef.current.style.top = `${topRef.current}px`;
domRef.current.style.left = `${leftRef.current}px`;
}
});

Expand Down Expand Up @@ -100,12 +106,13 @@ const FloatDomSingle = memo((props: { layer: IFloatDom; id: string }) => {
className={styles.floatDomWrapper}
style={{
position: 'absolute',
top: 0,
left: 0,
top: topRef.current,
left: leftRef.current,
width: Math.max(position.endX - position.startX - 2, 0),
height: Math.max(position.endY - position.startY - 2, 0),
transform: transformRef.current,
overflow: 'hidden',
transformOrigin: 'center center',
}}
onPointerMove={(e) => {
layer.onPointerMove(e.nativeEvent);
Expand Down

0 comments on commit 4e64ebb

Please sign in to comment.