diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/LineMapper.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/LineMapper.java index 8ddb19e600..31b6baa2b2 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/LineMapper.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/LineMapper.java @@ -112,7 +112,7 @@ class LineMapper { * ... */ LineOnOtherInfo lineOnOther(DisplaySide mySide, int line) { - List lineGaps = mySide == DisplaySide.A ? lineMapAtoB : lineMapBtoA; + List 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 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 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. diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ScrollSynchronizer.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ScrollSynchronizer.java index 898f06609a..033b08baf3 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ScrollSynchronizer.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ScrollSynchronizer.java @@ -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)); } } }