From 7ec1ec3c4332ab21e02ed3fd582fbe64c65e9bbd Mon Sep 17 00:00:00 2001 From: "Nicola Fiori (JD342)" Date: Tue, 25 Apr 2017 12:45:02 +0200 Subject: [PATCH 01/14] Implement smooth scrolling feature * Apply smooth scrolling in every scrollable element when mouse wheel scroll occurs * Apply smooth scrolling in editor view when large jumps occur (e.g. when the cursor is moved by pressing PageUp or PageDown) --- .../browser/ui/scrollbar/abstractScrollbar.ts | 6 +- .../ui/scrollbar/horizontalScrollbar.ts | 4 +- .../browser/ui/scrollbar/scrollableElement.ts | 15 ++- .../ui/scrollbar/scrollableElementOptions.ts | 12 ++ .../browser/ui/scrollbar/verticalScrollbar.ts | 4 +- src/vs/base/common/scrollable.ts | 109 ++++++++++++++++-- src/vs/editor/common/viewLayout/viewLayout.ts | 16 ++- 7 files changed, 143 insertions(+), 23 deletions(-) diff --git a/src/vs/base/browser/ui/scrollbar/abstractScrollbar.ts b/src/vs/base/browser/ui/scrollbar/abstractScrollbar.ts index 9d6879f176f3b..58746020ae035 100644 --- a/src/vs/base/browser/ui/scrollbar/abstractScrollbar.ts +++ b/src/vs/base/browser/ui/scrollbar/abstractScrollbar.ts @@ -236,11 +236,11 @@ export abstract class AbstractScrollbar extends Widget { return this._scrollbarState.validateScrollPosition(desiredScrollPosition); } - public setDesiredScrollPosition(desiredScrollPosition: number): boolean { + public setDesiredScrollPosition(desiredScrollPosition: number, smoothScrollDuration?: number): boolean { desiredScrollPosition = this.validateScrollPosition(desiredScrollPosition); let oldScrollPosition = this._getScrollPosition(); - this._setScrollPosition(desiredScrollPosition); + this._setScrollPosition(desiredScrollPosition, smoothScrollDuration); let newScrollPosition = this._getScrollPosition(); if (oldScrollPosition !== newScrollPosition) { @@ -258,5 +258,5 @@ export abstract class AbstractScrollbar extends Widget { protected abstract _sliderMousePosition(e: IMouseMoveEventData): number; protected abstract _sliderOrthogonalMousePosition(e: IMouseMoveEventData): number; protected abstract _getScrollPosition(): number; - protected abstract _setScrollPosition(elementScrollPosition: number): void; + protected abstract _setScrollPosition(elementScrollPosition: number, smoothScrollDuration?: number): void; } diff --git a/src/vs/base/browser/ui/scrollbar/horizontalScrollbar.ts b/src/vs/base/browser/ui/scrollbar/horizontalScrollbar.ts index bb0d255d2dee0..2d90176bf4eba 100644 --- a/src/vs/base/browser/ui/scrollbar/horizontalScrollbar.ts +++ b/src/vs/base/browser/ui/scrollbar/horizontalScrollbar.ts @@ -101,9 +101,9 @@ export class HorizontalScrollbar extends AbstractScrollbar { return scrollState.scrollLeft; } - protected _setScrollPosition(scrollPosition: number) { + protected _setScrollPosition(scrollPosition: number, smoothScrollDuration?: number) { this._scrollable.updateState({ scrollLeft: scrollPosition - }); + }, smoothScrollDuration); } } diff --git a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts index 356057395782e..097bf54aa17a6 100644 --- a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts +++ b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts @@ -23,6 +23,7 @@ import Event, { Emitter } from 'vs/base/common/event'; const HIDE_TIMEOUT = 500; const SCROLL_WHEEL_SENSITIVITY = 50; +const SCROLL_WHEEL_SMOOTH_SCROLL_TRESHOLD = 1.7; export interface IOverviewRulerLayoutInfo { parent: HTMLElement; @@ -240,7 +241,7 @@ export class ScrollableElement extends Widget { } } - const scrollState = this._scrollable.getState(); + const scrollState = this._scrollable.getSmoothScrollTargetState(); if (deltaY) { let currentScrollTop = scrollState.scrollTop; desiredScrollTop = this._verticalScrollbar.validateScrollPosition((desiredScrollTop !== -1 ? desiredScrollTop : currentScrollTop) - SCROLL_WHEEL_SENSITIVITY * deltaY); @@ -258,11 +259,17 @@ export class ScrollableElement extends Widget { if (desiredScrollTop !== -1 || desiredScrollLeft !== -1) { if (desiredScrollTop !== -1) { - this._shouldRender = this._verticalScrollbar.setDesiredScrollPosition(desiredScrollTop) || this._shouldRender; + // If |∆y| is too small then do not apply smooth scroll animation, because in that case the input source must be a touchpad or something similar. + const applySmoothScroll = this._options.mouseWheelSmoothScroll && Math.abs(deltaY) > SCROLL_WHEEL_SMOOTH_SCROLL_TRESHOLD; + const shouldRender = this._verticalScrollbar.setDesiredScrollPosition(desiredScrollTop, applySmoothScroll ? this._options.mouseWheelSmoothScrollDuration : undefined); + this._shouldRender = shouldRender || this._shouldRender; desiredScrollTop = -1; } if (desiredScrollLeft !== -1) { - this._shouldRender = this._horizontalScrollbar.setDesiredScrollPosition(desiredScrollLeft) || this._shouldRender; + // If |∆x| is too small then do not apply smooth scroll animation, because in that case the input source must be a touchpad or something similar. + const applySmoothScroll = this._options.mouseWheelSmoothScroll && Math.abs(deltaX) > SCROLL_WHEEL_SMOOTH_SCROLL_TRESHOLD; + const shouldRender = this._horizontalScrollbar.setDesiredScrollPosition(desiredScrollLeft, applySmoothScroll ? this._options.mouseWheelSmoothScrollDuration : undefined); + this._shouldRender = shouldRender || this._shouldRender; desiredScrollLeft = -1; } } @@ -407,6 +414,8 @@ function resolveOptions(opts: ScrollableElementCreationOptions): ScrollableEleme alwaysConsumeMouseWheel: (typeof opts.alwaysConsumeMouseWheel !== 'undefined' ? opts.alwaysConsumeMouseWheel : false), scrollYToX: (typeof opts.scrollYToX !== 'undefined' ? opts.scrollYToX : false), mouseWheelScrollSensitivity: (typeof opts.mouseWheelScrollSensitivity !== 'undefined' ? opts.mouseWheelScrollSensitivity : 1), + mouseWheelSmoothScroll: (typeof opts.mouseWheelSmoothScroll !== 'undefined' ? opts.mouseWheelSmoothScroll : true), + mouseWheelSmoothScrollDuration: (typeof opts.mouseWheelSmoothScrollDuration !== 'undefined' ? opts.mouseWheelSmoothScrollDuration : 100), arrowSize: (typeof opts.arrowSize !== 'undefined' ? opts.arrowSize : 11), listenOnDomNode: (typeof opts.listenOnDomNode !== 'undefined' ? opts.listenOnDomNode : null), diff --git a/src/vs/base/browser/ui/scrollbar/scrollableElementOptions.ts b/src/vs/base/browser/ui/scrollbar/scrollableElementOptions.ts index 032e637d5b51e..4cfacead9c5a9 100644 --- a/src/vs/base/browser/ui/scrollbar/scrollableElementOptions.ts +++ b/src/vs/base/browser/ui/scrollbar/scrollableElementOptions.ts @@ -30,6 +30,16 @@ export interface ScrollableElementCreationOptions { * Defaults to true */ handleMouseWheel?: boolean; + /** + * If mouse wheel is handled, make mouse wheel scrolling smooth. + * Defaults to true. + */ + mouseWheelSmoothScroll?: boolean; + /** + * Duration in milliseconds for mouse wheel smooth scrolling animation. + * Defaults to 100. + */ + mouseWheelSmoothScrollDuration?: number; /** * Flip axes. Treat vertical scrolling like horizontal and vice-versa. * Defaults to false. @@ -120,6 +130,8 @@ export interface ScrollableElementResolvedOptions { scrollYToX: boolean; alwaysConsumeMouseWheel: boolean; mouseWheelScrollSensitivity: number; + mouseWheelSmoothScroll: boolean; + mouseWheelSmoothScrollDuration: number; arrowSize: number; listenOnDomNode: HTMLElement; horizontal: ScrollbarVisibility; diff --git a/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts b/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts index e7b363ef16e9a..3939823cb4cdc 100644 --- a/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts +++ b/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts @@ -106,9 +106,9 @@ export class VerticalScrollbar extends AbstractScrollbar { return scrollState.scrollTop; } - protected _setScrollPosition(scrollPosition: number): void { + protected _setScrollPosition(scrollPosition: number, smoothScrollDuration?: number): void { this._scrollable.updateState({ scrollTop: scrollPosition - }); + }, smoothScrollDuration); } } diff --git a/src/vs/base/common/scrollable.ts b/src/vs/base/common/scrollable.ts index f7d327627f087..e7c8fe80bef27 100644 --- a/src/vs/base/common/scrollable.ts +++ b/src/vs/base/common/scrollable.ts @@ -95,6 +95,17 @@ export class ScrollState { ); } + public createUpdated(update: INewScrollState): ScrollState { + return new ScrollState( + (typeof update.width !== 'undefined' ? update.width : this.width), + (typeof update.scrollWidth !== 'undefined' ? update.scrollWidth : this.scrollWidth), + (typeof update.scrollLeft !== 'undefined' ? update.scrollLeft : this.scrollLeft), + (typeof update.height !== 'undefined' ? update.height : this.height), + (typeof update.scrollHeight !== 'undefined' ? update.scrollHeight : this.scrollHeight), + (typeof update.scrollTop !== 'undefined' ? update.scrollTop : this.scrollTop) + ); + } + public createScrollEvent(previous: ScrollState): ScrollEvent { let widthChanged = (this.width !== previous.width); let scrollWidthChanged = (this.scrollWidth !== previous.scrollWidth); @@ -140,6 +151,8 @@ export class Scrollable extends Disposable { _scrollableBrand: void; private _state: ScrollState; + private _smoothScrolling: boolean; + private _smoothScrollAnimationParams: ISmoothScrollAnimationParams; private _onScroll = this._register(new Emitter()); public onScroll: Event = this._onScroll.event; @@ -148,29 +161,101 @@ export class Scrollable extends Disposable { super(); this._state = new ScrollState(0, 0, 0, 0, 0, 0); + this._smoothScrolling = false; + this._smoothScrollAnimationParams = null; } public getState(): ScrollState { return this._state; } - public updateState(update: INewScrollState): void { - const oldState = this._state; - const newState = new ScrollState( - (typeof update.width !== 'undefined' ? update.width : oldState.width), - (typeof update.scrollWidth !== 'undefined' ? update.scrollWidth : oldState.scrollWidth), - (typeof update.scrollLeft !== 'undefined' ? update.scrollLeft : oldState.scrollLeft), - (typeof update.height !== 'undefined' ? update.height : oldState.height), - (typeof update.scrollHeight !== 'undefined' ? update.scrollHeight : oldState.scrollHeight), - (typeof update.scrollTop !== 'undefined' ? update.scrollTop : oldState.scrollTop) - ); + /** + * Returns the final scroll state that the instance will have once the smooth scroll animation concludes. + * If no scroll animation is occurring, it will return the actual scroll state instead. + */ + public getSmoothScrollTargetState(): ScrollState { + return this._smoothScrolling ? this._smoothScrollAnimationParams.newState : this._state; + } + + public updateState(update: INewScrollState, smoothScrollDuration?: number): void { + + // If smooth scroll duration is not specified, then assume that the invoker intends to do an immediate update. + if (smoothScrollDuration === undefined) { + const newState = this._state.createUpdated(update); - if (oldState.equals(newState)) { - // no change + // If smooth scrolling is in progress, terminate it. + if (this._smoothScrolling) { + this._smoothScrolling = false; + this._smoothScrollAnimationParams = null; + } + + // Update state immediately if it is different from the previous one. + if (!this._state.equals(newState)) { + this._updateState(newState); + } + } + // Otherwise update scroll state incrementally. + else { + const targetState = this.getSmoothScrollTargetState(); + const newTargetState = targetState.createUpdated(update); + + // Proceed only if the new target state differs from the current one. + if (!targetState.equals(newTargetState)) { + // Initialize/update smooth scroll parameters. + this._smoothScrollAnimationParams = { + oldState: this._state, + newState: newTargetState, + startTime: Date.now(), + duration: smoothScrollDuration, + }; + + // Invoke smooth scrolling functionality in the next frame if it is not already in progress. + if (!this._smoothScrolling) { + this._smoothScrolling = true; + requestAnimationFrame(() => { this._performSmoothScroll(); }); + } + } + } + } + + private _performSmoothScroll(): void { + if (!this._smoothScrolling) { + // Smooth scrolling has been terminated. return; } + const completion = (Date.now() - this._smoothScrollAnimationParams.startTime) / this._smoothScrollAnimationParams.duration; + const newState = this._smoothScrollAnimationParams.newState; + + if (completion < 1) { + const oldState = this._smoothScrollAnimationParams.oldState; + this._updateState(new ScrollState( + newState.width, + newState.scrollWidth, + oldState.scrollLeft + (newState.scrollLeft - oldState.scrollLeft) * completion, + newState.height, + newState.scrollHeight, + oldState.scrollTop + (newState.scrollTop - oldState.scrollTop) * completion + )); + requestAnimationFrame(() => { this._performSmoothScroll(); }); + } + else { + this._smoothScrolling = false; + this._smoothScrollAnimationParams = null; + this._updateState(newState); + } + } + + private _updateState(newState: ScrollState): void { + const oldState = this._state; this._state = newState; this._onScroll.fire(this._state.createScrollEvent(oldState)); } } + +interface ISmoothScrollAnimationParams { + oldState: ScrollState; + newState: ScrollState; + startTime: number; + duration: number; +} \ No newline at end of file diff --git a/src/vs/editor/common/viewLayout/viewLayout.ts b/src/vs/editor/common/viewLayout/viewLayout.ts index 072772279780b..d78d183c9c99b 100644 --- a/src/vs/editor/common/viewLayout/viewLayout.ts +++ b/src/vs/editor/common/viewLayout/viewLayout.ts @@ -237,6 +237,20 @@ export class LayoutProvider extends Disposable implements IViewLayout { } public setScrollPosition(position: editorCommon.INewScrollPosition): void { - this._scrollable.updateState(position); + const state = this._scrollable.getSmoothScrollTargetState(); + const xAbsChange = Math.abs(typeof position.scrollLeft !== 'undefined' ? position.scrollLeft - state.scrollLeft : 0); + const yAbsChange = Math.abs(typeof position.scrollTop !== 'undefined' ? position.scrollTop - state.scrollTop : 0); + + if (xAbsChange || yAbsChange) { + // If position change is big enough, then appply smooth scrolling + if (xAbsChange > state.width / 10 || + yAbsChange > state.height / 10) { + this._scrollable.updateState(position, 125); + } + // Otherwise update scroll position immediately + else { + this._scrollable.updateState(position); + } + } } } From b096f0d158d97542a5f11a768a5877ff05e7aa0a Mon Sep 17 00:00:00 2001 From: "Nicola Fiori (JD342)" Date: Wed, 26 Apr 2017 11:32:32 +0200 Subject: [PATCH 02/14] Fix typo --- src/vs/base/browser/ui/scrollbar/scrollableElement.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts index 097bf54aa17a6..cfa720f817f5f 100644 --- a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts +++ b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts @@ -23,7 +23,7 @@ import Event, { Emitter } from 'vs/base/common/event'; const HIDE_TIMEOUT = 500; const SCROLL_WHEEL_SENSITIVITY = 50; -const SCROLL_WHEEL_SMOOTH_SCROLL_TRESHOLD = 1.7; +const SCROLL_WHEEL_SMOOTH_SCROLL_THRESHOLD = 1.7; export interface IOverviewRulerLayoutInfo { parent: HTMLElement; @@ -260,14 +260,14 @@ export class ScrollableElement extends Widget { if (desiredScrollTop !== -1 || desiredScrollLeft !== -1) { if (desiredScrollTop !== -1) { // If |∆y| is too small then do not apply smooth scroll animation, because in that case the input source must be a touchpad or something similar. - const applySmoothScroll = this._options.mouseWheelSmoothScroll && Math.abs(deltaY) > SCROLL_WHEEL_SMOOTH_SCROLL_TRESHOLD; + const applySmoothScroll = this._options.mouseWheelSmoothScroll && Math.abs(deltaY) > SCROLL_WHEEL_SMOOTH_SCROLL_THRESHOLD; const shouldRender = this._verticalScrollbar.setDesiredScrollPosition(desiredScrollTop, applySmoothScroll ? this._options.mouseWheelSmoothScrollDuration : undefined); this._shouldRender = shouldRender || this._shouldRender; desiredScrollTop = -1; } if (desiredScrollLeft !== -1) { // If |∆x| is too small then do not apply smooth scroll animation, because in that case the input source must be a touchpad or something similar. - const applySmoothScroll = this._options.mouseWheelSmoothScroll && Math.abs(deltaX) > SCROLL_WHEEL_SMOOTH_SCROLL_TRESHOLD; + const applySmoothScroll = this._options.mouseWheelSmoothScroll && Math.abs(deltaX) > SCROLL_WHEEL_SMOOTH_SCROLL_THRESHOLD; const shouldRender = this._horizontalScrollbar.setDesiredScrollPosition(desiredScrollLeft, applySmoothScroll ? this._options.mouseWheelSmoothScrollDuration : undefined); this._shouldRender = shouldRender || this._shouldRender; desiredScrollLeft = -1; From a3358a4358d9a8b6adbb62902c2515498af16d10 Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Mon, 29 May 2017 18:30:35 +0200 Subject: [PATCH 03/14] Make smoothScrollDuration value explicit --- .../base/browser/ui/scrollbar/abstractScrollbar.ts | 10 +++++----- .../base/browser/ui/scrollbar/horizontalScrollbar.ts | 2 +- .../base/browser/ui/scrollbar/scrollableElement.ts | 6 +++--- .../base/browser/ui/scrollbar/verticalScrollbar.ts | 2 +- src/vs/base/common/scrollable.ts | 4 ++-- src/vs/editor/common/viewLayout/viewLayout.ts | 12 ++++++------ 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/vs/base/browser/ui/scrollbar/abstractScrollbar.ts b/src/vs/base/browser/ui/scrollbar/abstractScrollbar.ts index de216a8b03ef2..07dfbe84e1ed2 100644 --- a/src/vs/base/browser/ui/scrollbar/abstractScrollbar.ts +++ b/src/vs/base/browser/ui/scrollbar/abstractScrollbar.ts @@ -198,7 +198,7 @@ export abstract class AbstractScrollbar extends Widget { private _onMouseDown(e: IMouseEvent): void { let domNodePosition = DomUtils.getDomNodePagePosition(this.domNode.domNode); let desiredSliderPosition = this._mouseDownRelativePosition(e, domNodePosition) - this._scrollbarState.getArrowSize() - this._scrollbarState.getSliderSize() / 2; - this.setDesiredScrollPosition(this._scrollbarState.convertSliderPositionToScrollPosition(desiredSliderPosition)); + this.setDesiredScrollPosition(this._scrollbarState.convertSliderPositionToScrollPosition(desiredSliderPosition), 0/* immediate */); this._sliderMouseDown(e); } @@ -217,10 +217,10 @@ export abstract class AbstractScrollbar extends Widget { // console.log(initialMouseOrthogonalPosition + ' -> ' + mouseOrthogonalPosition + ': ' + mouseOrthogonalDelta); if (Platform.isWindows && mouseOrthogonalDelta > MOUSE_DRAG_RESET_DISTANCE) { // The mouse has wondered away from the scrollbar => reset dragging - this.setDesiredScrollPosition(initialScrollPosition); + this.setDesiredScrollPosition(initialScrollPosition, 0/* immediate */); } else { let desiredSliderPosition = this._sliderMousePosition(mouseMoveData) - draggingDelta; - this.setDesiredScrollPosition(this._scrollbarState.convertSliderPositionToScrollPosition(desiredSliderPosition)); + this.setDesiredScrollPosition(this._scrollbarState.convertSliderPositionToScrollPosition(desiredSliderPosition), 0/* immediate */); } }, () => { @@ -238,7 +238,7 @@ export abstract class AbstractScrollbar extends Widget { return this._scrollbarState.validateScrollPosition(desiredScrollPosition); } - public setDesiredScrollPosition(desiredScrollPosition: number, smoothScrollDuration?: number): boolean { + public setDesiredScrollPosition(desiredScrollPosition: number, smoothScrollDuration: number): boolean { desiredScrollPosition = this.validateScrollPosition(desiredScrollPosition); let oldScrollPosition = this._getScrollPosition(); @@ -260,5 +260,5 @@ export abstract class AbstractScrollbar extends Widget { protected abstract _sliderMousePosition(e: IMouseMoveEventData): number; protected abstract _sliderOrthogonalMousePosition(e: IMouseMoveEventData): number; protected abstract _getScrollPosition(): number; - protected abstract _setScrollPosition(elementScrollPosition: number, smoothScrollDuration?: number): void; + protected abstract _setScrollPosition(elementScrollPosition: number, smoothScrollDuration: number): void; } diff --git a/src/vs/base/browser/ui/scrollbar/horizontalScrollbar.ts b/src/vs/base/browser/ui/scrollbar/horizontalScrollbar.ts index 2d90176bf4eba..7eca11f98e9da 100644 --- a/src/vs/base/browser/ui/scrollbar/horizontalScrollbar.ts +++ b/src/vs/base/browser/ui/scrollbar/horizontalScrollbar.ts @@ -101,7 +101,7 @@ export class HorizontalScrollbar extends AbstractScrollbar { return scrollState.scrollLeft; } - protected _setScrollPosition(scrollPosition: number, smoothScrollDuration?: number) { + protected _setScrollPosition(scrollPosition: number, smoothScrollDuration: number) { this._scrollable.updateState({ scrollLeft: scrollPosition }, smoothScrollDuration); diff --git a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts index 080b4f458b0a8..852877764b3a9 100644 --- a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts +++ b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts @@ -151,7 +151,7 @@ export class ScrollableElement extends Widget { } public updateState(newState: INewScrollState): void { - this._scrollable.updateState(newState); + this._scrollable.updateState(newState, 0/* immediate */); } public getScrollState(): ScrollState { @@ -264,14 +264,14 @@ export class ScrollableElement extends Widget { if (desiredScrollTop !== -1) { // If |∆y| is too small then do not apply smooth scroll animation, because in that case the input source must be a touchpad or something similar. const applySmoothScroll = this._options.mouseWheelSmoothScroll && Math.abs(deltaY) > SCROLL_WHEEL_SMOOTH_SCROLL_THRESHOLD; - const shouldRender = this._verticalScrollbar.setDesiredScrollPosition(desiredScrollTop, applySmoothScroll ? this._options.mouseWheelSmoothScrollDuration : undefined); + const shouldRender = this._verticalScrollbar.setDesiredScrollPosition(desiredScrollTop, applySmoothScroll ? this._options.mouseWheelSmoothScrollDuration : 0/* immediate */); this._shouldRender = shouldRender || this._shouldRender; desiredScrollTop = -1; } if (desiredScrollLeft !== -1) { // If |∆x| is too small then do not apply smooth scroll animation, because in that case the input source must be a touchpad or something similar. const applySmoothScroll = this._options.mouseWheelSmoothScroll && Math.abs(deltaX) > SCROLL_WHEEL_SMOOTH_SCROLL_THRESHOLD; - const shouldRender = this._horizontalScrollbar.setDesiredScrollPosition(desiredScrollLeft, applySmoothScroll ? this._options.mouseWheelSmoothScrollDuration : undefined); + const shouldRender = this._horizontalScrollbar.setDesiredScrollPosition(desiredScrollLeft, applySmoothScroll ? this._options.mouseWheelSmoothScrollDuration : 0/* immediate */); this._shouldRender = shouldRender || this._shouldRender; desiredScrollLeft = -1; } diff --git a/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts b/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts index 3939823cb4cdc..4488c6769f1e3 100644 --- a/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts +++ b/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts @@ -106,7 +106,7 @@ export class VerticalScrollbar extends AbstractScrollbar { return scrollState.scrollTop; } - protected _setScrollPosition(scrollPosition: number, smoothScrollDuration?: number): void { + protected _setScrollPosition(scrollPosition: number, smoothScrollDuration: number): void { this._scrollable.updateState({ scrollTop: scrollPosition }, smoothScrollDuration); diff --git a/src/vs/base/common/scrollable.ts b/src/vs/base/common/scrollable.ts index e7c8fe80bef27..dd294321a00a1 100644 --- a/src/vs/base/common/scrollable.ts +++ b/src/vs/base/common/scrollable.ts @@ -177,10 +177,10 @@ export class Scrollable extends Disposable { return this._smoothScrolling ? this._smoothScrollAnimationParams.newState : this._state; } - public updateState(update: INewScrollState, smoothScrollDuration?: number): void { + public updateState(update: INewScrollState, smoothScrollDuration: number): void { // If smooth scroll duration is not specified, then assume that the invoker intends to do an immediate update. - if (smoothScrollDuration === undefined) { + if (smoothScrollDuration === 0) { const newState = this._state.createUpdated(update); // If smooth scrolling is in progress, terminate it. diff --git a/src/vs/editor/common/viewLayout/viewLayout.ts b/src/vs/editor/common/viewLayout/viewLayout.ts index c634a5333f058..3be7360c4dd8b 100644 --- a/src/vs/editor/common/viewLayout/viewLayout.ts +++ b/src/vs/editor/common/viewLayout/viewLayout.ts @@ -34,7 +34,7 @@ export class ViewLayout extends Disposable implements IViewLayout { this.scrollable.updateState({ width: configuration.editor.layoutInfo.contentWidth, height: configuration.editor.layoutInfo.contentHeight - }); + }, 0/* immediate */); this.onDidScroll = this.scrollable.onScroll; this._updateHeight(); @@ -62,7 +62,7 @@ export class ViewLayout extends Disposable implements IViewLayout { this.scrollable.updateState({ width: this._configuration.editor.layoutInfo.contentWidth, height: this._configuration.editor.layoutInfo.contentHeight - }); + }, 0/* immediate */); } this._updateHeight(); } @@ -109,7 +109,7 @@ export class ViewLayout extends Disposable implements IViewLayout { private _updateHeight(): void { this.scrollable.updateState({ scrollHeight: this._getTotalHeight() - }); + }, 0/* immediate */); } // ---- Layouting logic @@ -136,7 +136,7 @@ export class ViewLayout extends Disposable implements IViewLayout { let newScrollWidth = this._computeScrollWidth(maxLineWidth, this.getCurrentViewport().width); this.scrollable.updateState({ scrollWidth: newScrollWidth - }); + }, 0/* immediate */); // The height might depend on the fact that there is a horizontal scrollbar or not this._updateHeight(); @@ -164,7 +164,7 @@ export class ViewLayout extends Disposable implements IViewLayout { this.scrollable.updateState({ scrollLeft: state.scrollLeft, scrollTop: restoreScrollTop - }); + }, 0/* immediate */); } // ---- IVerticalLayoutProvider @@ -247,7 +247,7 @@ export class ViewLayout extends Disposable implements IViewLayout { } // Otherwise update scroll position immediately else { - this.scrollable.updateState(position); + this.scrollable.updateState(position, 0/* immediate */); } } } From 90377a73d4802984a0e6be852f70ea56924f81a1 Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Mon, 21 Aug 2017 18:25:03 +0200 Subject: [PATCH 04/14] * separate scroll dimensions and scroll positions * have explicit methods for changing the scroll position (delayed/smooth vs now/direct) that bubble up to the editor internals. * have all other clients of ScrollableElement not use smooth scrolling --- src/vs/base/browser/ui/list/listView.ts | 14 +- .../browser/ui/scrollbar/abstractScrollbar.ts | 26 +- .../ui/scrollbar/horizontalScrollbar.ts | 17 +- .../browser/ui/scrollbar/scrollableElement.ts | 136 +++++---- .../ui/scrollbar/scrollableElementOptions.ts | 6 - .../browser/ui/scrollbar/verticalScrollbar.ts | 17 +- src/vs/base/common/scrollable.ts | 268 ++++++++++++------ src/vs/base/parts/tree/browser/treeView.ts | 16 +- .../editor/browser/controller/mouseHandler.ts | 6 +- .../editor/browser/controller/mouseTarget.ts | 14 +- .../browser/controller/pointerHandler.ts | 18 +- .../editorScrollbar/editorScrollbar.ts | 12 +- .../browser/viewParts/lines/viewLines.ts | 10 +- .../browser/viewParts/minimap/minimap.ts | 4 +- src/vs/editor/common/commonCodeEditor.ts | 10 +- .../editor/common/controller/coreCommands.ts | 2 +- src/vs/editor/common/controller/cursor.ts | 2 +- .../editor/common/controller/cursorCommon.ts | 4 +- src/vs/editor/common/viewLayout/viewLayout.ts | 130 +++++---- src/vs/editor/common/viewModel/viewModel.ts | 14 +- .../editor/common/viewModel/viewModelImpl.ts | 2 +- .../browser/parts/editor/tabsTitleControl.ts | 10 +- .../extensions/browser/extensionEditor.ts | 4 +- .../electron-browser/walkThroughPart.ts | 54 ++-- 24 files changed, 443 insertions(+), 353 deletions(-) diff --git a/src/vs/base/browser/ui/list/listView.ts b/src/vs/base/browser/ui/list/listView.ts index ae867ee1e2147..235cd3fe9e935 100644 --- a/src/vs/base/browser/ui/list/listView.ts +++ b/src/vs/base/browser/ui/list/listView.ts @@ -124,7 +124,7 @@ export class ListView implements IDisposable { const scrollHeight = this.getContentHeight(); this.rowsContainer.style.height = `${scrollHeight}px`; - this.scrollableElement.updateState({ scrollHeight }); + this.scrollableElement.setScrollDimensions({ scrollHeight }); return deleted.map(i => i.element); } @@ -134,8 +134,8 @@ export class ListView implements IDisposable { } get renderHeight(): number { - const scrollState = this.scrollableElement.getScrollState(); - return scrollState.height; + const scrollDimensions = this.scrollableElement.getScrollDimensions(); + return scrollDimensions.height; } element(index: number): T { @@ -164,7 +164,7 @@ export class ListView implements IDisposable { } layout(height?: number): void { - this.scrollableElement.updateState({ + this.scrollableElement.setScrollDimensions({ height: height || DOM.getContentHeight(this._domNode) }); } @@ -221,12 +221,12 @@ export class ListView implements IDisposable { } getScrollTop(): number { - const scrollState = this.scrollableElement.getScrollState(); - return scrollState.scrollTop; + const scrollPosition = this.scrollableElement.getScrollPosition(); + return scrollPosition.scrollTop; } setScrollTop(scrollTop: number): void { - this.scrollableElement.updateState({ scrollTop }); + this.scrollableElement.setScrollPosition({ scrollTop }); } get scrollTop(): number { diff --git a/src/vs/base/browser/ui/scrollbar/abstractScrollbar.ts b/src/vs/base/browser/ui/scrollbar/abstractScrollbar.ts index 0473b04bf91b4..ce17af707bf2a 100644 --- a/src/vs/base/browser/ui/scrollbar/abstractScrollbar.ts +++ b/src/vs/base/browser/ui/scrollbar/abstractScrollbar.ts @@ -13,7 +13,7 @@ import { FastDomNode, createFastDomNode } from 'vs/base/browser/fastDomNode'; import { ScrollbarState } from 'vs/base/browser/ui/scrollbar/scrollbarState'; import { ScrollbarArrow, ScrollbarArrowOptions } from 'vs/base/browser/ui/scrollbar/scrollbarArrow'; import { ScrollbarVisibilityController } from 'vs/base/browser/ui/scrollbar/scrollbarVisibilityController'; -import { Scrollable, ScrollbarVisibility } from 'vs/base/common/scrollable'; +import { Scrollable, ScrollbarVisibility, INewScrollPosition } from 'vs/base/common/scrollable'; /** * The orthogonal distance to the slider at which dragging "resets". This implements "snapping" @@ -193,7 +193,7 @@ export abstract class AbstractScrollbar extends Widget { private _onMouseDown(e: IMouseEvent): void { let domNodePosition = DomUtils.getDomNodePagePosition(this.domNode.domNode); - this.setDesiredScrollPosition(this._scrollbarState.getDesiredScrollPositionFromOffset(this._mouseDownRelativePosition(e, domNodePosition)), 0/* immediate */); + this._setDesiredScrollPositionNow(this._scrollbarState.getDesiredScrollPositionFromOffset(this._mouseDownRelativePosition(e, domNodePosition))); if (e.leftButton) { e.preventDefault(); this._sliderMouseDown(e, () => { /*nothing to do*/ }); @@ -214,13 +214,13 @@ export abstract class AbstractScrollbar extends Widget { if (Platform.isWindows && mouseOrthogonalDelta > MOUSE_DRAG_RESET_DISTANCE) { // The mouse has wondered away from the scrollbar => reset dragging - this.setDesiredScrollPosition(initialScrollbarState.getScrollPosition(), 0/* immediate */); + this._setDesiredScrollPositionNow(initialScrollbarState.getScrollPosition()); return; } const mousePosition = this._sliderMousePosition(mouseMoveData); const mouseDelta = mousePosition - initialMousePosition; - this.setDesiredScrollPosition(initialScrollbarState.getDesiredScrollPositionFromDelta(mouseDelta), 0/* immediate */); + this._setDesiredScrollPositionNow(initialScrollbarState.getDesiredScrollPositionFromDelta(mouseDelta)); }, () => { this.slider.toggleClassName('active', false); @@ -232,18 +232,12 @@ export abstract class AbstractScrollbar extends Widget { this._host.onDragStart(); } - public setDesiredScrollPosition(desiredScrollPosition: number, smoothScrollDuration: number): boolean { - desiredScrollPosition = this.validateScrollPosition(desiredScrollPosition); + private _setDesiredScrollPositionNow(_desiredScrollPosition: number): void { - let oldScrollPosition = this._getScrollPosition(); - this._setScrollPosition(desiredScrollPosition, smoothScrollDuration); - let newScrollPosition = this._getScrollPosition(); + let desiredScrollPosition: INewScrollPosition = {}; + this.writeScrollPosition(desiredScrollPosition, _desiredScrollPosition); - if (oldScrollPosition !== newScrollPosition) { - this._onElementScrollPosition(this._getScrollPosition()); - return true; - } - return false; + this._scrollable.setScrollPositionNow(desiredScrollPosition); } // ----------------- Overwrite these @@ -255,7 +249,5 @@ export abstract class AbstractScrollbar extends Widget { protected abstract _sliderMousePosition(e: ISimplifiedMouseEvent): number; protected abstract _sliderOrthogonalMousePosition(e: ISimplifiedMouseEvent): number; - protected abstract _getScrollPosition(): number; - protected abstract _setScrollPosition(elementScrollPosition: number, smoothScrollDuration: number): void; - public abstract validateScrollPosition(desiredScrollPosition: number): number; + public abstract writeScrollPosition(target: INewScrollPosition, scrollPosition: number): void; } diff --git a/src/vs/base/browser/ui/scrollbar/horizontalScrollbar.ts b/src/vs/base/browser/ui/scrollbar/horizontalScrollbar.ts index 6b278275b7e7e..ac973201db59a 100644 --- a/src/vs/base/browser/ui/scrollbar/horizontalScrollbar.ts +++ b/src/vs/base/browser/ui/scrollbar/horizontalScrollbar.ts @@ -8,7 +8,7 @@ import { AbstractScrollbar, ScrollbarHost, ISimplifiedMouseEvent } from 'vs/base import { StandardMouseWheelEvent } from 'vs/base/browser/mouseEvent'; import { IDomNodePagePosition } from 'vs/base/browser/dom'; import { ScrollableElementResolvedOptions } from 'vs/base/browser/ui/scrollbar/scrollableElementOptions'; -import { Scrollable, ScrollEvent, ScrollbarVisibility } from 'vs/base/common/scrollable'; +import { Scrollable, ScrollEvent, ScrollbarVisibility, INewScrollPosition } from 'vs/base/common/scrollable'; import { ScrollbarState } from 'vs/base/browser/ui/scrollbar/scrollbarState'; import { ARROW_IMG_SIZE } from 'vs/base/browser/ui/scrollbar/scrollbarArrow'; @@ -89,18 +89,7 @@ export class HorizontalScrollbar extends AbstractScrollbar { return e.posy; } - protected _getScrollPosition(): number { - const scrollState = this._scrollable.getState(); - return scrollState.scrollLeft; - } - - protected _setScrollPosition(scrollPosition: number, smoothScrollDuration: number) { - this._scrollable.updateState({ - scrollLeft: scrollPosition - }, smoothScrollDuration); - } - - public validateScrollPosition(desiredScrollPosition: number): number { - return this._scrollable.validateScrollLeft(desiredScrollPosition); + public writeScrollPosition(target: INewScrollPosition, scrollPosition: number): void { + target.scrollLeft = scrollPosition; } } diff --git a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts index 10ca3fe886d51..b729ed33b05af 100644 --- a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts +++ b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts @@ -13,7 +13,7 @@ import { HorizontalScrollbar } from 'vs/base/browser/ui/scrollbar/horizontalScro import { VerticalScrollbar } from 'vs/base/browser/ui/scrollbar/verticalScrollbar'; import { ScrollableElementCreationOptions, ScrollableElementChangeOptions, ScrollableElementResolvedOptions } from 'vs/base/browser/ui/scrollbar/scrollableElementOptions'; import { IDisposable, dispose } from 'vs/base/common/lifecycle'; -import { Scrollable, ScrollState, ScrollEvent, INewScrollState, ScrollbarVisibility } from 'vs/base/common/scrollable'; +import { Scrollable, ScrollEvent, ScrollbarVisibility, INewScrollDimensions, IScrollDimensions, INewScrollPosition, IScrollPosition } from 'vs/base/common/scrollable'; import { Widget } from 'vs/base/browser/ui/widget'; import { TimeoutTimer } from 'vs/base/common/async'; import { FastDomNode, createFastDomNode } from 'vs/base/browser/fastDomNode'; @@ -29,41 +29,36 @@ export interface IOverviewRulerLayoutInfo { insertBefore: HTMLElement; } -export class ScrollableElement extends Widget { +export abstract class AbstractScrollableElement extends Widget { - private _options: ScrollableElementResolvedOptions; - private _scrollable: Scrollable; - private _verticalScrollbar: VerticalScrollbar; - private _horizontalScrollbar: HorizontalScrollbar; - private _domNode: HTMLElement; + private readonly _options: ScrollableElementResolvedOptions; + protected readonly _scrollable: Scrollable; + private readonly _verticalScrollbar: VerticalScrollbar; + private readonly _horizontalScrollbar: HorizontalScrollbar; + private readonly _domNode: HTMLElement; - private _leftShadowDomNode: FastDomNode; - private _topShadowDomNode: FastDomNode; - private _topLeftShadowDomNode: FastDomNode; + private readonly _leftShadowDomNode: FastDomNode; + private readonly _topShadowDomNode: FastDomNode; + private readonly _topLeftShadowDomNode: FastDomNode; - private _listenOnDomNode: HTMLElement; + private readonly _listenOnDomNode: HTMLElement; private _mouseWheelToDispose: IDisposable[]; private _isDragging: boolean; private _mouseIsOver: boolean; - private _hideTimeout: TimeoutTimer; + private readonly _hideTimeout: TimeoutTimer; private _shouldRender: boolean; - private _onScroll = this._register(new Emitter()); + private readonly _onScroll = this._register(new Emitter()); public onScroll: Event = this._onScroll.event; - constructor(element: HTMLElement, options: ScrollableElementCreationOptions, scrollable?: Scrollable) { + protected constructor(element: HTMLElement, options: ScrollableElementCreationOptions, scrollable?: Scrollable) { super(); element.style.overflow = 'hidden'; this._options = resolveOptions(options); - - if (typeof scrollable === 'undefined') { - this._scrollable = this._register(new Scrollable()); - } else { - this._scrollable = scrollable; - } + this._scrollable = scrollable; this._register(this._scrollable.onScroll((e) => { this._onDidScroll(e); @@ -153,12 +148,12 @@ export class ScrollableElement extends Widget { this._verticalScrollbar.delegateSliderMouseDown(e, onDragFinished); } - public updateState(newState: INewScrollState): void { - this._scrollable.updateState(newState, 0/* immediate */); + public getScrollDimensions(): IScrollDimensions { + return this._scrollable.getScrollDimensions(); } - public getScrollState(): ScrollState { - return this._scrollable.getState(); + public setScrollDimensions(dimensions: INewScrollDimensions): void { + this._scrollable.setScrollDimensions(dimensions); } /** @@ -215,8 +210,6 @@ export class ScrollableElement extends Widget { } private _onMouseWheel(e: StandardMouseWheelEvent): void { - let desiredScrollTop = -1; - let desiredScrollLeft = -1; if (e.deltaY || e.deltaX) { let deltaY = e.deltaY * this._options.mouseWheelScrollSensitivity; @@ -244,37 +237,39 @@ export class ScrollableElement extends Widget { } } - const scrollState = this._scrollable.getSmoothScrollTargetState(); + const futureScrollPosition = this._scrollable.getFutureScrollPosition(); + + let desiredScrollPosition: INewScrollPosition = {}; if (deltaY) { - let currentScrollTop = scrollState.scrollTop; - desiredScrollTop = this._verticalScrollbar.validateScrollPosition((desiredScrollTop !== -1 ? desiredScrollTop : currentScrollTop) - SCROLL_WHEEL_SENSITIVITY * deltaY); - if (desiredScrollTop === currentScrollTop) { - desiredScrollTop = -1; - } + const desiredScrollTop = futureScrollPosition.scrollTop - SCROLL_WHEEL_SENSITIVITY * deltaY; + this._verticalScrollbar.writeScrollPosition(desiredScrollPosition, desiredScrollTop); } if (deltaX) { - let currentScrollLeft = scrollState.scrollLeft; - desiredScrollLeft = this._horizontalScrollbar.validateScrollPosition((desiredScrollLeft !== -1 ? desiredScrollLeft : currentScrollLeft) - SCROLL_WHEEL_SENSITIVITY * deltaX); - if (desiredScrollLeft === currentScrollLeft) { - desiredScrollLeft = -1; - } + const desiredScrollLeft = futureScrollPosition.scrollLeft - SCROLL_WHEEL_SENSITIVITY * deltaX; + this._horizontalScrollbar.writeScrollPosition(desiredScrollPosition, desiredScrollLeft); } - if (desiredScrollTop !== -1 || desiredScrollLeft !== -1) { - if (desiredScrollTop !== -1) { - // If |∆y| is too small then do not apply smooth scroll animation, because in that case the input source must be a touchpad or something similar. - const applySmoothScroll = this._options.mouseWheelSmoothScroll && Math.abs(deltaY) > SCROLL_WHEEL_SMOOTH_SCROLL_THRESHOLD; - const shouldRender = this._verticalScrollbar.setDesiredScrollPosition(desiredScrollTop, applySmoothScroll ? this._options.mouseWheelSmoothScrollDuration : 0/* immediate */); - this._shouldRender = shouldRender || this._shouldRender; - desiredScrollTop = -1; - } - if (desiredScrollLeft !== -1) { - // If |∆x| is too small then do not apply smooth scroll animation, because in that case the input source must be a touchpad or something similar. - const applySmoothScroll = this._options.mouseWheelSmoothScroll && Math.abs(deltaX) > SCROLL_WHEEL_SMOOTH_SCROLL_THRESHOLD; - const shouldRender = this._horizontalScrollbar.setDesiredScrollPosition(desiredScrollLeft, applySmoothScroll ? this._options.mouseWheelSmoothScrollDuration : 0/* immediate */); - this._shouldRender = shouldRender || this._shouldRender; - desiredScrollLeft = -1; + // Check that we are scrolling towards a location which is valid + desiredScrollPosition = this._scrollable.validateScrollPosition(desiredScrollPosition); + + if (futureScrollPosition.scrollLeft !== desiredScrollPosition.scrollLeft || futureScrollPosition.scrollTop !== desiredScrollPosition.scrollTop) { + // TODO@smooth: [MUST] implement better heuristic for distinguishing inertia scrolling + // from physical mouse wheels + const ENABLE_MOUSE_WHEEL_SMOOTH = false; + + // If |∆x| and |∆y| are too small then do not apply smooth scroll animation, because in that case the input source must be a touchpad or something similar. + const smoothScrollThresholdReached = ( + Math.abs(deltaY) > SCROLL_WHEEL_SMOOTH_SCROLL_THRESHOLD + || Math.abs(deltaX) > SCROLL_WHEEL_SMOOTH_SCROLL_THRESHOLD + ); + + if (ENABLE_MOUSE_WHEEL_SMOOTH && this._options.mouseWheelSmoothScroll && smoothScrollThresholdReached) { + this._scrollable.setScrollPositionSmooth(desiredScrollPosition); + } else { + this._scrollable.setScrollPositionNow(desiredScrollPosition); } + + this._shouldRender = true; } } @@ -322,7 +317,7 @@ export class ScrollableElement extends Widget { this._verticalScrollbar.render(); if (this._options.useShadows) { - const scrollState = this._scrollable.getState(); + const scrollState = this._scrollable.getCurrentScrollPosition(); let enableTop = scrollState.scrollTop > 0; let enableLeft = scrollState.scrollLeft > 0; @@ -374,6 +369,33 @@ export class ScrollableElement extends Widget { } } +export class ScrollableElement extends AbstractScrollableElement { + + constructor(element: HTMLElement, options: ScrollableElementCreationOptions) { + options = options || {}; + options.mouseWheelSmoothScroll = false; + const scrollable = new Scrollable(0); + super(element, options, scrollable); + this._register(scrollable); + } + + public setScrollPosition(update: INewScrollPosition): void { + this._scrollable.setScrollPositionNow(update); + } + + public getScrollPosition(): IScrollPosition { + return this._scrollable.getCurrentScrollPosition(); + } +} + +export class SmoothScrollableElement extends AbstractScrollableElement { + + constructor(element: HTMLElement, options: ScrollableElementCreationOptions, scrollable: Scrollable) { + super(element, options, scrollable); + } + +} + export class DomScrollableElement extends ScrollableElement { private _element: HTMLElement; @@ -394,13 +416,14 @@ export class DomScrollableElement extends ScrollableElement { public scanDomNode(): void { // widh, scrollLeft, scrollWidth, height, scrollTop, scrollHeight - this.updateState({ + this.setScrollDimensions({ width: this._element.clientWidth, scrollWidth: this._element.scrollWidth, - scrollLeft: this._element.scrollLeft, - height: this._element.clientHeight, - scrollHeight: this._element.scrollHeight, + scrollHeight: this._element.scrollHeight + }); + this.setScrollPosition({ + scrollLeft: this._element.scrollLeft, scrollTop: this._element.scrollTop, }); } @@ -417,7 +440,6 @@ function resolveOptions(opts: ScrollableElementCreationOptions): ScrollableEleme scrollYToX: (typeof opts.scrollYToX !== 'undefined' ? opts.scrollYToX : false), mouseWheelScrollSensitivity: (typeof opts.mouseWheelScrollSensitivity !== 'undefined' ? opts.mouseWheelScrollSensitivity : 1), mouseWheelSmoothScroll: (typeof opts.mouseWheelSmoothScroll !== 'undefined' ? opts.mouseWheelSmoothScroll : true), - mouseWheelSmoothScrollDuration: (typeof opts.mouseWheelSmoothScrollDuration !== 'undefined' ? opts.mouseWheelSmoothScrollDuration : 100), arrowSize: (typeof opts.arrowSize !== 'undefined' ? opts.arrowSize : 11), listenOnDomNode: (typeof opts.listenOnDomNode !== 'undefined' ? opts.listenOnDomNode : null), diff --git a/src/vs/base/browser/ui/scrollbar/scrollableElementOptions.ts b/src/vs/base/browser/ui/scrollbar/scrollableElementOptions.ts index e28bbf90fed63..069a6ea6860da 100644 --- a/src/vs/base/browser/ui/scrollbar/scrollableElementOptions.ts +++ b/src/vs/base/browser/ui/scrollbar/scrollableElementOptions.ts @@ -31,11 +31,6 @@ export interface ScrollableElementCreationOptions { * Defaults to true. */ mouseWheelSmoothScroll?: boolean; - /** - * Duration in milliseconds for mouse wheel smooth scrolling animation. - * Defaults to 100. - */ - mouseWheelSmoothScrollDuration?: number; /** * Flip axes. Treat vertical scrolling like horizontal and vice-versa. * Defaults to false. @@ -125,7 +120,6 @@ export interface ScrollableElementResolvedOptions { alwaysConsumeMouseWheel: boolean; mouseWheelScrollSensitivity: number; mouseWheelSmoothScroll: boolean; - mouseWheelSmoothScrollDuration: number; arrowSize: number; listenOnDomNode: HTMLElement; horizontal: ScrollbarVisibility; diff --git a/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts b/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts index 53a76a0c0ff26..98d74206690b6 100644 --- a/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts +++ b/src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts @@ -8,7 +8,7 @@ import { AbstractScrollbar, ScrollbarHost, ISimplifiedMouseEvent } from 'vs/base import { StandardMouseWheelEvent } from 'vs/base/browser/mouseEvent'; import { IDomNodePagePosition } from 'vs/base/browser/dom'; import { ScrollableElementResolvedOptions } from 'vs/base/browser/ui/scrollbar/scrollableElementOptions'; -import { Scrollable, ScrollEvent, ScrollbarVisibility } from 'vs/base/common/scrollable'; +import { Scrollable, ScrollEvent, ScrollbarVisibility, INewScrollPosition } from 'vs/base/common/scrollable'; import { ScrollbarState } from 'vs/base/browser/ui/scrollbar/scrollbarState'; import { ARROW_IMG_SIZE } from 'vs/base/browser/ui/scrollbar/scrollbarArrow'; @@ -90,18 +90,7 @@ export class VerticalScrollbar extends AbstractScrollbar { return e.posx; } - protected _getScrollPosition(): number { - const scrollState = this._scrollable.getState(); - return scrollState.scrollTop; - } - - protected _setScrollPosition(scrollPosition: number, smoothScrollDuration: number): void { - this._scrollable.updateState({ - scrollTop: scrollPosition - }, smoothScrollDuration); - } - - public validateScrollPosition(desiredScrollPosition: number): number { - return this._scrollable.validateScrollTop(desiredScrollPosition); + public writeScrollPosition(target: INewScrollPosition, scrollPosition: number): void { + target.scrollTop = scrollPosition; } } diff --git a/src/vs/base/common/scrollable.ts b/src/vs/base/common/scrollable.ts index 2ce512911760b..d304a0386b664 100644 --- a/src/vs/base/common/scrollable.ts +++ b/src/vs/base/common/scrollable.ts @@ -31,7 +31,7 @@ export interface ScrollEvent { scrollTopChanged: boolean; } -export class ScrollState { +export class ScrollState implements IScrollDimensions, IScrollPosition { _scrollStateBrand: void; public readonly width: number; @@ -95,13 +95,24 @@ export class ScrollState { ); } - public createUpdated(update: INewScrollState): ScrollState { + public withScrollDimensions(update: INewScrollDimensions): ScrollState { return new ScrollState( (typeof update.width !== 'undefined' ? update.width : this.width), (typeof update.scrollWidth !== 'undefined' ? update.scrollWidth : this.scrollWidth), - (typeof update.scrollLeft !== 'undefined' ? update.scrollLeft : this.scrollLeft), + this.scrollLeft, (typeof update.height !== 'undefined' ? update.height : this.height), (typeof update.scrollHeight !== 'undefined' ? update.scrollHeight : this.scrollHeight), + this.scrollTop + ); + } + + public withScrollPosition(update: INewScrollPosition): ScrollState { + return new ScrollState( + this.width, + this.scrollWidth, + (typeof update.scrollLeft !== 'undefined' ? update.scrollLeft : this.scrollLeft), + this.height, + this.scrollHeight, (typeof update.scrollTop !== 'undefined' ? update.scrollTop : this.scrollTop) ); } @@ -136,13 +147,25 @@ export class ScrollState { } -export interface INewScrollState { +export interface IScrollDimensions { + readonly width: number; + readonly scrollWidth: number; + readonly height: number; + readonly scrollHeight: number; +} +export interface INewScrollDimensions { width?: number; scrollWidth?: number; - scrollLeft?: number; - height?: number; scrollHeight?: number; +} + +export interface IScrollPosition { + readonly scrollLeft: number; + readonly scrollTop: number; +} +export interface INewScrollPosition { + scrollLeft?: number; scrollTop?: number; } @@ -150,126 +173,193 @@ export class Scrollable extends Disposable { _scrollableBrand: void; + private readonly _smoothScrollDuration: number; private _state: ScrollState; - private _smoothScrolling: boolean; - private _smoothScrollAnimationParams: ISmoothScrollAnimationParams; + private _smoothScrolling: SmoothScrollingOperation; private _onScroll = this._register(new Emitter()); public onScroll: Event = this._onScroll.event; - constructor() { + constructor(smoothScrollDuration: number) { super(); + this._smoothScrollDuration = smoothScrollDuration; this._state = new ScrollState(0, 0, 0, 0, 0, 0); - this._smoothScrolling = false; - this._smoothScrollAnimationParams = null; + this._smoothScrolling = null; + } + + public dispose(): void { + if (this._smoothScrolling) { + this._smoothScrolling.dispose(); + this._smoothScrolling = null; + } + super.dispose(); + } + + public validateScrollPosition(scrollPosition: INewScrollPosition): IScrollPosition { + return this._state.withScrollPosition(scrollPosition); } - public getState(): ScrollState { + public getScrollDimensions(): IScrollDimensions { return this._state; } - public validateScrollTop(desiredScrollTop: number): number { - desiredScrollTop = Math.round(desiredScrollTop); - desiredScrollTop = Math.max(desiredScrollTop, 0); - desiredScrollTop = Math.min(desiredScrollTop, this._state.scrollHeight - this._state.height); - return desiredScrollTop; + public setScrollDimensions(dimensions: INewScrollDimensions): void { + const newState = this._state.withScrollDimensions(dimensions); + this._setState(newState); + + // TODO@smooth: [MUST] validate outstanding animated scroll position request target + // (in case it becomes invalid) } - public validateScrollLeft(desiredScrollLeft: number): number { - desiredScrollLeft = Math.round(desiredScrollLeft); - desiredScrollLeft = Math.max(desiredScrollLeft, 0); - desiredScrollLeft = Math.min(desiredScrollLeft, this._state.scrollWidth - this._state.width); - return desiredScrollLeft; + /** + * Returns the final scroll position that the instance will have once the smooth scroll animation concludes. + * If no scroll animation is occuring, it will return the current scroll position instead. + */ + public getFutureScrollPosition(): IScrollPosition { + if (this._smoothScrolling) { + return this._smoothScrolling.to; + } + return this._state; } /** - * Returns the final scroll state that the instance will have once the smooth scroll animation concludes. - * If no scroll animation is occurring, it will return the actual scroll state instead. + * Returns the current scroll position. + * Note: This result might be an intermediate scroll position, as there might be an ongoing smooth scroll animation. */ - public getSmoothScrollTargetState(): ScrollState { - return this._smoothScrolling ? this._smoothScrollAnimationParams.newState : this._state; + public getCurrentScrollPosition(): IScrollPosition { + return this._state; } - public updateState(update: INewScrollState, smoothScrollDuration: number): void { + public setScrollPositionNow(update: INewScrollPosition): void { + // no smooth scrolling requested + const newState = this._state.withScrollPosition(update); - // If smooth scroll duration is not specified, then assume that the invoker intends to do an immediate update. - if (smoothScrollDuration === 0) { - const newState = this._state.createUpdated(update); + // Terminate any outstanding smooth scrolling + if (this._smoothScrolling) { + this._smoothScrolling.dispose(); + this._smoothScrolling = null; + } - // If smooth scrolling is in progress, terminate it. - if (this._smoothScrolling) { - this._smoothScrolling = false; - this._smoothScrollAnimationParams = null; - } + this._setState(newState); + } - // Update state immediately if it is different from the previous one. - if (!this._state.equals(newState)) { - this._updateState(newState); - } + public setScrollPositionSmooth(update: INewScrollPosition): void { + if (this._smoothScrollDuration === 0) { + // Smooth scrolling not supported. + return this.setScrollPositionNow(update); } - // Otherwise update scroll state incrementally. - else { - const targetState = this.getSmoothScrollTargetState(); - const newTargetState = targetState.createUpdated(update); - - // Proceed only if the new target state differs from the current one. - if (!targetState.equals(newTargetState)) { - // Initialize/update smooth scroll parameters. - this._smoothScrollAnimationParams = { - oldState: this._state, - newState: newTargetState, - startTime: Date.now(), - duration: smoothScrollDuration, - }; - - // Invoke smooth scrolling functionality in the next frame if it is not already in progress. - if (!this._smoothScrolling) { - this._smoothScrolling = true; - requestAnimationFrame(() => { this._performSmoothScroll(); }); - } + + if (this._smoothScrolling) { + const oldSmoothScrolling = this._smoothScrolling; + const newSmoothScrolling = oldSmoothScrolling.combine(this._state, update, this._smoothScrollDuration); + if (oldSmoothScrolling.softEquals(newSmoothScrolling)) { + // No change + return; } + oldSmoothScrolling.dispose(); + this._smoothScrolling = newSmoothScrolling; + } else { + this._smoothScrolling = new SmoothScrollingOperation(this._state, update, this._smoothScrollDuration); } + + // Begin smooth scrolling animation + this._smoothScrolling.animationFrameToken = requestAnimationFrame(() => { + this._smoothScrolling.animationFrameToken = -1; + this._performSmoothScrolling(); + }); } - private _performSmoothScroll(): void { - if (!this._smoothScrolling) { - // Smooth scrolling has been terminated. - return; - } + private _performSmoothScrolling(): void { + const update = this._smoothScrolling.tick(); + const newState = this._state.withScrollPosition(update); - const completion = (Date.now() - this._smoothScrollAnimationParams.startTime) / this._smoothScrollAnimationParams.duration; - const newState = this._smoothScrollAnimationParams.newState; + this._setState(newState); - if (completion < 1) { - const oldState = this._smoothScrollAnimationParams.oldState; - this._updateState(new ScrollState( - newState.width, - newState.scrollWidth, - oldState.scrollLeft + (newState.scrollLeft - oldState.scrollLeft) * completion, - newState.height, - newState.scrollHeight, - oldState.scrollTop + (newState.scrollTop - oldState.scrollTop) * completion - )); - requestAnimationFrame(() => { this._performSmoothScroll(); }); - } - else { - this._smoothScrolling = false; - this._smoothScrollAnimationParams = null; - this._updateState(newState); + if (!update.isDone) { + // Continue smooth scrolling animation + this._smoothScrolling.animationFrameToken = requestAnimationFrame(() => { + this._smoothScrolling.animationFrameToken = -1; + this._performSmoothScrolling(); + }); } } - private _updateState(newState: ScrollState): void { + private _setState(newState: ScrollState): void { const oldState = this._state; + if (oldState.equals(newState)) { + // no change + return; + } this._state = newState; this._onScroll.fire(this._state.createScrollEvent(oldState)); } } -interface ISmoothScrollAnimationParams { - oldState: ScrollState; - newState: ScrollState; - startTime: number; - duration: number; -} \ No newline at end of file +class SmoothScrollingUpdate implements IScrollPosition { + + public readonly scrollLeft: number; + public readonly scrollTop: number; + public readonly isDone: boolean; + + constructor(scrollLeft: number, scrollTop: number, isDone: boolean) { + this.scrollLeft = scrollLeft; + this.scrollTop = scrollTop; + this.isDone = isDone; + } + +} + +class SmoothScrollingOperation { + + public readonly from: IScrollPosition; + public readonly to: IScrollPosition; + public readonly duration: number; + private readonly _startTime: number; + public animationFrameToken: number; + + constructor(from: ScrollState, to: INewScrollPosition, duration: number) { + this.from = from; + this.to = from.withScrollPosition(to); + this.duration = duration; + this._startTime = Date.now(); + this.animationFrameToken = -1; + } + + public softEquals(other: SmoothScrollingOperation): boolean { + return ( + this.to.scrollLeft === other.to.scrollLeft + && this.to.scrollTop === other.to.scrollTop + ); + } + + public dispose(): void { + if (this.animationFrameToken !== -1) { + cancelAnimationFrame(this.animationFrameToken); + this.animationFrameToken = -1; + } + } + + public tick(): SmoothScrollingUpdate { + const completion = (Date.now() - this._startTime) / this.duration; + + if (completion < 1) { + const newScrollLeft = this.from.scrollLeft + (this.to.scrollLeft - this.from.scrollLeft) * completion; + const newScrollTop = this.from.scrollTop + (this.to.scrollTop - this.from.scrollTop) * completion; + return new SmoothScrollingUpdate(newScrollLeft, newScrollTop, false); + } + + return new SmoothScrollingUpdate(this.to.scrollLeft, this.to.scrollTop, true); + } + + public combine(from: ScrollState, to: INewScrollPosition, duration: number): SmoothScrollingOperation { + // Combine our scrollLeft/scrollTop with incoming scrollLeft/scrollTop + to = { + scrollLeft: (typeof to.scrollLeft === 'undefined' ? this.to.scrollLeft : to.scrollLeft), + scrollTop: (typeof to.scrollTop === 'undefined' ? this.to.scrollTop : to.scrollTop) + }; + + // TODO@smooth: This is our opportunity to combine animations + return new SmoothScrollingOperation(from, to, duration); + } +} diff --git a/src/vs/base/parts/tree/browser/treeView.ts b/src/vs/base/parts/tree/browser/treeView.ts index 38a82069f3e13..ac5ba32ad5c7c 100644 --- a/src/vs/base/parts/tree/browser/treeView.ts +++ b/src/vs/base/parts/tree/browser/treeView.ts @@ -847,27 +847,29 @@ export class TreeView extends HeightMap { } public get viewHeight() { - const scrollState = this.scrollableElement.getScrollState(); - return scrollState.height; + const scrollDimensions = this.scrollableElement.getScrollDimensions(); + return scrollDimensions.height; } public set viewHeight(viewHeight: number) { - this.scrollableElement.updateState({ + this.scrollableElement.setScrollDimensions({ height: viewHeight, scrollHeight: this.getTotalHeight() }); } public get scrollTop(): number { - const scrollState = this.scrollableElement.getScrollState(); - return scrollState.scrollTop; + const scrollPosition = this.scrollableElement.getScrollPosition(); + return scrollPosition.scrollTop; } public set scrollTop(scrollTop: number) { - this.scrollableElement.updateState({ - scrollTop: scrollTop, + this.scrollableElement.setScrollDimensions({ scrollHeight: this.getTotalHeight() }); + this.scrollableElement.setScrollPosition({ + scrollTop: scrollTop + }); } public getScrollPosition(): number { diff --git a/src/vs/editor/browser/controller/mouseHandler.ts b/src/vs/editor/browser/controller/mouseHandler.ts index 60ff6e4d650c5..982043e7e5f7b 100644 --- a/src/vs/editor/browser/controller/mouseHandler.ts +++ b/src/vs/editor/browser/controller/mouseHandler.ts @@ -423,16 +423,16 @@ class MouseDownOperation extends Disposable { const mouseColumn = this._getMouseColumn(e); if (e.posy < editorContent.y) { - let aboveLineNumber = viewLayout.getLineNumberAtVerticalOffset(Math.max(viewLayout.getScrollTop() - (editorContent.y - e.posy), 0)); + let aboveLineNumber = viewLayout.getLineNumberAtVerticalOffset(Math.max(viewLayout.getCurrentScrollTop() - (editorContent.y - e.posy), 0)); return new MouseTarget(null, editorBrowser.MouseTargetType.OUTSIDE_EDITOR, mouseColumn, new Position(aboveLineNumber, 1)); } if (e.posy > editorContent.y + editorContent.height) { - let belowLineNumber = viewLayout.getLineNumberAtVerticalOffset(viewLayout.getScrollTop() + (e.posy - editorContent.y)); + let belowLineNumber = viewLayout.getLineNumberAtVerticalOffset(viewLayout.getCurrentScrollTop() + (e.posy - editorContent.y)); return new MouseTarget(null, editorBrowser.MouseTargetType.OUTSIDE_EDITOR, mouseColumn, new Position(belowLineNumber, model.getLineMaxColumn(belowLineNumber))); } - let possibleLineNumber = viewLayout.getLineNumberAtVerticalOffset(viewLayout.getScrollTop() + (e.posy - editorContent.y)); + let possibleLineNumber = viewLayout.getLineNumberAtVerticalOffset(viewLayout.getCurrentScrollTop() + (e.posy - editorContent.y)); if (e.posx < editorContent.x) { return new MouseTarget(null, editorBrowser.MouseTargetType.OUTSIDE_EDITOR, mouseColumn, new Position(possibleLineNumber, 1)); diff --git a/src/vs/editor/browser/controller/mouseTarget.ts b/src/vs/editor/browser/controller/mouseTarget.ts index 3626e6cfb7002..4290a20048704 100644 --- a/src/vs/editor/browser/controller/mouseTarget.ts +++ b/src/vs/editor/browser/controller/mouseTarget.ts @@ -327,12 +327,12 @@ class HitTestContext { return this._viewHelper.getPositionFromDOMInfo(spanNode, offset); } - public getScrollTop(): number { - return this._context.viewLayout.getScrollTop(); + public getCurrentScrollTop(): number { + return this._context.viewLayout.getCurrentScrollTop(); } - public getScrollLeft(): number { - return this._context.viewLayout.getScrollLeft(); + public getCurrentScrollLeft(): number { + return this._context.viewLayout.getCurrentScrollLeft(); } } @@ -351,8 +351,8 @@ abstract class BareHitTestRequest { this.editorPos = editorPos; this.pos = pos; - this.mouseVerticalOffset = Math.max(0, ctx.getScrollTop() + pos.y - editorPos.y); - this.mouseContentHorizontalOffset = ctx.getScrollLeft() + pos.x - editorPos.x - ctx.layoutInfo.contentLeft; + this.mouseVerticalOffset = Math.max(0, ctx.getCurrentScrollTop() + pos.y - editorPos.y); + this.mouseContentHorizontalOffset = ctx.getCurrentScrollLeft() + pos.x - editorPos.x - ctx.layoutInfo.contentLeft; this.isInMarginArea = (pos.x - editorPos.x < ctx.layoutInfo.contentLeft); this.isInContentArea = !this.isInMarginArea; this.mouseColumn = Math.max(0, MouseTargetFactory._getMouseColumn(this.mouseContentHorizontalOffset, ctx.typicalHalfwidthCharacterWidth)); @@ -649,7 +649,7 @@ export class MouseTargetFactory { public getMouseColumn(editorPos: EditorPagePosition, pos: PageCoordinates): number { let layoutInfo = this._context.configuration.editor.layoutInfo; - let mouseContentHorizontalOffset = this._context.viewLayout.getScrollLeft() + pos.x - editorPos.x - layoutInfo.contentLeft; + let mouseContentHorizontalOffset = this._context.viewLayout.getCurrentScrollLeft() + pos.x - editorPos.x - layoutInfo.contentLeft; return MouseTargetFactory._getMouseColumn(mouseContentHorizontalOffset, this._context.configuration.editor.fontInfo.typicalHalfwidthCharacterWidth); } diff --git a/src/vs/editor/browser/controller/pointerHandler.ts b/src/vs/editor/browser/controller/pointerHandler.ts index fdd809a2e1a8f..35b948c66340d 100644 --- a/src/vs/editor/browser/controller/pointerHandler.ts +++ b/src/vs/editor/browser/controller/pointerHandler.ts @@ -99,11 +99,7 @@ class MsPointerHandler extends MouseHandler implements IDisposable { } private _onGestureChange(e: IThrottledGestureEvent): void { - const viewLayout = this._context.viewLayout; - viewLayout.setScrollPosition({ - scrollLeft: viewLayout.getScrollLeft() - e.translationX, - scrollTop: viewLayout.getScrollTop() - e.translationY, - }); + this._context.viewLayout.deltaScrollNow(-e.translationX, -e.translationY); } public dispose(): void { @@ -181,11 +177,7 @@ class StandardPointerHandler extends MouseHandler implements IDisposable { } private _onGestureChange(e: IThrottledGestureEvent): void { - const viewLayout = this._context.viewLayout; - viewLayout.setScrollPosition({ - scrollLeft: viewLayout.getScrollLeft() - e.translationX, - scrollTop: viewLayout.getScrollTop() - e.translationY, - }); + this._context.viewLayout.deltaScrollNow(-e.translationX, -e.translationY); } public dispose(): void { @@ -227,11 +219,7 @@ class TouchHandler extends MouseHandler { } private onChange(e: GestureEvent): void { - const viewLayout = this._context.viewLayout; - viewLayout.setScrollPosition({ - scrollLeft: viewLayout.getScrollLeft() - e.translationX, - scrollTop: viewLayout.getScrollTop() - e.translationY, - }); + this._context.viewLayout.deltaScrollNow(-e.translationX, -e.translationY); } } diff --git a/src/vs/editor/browser/viewParts/editorScrollbar/editorScrollbar.ts b/src/vs/editor/browser/viewParts/editorScrollbar/editorScrollbar.ts index 4f5f918abe78e..552bb8f4d2aca 100644 --- a/src/vs/editor/browser/viewParts/editorScrollbar/editorScrollbar.ts +++ b/src/vs/editor/browser/viewParts/editorScrollbar/editorScrollbar.ts @@ -6,7 +6,7 @@ import * as dom from 'vs/base/browser/dom'; import { ScrollableElementCreationOptions, ScrollableElementChangeOptions } from 'vs/base/browser/ui/scrollbar/scrollableElementOptions'; -import { IOverviewRulerLayoutInfo, ScrollableElement } from 'vs/base/browser/ui/scrollbar/scrollableElement'; +import { IOverviewRulerLayoutInfo, SmoothScrollableElement } from 'vs/base/browser/ui/scrollbar/scrollableElement'; import { INewScrollPosition } from 'vs/editor/common/editorCommon'; import { ViewPart, PartFingerprint, PartFingerprints } from 'vs/editor/browser/view/viewPart'; import { ViewContext } from 'vs/editor/common/view/viewContext'; @@ -19,7 +19,7 @@ import { ISimplifiedMouseEvent } from 'vs/base/browser/ui/scrollbar/abstractScro export class EditorScrollbar extends ViewPart { - private scrollbar: ScrollableElement; + private scrollbar: SmoothScrollableElement; private scrollbarDomNode: FastDomNode; constructor( @@ -52,7 +52,7 @@ export class EditorScrollbar extends ViewPart { mouseWheelScrollSensitivity: configScrollbarOpts.mouseWheelScrollSensitivity, }; - this.scrollbar = this._register(new ScrollableElement(linesContent.domNode, scrollbarOptions, this._context.viewLayout.scrollable)); + this.scrollbar = this._register(new SmoothScrollableElement(linesContent.domNode, scrollbarOptions, this._context.viewLayout.scrollable)); PartFingerprints.write(this.scrollbar.getDomNode(), PartFingerprint.ScrollableElement); this.scrollbarDomNode = createFastDomNode(this.scrollbar.getDomNode()); @@ -69,7 +69,7 @@ export class EditorScrollbar extends ViewPart { if (lookAtScrollTop) { let deltaTop = domNode.scrollTop; if (deltaTop) { - newScrollPosition.scrollTop = this._context.viewLayout.getScrollTop() + deltaTop; + newScrollPosition.scrollTop = this._context.viewLayout.getCurrentScrollTop() + deltaTop; domNode.scrollTop = 0; } } @@ -77,12 +77,12 @@ export class EditorScrollbar extends ViewPart { if (lookAtScrollLeft) { let deltaLeft = domNode.scrollLeft; if (deltaLeft) { - newScrollPosition.scrollLeft = this._context.viewLayout.getScrollLeft() + deltaLeft; + newScrollPosition.scrollLeft = this._context.viewLayout.getCurrentScrollLeft() + deltaLeft; domNode.scrollLeft = 0; } } - this._context.viewLayout.setScrollPosition(newScrollPosition); + this._context.viewLayout.setScrollPositionNow(newScrollPosition); }; // I've seen this happen both on the view dom node & on the lines content dom node. diff --git a/src/vs/editor/browser/viewParts/lines/viewLines.ts b/src/vs/editor/browser/viewParts/lines/viewLines.ts index 60a1d51c3ed62..3ad8d11179ea6 100644 --- a/src/vs/editor/browser/viewParts/lines/viewLines.ts +++ b/src/vs/editor/browser/viewParts/lines/viewLines.ts @@ -205,7 +205,7 @@ export class ViewLines extends ViewPart implements IVisibleLinesHost, this._lastCursorRevealRangeHorizontallyEvent = e; } - this._context.viewLayout.setScrollPosition({ // TODO@Alex: scrolling vertically can be moved to the view model + this._context.viewLayout.setScrollPositionSmooth({ // TODO@Alex: scrolling vertically can be moved to the view model scrollTop: newScrollTop }); @@ -441,6 +441,8 @@ export class ViewLines extends ViewPart implements IVisibleLinesHost, // - this must happen after the lines are in the DOM since it might need a line that rendered just now // - it might change `scrollWidth` and `scrollLeft` if (this._lastCursorRevealRangeHorizontallyEvent) { + // TODO@smooth: [MUST] the line might not be visible due to our smooth scrolling to it!!! + let revealHorizontalRange = this._lastCursorRevealRangeHorizontallyEvent.range; this._lastCursorRevealRangeHorizontallyEvent = null; @@ -457,16 +459,16 @@ export class ViewLines extends ViewPart implements IVisibleLinesHost, } // set `scrollLeft` - this._context.viewLayout.setScrollPosition({ + this._context.viewLayout.setScrollPositionSmooth({ scrollLeft: newScrollLeft.scrollLeft }); } // (3) handle scrolling this._linesContent.setLayerHinting(this._canUseLayerHinting); - const adjustedScrollTop = this._context.viewLayout.getScrollTop() - viewportData.bigNumbersDelta; + const adjustedScrollTop = this._context.viewLayout.getCurrentScrollTop() - viewportData.bigNumbersDelta; this._linesContent.setTop(-adjustedScrollTop); - this._linesContent.setLeft(-this._context.viewLayout.getScrollLeft()); + this._linesContent.setLeft(-this._context.viewLayout.getCurrentScrollLeft()); // Update max line width (not so important, it is just so the horizontal scrollbar doesn't get too small) this._asyncUpdateLineWidths.schedule(); diff --git a/src/vs/editor/browser/viewParts/minimap/minimap.ts b/src/vs/editor/browser/viewParts/minimap/minimap.ts index 206351eb6dae7..0a0c8f77b8187 100644 --- a/src/vs/editor/browser/viewParts/minimap/minimap.ts +++ b/src/vs/editor/browser/viewParts/minimap/minimap.ts @@ -528,14 +528,14 @@ export class Minimap extends ViewPart { if (platform.isWindows && mouseOrthogonalDelta > MOUSE_DRAG_RESET_DISTANCE) { // The mouse has wondered away from the scrollbar => reset dragging - this._context.viewLayout.setScrollPosition({ + this._context.viewLayout.setScrollPositionNow({ scrollTop: initialSliderState.scrollTop }); return; } const mouseDelta = mouseMoveData.posy - initialMousePosition; - this._context.viewLayout.setScrollPosition({ + this._context.viewLayout.setScrollPositionNow({ scrollTop: initialSliderState.getDesiredScrollTopFromDelta(mouseDelta) }); }, diff --git a/src/vs/editor/common/commonCodeEditor.ts b/src/vs/editor/common/commonCodeEditor.ts index c636c9f8f5fc3..1fbfb9f968d5c 100644 --- a/src/vs/editor/common/commonCodeEditor.ts +++ b/src/vs/editor/common/commonCodeEditor.ts @@ -513,7 +513,7 @@ export abstract class CommonCodeEditor extends Disposable implements editorCommo if (!this.hasView) { return -1; } - return this.viewModel.viewLayout.getScrollLeft(); + return this.viewModel.viewLayout.getCurrentScrollLeft(); } public getScrollHeight(): number { @@ -526,7 +526,7 @@ export abstract class CommonCodeEditor extends Disposable implements editorCommo if (!this.hasView) { return -1; } - return this.viewModel.viewLayout.getScrollTop(); + return this.viewModel.viewLayout.getCurrentScrollTop(); } public setScrollLeft(newScrollLeft: number): void { @@ -536,7 +536,7 @@ export abstract class CommonCodeEditor extends Disposable implements editorCommo if (typeof newScrollLeft !== 'number') { throw new Error('Invalid arguments'); } - this.viewModel.viewLayout.setScrollPosition({ + this.viewModel.viewLayout.setScrollPositionNow({ scrollLeft: newScrollLeft }); } @@ -547,7 +547,7 @@ export abstract class CommonCodeEditor extends Disposable implements editorCommo if (typeof newScrollTop !== 'number') { throw new Error('Invalid arguments'); } - this.viewModel.viewLayout.setScrollPosition({ + this.viewModel.viewLayout.setScrollPositionNow({ scrollTop: newScrollTop }); } @@ -555,7 +555,7 @@ export abstract class CommonCodeEditor extends Disposable implements editorCommo if (!this.hasView) { return; } - this.viewModel.viewLayout.setScrollPosition(position); + this.viewModel.viewLayout.setScrollPositionNow(position); } public saveViewState(): editorCommon.ICodeEditorViewState { diff --git a/src/vs/editor/common/controller/coreCommands.ts b/src/vs/editor/common/controller/coreCommands.ts index f3eaebb76c4cb..9022c8747e4a7 100644 --- a/src/vs/editor/common/controller/coreCommands.ts +++ b/src/vs/editor/common/controller/coreCommands.ts @@ -1114,7 +1114,7 @@ export namespace CoreNavigationCommands { noOfLines = args.value; } const deltaLines = (args.direction === EditorScroll_.Direction.Up ? -1 : 1) * noOfLines; - return context.getScrollTop() + deltaLines * context.config.lineHeight; + return context.getCurrentScrollTop() + deltaLines * context.config.lineHeight; } } diff --git a/src/vs/editor/common/controller/cursor.ts b/src/vs/editor/common/controller/cursor.ts index 2aa2755fe6808..6cdcd4070ce1c 100644 --- a/src/vs/editor/common/controller/cursor.ts +++ b/src/vs/editor/common/controller/cursor.ts @@ -210,7 +210,7 @@ export class Cursor extends viewEvents.ViewEventEmitter implements ICursors { } public scrollTo(desiredScrollTop: number): void { - this._viewModel.viewLayout.setScrollPosition({ + this._viewModel.viewLayout.setScrollPositionSmooth({ scrollTop: desiredScrollTop }); } diff --git a/src/vs/editor/common/controller/cursorCommon.ts b/src/vs/editor/common/controller/cursorCommon.ts index eb7b37719647c..9196b6e88303e 100644 --- a/src/vs/editor/common/controller/cursorCommon.ts +++ b/src/vs/editor/common/controller/cursorCommon.ts @@ -304,8 +304,8 @@ export class CursorContext { return this.viewModel.coordinatesConverter.convertModelRangeToViewRange(modelRange); } - public getScrollTop(): number { - return this.viewModel.viewLayout.getScrollTop(); + public getCurrentScrollTop(): number { + return this.viewModel.viewLayout.getCurrentScrollTop(); } public getCompletelyVisibleViewRange(): Range { diff --git a/src/vs/editor/common/viewLayout/viewLayout.ts b/src/vs/editor/common/viewLayout/viewLayout.ts index 3be7360c4dd8b..cde2f0bf46e66 100644 --- a/src/vs/editor/common/viewLayout/viewLayout.ts +++ b/src/vs/editor/common/viewLayout/viewLayout.ts @@ -5,7 +5,7 @@ 'use strict'; import { Disposable } from 'vs/base/common/lifecycle'; -import { Scrollable, ScrollState, ScrollEvent, ScrollbarVisibility } from 'vs/base/common/scrollable'; +import { Scrollable, ScrollEvent, ScrollbarVisibility, IScrollDimensions } from 'vs/base/common/scrollable'; import * as editorCommon from 'vs/editor/common/editorCommon'; import { LinesLayout } from 'vs/editor/common/viewLayout/linesLayout'; import { IViewLayout, IViewWhitespaceViewportData, Viewport } from 'vs/editor/common/viewModel/viewModel'; @@ -30,11 +30,12 @@ export class ViewLayout extends Disposable implements IViewLayout { this._configuration = configuration; this._linesLayout = new LinesLayout(lineCount, this._configuration.editor.lineHeight); - this.scrollable = this._register(new Scrollable()); - this.scrollable.updateState({ + // TODO@smooth: [MUST] have an editor option for smooth scrolling + this.scrollable = this._register(new Scrollable(125)); + this.scrollable.setScrollDimensions({ width: configuration.editor.layoutInfo.contentWidth, height: configuration.editor.layoutInfo.contentHeight - }, 0/* immediate */); + }); this.onDidScroll = this.scrollable.onScroll; this._updateHeight(); @@ -59,10 +60,10 @@ export class ViewLayout extends Disposable implements IViewLayout { this._linesLayout.setLineHeight(this._configuration.editor.lineHeight); } if (e.layoutInfo) { - this.scrollable.updateState({ + this.scrollable.setScrollDimensions({ width: this._configuration.editor.layoutInfo.contentWidth, height: this._configuration.editor.layoutInfo.contentHeight - }, 0/* immediate */); + }); } this._updateHeight(); } @@ -81,12 +82,12 @@ export class ViewLayout extends Disposable implements IViewLayout { // ---- end view event handlers - private _getHorizontalScrollbarHeight(scrollState: ScrollState): number { + private _getHorizontalScrollbarHeight(scrollDimensions: IScrollDimensions): number { if (this._configuration.editor.viewInfo.scrollbar.horizontal === ScrollbarVisibility.Hidden) { // horizontal scrollbar not visible return 0; } - if (scrollState.width >= scrollState.scrollWidth) { + if (scrollDimensions.width >= scrollDimensions.scrollWidth) { // horizontal scrollbar not visible return 0; } @@ -94,33 +95,34 @@ export class ViewLayout extends Disposable implements IViewLayout { } private _getTotalHeight(): number { - const scrollState = this.scrollable.getState(); + const scrollDimensions = this.scrollable.getScrollDimensions(); let result = this._linesLayout.getLinesTotalHeight(); if (this._configuration.editor.viewInfo.scrollBeyondLastLine) { - result += scrollState.height - this._configuration.editor.lineHeight; + result += scrollDimensions.height - this._configuration.editor.lineHeight; } else { - result += this._getHorizontalScrollbarHeight(scrollState); + result += this._getHorizontalScrollbarHeight(scrollDimensions); } - return Math.max(scrollState.height, result); + return Math.max(scrollDimensions.height, result); } private _updateHeight(): void { - this.scrollable.updateState({ + this.scrollable.setScrollDimensions({ scrollHeight: this._getTotalHeight() - }, 0/* immediate */); + }); } // ---- Layouting logic public getCurrentViewport(): Viewport { - const scrollState = this.scrollable.getState(); + const scrollDimensions = this.scrollable.getScrollDimensions(); + const currentScrollPosition = this.scrollable.getCurrentScrollPosition(); return new Viewport( - scrollState.scrollTop, - scrollState.scrollLeft, - scrollState.width, - scrollState.height + currentScrollPosition.scrollTop, + currentScrollPosition.scrollLeft, + scrollDimensions.width, + scrollDimensions.height ); } @@ -134,9 +136,9 @@ export class ViewLayout extends Disposable implements IViewLayout { public onMaxLineWidthChanged(maxLineWidth: number): void { let newScrollWidth = this._computeScrollWidth(maxLineWidth, this.getCurrentViewport().width); - this.scrollable.updateState({ + this.scrollable.setScrollDimensions({ scrollWidth: newScrollWidth - }, 0/* immediate */); + }); // The height might depend on the fact that there is a horizontal scrollbar or not this._updateHeight(); @@ -145,14 +147,14 @@ export class ViewLayout extends Disposable implements IViewLayout { // ---- view state public saveState(): editorCommon.IViewState { - const scrollState = this.scrollable.getState(); - let scrollTop = scrollState.scrollTop; + const currentScrollPosition = this.scrollable.getFutureScrollPosition(); + let scrollTop = currentScrollPosition.scrollTop; let firstLineNumberInViewport = this._linesLayout.getLineNumberAtOrAfterVerticalOffset(scrollTop); let whitespaceAboveFirstLine = this._linesLayout.getWhitespaceAccumulatedHeightBeforeLineNumber(firstLineNumberInViewport); return { scrollTop: scrollTop, scrollTopWithoutViewZones: scrollTop - whitespaceAboveFirstLine, - scrollLeft: scrollState.scrollLeft + scrollLeft: currentScrollPosition.scrollLeft }; } @@ -161,10 +163,10 @@ export class ViewLayout extends Disposable implements IViewLayout { if (typeof state.scrollTopWithoutViewZones === 'number' && !this._linesLayout.hasWhitespace()) { restoreScrollTop = state.scrollTopWithoutViewZones; } - this.scrollable.updateState({ + this.scrollable.setScrollPositionNow({ scrollLeft: state.scrollLeft, scrollTop: restoreScrollTop - }, 0/* immediate */); + }); } // ---- IVerticalLayoutProvider @@ -197,14 +199,14 @@ export class ViewLayout extends Disposable implements IViewLayout { } public getLinesViewportDataAtScrollTop(scrollTop: number): IPartialViewLinesViewportData { // do some minimal validations on scrollTop - const scrollState = this.scrollable.getState(); - if (scrollTop + scrollState.height > scrollState.scrollHeight) { - scrollTop = scrollState.scrollHeight - scrollState.height; + const scrollDimensions = this.scrollable.getScrollDimensions(); + if (scrollTop + scrollDimensions.height > scrollDimensions.scrollHeight) { + scrollTop = scrollDimensions.scrollHeight - scrollDimensions.height; } if (scrollTop < 0) { scrollTop = 0; } - return this._linesLayout.getLinesViewportData(scrollTop, scrollTop + scrollState.height); + return this._linesLayout.getLinesViewportData(scrollTop, scrollTop + scrollDimensions.height); } public getWhitespaceViewportData(): IViewWhitespaceViewportData[] { const visibleBox = this.getCurrentViewport(); @@ -218,37 +220,45 @@ export class ViewLayout extends Disposable implements IViewLayout { public getScrollWidth(): number { - const scrollState = this.scrollable.getState(); - return scrollState.scrollWidth; - } - public getScrollLeft(): number { - const scrollState = this.scrollable.getState(); - return scrollState.scrollLeft; + const scrollDimensions = this.scrollable.getScrollDimensions(); + return scrollDimensions.scrollWidth; } public getScrollHeight(): number { - const scrollState = this.scrollable.getState(); - return scrollState.scrollHeight; - } - public getScrollTop(): number { - const scrollState = this.scrollable.getState(); - return scrollState.scrollTop; - } - - public setScrollPosition(position: editorCommon.INewScrollPosition): void { - const state = this.scrollable.getSmoothScrollTargetState(); - const xAbsChange = Math.abs(typeof position.scrollLeft !== 'undefined' ? position.scrollLeft - state.scrollLeft : 0); - const yAbsChange = Math.abs(typeof position.scrollTop !== 'undefined' ? position.scrollTop - state.scrollTop : 0); - - if (xAbsChange || yAbsChange) { - // If position change is big enough, then appply smooth scrolling - if (xAbsChange > state.width / 10 || - yAbsChange > state.height / 10) { - this.scrollable.updateState(position, 125); - } - // Otherwise update scroll position immediately - else { - this.scrollable.updateState(position, 0/* immediate */); - } - } + const scrollDimensions = this.scrollable.getScrollDimensions(); + return scrollDimensions.scrollHeight; + } + + public getCurrentScrollLeft(): number { + const currentScrollPosition = this.scrollable.getCurrentScrollPosition(); + return currentScrollPosition.scrollLeft; + } + public getCurrentScrollTop(): number { + const currentScrollPosition = this.scrollable.getCurrentScrollPosition(); + return currentScrollPosition.scrollTop; + } + + public getFutureScrollLeft(): number { + const currentScrollPosition = this.scrollable.getFutureScrollPosition(); + return currentScrollPosition.scrollLeft; + } + public getFutureScrollTop(): number { + const currentScrollPosition = this.scrollable.getFutureScrollPosition(); + return currentScrollPosition.scrollTop; + } + + public setScrollPositionNow(position: editorCommon.INewScrollPosition): void { + this.scrollable.setScrollPositionNow(position); + } + + public setScrollPositionSmooth(position: editorCommon.INewScrollPosition): void { + this.scrollable.setScrollPositionSmooth(position); + } + + public deltaScrollNow(deltaScrollLeft: number, deltaScrollTop: number): void { + const currentScrollPosition = this.scrollable.getCurrentScrollPosition(); + this.scrollable.setScrollPositionNow({ + scrollLeft: currentScrollPosition.scrollLeft + deltaScrollLeft, + scrollTop: currentScrollPosition.scrollTop + deltaScrollTop + }); } } diff --git a/src/vs/editor/common/viewModel/viewModel.ts b/src/vs/editor/common/viewModel/viewModel.ts index d7abff3dfd326..cde6379e0bda9 100644 --- a/src/vs/editor/common/viewModel/viewModel.ts +++ b/src/vs/editor/common/viewModel/viewModel.ts @@ -44,12 +44,20 @@ export interface IViewLayout { onMaxLineWidthChanged(width: number): void; - getScrollLeft(): number; getScrollWidth(): number; getScrollHeight(): number; - getScrollTop(): number; + + getCurrentScrollLeft(): number; + getCurrentScrollTop(): number; + + getFutureScrollLeft(): number; + getFutureScrollTop(): number; + getCurrentViewport(): Viewport; - setScrollPosition(position: INewScrollPosition): void; + + setScrollPositionNow(position: INewScrollPosition): void; + setScrollPositionSmooth(position: INewScrollPosition): void; + deltaScrollNow(deltaScrollLeft: number, deltaScrollTop: number): void; getLinesViewportData(): IPartialViewLinesViewportData; getLinesViewportDataAtScrollTop(scrollTop: number): IPartialViewLinesViewportData; diff --git a/src/vs/editor/common/viewModel/viewModelImpl.ts b/src/vs/editor/common/viewModel/viewModelImpl.ts index 12b09a4a62d70..1b6adb485de7d 100644 --- a/src/vs/editor/common/viewModel/viewModelImpl.ts +++ b/src/vs/editor/common/viewModel/viewModelImpl.ts @@ -125,7 +125,7 @@ export class ViewModel extends viewEvents.ViewEventEmitter implements IViewModel this.decorations.onLineMappingChanged(); this.viewLayout.onFlushed(this.getLineCount()); - if (this.viewLayout.getScrollTop() !== 0) { + if (this.viewLayout.getCurrentScrollTop() !== 0) { // Never change the scroll position from 0 to something else... revealPreviousCenteredModelRange = true; } diff --git a/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts b/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts index c0eb5a14a1b8a..bb00ee4d2b9f9 100644 --- a/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts +++ b/src/vs/workbench/browser/parts/editor/tabsTitleControl.ts @@ -127,7 +127,7 @@ export class TabsTitleControl extends TitleControl { // Forward scrolling inside the container to our custom scrollbar this.toUnbind.push(DOM.addDisposableListener(this.tabsContainer, DOM.EventType.SCROLL, e => { if (DOM.hasClass(this.tabsContainer, 'scroll')) { - this.scrollbar.updateState({ + this.scrollbar.setScrollPosition({ scrollLeft: this.tabsContainer.scrollLeft // during DND the container gets scrolled so we need to update the custom scrollbar }); } @@ -458,7 +458,7 @@ export class TabsTitleControl extends TitleControl { const totalContainerWidth = this.tabsContainer.scrollWidth; // Update scrollbar - this.scrollbar.updateState({ + this.scrollbar.setScrollDimensions({ width: visibleContainerWidth, scrollWidth: totalContainerWidth }); @@ -478,14 +478,14 @@ export class TabsTitleControl extends TitleControl { // Tab is overflowing to the right: Scroll minimally until the element is fully visible to the right // Note: only try to do this if we actually have enough width to give to show the tab fully! if (activeTabFits && containerScrollPosX + visibleContainerWidth < activeTabPosX + activeTabWidth) { - this.scrollbar.updateState({ + this.scrollbar.setScrollPosition({ scrollLeft: containerScrollPosX + ((activeTabPosX + activeTabWidth) /* right corner of tab */ - (containerScrollPosX + visibleContainerWidth) /* right corner of view port */) }); } // Tab is overlflowng to the left or does not fit: Scroll it into view to the left else if (containerScrollPosX > activeTabPosX || !activeTabFits) { - this.scrollbar.updateState({ + this.scrollbar.setScrollPosition({ scrollLeft: this.activeTab.offsetLeft }); } @@ -565,7 +565,7 @@ export class TabsTitleControl extends TitleControl { } // moving in the tabs container can have an impact on scrolling position, so we need to update the custom scrollbar - this.scrollbar.updateState({ + this.scrollbar.setScrollPosition({ scrollLeft: this.tabsContainer.scrollLeft }); })); diff --git a/src/vs/workbench/parts/extensions/browser/extensionEditor.ts b/src/vs/workbench/parts/extensions/browser/extensionEditor.ts index c947f78d20df0..fb9961d043e61 100644 --- a/src/vs/workbench/parts/extensions/browser/extensionEditor.ts +++ b/src/vs/workbench/parts/extensions/browser/extensionEditor.ts @@ -445,8 +445,8 @@ export class ExtensionEditor extends BaseEditor { const tree = this.renderDependencies(content, extensionDependencies); const layout = () => { scrollableContent.scanDomNode(); - const scrollState = scrollableContent.getScrollState(); - tree.layout(scrollState.height); + const scrollDimensions = scrollableContent.getScrollDimensions(); + tree.layout(scrollDimensions.height); }; const removeLayoutParticipant = arrays.insert(this.layoutParticipants, { layout }); this.contentDisposables.push(toDisposable(removeLayoutParticipant)); diff --git a/src/vs/workbench/parts/welcome/walkThrough/electron-browser/walkThroughPart.ts b/src/vs/workbench/parts/welcome/walkThrough/electron-browser/walkThroughPart.ts index 763338760e905..c8c2a0ebe74a5 100644 --- a/src/vs/workbench/parts/welcome/walkThrough/electron-browser/walkThroughPart.ts +++ b/src/vs/workbench/parts/welcome/walkThrough/electron-browser/walkThroughPart.ts @@ -134,11 +134,12 @@ export class WalkThroughPart extends BaseEditor { } private updatedScrollPosition() { - const scrollState = this.scrollbar.getScrollState(); - const scrollHeight = scrollState.scrollHeight; + const scrollDimensions = this.scrollbar.getScrollDimensions(); + const scrollPosition = this.scrollbar.getScrollPosition(); + const scrollHeight = scrollDimensions.scrollHeight; if (scrollHeight && this.input instanceof WalkThroughInput) { - const scrollTop = scrollState.scrollTop; - const height = scrollState.height; + const scrollTop = scrollPosition.scrollTop; + const height = scrollDimensions.height; this.input.relativeScrollPosition(scrollTop / scrollHeight, (scrollTop + height) / scrollHeight); } } @@ -163,9 +164,9 @@ export class WalkThroughPart extends BaseEditor { this.disposables.push(this.addEventListener(this.content, 'focusin', e => { // Work around scrolling as side-effect of setting focus on the offscreen zone widget (#18929) if (e.target instanceof HTMLElement && e.target.classList.contains('zone-widget-container')) { - let scrollState = this.scrollbar.getScrollState(); - this.content.scrollTop = scrollState.scrollTop; - this.content.scrollLeft = scrollState.scrollLeft; + const scrollPosition = this.scrollbar.getScrollPosition(); + this.content.scrollTop = scrollPosition.scrollTop; + this.content.scrollLeft = scrollPosition.scrollLeft; } })); } @@ -186,7 +187,7 @@ export class WalkThroughPart extends BaseEditor { if (scrollTarget && innerContent) { const targetTop = scrollTarget.getBoundingClientRect().top - 20; const containerTop = innerContent.getBoundingClientRect().top; - this.scrollbar.updateState({ scrollTop: targetTop - containerTop }); + this.scrollbar.setScrollPosition({ scrollTop: targetTop - containerTop }); } } else { this.open(URI.parse(node.href)); @@ -261,13 +262,13 @@ export class WalkThroughPart extends BaseEditor { } arrowUp() { - const scrollState = this.scrollbar.getScrollState(); - this.scrollbar.updateState({ scrollTop: scrollState.scrollTop - this.getArrowScrollHeight() }); + const scrollPosition = this.scrollbar.getScrollPosition(); + this.scrollbar.setScrollPosition({ scrollTop: scrollPosition.scrollTop - this.getArrowScrollHeight() }); } arrowDown() { - const scrollState = this.scrollbar.getScrollState(); - this.scrollbar.updateState({ scrollTop: scrollState.scrollTop + this.getArrowScrollHeight() }); + const scrollPosition = this.scrollbar.getScrollPosition(); + this.scrollbar.setScrollPosition({ scrollTop: scrollPosition.scrollTop + this.getArrowScrollHeight() }); } private getArrowScrollHeight() { @@ -279,13 +280,15 @@ export class WalkThroughPart extends BaseEditor { } pageUp() { - const scrollState = this.scrollbar.getScrollState(); - this.scrollbar.updateState({ scrollTop: scrollState.scrollTop - scrollState.height }); + const scrollDimensions = this.scrollbar.getScrollDimensions(); + const scrollPosition = this.scrollbar.getScrollPosition(); + this.scrollbar.setScrollPosition({ scrollTop: scrollPosition.scrollTop - scrollDimensions.height }); } pageDown() { - const scrollState = this.scrollbar.getScrollState(); - this.scrollbar.updateState({ scrollTop: scrollState.scrollTop + scrollState.height }); + const scrollDimensions = this.scrollbar.getScrollDimensions(); + const scrollPosition = this.scrollbar.getScrollPosition(); + this.scrollbar.setScrollPosition({ scrollTop: scrollPosition.scrollTop + scrollDimensions.height }); } setInput(input: WalkThroughInput, options: EditorOptions): TPromise { @@ -367,13 +370,14 @@ export class WalkThroughPart extends BaseEditor { const lineHeight = editor.getConfiguration().lineHeight; const lineTop = (targetTop + (e.position.lineNumber - 1) * lineHeight) - containerTop; const lineBottom = lineTop + lineHeight; - const scrollState = this.scrollbar.getScrollState(); - const scrollTop = scrollState.scrollTop; - const height = scrollState.height; + const scrollDimensions = this.scrollbar.getScrollDimensions(); + const scrollPosition = this.scrollbar.getScrollPosition(); + const scrollTop = scrollPosition.scrollTop; + const height = scrollDimensions.height; if (scrollTop > lineTop) { - this.scrollbar.updateState({ scrollTop: lineTop }); + this.scrollbar.setScrollPosition({ scrollTop: lineTop }); } else if (scrollTop < lineBottom - height) { - this.scrollbar.updateState({ scrollTop: lineBottom - height }); + this.scrollbar.setScrollPosition({ scrollTop: lineBottom - height }); } } })); @@ -485,11 +489,11 @@ export class WalkThroughPart extends BaseEditor { memento[WALK_THROUGH_EDITOR_VIEW_STATE_PREFERENCE_KEY] = editorViewStateMemento; } - const scrollState = this.scrollbar.getScrollState(); + const scrollPosition = this.scrollbar.getScrollPosition(); const editorViewState: IWalkThroughEditorViewState = { viewState: { - scrollTop: scrollState.scrollTop, - scrollLeft: scrollState.scrollLeft + scrollTop: scrollPosition.scrollTop, + scrollLeft: scrollPosition.scrollLeft } }; @@ -512,7 +516,7 @@ export class WalkThroughPart extends BaseEditor { if (fileViewState) { const state: IWalkThroughEditorViewState = fileViewState[this.position]; if (state) { - this.scrollbar.updateState(state.viewState); + this.scrollbar.setScrollPosition(state.viewState); } } } From 53a1404a86ff682a0557973f3000b295577fb633 Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Tue, 22 Aug 2017 11:00:20 +0200 Subject: [PATCH 05/14] Validate outstanding animated scroll position target when the scroll dimensions change --- src/vs/base/common/scrollable.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/vs/base/common/scrollable.ts b/src/vs/base/common/scrollable.ts index d304a0386b664..a2c4282daa22d 100644 --- a/src/vs/base/common/scrollable.ts +++ b/src/vs/base/common/scrollable.ts @@ -208,8 +208,10 @@ export class Scrollable extends Disposable { const newState = this._state.withScrollDimensions(dimensions); this._setState(newState); - // TODO@smooth: [MUST] validate outstanding animated scroll position request target - // (in case it becomes invalid) + // Validate outstanding animated scroll position target + if (this._smoothScrolling) { + this._smoothScrolling.acceptScrollDimensions(this._state); + } } /** @@ -313,7 +315,7 @@ class SmoothScrollingUpdate implements IScrollPosition { class SmoothScrollingOperation { public readonly from: IScrollPosition; - public readonly to: IScrollPosition; + public to: IScrollPosition; public readonly duration: number; private readonly _startTime: number; public animationFrameToken: number; @@ -340,6 +342,10 @@ class SmoothScrollingOperation { } } + public acceptScrollDimensions(state: ScrollState): void { + this.to = state.withScrollPosition(this.to); + } + public tick(): SmoothScrollingUpdate { const completion = (Date.now() - this._startTime) / this.duration; From b34a82809b67b178c2ce7f97f57f6b33b448e72d Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Tue, 22 Aug 2017 11:02:12 +0200 Subject: [PATCH 06/14] Use a cubic ease-in-out function for the smooth scrolling animation --- src/vs/base/common/scrollable.ts | 37 ++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/src/vs/base/common/scrollable.ts b/src/vs/base/common/scrollable.ts index a2c4282daa22d..5c41ae736b186 100644 --- a/src/vs/base/common/scrollable.ts +++ b/src/vs/base/common/scrollable.ts @@ -262,7 +262,7 @@ export class Scrollable extends Disposable { oldSmoothScrolling.dispose(); this._smoothScrolling = newSmoothScrolling; } else { - this._smoothScrolling = new SmoothScrollingOperation(this._state, update, this._smoothScrollDuration); + this._smoothScrolling = SmoothScrollingOperation.start(this._state, update, this._smoothScrollDuration); } // Begin smooth scrolling animation @@ -320,11 +320,11 @@ class SmoothScrollingOperation { private readonly _startTime: number; public animationFrameToken: number; - constructor(from: ScrollState, to: INewScrollPosition, duration: number) { + private constructor(from: IScrollPosition, to: IScrollPosition, startTime: number, duration: number) { this.from = from; - this.to = from.withScrollPosition(to); + this.to = to; this.duration = duration; - this._startTime = Date.now(); + this._startTime = startTime; this.animationFrameToken = -1; } @@ -350,8 +350,9 @@ class SmoothScrollingOperation { const completion = (Date.now() - this._startTime) / this.duration; if (completion < 1) { - const newScrollLeft = this.from.scrollLeft + (this.to.scrollLeft - this.from.scrollLeft) * completion; - const newScrollTop = this.from.scrollTop + (this.to.scrollTop - this.from.scrollTop) * completion; + const t = easeInOutCubic(completion); + const newScrollLeft = this.from.scrollLeft + (this.to.scrollLeft - this.from.scrollLeft) * t; + const newScrollTop = this.from.scrollTop + (this.to.scrollTop - this.from.scrollTop) * t; return new SmoothScrollingUpdate(newScrollLeft, newScrollTop, false); } @@ -365,7 +366,29 @@ class SmoothScrollingOperation { scrollTop: (typeof to.scrollTop === 'undefined' ? this.to.scrollTop : to.scrollTop) }; + // Validate `to` + const validTarget = from.withScrollPosition(to); + // TODO@smooth: This is our opportunity to combine animations - return new SmoothScrollingOperation(from, to, duration); + return new SmoothScrollingOperation(from, validTarget, Date.now(), duration); + } + + public static start(from: ScrollState, to: INewScrollPosition, duration: number): SmoothScrollingOperation { + + // Validate `to` + const validTarget = from.withScrollPosition(to); + + return new SmoothScrollingOperation(from, validTarget, Date.now(), duration); + } +} + +function easeInCubic(t) { + return Math.pow(t, 3); +} + +function easeInOutCubic(t) { + if (t < 0.5) { + return easeInCubic(t * 2) / 2; } + return 1 - easeInCubic((1 - t) * 2) / 2; } From ac99d550060cb4c9ac4679e77da71b71573e9b7f Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Tue, 22 Aug 2017 12:45:57 +0200 Subject: [PATCH 07/14] * switch to using ease-out-cubic in smooth scrolling * use `dom.scheduleAtNextAnimationFrame` --- .../browser/ui/scrollbar/scrollableElement.ts | 2 +- src/vs/base/common/scrollable.ts | 98 +++++++++---------- .../editor/browser/widget/codeEditorWidget.ts | 6 +- src/vs/editor/common/commonCodeEditor.ts | 3 +- src/vs/editor/common/viewLayout/viewLayout.ts | 6 +- .../editor/common/viewModel/viewModelImpl.ts | 5 +- .../test/common/controller/cursor.test.ts | 12 +-- .../controller/cursorMoveCommand.test.ts | 2 +- .../test/common/mocks/mockCodeEditor.ts | 2 + .../test/common/viewModel/testViewModel.ts | 2 +- 10 files changed, 70 insertions(+), 68 deletions(-) diff --git a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts index b729ed33b05af..cb5370ff02170 100644 --- a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts +++ b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts @@ -374,7 +374,7 @@ export class ScrollableElement extends AbstractScrollableElement { constructor(element: HTMLElement, options: ScrollableElementCreationOptions) { options = options || {}; options.mouseWheelSmoothScroll = false; - const scrollable = new Scrollable(0); + const scrollable = new Scrollable(0, (callback) => DomUtils.scheduleAtNextAnimationFrame(callback)); super(element, options, scrollable); this._register(scrollable); } diff --git a/src/vs/base/common/scrollable.ts b/src/vs/base/common/scrollable.ts index 5c41ae736b186..4da00defa3a2d 100644 --- a/src/vs/base/common/scrollable.ts +++ b/src/vs/base/common/scrollable.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ 'use strict'; -import { Disposable } from 'vs/base/common/lifecycle'; +import { Disposable, IDisposable } from 'vs/base/common/lifecycle'; import Event, { Emitter } from 'vs/base/common/event'; export enum ScrollbarVisibility { @@ -174,16 +174,18 @@ export class Scrollable extends Disposable { _scrollableBrand: void; private readonly _smoothScrollDuration: number; + private readonly _scheduleAtNextAnimationFrame: (callback: () => void) => IDisposable; private _state: ScrollState; private _smoothScrolling: SmoothScrollingOperation; private _onScroll = this._register(new Emitter()); public onScroll: Event = this._onScroll.event; - constructor(smoothScrollDuration: number) { + constructor(smoothScrollDuration: number, scheduleAtNextAnimationFrame: (callback: () => void) => IDisposable) { super(); this._smoothScrollDuration = smoothScrollDuration; + this._scheduleAtNextAnimationFrame = scheduleAtNextAnimationFrame; this._state = new ScrollState(0, 0, 0, 0, 0, 0); this._smoothScrolling = null; } @@ -253,21 +255,33 @@ export class Scrollable extends Disposable { } if (this._smoothScrolling) { - const oldSmoothScrolling = this._smoothScrolling; - const newSmoothScrolling = oldSmoothScrolling.combine(this._state, update, this._smoothScrollDuration); - if (oldSmoothScrolling.softEquals(newSmoothScrolling)) { - // No change + // Combine our pending scrollLeft/scrollTop with incoming scrollLeft/scrollTop + update = { + scrollLeft: (typeof update.scrollLeft === 'undefined' ? this._smoothScrolling.to.scrollLeft : update.scrollLeft), + scrollTop: (typeof update.scrollTop === 'undefined' ? this._smoothScrolling.to.scrollTop : update.scrollTop) + }; + + // Validate `update` + const validTarget = this._state.withScrollPosition(update); + + if (this._smoothScrolling.to.scrollLeft === validTarget.scrollLeft && this._smoothScrolling.to.scrollTop === validTarget.scrollTop) { + // No need to interrupt or extend the current animation since we're going to the same place return; } - oldSmoothScrolling.dispose(); + + const newSmoothScrolling = this._smoothScrolling.combine(this._state, validTarget, this._smoothScrollDuration); + this._smoothScrolling.dispose(); this._smoothScrolling = newSmoothScrolling; } else { - this._smoothScrolling = SmoothScrollingOperation.start(this._state, update, this._smoothScrollDuration); + // Validate `update` + const validTarget = this._state.withScrollPosition(update); + + this._smoothScrolling = SmoothScrollingOperation.start(this._state, validTarget, this._smoothScrollDuration); } // Begin smooth scrolling animation - this._smoothScrolling.animationFrameToken = requestAnimationFrame(() => { - this._smoothScrolling.animationFrameToken = -1; + this._smoothScrolling.animationFrameDisposable = this._scheduleAtNextAnimationFrame(() => { + this._smoothScrolling.animationFrameDisposable = null; this._performSmoothScrolling(); }); } @@ -278,13 +292,17 @@ export class Scrollable extends Disposable { this._setState(newState); - if (!update.isDone) { - // Continue smooth scrolling animation - this._smoothScrolling.animationFrameToken = requestAnimationFrame(() => { - this._smoothScrolling.animationFrameToken = -1; - this._performSmoothScrolling(); - }); + if (update.isDone) { + this._smoothScrolling.dispose(); + this._smoothScrolling = null; + return; } + + // Continue smooth scrolling animation + this._smoothScrolling.animationFrameDisposable = this._scheduleAtNextAnimationFrame(() => { + this._smoothScrolling.animationFrameDisposable = null; + this._performSmoothScrolling(); + }); } private _setState(newState: ScrollState): void { @@ -318,27 +336,20 @@ class SmoothScrollingOperation { public to: IScrollPosition; public readonly duration: number; private readonly _startTime: number; - public animationFrameToken: number; + public animationFrameDisposable: IDisposable; private constructor(from: IScrollPosition, to: IScrollPosition, startTime: number, duration: number) { this.from = from; this.to = to; this.duration = duration; this._startTime = startTime; - this.animationFrameToken = -1; - } - - public softEquals(other: SmoothScrollingOperation): boolean { - return ( - this.to.scrollLeft === other.to.scrollLeft - && this.to.scrollTop === other.to.scrollTop - ); + this.animationFrameDisposable = null; } public dispose(): void { - if (this.animationFrameToken !== -1) { - cancelAnimationFrame(this.animationFrameToken); - this.animationFrameToken = -1; + if (this.animationFrameDisposable !== null) { + this.animationFrameDisposable.dispose(); + this.animationFrameDisposable = null; } } @@ -350,7 +361,7 @@ class SmoothScrollingOperation { const completion = (Date.now() - this._startTime) / this.duration; if (completion < 1) { - const t = easeInOutCubic(completion); + const t = easeOutCubic(completion); const newScrollLeft = this.from.scrollLeft + (this.to.scrollLeft - this.from.scrollLeft) * t; const newScrollTop = this.from.scrollTop + (this.to.scrollTop - this.from.scrollTop) * t; return new SmoothScrollingUpdate(newScrollLeft, newScrollTop, false); @@ -359,26 +370,12 @@ class SmoothScrollingOperation { return new SmoothScrollingUpdate(this.to.scrollLeft, this.to.scrollTop, true); } - public combine(from: ScrollState, to: INewScrollPosition, duration: number): SmoothScrollingOperation { - // Combine our scrollLeft/scrollTop with incoming scrollLeft/scrollTop - to = { - scrollLeft: (typeof to.scrollLeft === 'undefined' ? this.to.scrollLeft : to.scrollLeft), - scrollTop: (typeof to.scrollTop === 'undefined' ? this.to.scrollTop : to.scrollTop) - }; - - // Validate `to` - const validTarget = from.withScrollPosition(to); - - // TODO@smooth: This is our opportunity to combine animations - return new SmoothScrollingOperation(from, validTarget, Date.now(), duration); + public combine(from: IScrollPosition, to: IScrollPosition, duration: number): SmoothScrollingOperation { + return SmoothScrollingOperation.start(from, to, duration); } - public static start(from: ScrollState, to: INewScrollPosition, duration: number): SmoothScrollingOperation { - - // Validate `to` - const validTarget = from.withScrollPosition(to); - - return new SmoothScrollingOperation(from, validTarget, Date.now(), duration); + public static start(from: IScrollPosition, to: IScrollPosition, duration: number): SmoothScrollingOperation { + return new SmoothScrollingOperation(from, to, Date.now(), duration); } } @@ -386,9 +383,6 @@ function easeInCubic(t) { return Math.pow(t, 3); } -function easeInOutCubic(t) { - if (t < 0.5) { - return easeInCubic(t * 2) / 2; - } - return 1 - easeInCubic((1 - t) * 2) / 2; +function easeOutCubic(t) { + return 1 - easeInCubic(1 - t); } diff --git a/src/vs/editor/browser/widget/codeEditorWidget.ts b/src/vs/editor/browser/widget/codeEditorWidget.ts index 6717aa0219366..860876c299a7e 100644 --- a/src/vs/editor/browser/widget/codeEditorWidget.ts +++ b/src/vs/editor/browser/widget/codeEditorWidget.ts @@ -21,7 +21,7 @@ import { ICodeEditorService } from 'vs/editor/common/services/codeEditorService' import { Configuration } from 'vs/editor/browser/config/configuration'; import * as editorBrowser from 'vs/editor/browser/editorBrowser'; import { View, IOverlayWidgetData, IContentWidgetData } from 'vs/editor/browser/view/viewImpl'; -import { Disposable } from 'vs/base/common/lifecycle'; +import { Disposable, IDisposable } from 'vs/base/common/lifecycle'; import Event, { Emitter } from 'vs/base/common/event'; import { IKeyboardEvent } from 'vs/base/browser/keyboardEvent'; import { InternalEditorAction } from 'vs/editor/common/editorAction'; @@ -403,6 +403,10 @@ export abstract class CodeEditorWidget extends CommonCodeEditor implements edito } } + protected _scheduleAtNextAnimationFrame(callback: () => void): IDisposable { + return dom.scheduleAtNextAnimationFrame(callback); + } + protected _createView(): void { this._view = new View( this._commandService, diff --git a/src/vs/editor/common/commonCodeEditor.ts b/src/vs/editor/common/commonCodeEditor.ts index 1fbfb9f968d5c..f3a58e289d594 100644 --- a/src/vs/editor/common/commonCodeEditor.ts +++ b/src/vs/editor/common/commonCodeEditor.ts @@ -862,7 +862,7 @@ export abstract class CommonCodeEditor extends Disposable implements editorCommo this.model.onBeforeAttached(); - this.viewModel = new ViewModel(this.id, this._configuration, this.model); + this.viewModel = new ViewModel(this.id, this._configuration, this.model, (callback) => this._scheduleAtNextAnimationFrame(callback)); this.listenersToRemove.push(this.model.addBulkListener((events) => { for (let i = 0, len = events.length; i < len; i++) { @@ -935,6 +935,7 @@ export abstract class CommonCodeEditor extends Disposable implements editorCommo } } + protected abstract _scheduleAtNextAnimationFrame(callback: () => void): IDisposable; protected abstract _createView(): void; protected _postDetachModelCleanup(detachedModel: editorCommon.IModel): void { diff --git a/src/vs/editor/common/viewLayout/viewLayout.ts b/src/vs/editor/common/viewLayout/viewLayout.ts index cde2f0bf46e66..ef506cc7284cf 100644 --- a/src/vs/editor/common/viewLayout/viewLayout.ts +++ b/src/vs/editor/common/viewLayout/viewLayout.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ 'use strict'; -import { Disposable } from 'vs/base/common/lifecycle'; +import { Disposable, IDisposable } from 'vs/base/common/lifecycle'; import { Scrollable, ScrollEvent, ScrollbarVisibility, IScrollDimensions } from 'vs/base/common/scrollable'; import * as editorCommon from 'vs/editor/common/editorCommon'; import { LinesLayout } from 'vs/editor/common/viewLayout/linesLayout'; @@ -24,14 +24,14 @@ export class ViewLayout extends Disposable implements IViewLayout { public readonly scrollable: Scrollable; public readonly onDidScroll: Event; - constructor(configuration: editorCommon.IConfiguration, lineCount: number) { + constructor(configuration: editorCommon.IConfiguration, lineCount: number, scheduleAtNextAnimationFrame: (callback: () => void) => IDisposable) { super(); this._configuration = configuration; this._linesLayout = new LinesLayout(lineCount, this._configuration.editor.lineHeight); // TODO@smooth: [MUST] have an editor option for smooth scrolling - this.scrollable = this._register(new Scrollable(125)); + this.scrollable = this._register(new Scrollable(125, scheduleAtNextAnimationFrame)); this.scrollable.setScrollDimensions({ width: configuration.editor.layoutInfo.contentWidth, height: configuration.editor.layoutInfo.contentHeight diff --git a/src/vs/editor/common/viewModel/viewModelImpl.ts b/src/vs/editor/common/viewModel/viewModelImpl.ts index 1b6adb485de7d..aaef276893278 100644 --- a/src/vs/editor/common/viewModel/viewModelImpl.ts +++ b/src/vs/editor/common/viewModel/viewModelImpl.ts @@ -21,6 +21,7 @@ import { IConfigurationChangedEvent } from 'vs/editor/common/config/editorOption import { CharacterHardWrappingLineMapperFactory } from 'vs/editor/common/viewModel/characterHardWrappingLineMapper'; import { ViewLayout } from 'vs/editor/common/viewLayout/viewLayout'; import { Color } from 'vs/base/common/color'; +import { IDisposable } from "vs/base/common/lifecycle"; const USE_IDENTITY_LINES_COLLECTION = true; @@ -38,7 +39,7 @@ export class ViewModel extends viewEvents.ViewEventEmitter implements IViewModel private _isDisposing: boolean; private _centeredViewLine: number; - constructor(editorId: number, configuration: editorCommon.IConfiguration, model: editorCommon.IModel) { + constructor(editorId: number, configuration: editorCommon.IConfiguration, model: editorCommon.IModel, scheduleAtNextAnimationFrame: (callback: () => void) => IDisposable) { super(); this.editorId = editorId; @@ -70,7 +71,7 @@ export class ViewModel extends viewEvents.ViewEventEmitter implements IViewModel this.coordinatesConverter = this.lines.createCoordinatesConverter(); - this.viewLayout = this._register(new ViewLayout(this.configuration, this.getLineCount())); + this.viewLayout = this._register(new ViewLayout(this.configuration, this.getLineCount(), scheduleAtNextAnimationFrame)); this._register(this.viewLayout.onDidScroll((e) => { this._emit([new viewEvents.ViewScrollChangedEvent(e)]); diff --git a/src/vs/editor/test/common/controller/cursor.test.ts b/src/vs/editor/test/common/controller/cursor.test.ts index 4b18c7cdc14b6..b71db2b9c93c2 100644 --- a/src/vs/editor/test/common/controller/cursor.test.ts +++ b/src/vs/editor/test/common/controller/cursor.test.ts @@ -152,7 +152,7 @@ suite('Editor Controller - Cursor', () => { thisModel = Model.createFromString(text); thisConfiguration = new TestConfiguration(null); - thisViewModel = new ViewModel(0, thisConfiguration, thisModel); + thisViewModel = new ViewModel(0, thisConfiguration, thisModel, null); thisCursor = new Cursor(thisConfiguration, thisModel, thisViewModel); }); @@ -736,7 +736,7 @@ suite('Editor Controller - Cursor', () => { 'var newer = require("gulp-newer");', ].join('\n')); const config = new TestConfiguration(null); - const viewModel = new ViewModel(0, config, model); + const viewModel = new ViewModel(0, config, model, null); const cursor = new Cursor(config, model, viewModel); moveTo(cursor, 1, 4, false); @@ -775,7 +775,7 @@ suite('Editor Controller - Cursor', () => { '', ].join('\n')); const config = new TestConfiguration(null); - const viewModel = new ViewModel(0, config, model); + const viewModel = new ViewModel(0, config, model, null); const cursor = new Cursor(config, model, viewModel); moveTo(cursor, 10, 10, false); @@ -837,7 +837,7 @@ suite('Editor Controller - Cursor', () => { '', ].join('\n')); const config = new TestConfiguration(null); - const viewModel = new ViewModel(0, config, model); + const viewModel = new ViewModel(0, config, model, null); const cursor = new Cursor(config, model, viewModel); moveTo(cursor, 10, 10, false); @@ -886,7 +886,7 @@ suite('Editor Controller - Cursor', () => { 'var newer = require("gulp-newer");', ].join('\n')); const config = new TestConfiguration(null); - const viewModel = new ViewModel(0, config, model); + const viewModel = new ViewModel(0, config, model, null); const cursor = new Cursor(config, model, viewModel); moveTo(cursor, 1, 4, false); @@ -3118,7 +3118,7 @@ function usingCursor(opts: ICursorOpts, callback: (model: Model, cursor: Cursor) let model = Model.createFromString(opts.text.join('\n'), opts.modelOpts, opts.languageIdentifier); model.forceTokenization(model.getLineCount()); let config = new TestConfiguration(opts.editorOpts); - let viewModel = new ViewModel(0, config, model); + let viewModel = new ViewModel(0, config, model, null); let cursor = new Cursor(config, model, viewModel); callback(model, cursor); diff --git a/src/vs/editor/test/common/controller/cursorMoveCommand.test.ts b/src/vs/editor/test/common/controller/cursorMoveCommand.test.ts index 2b9ac20069d6f..57fe5b79395dd 100644 --- a/src/vs/editor/test/common/controller/cursorMoveCommand.test.ts +++ b/src/vs/editor/test/common/controller/cursorMoveCommand.test.ts @@ -33,7 +33,7 @@ suite('Cursor move command test', () => { thisModel = Model.createFromString(text); thisConfiguration = new TestConfiguration(null); - thisViewModel = new ViewModel(0, thisConfiguration, thisModel); + thisViewModel = new ViewModel(0, thisConfiguration, thisModel, null); thisCursor = new Cursor(thisConfiguration, thisModel, thisViewModel); }); diff --git a/src/vs/editor/test/common/mocks/mockCodeEditor.ts b/src/vs/editor/test/common/mocks/mockCodeEditor.ts index 52e03d799064c..bc2370fcd46a0 100644 --- a/src/vs/editor/test/common/mocks/mockCodeEditor.ts +++ b/src/vs/editor/test/common/mocks/mockCodeEditor.ts @@ -15,6 +15,7 @@ import * as editorCommon from 'vs/editor/common/editorCommon'; import { Model } from 'vs/editor/common/model/model'; import { TestConfiguration } from 'vs/editor/test/common/mocks/testConfiguration'; import * as editorOptions from 'vs/editor/common/config/editorOptions'; +import { IDisposable } from "vs/base/common/lifecycle"; export class MockCodeEditor extends CommonCodeEditor { protected _createConfiguration(options: editorOptions.IEditorOptions): CommonEditorConfiguration { @@ -28,6 +29,7 @@ export class MockCodeEditor extends CommonCodeEditor { public hasWidgetFocus(): boolean { return true; }; protected _enableEmptySelectionClipboard(): boolean { return false; } + protected _scheduleAtNextAnimationFrame(callback: () => void): IDisposable { throw new Error('Notimplemented'); } protected _createView(): void { } protected _registerDecorationType(key: string, options: editorCommon.IDecorationRenderOptions, parentTypeKey?: string): void { throw new Error('NotImplemented'); } diff --git a/src/vs/editor/test/common/viewModel/testViewModel.ts b/src/vs/editor/test/common/viewModel/testViewModel.ts index a041d80037a54..9584e043983df 100644 --- a/src/vs/editor/test/common/viewModel/testViewModel.ts +++ b/src/vs/editor/test/common/viewModel/testViewModel.ts @@ -16,7 +16,7 @@ export function testViewModel(text: string[], options: MockCodeEditorCreationOpt let model = Model.createFromString(text.join('\n')); - let viewModel = new ViewModel(EDITOR_ID, configuration, model); + let viewModel = new ViewModel(EDITOR_ID, configuration, model, null); callback(viewModel, model); From 57f04c62b235d35be0410e0396d29b3096585253 Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Tue, 22 Aug 2017 12:47:27 +0200 Subject: [PATCH 08/14] No JS smooth scrolling in the `mousewheel` case --- .../browser/ui/scrollbar/scrollableElement.ts | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts index cb5370ff02170..4454347c33e41 100644 --- a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts +++ b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts @@ -22,7 +22,6 @@ import Event, { Emitter } from 'vs/base/common/event'; const HIDE_TIMEOUT = 500; const SCROLL_WHEEL_SENSITIVITY = 50; -const SCROLL_WHEEL_SMOOTH_SCROLL_THRESHOLD = 1.7; export interface IOverviewRulerLayoutInfo { parent: HTMLElement; @@ -253,22 +252,7 @@ export abstract class AbstractScrollableElement extends Widget { desiredScrollPosition = this._scrollable.validateScrollPosition(desiredScrollPosition); if (futureScrollPosition.scrollLeft !== desiredScrollPosition.scrollLeft || futureScrollPosition.scrollTop !== desiredScrollPosition.scrollTop) { - // TODO@smooth: [MUST] implement better heuristic for distinguishing inertia scrolling - // from physical mouse wheels - const ENABLE_MOUSE_WHEEL_SMOOTH = false; - - // If |∆x| and |∆y| are too small then do not apply smooth scroll animation, because in that case the input source must be a touchpad or something similar. - const smoothScrollThresholdReached = ( - Math.abs(deltaY) > SCROLL_WHEEL_SMOOTH_SCROLL_THRESHOLD - || Math.abs(deltaX) > SCROLL_WHEEL_SMOOTH_SCROLL_THRESHOLD - ); - - if (ENABLE_MOUSE_WHEEL_SMOOTH && this._options.mouseWheelSmoothScroll && smoothScrollThresholdReached) { - this._scrollable.setScrollPositionSmooth(desiredScrollPosition); - } else { - this._scrollable.setScrollPositionNow(desiredScrollPosition); - } - + this._scrollable.setScrollPositionNow(desiredScrollPosition); this._shouldRender = true; } } From a6623de2b2abfc9ee9c1e46dd1263cbdbdd477f7 Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Tue, 22 Aug 2017 13:01:23 +0200 Subject: [PATCH 09/14] Add `editor.smoothScrolling` that controls if smooth scrolling should be on/off --- src/vs/base/common/scrollable.ts | 6 +++++- src/vs/editor/common/config/commonEditorConfig.ts | 5 +++++ src/vs/editor/common/config/editorOptions.ts | 10 ++++++++++ src/vs/editor/common/viewLayout/viewLayout.ts | 14 ++++++++++++-- src/vs/monaco.d.ts | 6 ++++++ 5 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/vs/base/common/scrollable.ts b/src/vs/base/common/scrollable.ts index 4da00defa3a2d..c9f3dc2fb6288 100644 --- a/src/vs/base/common/scrollable.ts +++ b/src/vs/base/common/scrollable.ts @@ -173,7 +173,7 @@ export class Scrollable extends Disposable { _scrollableBrand: void; - private readonly _smoothScrollDuration: number; + private _smoothScrollDuration: number; private readonly _scheduleAtNextAnimationFrame: (callback: () => void) => IDisposable; private _state: ScrollState; private _smoothScrolling: SmoothScrollingOperation; @@ -198,6 +198,10 @@ export class Scrollable extends Disposable { super.dispose(); } + public setSmoothScrollDuration(smoothScrollDuration: number): void { + this._smoothScrollDuration = smoothScrollDuration; + } + public validateScrollPosition(scrollPosition: INewScrollPosition): IScrollPosition { return this._state.withScrollPosition(scrollPosition); } diff --git a/src/vs/editor/common/config/commonEditorConfig.ts b/src/vs/editor/common/config/commonEditorConfig.ts index c0666b78a986c..11ef7a113739b 100644 --- a/src/vs/editor/common/config/commonEditorConfig.ts +++ b/src/vs/editor/common/config/commonEditorConfig.ts @@ -248,6 +248,11 @@ const editorConfiguration: IConfigurationNode = { 'default': EDITOR_DEFAULTS.viewInfo.scrollBeyondLastLine, 'description': nls.localize('scrollBeyondLastLine', "Controls if the editor will scroll beyond the last line") }, + 'editor.smoothScrolling': { + 'type': 'boolean', + 'default': EDITOR_DEFAULTS.viewInfo.smoothScrolling, + 'description': nls.localize('smoothScrolling', "Controls if the editor will scroll using an animation") + }, 'editor.minimap.enabled': { 'type': 'boolean', 'default': EDITOR_DEFAULTS.viewInfo.minimap.enabled, diff --git a/src/vs/editor/common/config/editorOptions.ts b/src/vs/editor/common/config/editorOptions.ts index 5c90ebb782f30..5be479dd74198 100644 --- a/src/vs/editor/common/config/editorOptions.ts +++ b/src/vs/editor/common/config/editorOptions.ts @@ -266,6 +266,11 @@ export interface IEditorOptions { * Defaults to true. */ scrollBeyondLastLine?: boolean; + /** + * Enable that the editor animates scrolling to a position. + * Defaults to true. + */ + smoothScrolling?: boolean; /** * Enable that the editor will install an interval to check if its container dom node size has changed. * Enabling this might have a severe performance impact. @@ -748,6 +753,7 @@ export interface InternalEditorViewOptions { readonly cursorStyle: TextEditorCursorStyle; readonly hideCursorInOverviewRuler: boolean; readonly scrollBeyondLastLine: boolean; + readonly smoothScrolling: boolean; readonly stopRenderingLineAfter: number; readonly renderWhitespace: 'none' | 'boundary' | 'all'; readonly renderControlCharacters: boolean; @@ -1014,6 +1020,7 @@ export class InternalEditorOptions { && a.cursorStyle === b.cursorStyle && a.hideCursorInOverviewRuler === b.hideCursorInOverviewRuler && a.scrollBeyondLastLine === b.scrollBeyondLastLine + && a.smoothScrolling === b.smoothScrolling && a.stopRenderingLineAfter === b.stopRenderingLineAfter && a.renderWhitespace === b.renderWhitespace && a.renderControlCharacters === b.renderControlCharacters @@ -1609,6 +1616,7 @@ export class EditorOptionsValidator { cursorStyle: _cursorStyleFromString(opts.cursorStyle, defaults.cursorStyle), hideCursorInOverviewRuler: _boolean(opts.hideCursorInOverviewRuler, defaults.hideCursorInOverviewRuler), scrollBeyondLastLine: _boolean(opts.scrollBeyondLastLine, defaults.scrollBeyondLastLine), + smoothScrolling: _boolean(opts.smoothScrolling, defaults.smoothScrolling), stopRenderingLineAfter: _clampedInt(opts.stopRenderingLineAfter, defaults.stopRenderingLineAfter, -1, Constants.MAX_SAFE_SMALL_INTEGER), renderWhitespace: renderWhitespace, renderControlCharacters: _boolean(opts.renderControlCharacters, defaults.renderControlCharacters), @@ -1709,6 +1717,7 @@ export class InternalEditorOptionsFactory { cursorStyle: opts.viewInfo.cursorStyle, hideCursorInOverviewRuler: opts.viewInfo.hideCursorInOverviewRuler, scrollBeyondLastLine: opts.viewInfo.scrollBeyondLastLine, + smoothScrolling: opts.viewInfo.smoothScrolling, stopRenderingLineAfter: opts.viewInfo.stopRenderingLineAfter, renderWhitespace: (accessibilityIsOn ? 'none' : opts.viewInfo.renderWhitespace), // DISABLED WHEN SCREEN READER IS ATTACHED renderControlCharacters: (accessibilityIsOn ? false : opts.viewInfo.renderControlCharacters), // DISABLED WHEN SCREEN READER IS ATTACHED @@ -2130,6 +2139,7 @@ export const EDITOR_DEFAULTS: IValidatedEditorOptions = { cursorStyle: TextEditorCursorStyle.Line, hideCursorInOverviewRuler: false, scrollBeyondLastLine: true, + smoothScrolling: true, stopRenderingLineAfter: 10000, renderWhitespace: 'none', renderControlCharacters: false, diff --git a/src/vs/editor/common/viewLayout/viewLayout.ts b/src/vs/editor/common/viewLayout/viewLayout.ts index ef506cc7284cf..3172366b9812a 100644 --- a/src/vs/editor/common/viewLayout/viewLayout.ts +++ b/src/vs/editor/common/viewLayout/viewLayout.ts @@ -14,6 +14,8 @@ import { IEditorWhitespace } from 'vs/editor/common/viewLayout/whitespaceCompute import Event from 'vs/base/common/event'; import { IConfigurationChangedEvent } from 'vs/editor/common/config/editorOptions'; +const SMOOTH_SCROLLING_TIME = 125; + export class ViewLayout extends Disposable implements IViewLayout { static LINES_HORIZONTAL_EXTRA_PX = 30; @@ -30,8 +32,9 @@ export class ViewLayout extends Disposable implements IViewLayout { this._configuration = configuration; this._linesLayout = new LinesLayout(lineCount, this._configuration.editor.lineHeight); - // TODO@smooth: [MUST] have an editor option for smooth scrolling - this.scrollable = this._register(new Scrollable(125, scheduleAtNextAnimationFrame)); + this.scrollable = this._register(new Scrollable(0, scheduleAtNextAnimationFrame)); + this._configureSmoothScrollDuration(); + this.scrollable.setScrollDimensions({ width: configuration.editor.layoutInfo.contentWidth, height: configuration.editor.layoutInfo.contentHeight @@ -53,6 +56,10 @@ export class ViewLayout extends Disposable implements IViewLayout { this._updateHeight(); } + private _configureSmoothScrollDuration(): void { + this.scrollable.setSmoothScrollDuration(this._configuration.editor.viewInfo.smoothScrolling ? SMOOTH_SCROLLING_TIME : 0); + } + // ---- begin view event handlers public onConfigurationChanged(e: IConfigurationChangedEvent): void { @@ -65,6 +72,9 @@ export class ViewLayout extends Disposable implements IViewLayout { height: this._configuration.editor.layoutInfo.contentHeight }); } + if (e.viewInfo) { + this._configureSmoothScrollDuration(); + } this._updateHeight(); } public onFlushed(lineCount: number): void { diff --git a/src/vs/monaco.d.ts b/src/vs/monaco.d.ts index 97c8a13a94d56..b137933244cac 100644 --- a/src/vs/monaco.d.ts +++ b/src/vs/monaco.d.ts @@ -2824,6 +2824,11 @@ declare module monaco.editor { * Defaults to true. */ scrollBeyondLastLine?: boolean; + /** + * Enable that the editor animates scrolling to a position. + * Defaults to true. + */ + smoothScrolling?: boolean; /** * Enable that the editor will install an interval to check if its container dom node size has changed. * Enabling this might have a severe performance impact. @@ -3245,6 +3250,7 @@ declare module monaco.editor { readonly cursorStyle: TextEditorCursorStyle; readonly hideCursorInOverviewRuler: boolean; readonly scrollBeyondLastLine: boolean; + readonly smoothScrolling: boolean; readonly stopRenderingLineAfter: number; readonly renderWhitespace: 'none' | 'boundary' | 'all'; readonly renderControlCharacters: boolean; From 3b433ea24da6791002c728e19ea9b6531ba0f4b7 Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Tue, 22 Aug 2017 15:35:54 +0200 Subject: [PATCH 10/14] Fix horizontal revealing of a range (in the smooth scrolling case where the line containing the range might come in the viewport at a later time) --- .../browser/ui/scrollbar/scrollableElement.ts | 1 + .../browser/viewParts/lines/viewLines.ts | 107 ++++++++++++------ src/vs/editor/common/viewLayout/viewLayout.ts | 11 +- src/vs/editor/common/viewModel/viewModel.ts | 7 +- 4 files changed, 79 insertions(+), 47 deletions(-) diff --git a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts index 4454347c33e41..1690d4e3a91aa 100644 --- a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts +++ b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts @@ -252,6 +252,7 @@ export abstract class AbstractScrollableElement extends Widget { desiredScrollPosition = this._scrollable.validateScrollPosition(desiredScrollPosition); if (futureScrollPosition.scrollLeft !== desiredScrollPosition.scrollLeft || futureScrollPosition.scrollTop !== desiredScrollPosition.scrollTop) { + // TODO@smooth: [MUST] detect if the source of the `mousewheel` is intertial or not and use setScrollPositionSmooth this._scrollable.setScrollPositionNow(desiredScrollPosition); this._shouldRender = true; } diff --git a/src/vs/editor/browser/viewParts/lines/viewLines.ts b/src/vs/editor/browser/viewParts/lines/viewLines.ts index 3ad8d11179ea6..f7db650f05e49 100644 --- a/src/vs/editor/browser/viewParts/lines/viewLines.ts +++ b/src/vs/editor/browser/viewParts/lines/viewLines.ts @@ -36,6 +36,23 @@ class LastRenderedData { } } +class HorizontalRevealRequest { + + public readonly lineNumber: number; + public readonly startColumn: number; + public readonly endColumn: number; + public readonly startScrollTop: number; + public readonly stopScrollTop: number; + + constructor(lineNumber: number, startColumn: number, endColumn: number, startScrollTop: number, stopScrollTop: number) { + this.lineNumber = lineNumber; + this.startColumn = startColumn; + this.endColumn = endColumn; + this.startScrollTop = startScrollTop; + this.stopScrollTop = stopScrollTop; + } +} + export class ViewLines extends ViewPart implements IVisibleLinesHost, IViewLines { /** * Adds this ammount of pixels to the right of lines (no-one wants to type near the edge of the viewport) @@ -59,7 +76,7 @@ export class ViewLines extends ViewPart implements IVisibleLinesHost, private _maxLineWidth: number; private _asyncUpdateLineWidths: RunOnceScheduler; - private _lastCursorRevealRangeHorizontallyEvent: viewEvents.ViewRevealRangeRequestEvent; + private _horizontalRevealRequest: HorizontalRevealRequest; private _lastRenderedData: LastRenderedData; constructor(context: ViewContext, linesContent: FastDomNode) { @@ -90,7 +107,7 @@ export class ViewLines extends ViewPart implements IVisibleLinesHost, this._lastRenderedData = new LastRenderedData(); - this._lastCursorRevealRangeHorizontallyEvent = null; + this._horizontalRevealRequest = null; } public dispose(): void { @@ -199,19 +216,43 @@ export class ViewLines extends ViewPart implements IVisibleLinesHost, return this._visibleLines.onLinesInserted(e); } public onRevealRangeRequest(e: viewEvents.ViewRevealRangeRequestEvent): boolean { - let newScrollTop = this._computeScrollTopToRevealRange(this._context.viewLayout.getCurrentViewport(), e.range, e.verticalType); + let desiredScrollTop = this._computeScrollTopToRevealRange(this._context.viewLayout.getCurrentViewport(), e.range, e.verticalType); + + // validate the new desired scroll top + let newScrollPosition = this._context.viewLayout.validateScrollPosition({ scrollTop: desiredScrollTop }); if (e.revealHorizontal) { - this._lastCursorRevealRangeHorizontallyEvent = e; + if (e.range.startLineNumber !== e.range.endLineNumber) { + // Two or more lines? => scroll to base (That's how you see most of the two lines) + newScrollPosition = { + scrollTop: newScrollPosition.scrollTop, + scrollLeft: 0 + }; + } else { + // We don't necessarily know the horizontal offset of this range since the line might not be in the view... + this._horizontalRevealRequest = new HorizontalRevealRequest(e.range.startLineNumber, e.range.startColumn, e.range.endColumn, this._context.viewLayout.getCurrentScrollTop(), newScrollPosition.scrollTop); + } + } else { + this._horizontalRevealRequest = null; } - this._context.viewLayout.setScrollPositionSmooth({ // TODO@Alex: scrolling vertically can be moved to the view model - scrollTop: newScrollTop - }); + this._context.viewLayout.setScrollPositionSmooth(newScrollPosition); return true; } public onScrollChanged(e: viewEvents.ViewScrollChangedEvent): boolean { + if (this._horizontalRevealRequest && e.scrollLeftChanged) { + // cancel any outstanding horizontal reveal request if someone else scrolls horizontally. + this._horizontalRevealRequest = null; + } + if (this._horizontalRevealRequest && e.scrollTopChanged) { + const min = Math.min(this._horizontalRevealRequest.startScrollTop, this._horizontalRevealRequest.stopScrollTop); + const max = Math.max(this._horizontalRevealRequest.startScrollTop, this._horizontalRevealRequest.stopScrollTop); + if (e.scrollTop < min || e.scrollTop > max) { + // cancel any outstanding horizontal reveal request if someone else scrolls vertically. + this._horizontalRevealRequest = null; + } + } this.domNode.setWidth(e.scrollWidth); return this._visibleLines.onScrollChanged(e) || true; } @@ -440,28 +481,34 @@ export class ViewLines extends ViewPart implements IVisibleLinesHost, // (2) compute horizontal scroll position: // - this must happen after the lines are in the DOM since it might need a line that rendered just now // - it might change `scrollWidth` and `scrollLeft` - if (this._lastCursorRevealRangeHorizontallyEvent) { - // TODO@smooth: [MUST] the line might not be visible due to our smooth scrolling to it!!! + if (this._horizontalRevealRequest) { - let revealHorizontalRange = this._lastCursorRevealRangeHorizontallyEvent.range; - this._lastCursorRevealRangeHorizontallyEvent = null; + const revealLineNumber = this._horizontalRevealRequest.lineNumber; + const revealStartColumn = this._horizontalRevealRequest.startColumn; + const revealEndColumn = this._horizontalRevealRequest.endColumn; - // allow `visibleRangesForRange2` to work - this.onDidRender(); + // Check that we have the line that contains the horizontal range in the viewport + if (viewportData.startLineNumber <= revealLineNumber && revealLineNumber <= viewportData.endLineNumber) { - // compute new scroll position - let newScrollLeft = this._computeScrollLeftToRevealRange(revealHorizontalRange); + this._horizontalRevealRequest = null; - let isViewportWrapping = this._isViewportWrapping; - if (!isViewportWrapping) { - // ensure `scrollWidth` is large enough - this._ensureMaxLineWidth(newScrollLeft.maxHorizontalOffset); - } + // allow `visibleRangesForRange2` to work + this.onDidRender(); + + // compute new scroll position + let newScrollLeft = this._computeScrollLeftToRevealRange(revealLineNumber, revealStartColumn, revealEndColumn); - // set `scrollLeft` - this._context.viewLayout.setScrollPositionSmooth({ - scrollLeft: newScrollLeft.scrollLeft - }); + let isViewportWrapping = this._isViewportWrapping; + if (!isViewportWrapping) { + // ensure `scrollWidth` is large enough + this._ensureMaxLineWidth(newScrollLeft.maxHorizontalOffset); + } + + // set `scrollLeft` + this._context.viewLayout.setScrollPositionSmooth({ + scrollLeft: newScrollLeft.scrollLeft + }); + } } // (3) handle scrolling @@ -517,23 +564,15 @@ export class ViewLines extends ViewPart implements IVisibleLinesHost, return newScrollTop; } - private _computeScrollLeftToRevealRange(range: Range): { scrollLeft: number; maxHorizontalOffset: number; } { + private _computeScrollLeftToRevealRange(lineNumber: number, startColumn: number, endColumn: number): { scrollLeft: number; maxHorizontalOffset: number; } { let maxHorizontalOffset = 0; - if (range.startLineNumber !== range.endLineNumber) { - // Two or more lines? => scroll to base (That's how you see most of the two lines) - return { - scrollLeft: 0, - maxHorizontalOffset: maxHorizontalOffset - }; - } - let viewport = this._context.viewLayout.getCurrentViewport(); let viewportStartX = viewport.left; let viewportEndX = viewportStartX + viewport.width; - let visibleRanges = this.visibleRangesForRange2(range); + let visibleRanges = this.visibleRangesForRange2(new Range(lineNumber, startColumn, lineNumber, endColumn)); let boxStartX = Number.MAX_VALUE; let boxEndX = 0; diff --git a/src/vs/editor/common/viewLayout/viewLayout.ts b/src/vs/editor/common/viewLayout/viewLayout.ts index 3172366b9812a..17575467cf08a 100644 --- a/src/vs/editor/common/viewLayout/viewLayout.ts +++ b/src/vs/editor/common/viewLayout/viewLayout.ts @@ -5,7 +5,7 @@ 'use strict'; import { Disposable, IDisposable } from 'vs/base/common/lifecycle'; -import { Scrollable, ScrollEvent, ScrollbarVisibility, IScrollDimensions } from 'vs/base/common/scrollable'; +import { Scrollable, ScrollEvent, ScrollbarVisibility, IScrollDimensions, IScrollPosition } from 'vs/base/common/scrollable'; import * as editorCommon from 'vs/editor/common/editorCommon'; import { LinesLayout } from 'vs/editor/common/viewLayout/linesLayout'; import { IViewLayout, IViewWhitespaceViewportData, Viewport } from 'vs/editor/common/viewModel/viewModel'; @@ -247,13 +247,8 @@ export class ViewLayout extends Disposable implements IViewLayout { return currentScrollPosition.scrollTop; } - public getFutureScrollLeft(): number { - const currentScrollPosition = this.scrollable.getFutureScrollPosition(); - return currentScrollPosition.scrollLeft; - } - public getFutureScrollTop(): number { - const currentScrollPosition = this.scrollable.getFutureScrollPosition(); - return currentScrollPosition.scrollTop; + public validateScrollPosition(scrollPosition: editorCommon.INewScrollPosition): IScrollPosition { + return this.scrollable.validateScrollPosition(scrollPosition); } public setScrollPositionNow(position: editorCommon.INewScrollPosition): void { diff --git a/src/vs/editor/common/viewModel/viewModel.ts b/src/vs/editor/common/viewModel/viewModel.ts index cde6379e0bda9..e506f578b2ecd 100644 --- a/src/vs/editor/common/viewModel/viewModel.ts +++ b/src/vs/editor/common/viewModel/viewModel.ts @@ -11,7 +11,7 @@ import { Range } from 'vs/editor/common/core/range'; import { Selection } from 'vs/editor/common/core/selection'; import { ViewEvent, IViewEventListener } from 'vs/editor/common/view/viewEvents'; import { IDisposable } from 'vs/base/common/lifecycle'; -import { Scrollable } from 'vs/base/common/scrollable'; +import { Scrollable, IScrollPosition } from 'vs/base/common/scrollable'; import { IPartialViewLinesViewportData } from 'vs/editor/common/viewLayout/viewLinesViewportData'; import { IEditorWhitespace } from 'vs/editor/common/viewLayout/whitespaceComputer'; @@ -49,12 +49,9 @@ export interface IViewLayout { getCurrentScrollLeft(): number; getCurrentScrollTop(): number; - - getFutureScrollLeft(): number; - getFutureScrollTop(): number; - getCurrentViewport(): Viewport; + validateScrollPosition(scrollPosition: INewScrollPosition): IScrollPosition; setScrollPositionNow(position: INewScrollPosition): void; setScrollPositionSmooth(position: INewScrollPosition): void; deltaScrollNow(deltaScrollLeft: number, deltaScrollTop: number): void; From b38ba93f0030da2526fe6c3d429bcc93defe140b Mon Sep 17 00:00:00 2001 From: "Nicola Fiori (JD342)" Date: Tue, 22 Aug 2017 15:49:14 +0200 Subject: [PATCH 11/14] Add smooth scroll on mousewheel with new heuristic --- .../browser/ui/scrollbar/scrollableElement.ts | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts index 1690d4e3a91aa..07c6ec9fc78e3 100644 --- a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts +++ b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts @@ -22,6 +22,9 @@ import Event, { Emitter } from 'vs/base/common/event'; const HIDE_TIMEOUT = 500; const SCROLL_WHEEL_SENSITIVITY = 50; +const SCROLL_WHEEL_SMOOTH_SCROLL_ENABLED = true; +const SCROLL_WHEEL_SMOOTH_SCROLL_MS_TIME_TRESHOLD = 4; +const SCROLL_WHEEL_SMOOTH_SCROLL_PX_SPACE_TRESHOLD = 7; export interface IOverviewRulerLayoutInfo { parent: HTMLElement; @@ -44,6 +47,8 @@ export abstract class AbstractScrollableElement extends Widget { private _mouseWheelToDispose: IDisposable[]; + private _previousMouseWheelEventTime: number = 0; + private _isDragging: boolean; private _mouseIsOver: boolean; @@ -209,6 +214,7 @@ export abstract class AbstractScrollableElement extends Widget { } private _onMouseWheel(e: StandardMouseWheelEvent): void { + const currentMouseWheelEventTime = Date.now(); if (e.deltaY || e.deltaX) { let deltaY = e.deltaY * this._options.mouseWheelScrollSensitivity; @@ -253,15 +259,31 @@ export abstract class AbstractScrollableElement extends Widget { if (futureScrollPosition.scrollLeft !== desiredScrollPosition.scrollLeft || futureScrollPosition.scrollTop !== desiredScrollPosition.scrollTop) { // TODO@smooth: [MUST] detect if the source of the `mousewheel` is intertial or not and use setScrollPositionSmooth - this._scrollable.setScrollPositionNow(desiredScrollPosition); + const deltaT = currentMouseWheelEventTime - this._previousMouseWheelEventTime; + const canPerformSmoothScroll = ( + SCROLL_WHEEL_SMOOTH_SCROLL_ENABLED + && this._options.mouseWheelSmoothScroll + // If either |∆x|, |∆y| or ∆t are too small then do not apply smooth scroll animation, because in that case the input source must be a touchpad or something similar. + && Math.max(Math.abs(deltaX), Math.abs(deltaY)) * SCROLL_WHEEL_SENSITIVITY > SCROLL_WHEEL_SMOOTH_SCROLL_PX_SPACE_TRESHOLD + && deltaT > SCROLL_WHEEL_SMOOTH_SCROLL_MS_TIME_TRESHOLD + ); + + if (canPerformSmoothScroll) { + this._scrollable.setScrollPositionSmooth(desiredScrollPosition); + } else { + this._scrollable.setScrollPositionNow(desiredScrollPosition); + } this._shouldRender = true; } + } if (this._options.alwaysConsumeMouseWheel || this._shouldRender) { e.preventDefault(); e.stopPropagation(); } + + this._previousMouseWheelEventTime = currentMouseWheelEventTime; } private _onDidScroll(e: ScrollEvent): void { From d73f9a6f66d6e0d35c8662c64171bd30e425b109 Mon Sep 17 00:00:00 2001 From: "Nicola Fiori (JD342)" Date: Tue, 22 Aug 2017 16:07:58 +0200 Subject: [PATCH 12/14] Remove unnecessary line --- src/vs/base/browser/ui/scrollbar/scrollableElement.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts index 07c6ec9fc78e3..eb96b5cb1831c 100644 --- a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts +++ b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts @@ -275,7 +275,6 @@ export abstract class AbstractScrollableElement extends Widget { } this._shouldRender = true; } - } if (this._options.alwaysConsumeMouseWheel || this._shouldRender) { From 9c46f948fa2a3680b88ca878e4733fd82e0f9c56 Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Tue, 22 Aug 2017 17:48:16 +0200 Subject: [PATCH 13/14] Add a physical mouse wheel classifier with tests --- .../browser/ui/scrollbar/scrollableElement.ts | 122 ++++++++++ .../ui/scrollbar/scrollableElement.test.ts | 210 ++++++++++++++++++ 2 files changed, 332 insertions(+) create mode 100644 src/vs/base/test/browser/ui/scrollbar/scrollableElement.test.ts diff --git a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts index eb96b5cb1831c..49557d73c8426 100644 --- a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts +++ b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts @@ -31,6 +31,126 @@ export interface IOverviewRulerLayoutInfo { insertBefore: HTMLElement; } +class MouseWheelClassifierItem { + public timestamp: number; + public deltaX: number; + public deltaY: number; + public score: number; + + constructor(timestamp: number, deltaX: number, deltaY: number) { + this.timestamp = timestamp; + this.deltaX = deltaX; + this.deltaY = deltaY; + this.score = 0; + } +} + +export class MouseWheelClassifier { + + private readonly _capacity: number; + private _memory: MouseWheelClassifierItem[]; + private _front: number; + private _rear: number; + + constructor() { + this._capacity = 5; + this._memory = []; + this._front = -1; + this._rear = -1; + } + + // private isE + + public isPhysicalMouseWheel(): boolean { + if (this._front === -1 && this._rear === -1) { + // no elements + return false; + } + + // 0.5 * last + 0.25 * before last + 0.125 * before before last + ... + let remainingInfluence = 1; + let score = 0; + let iteration = 1; + + let index = this._rear; + do { + const influence = (index === this._front ? remainingInfluence : Math.pow(2, -iteration)); + remainingInfluence -= influence; + score += this._memory[index].score * influence; + + if (index === this._front) { + break; + } + + index = (this._capacity + index - 1) % this._capacity; + iteration++; + } while (true); + + return (score <= 0.5); + } + + public accept(timestamp: number, deltaX: number, deltaY: number): void { + const item = new MouseWheelClassifierItem(timestamp, deltaX, deltaY); + item.score = this._computeScore(item); + + if (this._front === -1 && this._rear === -1) { + this._memory[0] = item; + this._front = 0; + this._rear = 0; + } else { + this._rear = (this._rear + 1) % this._capacity; + if (this._rear === this._front) { + // Drop oldest + this._front = (this._front + 1) % this._capacity; + } + this._memory[this._rear] = item; + + // if () + // if (this._front === this._rea) + } + // this._lastIndex = (this._lastIndex + 1) % this._capacity; + // if (this._firstIndex === this._lastIndex) { + // this._firstIndex = (this._firstIndex + 1) % this._capacity; + // } + // this._memory[this._lastIndex] = item; + } + + /** + * A score between 0 and 1 for `item`. + * - a score towards 0 indicates that the source appears to be a physical mouse wheel + * - a score towards 1 indicates that the source appears to be a touchpad or magic mouse, etc. + */ + private _computeScore(item: MouseWheelClassifierItem): number { + + if (Math.abs(item.deltaX) > 0 && Math.abs(item.deltaY) > 0) { + // both axes exercised => definitely not a physical mouse wheel + return 1; + } + + let score: number = 0.5; + const prev = (this._front === -1 && this._rear === -1 ? null : this._memory[this._rear]); + if (prev) { + // const deltaT = item.timestamp - prev.timestamp; + // if (deltaT < 1000 / 30) { + // // sooner than X times per second => indicator that this is not a physical mouse wheel + // score += 0.25; + // } + + // if (item.deltaX === prev.deltaX && item.deltaY === prev.deltaY) { + // // equal amplitude => indicator that this is a physical mouse wheel + // score -= 0.25; + // } + } + + if (Math.abs(item.deltaX - Math.round(item.deltaX)) > 0 || Math.abs(item.deltaY - Math.round(item.deltaY)) > 0) { + // non-integer deltas => indicator that this is not a physical mouse wheel + score += 0.25; + } + + return Math.min(Math.max(score, 0), 1); + } +} + export abstract class AbstractScrollableElement extends Widget { private readonly _options: ScrollableElementResolvedOptions; @@ -216,6 +336,8 @@ export abstract class AbstractScrollableElement extends Widget { private _onMouseWheel(e: StandardMouseWheelEvent): void { const currentMouseWheelEventTime = Date.now(); + // console.log(`${Date.now()}, ${e.deltaY}, ${e.deltaX}`); + if (e.deltaY || e.deltaX) { let deltaY = e.deltaY * this._options.mouseWheelScrollSensitivity; let deltaX = e.deltaX * this._options.mouseWheelScrollSensitivity; diff --git a/src/vs/base/test/browser/ui/scrollbar/scrollableElement.test.ts b/src/vs/base/test/browser/ui/scrollbar/scrollableElement.test.ts new file mode 100644 index 0000000000000..1f5e737d70e13 --- /dev/null +++ b/src/vs/base/test/browser/ui/scrollbar/scrollableElement.test.ts @@ -0,0 +1,210 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ +'use strict'; + +import { MouseWheelClassifier } from "vs/base/browser/ui/scrollbar/scrollableElement"; +import * as assert from 'assert'; + +export type IMouseWheelEvent = [number, number, number]; + +suite('MouseWheelClassifier', () => { + + test('Apple Magic Mouse', () => { + const testData: IMouseWheelEvent[] = [ + [1503409622410, -0.025, 0], + [1503409622435, -0.175, 0], + [1503409622446, -0.225, 0], + [1503409622489, -0.65, 0], + [1503409622514, -1.225, 0], + [1503409622537, -1.025, 0], + [1503409622543, -0.55, 0], + [1503409622587, -0.75, 0], + [1503409622623, -1.45, 0], + [1503409622641, -1.325, 0], + [1503409622663, -0.6, 0], + [1503409622681, -1.125, 0], + [1503409622703, -0.5166666666666667, 0], + [1503409622721, -0.475, 0], + [1503409622822, -0.425, 0], + [1503409622871, -1.9916666666666667, 0], + [1503409622933, -0.7, 0], + [1503409622991, -0.725, 0], + [1503409623032, -0.45, 0], + [1503409623083, -0.25, 0], + [1503409623122, -0.4, 0], + [1503409623176, -0.2, 0], + [1503409623197, -0.225, 0], + [1503409623219, -0.05, 0], + [1503409623249, -0.1, 0], + [1503409623278, -0.1, 0], + [1503409623292, -0.025, 0], + [1503409623315, -0.025, 0], + [1503409623324, -0.05, 0], + [1503409623356, -0.025, 0], + [1503409623415, -0.025, 0], + [1503409623443, -0.05, 0], + [1503409623452, -0.025, 0], + ]; + + const classifier = new MouseWheelClassifier(); + for (let i = 0, len = testData.length; i < len; i++) { + const [timestamp, deltaY, deltaX] = testData[i]; + classifier.accept(timestamp, deltaX, deltaY); + + const actual = classifier.isPhysicalMouseWheel(); + assert.equal(actual, false); + } + }); + + test('Apple Touch Pad', () => { + const testData: IMouseWheelEvent[] = [ + [1503409780792, 0.025, 0], + [1503409780808, 0.175, -0.025], + [1503409780811, 0.35, -0.05], + [1503409780816, 0.55, -0.075], + [1503409780836, 0.825, -0.1], + [1503409780840, 0.725, -0.075], + [1503409780842, 1.5, -0.125], + [1503409780848, 1.1, -0.1], + [1503409780877, 2.05, -0.1], + [1503409780882, 3.9, 0], + [1503409780908, 3.825, 0], + [1503409780915, 3.65, 0], + [1503409780940, 3.45, 0], + [1503409780949, 3.25, 0], + [1503409780979, 3.075, 0], + [1503409780982, 2.9, 0], + [1503409781016, 2.75, 0], + [1503409781018, 2.625, 0], + [1503409781051, 2.5, 0], + [1503409781071, 2.4, 0], + [1503409781089, 2.3, 0], + [1503409781111, 2.175, 0], + [1503409781140, 3.975, 0], + [1503409781165, 1.8, 0], + [1503409781183, 3.3, 0], + [1503409781202, 1.475, 0], + [1503409781223, 1.375, 0], + [1503409781244, 1.275, 0], + [1503409781269, 2.25, 0], + [1503409781285, 1.025, 0], + [1503409781300, 0.925, 0], + [1503409781303, 0.875, 0], + [1503409781321, 0.8, 0], + [1503409781333, 0.725, 0], + [1503409781355, 0.65, 0], + [1503409781370, 0.6, 0], + [1503409781384, 0.55, 0], + [1503409781410, 0.5, 0], + [1503409781422, 0.475, 0], + [1503409781435, 0.425, 0], + [1503409781454, 0.4, 0], + [1503409781470, 0.35, 0], + [1503409781486, 0.325, 0], + [1503409781501, 0.3, 0], + [1503409781519, 0.275, 0], + [1503409781534, 0.25, 0], + [1503409781553, 0.225, 0], + [1503409781569, 0.2, 0], + [1503409781589, 0.2, 0], + [1503409781601, 0.175, 0], + [1503409781621, 0.15, 0], + [1503409781631, 0.15, 0], + [1503409781652, 0.125, 0], + [1503409781667, 0.125, 0], + [1503409781685, 0.125, 0], + [1503409781703, 0.1, 0], + [1503409781715, 0.1, 0], + [1503409781734, 0.1, 0], + [1503409781753, 0.075, 0], + [1503409781768, 0.075, 0], + [1503409781783, 0.075, 0], + [1503409781801, 0.075, 0], + [1503409781815, 0.05, 0], + [1503409781836, 0.05, 0], + [1503409781850, 0.05, 0], + [1503409781865, 0.05, 0], + [1503409781880, 0.05, 0], + [1503409781899, 0.025, 0], + [1503409781916, 0.025, 0], + [1503409781933, 0.025, 0], + [1503409781952, 0.025, 0], + [1503409781965, 0.025, 0], + [1503409781996, 0.025, 0], + [1503409782015, 0.025, 0], + [1503409782045, 0.025, 0], + ]; + + const classifier = new MouseWheelClassifier(); + for (let i = 0, len = testData.length; i < len; i++) { + const [timestamp, deltaY, deltaX] = testData[i]; + classifier.accept(timestamp, deltaX, deltaY); + + const actual = classifier.isPhysicalMouseWheel(); + assert.equal(actual, false); + } + }); + + test('Apple Physical Mouse Wheel', () => { + const testData: IMouseWheelEvent[] = [ + [1503409880776, -1, 0], + [1503409880791, -1, 0], + [1503409880810, -4, 0], + [1503409880820, -5, 0], + [1503409880848, -6, 0], + [1503409880876, -7, 0], + [1503409881319, -1, 0], + [1503409881387, -1, 0], + [1503409881407, -2, 0], + [1503409881443, -4, 0], + [1503409881444, -5, 0], + [1503409881470, -6, 0], + [1503409881496, -7, 0], + [1503409881812, -1, 0], + [1503409881829, -1, 0], + [1503409881850, -4, 0], + [1503409881871, -5, 0], + [1503409881896, -13, 0], + [1503409881914, -16, 0], + [1503409882551, -1, 0], + [1503409882589, -1, 0], + [1503409882625, -2, 0], + [1503409883035, -1, 0], + [1503409883098, -1, 0], + [1503409883143, -2, 0], + [1503409883217, -2, 0], + [1503409883270, -3, 0], + [1503409883388, -3, 0], + [1503409883531, -3, 0], + [1503409884095, -1, 0], + [1503409884122, -1, 0], + [1503409884160, -3, 0], + [1503409884208, -4, 0], + [1503409884292, -4, 0], + [1503409884447, -1, 0], + [1503409884788, -1, 0], + [1503409884835, -1, 0], + [1503409884898, -2, 0], + [1503409884965, -3, 0], + [1503409885085, -2, 0], + [1503409885552, -1, 0], + [1503409885619, -1, 0], + [1503409885670, -1, 0], + [1503409885733, -2, 0], + [1503409885784, -4, 0], + [1503409885916, -3, 0], + ]; + + const classifier = new MouseWheelClassifier(); + for (let i = 0, len = testData.length; i < len; i++) { + const [timestamp, deltaY, deltaX] = testData[i]; + classifier.accept(timestamp, deltaX, deltaY); + + const actual = classifier.isPhysicalMouseWheel(); + assert.equal(actual, true); + } + }); + +}); From 835e76458a896771f7f4e9316ec558d2596d2525 Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Tue, 22 Aug 2017 18:39:38 +0200 Subject: [PATCH 14/14] * more windows tests for the physical mouse wheel classifier; * adopt the classifier; --- .../browser/ui/scrollbar/scrollableElement.ts | 33 +- .../ui/scrollbar/scrollableElement.test.ts | 321 +++++++++++++++++- 2 files changed, 327 insertions(+), 27 deletions(-) diff --git a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts index 49557d73c8426..d45f05a670326 100644 --- a/src/vs/base/browser/ui/scrollbar/scrollableElement.ts +++ b/src/vs/base/browser/ui/scrollbar/scrollableElement.ts @@ -23,8 +23,6 @@ import Event, { Emitter } from 'vs/base/common/event'; const HIDE_TIMEOUT = 500; const SCROLL_WHEEL_SENSITIVITY = 50; const SCROLL_WHEEL_SMOOTH_SCROLL_ENABLED = true; -const SCROLL_WHEEL_SMOOTH_SCROLL_MS_TIME_TRESHOLD = 4; -const SCROLL_WHEEL_SMOOTH_SCROLL_PX_SPACE_TRESHOLD = 7; export interface IOverviewRulerLayoutInfo { parent: HTMLElement; @@ -47,6 +45,8 @@ class MouseWheelClassifierItem { export class MouseWheelClassifier { + public static INSTANCE = new MouseWheelClassifier(); + private readonly _capacity: number; private _memory: MouseWheelClassifierItem[]; private _front: number; @@ -59,8 +59,6 @@ export class MouseWheelClassifier { this._rear = -1; } - // private isE - public isPhysicalMouseWheel(): boolean { if (this._front === -1 && this._rear === -1) { // no elements @@ -104,15 +102,7 @@ export class MouseWheelClassifier { this._front = (this._front + 1) % this._capacity; } this._memory[this._rear] = item; - - // if () - // if (this._front === this._rea) } - // this._lastIndex = (this._lastIndex + 1) % this._capacity; - // if (this._firstIndex === this._lastIndex) { - // this._firstIndex = (this._firstIndex + 1) % this._capacity; - // } - // this._memory[this._lastIndex] = item; } /** @@ -167,8 +157,6 @@ export abstract class AbstractScrollableElement extends Widget { private _mouseWheelToDispose: IDisposable[]; - private _previousMouseWheelEventTime: number = 0; - private _isDragging: boolean; private _mouseIsOver: boolean; @@ -189,8 +177,6 @@ export abstract class AbstractScrollableElement extends Widget { this._onScroll.fire(e); })); - // this._scrollable = this._register(new DelegateScrollable(scrollable, () => this._onScroll())); - let scrollbarHost: ScrollbarHost = { onMouseWheel: (mouseWheelEvent: StandardMouseWheelEvent) => this._onMouseWheel(mouseWheelEvent), onDragStart: () => this._onDragStart(), @@ -334,7 +320,11 @@ export abstract class AbstractScrollableElement extends Widget { } private _onMouseWheel(e: StandardMouseWheelEvent): void { - const currentMouseWheelEventTime = Date.now(); + + const classifier = MouseWheelClassifier.INSTANCE; + if (SCROLL_WHEEL_SMOOTH_SCROLL_ENABLED) { + classifier.accept(Date.now(), e.deltaX, e.deltaY); + } // console.log(`${Date.now()}, ${e.deltaY}, ${e.deltaX}`); @@ -380,14 +370,11 @@ export abstract class AbstractScrollableElement extends Widget { desiredScrollPosition = this._scrollable.validateScrollPosition(desiredScrollPosition); if (futureScrollPosition.scrollLeft !== desiredScrollPosition.scrollLeft || futureScrollPosition.scrollTop !== desiredScrollPosition.scrollTop) { - // TODO@smooth: [MUST] detect if the source of the `mousewheel` is intertial or not and use setScrollPositionSmooth - const deltaT = currentMouseWheelEventTime - this._previousMouseWheelEventTime; + const canPerformSmoothScroll = ( SCROLL_WHEEL_SMOOTH_SCROLL_ENABLED && this._options.mouseWheelSmoothScroll - // If either |∆x|, |∆y| or ∆t are too small then do not apply smooth scroll animation, because in that case the input source must be a touchpad or something similar. - && Math.max(Math.abs(deltaX), Math.abs(deltaY)) * SCROLL_WHEEL_SENSITIVITY > SCROLL_WHEEL_SMOOTH_SCROLL_PX_SPACE_TRESHOLD - && deltaT > SCROLL_WHEEL_SMOOTH_SCROLL_MS_TIME_TRESHOLD + && classifier.isPhysicalMouseWheel() ); if (canPerformSmoothScroll) { @@ -403,8 +390,6 @@ export abstract class AbstractScrollableElement extends Widget { e.preventDefault(); e.stopPropagation(); } - - this._previousMouseWheelEventTime = currentMouseWheelEventTime; } private _onDidScroll(e: ScrollEvent): void { diff --git a/src/vs/base/test/browser/ui/scrollbar/scrollableElement.test.ts b/src/vs/base/test/browser/ui/scrollbar/scrollableElement.test.ts index 1f5e737d70e13..046f8cc3c1014 100644 --- a/src/vs/base/test/browser/ui/scrollbar/scrollableElement.test.ts +++ b/src/vs/base/test/browser/ui/scrollbar/scrollableElement.test.ts @@ -11,7 +11,7 @@ export type IMouseWheelEvent = [number, number, number]; suite('MouseWheelClassifier', () => { - test('Apple Magic Mouse', () => { + test('OSX - Apple Magic Mouse', () => { const testData: IMouseWheelEvent[] = [ [1503409622410, -0.025, 0], [1503409622435, -0.175, 0], @@ -58,7 +58,7 @@ suite('MouseWheelClassifier', () => { } }); - test('Apple Touch Pad', () => { + test('OSX - Apple Touch Pad', () => { const testData: IMouseWheelEvent[] = [ [1503409780792, 0.025, 0], [1503409780808, 0.175, -0.025], @@ -147,7 +147,7 @@ suite('MouseWheelClassifier', () => { } }); - test('Apple Physical Mouse Wheel', () => { + test('OSX - Razer Physical Mouse Wheel', () => { const testData: IMouseWheelEvent[] = [ [1503409880776, -1, 0], [1503409880791, -1, 0], @@ -207,4 +207,319 @@ suite('MouseWheelClassifier', () => { } }); + test('Windows - Microsoft Arc Touch', () => { + const testData: IMouseWheelEvent[] = [ + [1503418316909, -2, 0], + [1503418316985, -2, 0], + [1503418316988, -4, 0], + [1503418317034, -2, 0], + [1503418317071, -2, 0], + [1503418317094, -2, 0], + [1503418317133, -2, 0], + [1503418317170, -2, 0], + [1503418317192, -2, 0], + [1503418317265, -2, 0], + [1503418317289, -2, 0], + [1503418317365, -2, 0], + [1503418317414, -2, 0], + [1503418317458, -2, 0], + [1503418317513, -2, 0], + [1503418317583, -2, 0], + [1503418317637, -2, 0], + [1503418317720, -2, 0], + [1503418317786, -2, 0], + [1503418317832, -2, 0], + [1503418317933, -2, 0], + [1503418318037, -2, 0], + [1503418318134, -2, 0], + [1503418318267, -2, 0], + [1503418318411, -2, 0], + ]; + + const classifier = new MouseWheelClassifier(); + for (let i = 0, len = testData.length; i < len; i++) { + const [timestamp, deltaY, deltaX] = testData[i]; + classifier.accept(timestamp, deltaX, deltaY); + + const actual = classifier.isPhysicalMouseWheel(); + assert.equal(actual, true); + } + }); + + test('Windows - SurfaceBook TouchPad', () => { + const testData: IMouseWheelEvent[] = [ + [1503418499174, -3.35, 0], + [1503418499177, -0.9333333333333333, 0], + [1503418499222, -2.091666666666667, 0], + [1503418499238, -1.5666666666666667, 0], + [1503418499242, -1.8, 0], + [1503418499271, -2.5166666666666666, 0], + [1503418499283, -0.7666666666666667, 0], + [1503418499308, -2.033333333333333, 0], + [1503418499320, -2.85, 0], + [1503418499372, -1.5333333333333334, 0], + [1503418499373, -2.8, 0], + [1503418499411, -1.6166666666666667, 0], + [1503418499413, -1.9166666666666667, 0], + [1503418499443, -0.9333333333333333, 0], + [1503418499446, -0.9833333333333333, 0], + [1503418499458, -0.7666666666666667, 0], + [1503418499482, -0.9666666666666667, 0], + [1503418499485, -0.36666666666666664, 0], + [1503418499508, -0.5833333333333334, 0], + [1503418499532, -0.48333333333333334, 0], + [1503418499541, -0.6333333333333333, 0], + [1503418499571, -0.18333333333333332, 0], + [1503418499573, -0.4, 0], + [1503418499595, -0.15, 0], + [1503418499608, -0.23333333333333334, 0], + [1503418499625, -0.18333333333333332, 0], + [1503418499657, -0.13333333333333333, 0], + [1503418499674, -0.15, 0], + [1503418499676, -0.03333333333333333, 0], + [1503418499691, -0.016666666666666666, 0], + ]; + + const classifier = new MouseWheelClassifier(); + for (let i = 0, len = testData.length; i < len; i++) { + const [timestamp, deltaY, deltaX] = testData[i]; + classifier.accept(timestamp, deltaX, deltaY); + + const actual = classifier.isPhysicalMouseWheel(); + assert.equal(actual, false); + } + }); + + test('Windows - Razer physical wheel', () => { + const testData: IMouseWheelEvent[] = [ + [1503418638271, -2, 0], + [1503418638317, -2, 0], + [1503418638336, -2, 0], + [1503418638350, -2, 0], + [1503418638360, -2, 0], + [1503418638366, -2, 0], + [1503418638407, -2, 0], + [1503418638694, -2, 0], + [1503418638742, -2, 0], + [1503418638744, -2, 0], + [1503418638746, -2, 0], + [1503418638780, -2, 0], + [1503418638782, -2, 0], + [1503418638810, -2, 0], + [1503418639127, -2, 0], + [1503418639168, -2, 0], + [1503418639194, -2, 0], + [1503418639197, -4, 0], + [1503418639244, -2, 0], + [1503418639248, -2, 0], + [1503418639586, -2, 0], + [1503418639653, -2, 0], + [1503418639667, -4, 0], + [1503418639677, -2, 0], + [1503418639681, -2, 0], + [1503418639728, -2, 0], + [1503418639997, -2, 0], + [1503418640034, -2, 0], + [1503418640039, -2, 0], + [1503418640065, -2, 0], + [1503418640080, -2, 0], + [1503418640097, -2, 0], + [1503418640141, -2, 0], + [1503418640413, -2, 0], + [1503418640456, -2, 0], + [1503418640490, -2, 0], + [1503418640492, -4, 0], + [1503418640494, -2, 0], + [1503418640546, -2, 0], + [1503418640781, -2, 0], + [1503418640823, -2, 0], + [1503418640824, -2, 0], + [1503418640829, -2, 0], + [1503418640864, -2, 0], + [1503418640874, -2, 0], + [1503418640876, -2, 0], + [1503418641168, -2, 0], + [1503418641203, -2, 0], + [1503418641224, -2, 0], + [1503418641240, -2, 0], + [1503418641254, -4, 0], + [1503418641270, -2, 0], + [1503418641546, -2, 0], + [1503418641612, -2, 0], + [1503418641625, -6, 0], + [1503418641634, -2, 0], + [1503418641680, -2, 0], + [1503418641961, -2, 0], + [1503418642004, -2, 0], + [1503418642016, -4, 0], + [1503418642044, -2, 0], + [1503418642065, -2, 0], + [1503418642083, -2, 0], + [1503418642349, -2, 0], + [1503418642378, -2, 0], + [1503418642390, -2, 0], + [1503418642408, -2, 0], + [1503418642413, -2, 0], + [1503418642448, -2, 0], + [1503418642468, -2, 0], + [1503418642746, -2, 0], + [1503418642800, -2, 0], + [1503418642814, -4, 0], + [1503418642816, -2, 0], + [1503418642857, -2, 0], + ]; + + const classifier = new MouseWheelClassifier(); + for (let i = 0, len = testData.length; i < len; i++) { + const [timestamp, deltaY, deltaX] = testData[i]; + classifier.accept(timestamp, deltaX, deltaY); + + const actual = classifier.isPhysicalMouseWheel(); + assert.equal(actual, true); + } + }); + + test('Windows - Logitech physical wheel', () => { + const testData: IMouseWheelEvent[] = [ + [1503418872930, -2, 0], + [1503418872952, -2, 0], + [1503418872969, -2, 0], + [1503418873022, -2, 0], + [1503418873042, -2, 0], + [1503418873076, -2, 0], + [1503418873368, -2, 0], + [1503418873393, -2, 0], + [1503418873404, -2, 0], + [1503418873425, -2, 0], + [1503418873479, -2, 0], + [1503418873520, -2, 0], + [1503418873758, -2, 0], + [1503418873759, -2, 0], + [1503418873762, -2, 0], + [1503418873807, -2, 0], + [1503418873830, -4, 0], + [1503418873850, -2, 0], + [1503418874076, -2, 0], + [1503418874116, -2, 0], + [1503418874136, -4, 0], + [1503418874148, -2, 0], + [1503418874150, -2, 0], + [1503418874409, -2, 0], + [1503418874452, -2, 0], + [1503418874472, -2, 0], + [1503418874474, -4, 0], + [1503418874543, -2, 0], + [1503418874566, -2, 0], + [1503418874778, -2, 0], + [1503418874780, -2, 0], + [1503418874801, -2, 0], + [1503418874822, -2, 0], + [1503418874832, -2, 0], + [1503418874845, -2, 0], + [1503418875122, -2, 0], + [1503418875158, -2, 0], + [1503418875180, -2, 0], + [1503418875195, -4, 0], + [1503418875239, -2, 0], + [1503418875260, -2, 0], + [1503418875490, -2, 0], + [1503418875525, -2, 0], + [1503418875547, -4, 0], + [1503418875556, -4, 0], + [1503418875630, -2, 0], + [1503418875852, -2, 0], + [1503418875895, -2, 0], + [1503418875935, -2, 0], + [1503418875941, -4, 0], + [1503418876198, -2, 0], + [1503418876242, -2, 0], + [1503418876270, -4, 0], + [1503418876279, -2, 0], + [1503418876333, -2, 0], + [1503418876342, -2, 0], + [1503418876585, -2, 0], + [1503418876609, -2, 0], + [1503418876623, -2, 0], + [1503418876644, -2, 0], + [1503418876646, -2, 0], + [1503418876678, -2, 0], + [1503418877330, -2, 0], + [1503418877354, -2, 0], + [1503418877368, -2, 0], + [1503418877397, -2, 0], + [1503418877411, -2, 0], + [1503418877748, -2, 0], + [1503418877756, -2, 0], + [1503418877778, -2, 0], + [1503418877793, -2, 0], + [1503418877807, -2, 0], + [1503418878091, -2, 0], + [1503418878133, -2, 0], + [1503418878137, -4, 0], + [1503418878181, -2, 0], + ]; + + const classifier = new MouseWheelClassifier(); + for (let i = 0, len = testData.length; i < len; i++) { + const [timestamp, deltaY, deltaX] = testData[i]; + classifier.accept(timestamp, deltaX, deltaY); + + const actual = classifier.isPhysicalMouseWheel(); + assert.equal(actual, true); + } + }); + + test('Windows - Microsoft basic v2 physical wheel', () => { + const testData: IMouseWheelEvent[] = [ + [1503418994564, -2, 0], + [1503418994643, -2, 0], + [1503418994676, -2, 0], + [1503418994691, -2, 0], + [1503418994727, -2, 0], + [1503418994799, -2, 0], + [1503418994850, -2, 0], + [1503418995259, -2, 0], + [1503418995321, -2, 0], + [1503418995328, -2, 0], + [1503418995343, -2, 0], + [1503418995402, -2, 0], + [1503418995454, -2, 0], + [1503418996052, -2, 0], + [1503418996095, -2, 0], + [1503418996107, -2, 0], + [1503418996120, -2, 0], + [1503418996146, -2, 0], + [1503418996471, -2, 0], + [1503418996530, -2, 0], + [1503418996549, -2, 0], + [1503418996561, -2, 0], + [1503418996571, -2, 0], + [1503418996636, -2, 0], + [1503418996936, -2, 0], + [1503418997002, -2, 0], + [1503418997006, -2, 0], + [1503418997043, -2, 0], + [1503418997045, -2, 0], + [1503418997092, -2, 0], + [1503418997357, -2, 0], + [1503418997394, -2, 0], + [1503418997410, -2, 0], + [1503418997426, -2, 0], + [1503418997442, -2, 0], + [1503418997486, -2, 0], + [1503418997757, -2, 0], + [1503418997807, -2, 0], + [1503418997813, -2, 0], + [1503418997850, -2, 0], + ]; + + const classifier = new MouseWheelClassifier(); + for (let i = 0, len = testData.length; i < len; i++) { + const [timestamp, deltaY, deltaX] = testData[i]; + classifier.accept(timestamp, deltaX, deltaY); + + const actual = classifier.isPhysicalMouseWheel(); + assert.equal(actual, true); + } + }); });