Skip to content

Commit

Permalink
re microsoft#126880. listener on cell view model triggers mem leak mo…
Browse files Browse the repository at this point in the history
…nitor which slows down scrolling.
  • Loading branch information
rebornix authored and utajum committed Sep 13, 2021
1 parent 9f19c19 commit d087efc
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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 = {
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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));
}
}));
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export interface NotebookLayoutConfiguration {
editorOptionsCustomizations: any | undefined;
}

interface NotebookOptionsChangeEvent {
export interface NotebookOptionsChangeEvent {
cellStatusBarVisibility?: boolean;
cellToolbarLocation?: boolean;
cellToolbarInteraction?: boolean;
Expand Down

0 comments on commit d087efc

Please sign in to comment.