SideBySide2: Fix scrolling alignment errors and reduce jank
CM3 does not compute the height of line widgets if the widget is offscreen and has not been rendered yet. This allows rendering large documents faster, but means Y coordinates from one CM3 do not align common lines of code in the other CM3 instance. During scrolling determine the line visible at the top of the viewport. Rely on the LineMapper data to locate the common line either at this position, or the first common line before it. With the nearest prior common line in hand, compute the distance between the viewport top and this common line, and add that to the position of the common line on the other CM3. This should pixel align both CM3 instances even if other widgets are not properly measured. Using this alignment method the view scrolls almost "butter smooth" under Chrome on a modern MacBook Pro. It is difficult to tell there are two synchronized CM3 instances. Keep the 20ms/40ms timer running during scrolling. The timer adjusts the other CM3 every 20ms to ensure it is aligned and cancels after 40ms of no scrolling activity. Running the timer is a backup measure to ensure the panes do not fall out of sync. The 40ms cancel timer must be kept to "disconnect" the current scrolling event and allow the other CM3 to control a future scroll attempt. Most jank and artifacting during throw scrolling is now gone. This commit was tested on an edit of a 12,000 line text file containing an insertion at line 7, and another at 11,838, and 200 lines of context. Change-Id: I4d2305abcd16040a67a4d3534981d75dd2c3a5d9
This commit is contained in:
@@ -112,7 +112,7 @@ class LineMapper {
|
||||
* ...
|
||||
*/
|
||||
LineOnOtherInfo lineOnOther(DisplaySide mySide, int line) {
|
||||
List<LineGap> lineGaps = mySide == DisplaySide.A ? lineMapAtoB : lineMapBtoA;
|
||||
List<LineGap> lineGaps = gapList(mySide);
|
||||
// Create a dummy LineGap for the search.
|
||||
int ret = Collections.binarySearch(lineGaps, new LineGap(line));
|
||||
if (ret == -1) {
|
||||
@@ -130,6 +130,38 @@ class LineMapper {
|
||||
}
|
||||
}
|
||||
|
||||
AlignedPair align(DisplaySide mySide, int line) {
|
||||
List<LineGap> gaps = gapList(mySide);
|
||||
int idx = Collections.binarySearch(gaps, new LineGap(line));
|
||||
if (idx == -1) {
|
||||
return new AlignedPair(line, line);
|
||||
}
|
||||
|
||||
LineGap g = gaps.get(0 <= idx ? idx : -idx - 2);
|
||||
if (g.start <= line && line <= g.end && g.end != -1) {
|
||||
if (0 < g.start) {
|
||||
// Line falls within this gap, use alignment before.
|
||||
return new AlignedPair(g.start - 1, g.end + g.delta);
|
||||
}
|
||||
return new AlignedPair(g.end, g.end + g.delta + 1);
|
||||
}
|
||||
return new AlignedPair(line, line + g.delta);
|
||||
}
|
||||
|
||||
private List<LineGap> gapList(DisplaySide mySide) {
|
||||
return mySide == DisplaySide.A ? lineMapAtoB : lineMapBtoA;
|
||||
}
|
||||
|
||||
static class AlignedPair {
|
||||
final int src;
|
||||
final int dst;
|
||||
|
||||
AlignedPair(int s, int d) {
|
||||
src = s;
|
||||
dst = d;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @field line The line number on the other side.
|
||||
* @field aligned Whether the two lines are at the same height when displayed.
|
||||
|
@@ -14,11 +14,9 @@
|
||||
|
||||
package com.google.gerrit.client.diff;
|
||||
|
||||
import com.google.gerrit.client.diff.LineMapper.LineOnOtherInfo;
|
||||
import com.google.gwt.user.client.Timer;
|
||||
|
||||
import net.codemirror.lib.CodeMirror;
|
||||
import net.codemirror.lib.CodeMirror.Viewport;
|
||||
import net.codemirror.lib.ScrollInfo;
|
||||
|
||||
class ScrollSynchronizer {
|
||||
@@ -75,7 +73,7 @@ class ScrollSynchronizer {
|
||||
if (active == this) {
|
||||
ScrollInfo si = src.getScrollInfo();
|
||||
updateScreenHeader(si);
|
||||
dst.scrollTo(si.getLeft(), si.getTop());
|
||||
dst.scrollTo(si.getLeft(), align(si.getTop()));
|
||||
state = 0;
|
||||
}
|
||||
}
|
||||
@@ -84,37 +82,32 @@ class ScrollSynchronizer {
|
||||
switch (state) {
|
||||
case 0:
|
||||
state = 1;
|
||||
dst.scrollToY(align(src.getScrollInfo().getTop()));
|
||||
break;
|
||||
case 1:
|
||||
state = 2;
|
||||
return;
|
||||
break;
|
||||
case 2:
|
||||
active = null;
|
||||
fixup.cancel();
|
||||
return;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
private double align(double srcTop) {
|
||||
// Since CM doesn't always take the height of line widgets into
|
||||
// account when calculating scrollInfo when scrolling too fast (e.g.
|
||||
// throw scrolling), simply setting scrollTop to be the same doesn't
|
||||
// guarantee alignment.
|
||||
//
|
||||
// Iterate over the viewport to find the first line that isn't part of
|
||||
// an insertion or deletion gap, for which isAligned() will be true.
|
||||
// We then manually examine if the lines that should be aligned are at
|
||||
// the same height. If not, perform additional scrolling.
|
||||
Viewport fromTo = src.getViewport();
|
||||
for (int line = fromTo.getFrom(); line <= fromTo.getTo(); line++) {
|
||||
LineOnOtherInfo info = mapper.lineOnOther(srcSide, line);
|
||||
if (info.isAligned()) {
|
||||
double sy = src.heightAtLine(line);
|
||||
double dy = dst.heightAtLine(info.getLine());
|
||||
if (Math.abs(dy - sy) >= 1) {
|
||||
dst.scrollToY(dst.getScrollInfo().getTop() + (dy - sy));
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
// Find a pair of lines that are aligned and near the top of
|
||||
// the viewport. Use that distance to correct the Y coordinate.
|
||||
int line = src.lineAtHeight(srcTop, "local");
|
||||
LineMapper.AlignedPair p = mapper.align(srcSide, line);
|
||||
|
||||
double sy = src.heightAtLine(p.src, "local");
|
||||
double dy = dst.heightAtLine(p.dst, "local");
|
||||
return Math.max(0, dy + (srcTop - sy));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
Reference in New Issue
Block a user