Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(sheet): optimize memory release and resolve issues with the editor being recreated repeatedly #1432

Merged
merged 6 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 27 additions & 22 deletions packages/docs-ui/src/controllers/page-render.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
IUniverInstanceService,
LifecycleStages,
OnLifecycle,
toDisposable,
} from '@univerjs/core';
import type { Documents, IPageRenderConfig } from '@univerjs/engine-render';
import { IRenderManagerService, Rect } from '@univerjs/engine-render';
Expand Down Expand Up @@ -68,28 +69,32 @@ export class PageRenderController extends Disposable {

const pageSize = docsComponent.getSkeleton()?.getPageSize();

docsComponent.onPageRenderObservable.add((config: IPageRenderConfig) => {
if (this._editorService.isEditor(unitId)) {
return;
}

// Draw page borders
const { page, pageLeft, pageTop, ctx } = config;
const { width, pageWidth, height, pageHeight } = page;

ctx.save();

ctx.translate(pageLeft - 0.5, pageTop - 0.5);
Rect.drawWith(ctx, {
width: pageSize?.width ?? pageWidth ?? width,
height: pageSize?.height ?? pageHeight ?? height,
strokeWidth: 1,
stroke: PAGE_STROKE_COLOR,
fill: PAGE_FILL_COLOR,
zIndex: 3,
});
ctx.restore();
});
this.disposeWithMe(
toDisposable(
docsComponent.onPageRenderObservable.add((config: IPageRenderConfig) => {
if (this._editorService.isEditor(unitId)) {
return;
}

// Draw page borders
const { page, pageLeft, pageTop, ctx } = config;
const { width, pageWidth, height, pageHeight } = page;

ctx.save();

ctx.translate(pageLeft - 0.5, pageTop - 0.5);
Rect.drawWith(ctx, {
width: pageSize?.width ?? pageWidth ?? width,
height: pageSize?.height ?? pageHeight ?? height,
strokeWidth: 1,
stroke: PAGE_STROKE_COLOR,
fill: PAGE_FILL_COLOR,
zIndex: 3,
});
ctx.restore();
})
)
);
});
}

Expand Down
5 changes: 1 addition & 4 deletions packages/docs-ui/src/views/doc-canvas-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ export class DocCanvasView extends RxDisposable {

private _currentDocumentModel!: DocumentDataModel;

private _loadedMap = new Set();

private readonly _fps$ = new BehaviorSubject<string>('');

readonly fps$ = this._fps$.asObservable();
Expand Down Expand Up @@ -79,9 +77,8 @@ export class DocCanvasView extends RxDisposable {

this._currentDocumentModel = model;

if (!this._loadedMap.has(unitId)) {
if (!this._renderManagerService.has(unitId)) {
this._addNewRender();
this._loadedMap.add(unitId);
}
}

Expand Down
13 changes: 9 additions & 4 deletions packages/docs/src/controllers/text-selection.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import type { ICommandInfo, Nullable } from '@univerjs/core';
import { Disposable, ICommandService, IUniverInstanceService, LifecycleStages, OnLifecycle, toDisposable } from '@univerjs/core';
import type { Documents, IMouseEvent, IPointerEvent } from '@univerjs/engine-render';
import type { Documents, IMouseEvent, IPointerEvent, RenderComponentType } from '@univerjs/engine-render';
import { CURSOR_TYPE, IRenderManagerService, ITextSelectionRenderManager } from '@univerjs/engine-render';
import { Inject } from '@wendellhu/redi';

Expand All @@ -28,7 +28,7 @@ import { TextSelectionManagerService } from '../services/text-selection-manager.

@OnLifecycle(LifecycleStages.Rendered, TextSelectionController)
export class TextSelectionController extends Disposable {
private _loadedMap = new Set();
private _loadedMap = new WeakSet<RenderComponentType>();

constructor(
@Inject(DocSkeletonManagerService) private readonly _docSkeletonManagerService: DocSkeletonManagerService,
Expand Down Expand Up @@ -71,9 +71,14 @@ export class TextSelectionController extends Disposable {
return;
}

if (!this._loadedMap.has(unitId)) {
const docObject = this._getDocObjectById(unitId);
if (docObject == null || docObject.document == null) {
return;
}

if (!this._loadedMap.has(docObject.document)) {
Jocs marked this conversation as resolved.
Show resolved Hide resolved
this._initialMain(unitId);
this._loadedMap.add(unitId);
this._loadedMap.add(docObject.document);
}
}

Expand Down
11 changes: 9 additions & 2 deletions packages/docs/src/services/doc-skeleton-manager.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import type { Nullable } from '@univerjs/core';
import { LocaleService, RxDisposable } from '@univerjs/core';
import { IUniverInstanceService, LocaleService, RxDisposable } from '@univerjs/core';
import type { DocumentViewModel } from '@univerjs/engine-render';
import { DocumentSkeleton } from '@univerjs/engine-render';
import { Inject } from '@wendellhu/redi';
Expand Down Expand Up @@ -48,7 +48,8 @@ export class DocSkeletonManagerService extends RxDisposable {

constructor(
@Inject(LocaleService) private readonly _localeService: LocaleService,
@Inject(DocViewModelManagerService) private readonly _docViewModelManagerService: DocViewModelManagerService
@Inject(DocViewModelManagerService) private readonly _docViewModelManagerService: DocViewModelManagerService,
@IUniverInstanceService private readonly _currentUniverService: IUniverInstanceService
) {
super();
this._initialize();
Expand Down Expand Up @@ -82,6 +83,12 @@ export class DocSkeletonManagerService extends RxDisposable {

Jocs marked this conversation as resolved.
Show resolved Hide resolved
this._setCurrent(docViewModel);
});

this._currentUniverService.docDisposed$.pipe(takeUntil(this.dispose$)).subscribe((documentModel) => {
this._docSkeletonMap.delete(documentModel.getUnitId());

this._currentSkeletonUnitId = this._currentUniverService.getCurrentUniverDocInstance().getUnitId();
Jocs marked this conversation as resolved.
Show resolved Hide resolved
});
}

getCurrent(): Nullable<IDocSkeletonManagerParam> {
Expand Down
13 changes: 5 additions & 8 deletions packages/docs/src/services/doc-view-model-manager.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ export interface IDocumentViewModelManagerParam {
* The view model manager is used to manage Doc view model. has a one-to-one correspondence with the doc skeleton.
*/
export class DocViewModelManagerService extends RxDisposable {
private _currentViewModelUnitId: string = '';
private _docViewModelMap: Map<string, IDocumentViewModelManagerParam> = new Map();

private readonly _currentDocViewModel$ = new BehaviorSubject<Nullable<IDocumentViewModelManagerParam>>(null);
Expand Down Expand Up @@ -56,6 +55,10 @@ export class DocViewModelManagerService extends RxDisposable {
this._currentUniverService.getAllUniverDocsInstance().forEach((documentModel) => {
this._create(documentModel);
});

this._currentUniverService.docDisposed$.pipe(takeUntil(this.dispose$)).subscribe((documentModel) => {
this._docViewModelMap.delete(documentModel.getUnitId());
});
}

private _create(documentModel: Nullable<DocumentDataModel>) {
Expand All @@ -68,10 +71,6 @@ export class DocViewModelManagerService extends RxDisposable {
this._setCurrent(unitId);
}

getCurrent() {
return this._docViewModelMap.get(this._currentViewModelUnitId);
}

getAllModel() {
return this._docViewModelMap;
}
Expand Down Expand Up @@ -111,9 +110,7 @@ export class DocViewModelManagerService extends RxDisposable {
docViewModel.reset(documentDataModel);
}

this._currentViewModelUnitId = unitId;

this._currentDocViewModel$.next(this.getCurrent());
this._currentDocViewModel$.next(this._docViewModelMap.get(unitId));
}

private _buildDocViewModel(documentDataModel: DocumentDataModel) {
Expand Down
3 changes: 3 additions & 0 deletions packages/engine-render/src/base-object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,7 @@ export abstract class BaseObject {
}

dispose() {
this.onTransformChangeObservable.clear();
this.onPointerDownObserver.clear();
this.onPointerMoveObserver.clear();
this.onPointerUpObserver.clear();
Expand All @@ -702,6 +703,8 @@ export abstract class BaseObject {
this.onDisposeObserver.notifyObservers(this);

this._makeDirtyMix();

this.onDisposeObserver.clear();
}

toJson() {
Expand Down
18 changes: 13 additions & 5 deletions packages/engine-render/src/layer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@
*/

import type { Nullable } from '@univerjs/core';
import { sortRules } from '@univerjs/core';
import { Disposable, sortRules, toDisposable } from '@univerjs/core';

import { BaseObject } from './base-object';
import { RENDER_CLASS_TYPE } from './basics/const';
import { Canvas } from './canvas';
import type { UniverRenderingContext } from './context';
import type { ThinScene } from './thin-scene';

export class Layer {
export class Layer extends Disposable {
private _objects: BaseObject[] = [];

private _cacheCanvas: Nullable<Canvas>;
Expand All @@ -36,6 +36,8 @@ export class Layer {
private _zIndex: number = 1,
private _allowCache: boolean = false
) {
super();

this.addObjects(objects);

if (this._allowCache) {
Expand Down Expand Up @@ -203,9 +205,13 @@ export class Layer {

private _initialCacheCanvas() {
this._cacheCanvas = new Canvas();
this._scene.getEngine().onTransformChangeObservable.add(() => {
this._resizeCacheCanvas();
});
this.disposeWithMe(
toDisposable(
this._scene.getEngine().onTransformChangeObservable.add(() => {
this._resizeCacheCanvas();
})
)
);
}

private _draw(mainCtx: UniverRenderingContext, isMaxLayer: boolean) {
Expand All @@ -232,6 +238,8 @@ export class Layer {
}

dispose() {
super.dispose();

this.getObjects().forEach((o) => {
o.dispose();
});
Expand Down
5 changes: 5 additions & 0 deletions packages/engine-render/src/render-manager.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export interface IRenderManagerService {
create(unitId: Nullable<string>): void;
getCurrent(): Nullable<IRender>;
getFirst(): Nullable<IRender>;
has(unitId: string): boolean;
}

export type RenderComponentType = SheetComponent | DocComponent | Slide | BaseObject;
Expand Down Expand Up @@ -166,6 +167,10 @@ export class RenderManagerService implements IRenderManagerService {
this._renderMap.delete(unitId);
}

has(unitId: string) {
return this._renderMap.has(unitId);
}

setCurrent(unitId: string) {
this._currentUnitId = unitId;

Expand Down
Loading
Loading