maxmustermann/vscode
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Use the future scroll position to compute the scroll top and reuse the current smooth scrolling animation (#104144, #107704, #104284)

Browse source 
Co-authored-by: Joao Moreno <joao.moreno@microsoft.com>
This commit is contained in:
Alex Dima 2020-12-09 12:25:27 +01:00
parent 5df492ff59
commit eec97b824f
No known key found for this signature in database
GPG key ID: 6E58D7B045760DA0
3 changed files with 39 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
src/vs/base/browser/ui/list/listView.ts
View file

@ -809,14 +809,14 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
return scrollPosition.scrollTop;
}
setScrollTop(scrollTop: number): void {
setScrollTop(scrollTop: number, reuseAnimation?: boolean): void {
if (this.scrollableElementUpdateDisposable) {
this.scrollableElementUpdateDisposable.dispose();
this.scrollableElementUpdateDisposable = null;
this.scrollableElement.setScrollDimensions({ scrollHeight: this.scrollHeight });
}
this.scrollableElement.setScrollPosition({ scrollTop });
this.scrollableElement.setScrollPosition({ scrollTop, reuseAnimation });
}
getScrollLeft(): number {
@ -895,9 +895,7 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
this.render(previousRenderRange, e.scrollTop, e.height, e.scrollLeft, e.scrollWidth);
if (this.supportDynamicHeights) {
// Don't update scrollTop from within an scroll event
// so we don't break smooth scrolling. #104144
this._rerender(e.scrollTop, e.height, false);
this._rerender(e.scrollTop, e.height, e.inSmoothScrolling);
}
} catch (err) {
console.error('Got bad scroll event:', e);
@ -1167,7 +1165,7 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
* Given a stable rendered state, checks every rendered element whether it needs
* to be probed for dynamic height. Adjusts scroll height and top if necessary.
*/
private _rerender(renderTop: number, renderHeight: number, updateScrollTop: boolean = true): void {
private _rerender(renderTop: number, renderHeight: number, inSmoothScrolling?: boolean): void {
const previousRenderRange = this.getRenderRange(renderTop, renderHeight);
// Let's remember the second element's position, this helps in scrolling up
@ -1233,8 +1231,15 @@ export class ListView<T> implements ISpliceable<T>, IDisposable {
}
}
if (updateScrollTop && typeof anchorElementIndex === 'number') {
this.scrollTop = this.elementTop(anchorElementIndex) - anchorElementTopDelta!;
if (typeof anchorElementIndex === 'number') {
// To compute a destination scroll top, we need to take into account the current smooth scrolling
// animation, and then reuse it with a new target (to avoid prolonging the scroll)
// See https://github.com/microsoft/vscode/issues/104144
// See https://github.com/microsoft/vscode/pull/104284
// See https://github.com/microsoft/vscode/issues/107704
const deltaScrollTop = this.scrollable.getFutureScrollPosition().scrollTop - renderTop;
const newScrollTop = this.elementTop(anchorElementIndex) - anchorElementTopDelta! + deltaScrollTop;
this.setScrollTop(newScrollTop, inSmoothScrolling);
}
this._onDidChangeContentHeight.fire(this.contentHeight);

8
src/vs/base/browser/ui/scrollbar/scrollableElement.ts
View file

@ -549,8 +549,12 @@ export class SmoothScrollableElement extends AbstractScrollableElement {
super(element, options, scrollable);
}
public setScrollPosition(update: INewScrollPosition): void {
this._scrollable.setScrollPositionNow(update);
public setScrollPosition(update: INewScrollPosition & { reuseAnimation?: boolean }): void {
if (update.reuseAnimation) {
this._scrollable.setScrollPositionSmooth(update, update.reuseAnimation);
} else {
this._scrollable.setScrollPositionNow(update);
}
}
public getScrollPosition(): IScrollPosition {

33
src/vs/base/common/scrollable.ts
View file

@ -13,6 +13,8 @@ export const enum ScrollbarVisibility {
}
export interface ScrollEvent {
inSmoothScrolling: boolean;
oldWidth: number;
oldScrollWidth: number;
oldScrollLeft: number;
@ -132,7 +134,7 @@ export class ScrollState implements IScrollDimensions, IScrollPosition {
);
}
public createScrollEvent(previous: ScrollState): ScrollEvent {
public createScrollEvent(previous: ScrollState, inSmoothScrolling: boolean): ScrollEvent {
const widthChanged = (this.width !== previous.width);
const scrollWidthChanged = (this.scrollWidth !== previous.scrollWidth);
const scrollLeftChanged = (this.scrollLeft !== previous.scrollLeft);
@ -142,6 +144,7 @@ export class ScrollState implements IScrollDimensions, IScrollPosition {
const scrollTopChanged = (this.scrollTop !== previous.scrollTop);
return {
inSmoothScrolling: inSmoothScrolling,
oldWidth: previous.width,
oldScrollWidth: previous.scrollWidth,
oldScrollLeft: previous.scrollLeft,
@ -242,7 +245,7 @@ export class Scrollable extends Disposable {
public setScrollDimensions(dimensions: INewScrollDimensions, useRawScrollPositions: boolean): void {
const newState = this._state.withScrollDimensions(dimensions, useRawScrollPositions);
this._setState(newState);
this._setState(newState, Boolean(this._smoothScrolling));
// Validate outstanding animated scroll position target
if (this._smoothScrolling) {
@ -279,10 +282,10 @@ export class Scrollable extends Disposable {
this._smoothScrolling = null;
}
this._setState(newState);
this._setState(newState, false);
}
public setScrollPositionSmooth(update: INewScrollPosition): void {
public setScrollPositionSmooth(update: INewScrollPosition, reuseAnimation?: boolean): void {
if (this._smoothScrollDuration === 0) {
// Smooth scrolling not supported.
return this.setScrollPositionNow(update);
@ -302,8 +305,12 @@ export class Scrollable extends Disposable {
// No need to interrupt or extend the current animation since we're going to the same place
return;
}
const newSmoothScrolling = this._smoothScrolling.combine(this._state, validTarget, this._smoothScrollDuration);
let newSmoothScrolling: SmoothScrollingOperation;
if (reuseAnimation) {
newSmoothScrolling = new SmoothScrollingOperation(this._smoothScrolling.from, validTarget, this._smoothScrolling.startTime, this._smoothScrolling.duration);
} else {
newSmoothScrolling = this._smoothScrolling.combine(this._state, validTarget, this._smoothScrollDuration);
}
this._smoothScrolling.dispose();
this._smoothScrolling = newSmoothScrolling;
} else {
@ -330,7 +337,7 @@ export class Scrollable extends Disposable {
const update = this._smoothScrolling.tick();
const newState = this._state.withScrollPosition(update);
this._setState(newState);
this._setState(newState, true);
if (!this._smoothScrolling) {
// Looks like someone canceled the smooth scrolling
@ -354,14 +361,14 @@ export class Scrollable extends Disposable {
});
}
private _setState(newState: ScrollState): void {
private _setState(newState: ScrollState, inSmoothScrolling: boolean): void {
const oldState = this._state;
if (oldState.equals(newState)) {
// no change
return;
}
this._state = newState;
this._onScroll.fire(this._state.createScrollEvent(oldState));
this._onScroll.fire(this._state.createScrollEvent(oldState, inSmoothScrolling));
}
}
@ -404,17 +411,17 @@ export class SmoothScrollingOperation {
public readonly from: ISmoothScrollPosition;
public to: ISmoothScrollPosition;
public readonly duration: number;
private readonly _startTime: number;
public readonly startTime: number;
public animationFrameDisposable: IDisposable | null;
private scrollLeft!: IAnimation;
private scrollTop!: IAnimation;
protected constructor(from: ISmoothScrollPosition, to: ISmoothScrollPosition, startTime: number, duration: number) {
constructor(from: ISmoothScrollPosition, to: ISmoothScrollPosition, startTime: number, duration: number) {
this.from = from;
this.to = to;
this.duration = duration;
this._startTime = startTime;
this.startTime = startTime;
this.animationFrameDisposable = null;
@ -460,7 +467,7 @@ export class SmoothScrollingOperation {
}
protected _tick(now: number): SmoothScrollingUpdate {
const completion = (now - this._startTime) / this.duration;
const completion = (now - this.startTime) / this.duration;
if (completion < 1) {
const newScrollLeft = this.scrollLeft(completion);