From d087efc95da941a0f4734257ffe819e0b81a10d2 Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 9 Sep 2021 14:42:05 -0700 Subject: [PATCH] re #126880. listener on cell view model triggers mem leak monitor which slows down scrolling. --- .../browser/viewModel/baseCellViewModel.ts | 3 +- .../browser/viewModel/codeCellViewModel.ts | 13 ++++--- .../browser/viewModel/markupCellViewModel.ts | 39 ++++++++++--------- .../browser/viewModel/notebookViewModel.ts | 8 ++++ .../notebook/common/notebookOptions.ts | 2 +- 5 files changed, 38 insertions(+), 27 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/baseCellViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/baseCellViewModel.ts index 6be6cc90e1fd9e..da24f732d410cc 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/baseCellViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/baseCellViewModel.ts @@ -20,6 +20,7 @@ import { CellEditState, CellFocusMode, CellViewModelStateChangeEvent, CursorAtBo import { ViewContext } from 'vs/workbench/contrib/notebook/browser/viewModel/viewContext'; import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; import { CellKind, INotebookCellStatusBarItem, INotebookSearchOptions } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { NotebookOptionsChangeEvent } from 'vs/workbench/contrib/notebook/common/notebookOptions'; export abstract class BaseCellViewModel extends Disposable { @@ -178,7 +179,7 @@ export abstract class BaseCellViewModel extends Disposable { } - + abstract updateOptions(e: NotebookOptionsChangeEvent): void; abstract hasDynamicHeight(): boolean; abstract getHeight(lineHeight: number): number; abstract onDeselect(): void; diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts index 63f29f7dd57a39..9de1fc92babd95 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts @@ -17,6 +17,7 @@ import { ViewContext } from 'vs/workbench/contrib/notebook/browser/viewModel/vie import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; import { CellKind, INotebookSearchOptions, NotebookCellOutputsSplice } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookKeymapService } from 'vs/workbench/contrib/notebook/common/notebookKeymapService'; +import { NotebookOptionsChangeEvent } from 'vs/workbench/contrib/notebook/common/notebookOptions'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; import { BaseCellViewModel } from './baseCellViewModel'; @@ -134,12 +135,6 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod } })); - this._register(this.viewContext.notebookOptions.onDidChangeOptions(e => { - if (e.cellStatusBarVisibility || e.insertToolbarPosition || e.cellToolbarLocation) { - this.layoutChange({}); - } - })); - this._outputCollection = new Array(this.model.outputs.length); this._layoutInfo = { @@ -159,6 +154,12 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod }; } + updateOptions(e: NotebookOptionsChangeEvent) { + if (e.cellStatusBarVisibility || e.insertToolbarPosition || e.cellToolbarLocation) { + this.layoutChange({}); + } + } + layoutChange(state: CodeCellLayoutChangeEvent, source?: string) { // recompute this._ensureOutputsTop(); diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/markupCellViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/markupCellViewModel.ts index e6b1d1d05d0854..629b7e81072f2c 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/markupCellViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/markupCellViewModel.ts @@ -16,6 +16,7 @@ import { ITextModelService } from 'vs/editor/common/services/resolverService'; import { ViewContext } from 'vs/workbench/contrib/notebook/browser/viewModel/viewContext'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IUndoRedoService } from 'vs/platform/undoRedo/common/undoRedo'; +import { NotebookOptionsChangeEvent } from 'vs/workbench/contrib/notebook/common/notebookOptions'; export class MarkupCellViewModel extends BaseCellViewModel implements ICellViewModel { @@ -141,27 +142,27 @@ export class MarkupCellViewModel extends BaseCellViewModel implements ICellViewM this._onDidHideInput.fire(); } })); + } + + updateOptions(e: NotebookOptionsChangeEvent) { + if (e.cellStatusBarVisibility || e.insertToolbarPosition || e.cellToolbarLocation) { + const layoutConfiguration = this.viewContext.notebookOptions.getLayoutConfiguration(); + const { bottomToolbarGap } = this.viewContext.notebookOptions.computeBottomToolbarDimensions(this.viewType); - this._register(this.viewContext.notebookOptions.onDidChangeOptions(e => { - if (e.cellStatusBarVisibility || e.insertToolbarPosition || e.cellToolbarLocation) { - const layoutConfiguration = this.viewContext.notebookOptions.getLayoutConfiguration(); - const { bottomToolbarGap } = this.viewContext.notebookOptions.computeBottomToolbarDimensions(this.viewType); - - if (this.getEditState() === CellEditState.Editing) { - this._updateTotalHeight(this._editorHeight - + layoutConfiguration.markdownCellTopMargin - + layoutConfiguration.markdownCellBottomMargin - + bottomToolbarGap - + this.viewContext.notebookOptions.computeStatusBarHeight()); - } else { - // @rebornix - // On file open, the previewHeight + bottomToolbarGap for a cell out of viewport can be 0 - // When it's 0, the list view will never try to render it anymore even if we scroll the cell into view. - // Thus we make sure it's greater than 0 - this._updateTotalHeight(Math.max(1, this._previewHeight + bottomToolbarGap)); - } + if (this.getEditState() === CellEditState.Editing) { + this._updateTotalHeight(this._editorHeight + + layoutConfiguration.markdownCellTopMargin + + layoutConfiguration.markdownCellBottomMargin + + bottomToolbarGap + + this.viewContext.notebookOptions.computeStatusBarHeight()); + } else { + // @rebornix + // On file open, the previewHeight + bottomToolbarGap for a cell out of viewport can be 0 + // When it's 0, the list view will never try to render it anymore even if we scroll the cell into view. + // Thus we make sure it's greater than 0 + this._updateTotalHeight(Math.max(1, this._previewHeight + bottomToolbarGap)); } - })); + } } /** diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts index 3bb0dfb7e9581b..9e9f851f86c9b8 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel.ts @@ -349,6 +349,14 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD }); })); + this._register(this._viewContext.notebookOptions.onDidChangeOptions(e => { + for (let i = 0; i < this.length; i++) { + const cell = this._viewCells[i]; + cell.updateOptions(e); + } + })); + + this._register(this._selectionCollection.onDidChangeSelection(e => { this._onDidChangeSelection.fire(e); })); diff --git a/src/vs/workbench/contrib/notebook/common/notebookOptions.ts b/src/vs/workbench/contrib/notebook/common/notebookOptions.ts index c98433465a4afc..d3632a4dd97285 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookOptions.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookOptions.ts @@ -61,7 +61,7 @@ export interface NotebookLayoutConfiguration { editorOptionsCustomizations: any | undefined; } -interface NotebookOptionsChangeEvent { +export interface NotebookOptionsChangeEvent { cellStatusBarVisibility?: boolean; cellToolbarLocation?: boolean; cellToolbarInteraction?: boolean;