Optimize the performance of Escalator (#12645)

After noticing that getScrollTop and getScrollLeft took an ungodly
amount of execution time, I decided to add a local cache for
ScrollbarBundle. Also, on a touch scroll, the event isn't processed
immediately, but put into a queue, waiting for the next rendered frame.

Testing by scrolling up/down and in circles for one minute on a plain
Escalator with only text, 20 visible rows and 10 columns:
Before: iOS browser self time 43.78s/1.1min (69.37%)
After: iOS browser self time 50.19s/1.1min (78.31%)

This means that the optimizations added almost 10 percentage points more
time for the application to render.

Change-Id: I0bb65646852321c0df706ecac0c546f0d8324865

client/src/com/vaadin/client/ui/grid/Escalator.java

@@ -26,6 +26,7 @@ import java.util.logging.Logger;
 
 import com.google.gwt.animation.client.AnimationScheduler;
 import com.google.gwt.animation.client.AnimationScheduler.AnimationCallback;
+import com.google.gwt.animation.client.AnimationScheduler.AnimationHandle;
 import com.google.gwt.core.client.Duration;
 import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwt.dom.client.Document;
@@ -303,6 +304,54 @@ public class Escalator extends Widget {
             private double deltaY = 0;
 
             private final Escalator escalator;
+            private CustomTouchEvent latestTouchMoveEvent;
+            private AnimationCallback mover = new AnimationCallback() {
+                @Override
+                public void execute(double timestamp) {
+                    if (touches != 1) {
+                        return;
+                    }
+
+                    final int x = latestTouchMoveEvent.getPageX();
+                    final int y = latestTouchMoveEvent.getPageY();
+                    deltaX = x - lastX;
+                    deltaY = y - lastY;
+                    lastX = x;
+                    lastY = y;
+                    lastTime = timestamp;
+
+                    // snap the scroll to the major axes, at first.
+                    if (snappedScrollEnabled) {
+                        final double oldDeltaX = deltaX;
+                        final double oldDeltaY = deltaY;
+
+                        /*
+                         * Scrolling snaps to 40 degrees vs. flick scroll's 30
+                         * degrees, since slow movements have poor resolution -
+                         * it's easy to interpret a slight angle as a steep
+                         * angle, since the sample rate is "unnecessarily" high.
+                         * 40 simply felt better than 30.
+                         */
+                        final double[] snapped = Escalator.snapDeltas(deltaX,
+                                deltaY, RATIO_OF_40_DEGREES);
+                        deltaX = snapped[0];
+                        deltaY = snapped[1];
+
+                        /*
+                         * if the snap failed once, let's follow the pointer
+                         * from now on.
+                         */
+                        if (oldDeltaX != 0 && deltaX == oldDeltaX
+                                && oldDeltaY != 0 && deltaY == oldDeltaY) {
+                            snappedScrollEnabled = false;
+                        }
+                    }
+
+                    moveScrollFromEvent(escalator, -deltaX, -deltaY,
+                            latestTouchMoveEvent.getNativeEvent());
+                }
+            };
+            private AnimationHandle animationHandle;
 
             public TouchHandlerBundle(final Escalator escalator) {
                 this.escalator = escalator;
@@ -350,47 +399,20 @@ public class Escalator extends Widget {
             }
 
             public void touchMove(final CustomTouchEvent event) {
-                if (touches != 1) {
-                    return;
-                }
-
-                final int x = event.getPageX();
-                final int y = event.getPageY();
-                deltaX = x - lastX;
-                deltaY = y - lastY;
-                lastX = x;
-                lastY = y;
-                lastTime = Duration.currentTimeMillis();
-
-                // snap the scroll to the major axes, at first.
-                if (snappedScrollEnabled) {
-                    final double oldDeltaX = deltaX;
-                    final double oldDeltaY = deltaY;
-
-                    /*
-                     * Scrolling snaps to 40 degrees vs. flick scroll's 30
-                     * degrees, since slow movements have poor resolution - it's
-                     * easy to interpret a slight angle as a steep angle, since
-                     * the sample rate is "unnecessarily" high. 40 simply felt
-                     * better than 30.
-                     */
-                    final double[] snapped = Escalator.snapDeltas(deltaX,
-                            deltaY, RATIO_OF_40_DEGREES);
-                    deltaX = snapped[0];
-                    deltaY = snapped[1];
+                /*
+                 * since we only use the getPageX/Y, and calculate the diff
+                 * within the handler, we don't need to calculate any
+                 * intermediate deltas.
+                 */
+                latestTouchMoveEvent = event;
 
-                    /*
-                     * if the snap failed once, let's follow the pointer from
-                     * now on.
-                     */
-                    if (oldDeltaX != 0 && deltaX == oldDeltaX && oldDeltaY != 0
-                            && deltaY == oldDeltaY) {
-                        snappedScrollEnabled = false;
-                    }
+                if (animationHandle != null) {
+                    animationHandle.cancel();
                 }
-
-                moveScrollFromEvent(escalator, -deltaX, -deltaY,
-                        event.getNativeEvent());
+                animationHandle = AnimationScheduler.get()
+                        .requestAnimationFrame(mover, escalator.bodyElem);
+                event.getNativeEvent().preventDefault();
+                mover.execute(Duration.currentTimeMillis());
             }
 
             public void touchEnd(@SuppressWarnings("unused")
@@ -538,7 +560,24 @@ public class Escalator extends Widget {
         protected native JavaScriptObject createScrollListenerFunction(
                 Escalator esc)
         /*-{
+            var vScroll = esc.@com.vaadin.client.ui.grid.Escalator::verticalScrollbar;
+            var vScrollElem = vScroll.@com.vaadin.client.ui.grid.ScrollbarBundle::getElement()();
+
+            var hScroll = esc.@com.vaadin.client.ui.grid.Escalator::horizontalScrollbar;
+            var hScrollElem = hScroll.@com.vaadin.client.ui.grid.ScrollbarBundle::getElement()();
+
             return $entry(function(e) {
+                // in case the scroll event was native (i.e. scrollbars were dragged, or
+                // the scrollTop/Left was manually modified), the bundles have old cache
+                // values. We need to make sure that the caches are kept up to date.
+                if (e.target === vScrollElem) {
+                    vScroll.@com.vaadin.client.ui.grid.ScrollbarBundle::updateScrollPosFromDom()();
+                } else if (e.target === hScrollElem) {
+                    hScroll.@com.vaadin.client.ui.grid.ScrollbarBundle::updateScrollPosFromDom()();
+                } else {
+                    $wnd.console.error("unexpected scroll target: "+e.target);
+                }
+
                 esc.@com.vaadin.client.ui.grid.Escalator::onScroll()();
             });
         }-*/;
@@ -612,6 +651,9 @@ public class Escalator extends Widget {
 
             // we might've got new or got rid of old scrollbars.
             recalculateTableWrapperSize();
+
+            verticalScrollbar.recalculateMaxScrollPos();
+            horizontalScrollbar.recalculateMaxScrollPos();
         }
 
         /**
@@ -2966,7 +3008,22 @@ public class Escalator extends Widget {
     /** The {@code <tfoot/>} tag. */
     private final Element footElem = DOM.createTFoot();
 
+    /**
+     * TODO: investigate whether this field is now unnecessary, as
+     * {@link ScrollbarBundle} now caches its values.
+     * 
+     * @deprecated maybe...
+     */
+    @Deprecated
     private int tBodyScrollTop = 0;
+
+    /**
+     * TODO: investigate whether this field is now unnecessary, as
+     * {@link ScrollbarBundle} now caches its values.
+     * 
+     * @deprecated maybe...
+     */
+    @Deprecated
     private int tBodyScrollLeft = 0;
 
     private final VerticalScrollbarBundle verticalScrollbar = new VerticalScrollbarBundle();

client/src/com/vaadin/client/ui/grid/ScrollbarBundle.java

@@ -57,12 +57,12 @@ abstract class ScrollbarBundle {
         }
 
         @Override
-        public void setScrollPos(int px) {
+        protected void internalSetScrollPos(int px) {
             root.setScrollTop(px);
         }
 
         @Override
-        public int getScrollPos() {
+        protected int internalGetScrollPos() {
             return root.getScrollTop();
         }
 
@@ -121,12 +121,12 @@ abstract class ScrollbarBundle {
         }
 
         @Override
-        public void setScrollPos(int px) {
+        protected void internalSetScrollPos(int px) {
             root.setScrollLeft(px);
         }
 
         @Override
-        public int getScrollPos() {
+        protected int internalGetScrollPos() {
             return root.getScrollLeft();
         }
 
@@ -175,6 +175,9 @@ abstract class ScrollbarBundle {
     protected final Element scrollSizeElement = DOM.createDiv();
     protected boolean isInvisibleScrollbar = false;
 
+    private int scrollPos = 0;
+    private int maxScrollPos = 0;
+
     private ScrollbarBundle() {
         root.appendChild(scrollSizeElement);
     }
@@ -194,7 +197,7 @@ abstract class ScrollbarBundle {
      * 
      * @return the root element
      */
-    public Element getElement() {
+    public final Element getElement() {
         return root;
     }
 
@@ -204,12 +207,8 @@ abstract class ScrollbarBundle {
      * @param delta
      *            the delta in pixels to change the scroll position by
      */
-    public void setScrollPosByDelta(int delta) {
+    public final void setScrollPosByDelta(int delta) {
         if (delta != 0) {
-            /*
-             * TODO [[optimize]]: rewrite this so that we can evoid a potential
-             * reflow from getScrollPos()
-             */
             setScrollPos(getScrollPos() + delta);
         }
     }
@@ -230,9 +229,10 @@ abstract class ScrollbarBundle {
      * @param px
      *            the length of the scrollbar in pixels
      */
-    public void setOffsetSize(int px) {
+    public final void setOffsetSize(int px) {
         internalSetOffsetSize(px);
         forceScrollbar(needsScrollbars());
+        recalculateMaxScrollPos();
     }
 
     /**
@@ -259,7 +259,16 @@ abstract class ScrollbarBundle {
      * @param px
      *            the new scroll position in pixels
      */
-    public abstract void setScrollPos(int px);
+    public final void setScrollPos(int px) {
+        int oldScrollPos = scrollPos;
+        scrollPos = Math.max(0, Math.min(maxScrollPos, px));
+
+        if (oldScrollPos != scrollPos) {
+            internalSetScrollPos(px);
+        }
+    }
+
+    protected abstract void internalSetScrollPos(int px);
 
     /**
      * Gets the scroll position of the scrollbar in the axis the scrollbar is
@@ -267,7 +276,12 @@ abstract class ScrollbarBundle {
      * 
      * @return the new scroll position in pixels
      */
-    public abstract int getScrollPos();
+    public final int getScrollPos() {
+        assert internalGetScrollPos() == scrollPos : "calculated scroll position did not match the actual scroll position";
+        return scrollPos;
+    }
+
+    protected abstract int internalGetScrollPos();
 
     /**
      * Modifies {@link #scrollSizeElement scrollSizeElement's} dimensions in
@@ -288,9 +302,10 @@ abstract class ScrollbarBundle {
      *            the number of pixels the scrollbar should be able to scroll
      *            through
      */
-    public void setScrollSize(int px) {
+    public final void setScrollSize(int px) {
         internalSetScrollSize(px);
         forceScrollbar(needsScrollbars());
+        recalculateMaxScrollPos();
     }
 
     /**
@@ -323,7 +338,7 @@ abstract class ScrollbarBundle {
      * @param px
      *            the scrollbar's thickness in pixels
      */
-    public void setScrollbarThickness(int px) {
+    public final void setScrollbarThickness(int px) {
         isInvisibleScrollbar = (px == 0);
         internalSetScrollbarThickness(px != 0 ? px
                 : OSX_INVISIBLE_SCROLLBAR_FAKE_SIZE_PX);
@@ -345,7 +360,7 @@ abstract class ScrollbarBundle {
      * 
      * @return the scrollbar's thickness in pixels
      */
-    public int getScrollbarThickness() {
+    public final int getScrollbarThickness() {
         if (!isInvisibleScrollbar) {
             return internalGetScrollbarThickness();
         } else {
@@ -362,4 +377,22 @@ abstract class ScrollbarBundle {
     protected boolean needsScrollbars() {
         return getOffsetSize() < getScrollSize();
     }
+
+    public void recalculateMaxScrollPos() {
+        int scrollSize = getScrollSize();
+        int offsetSize = getOffsetSize();
+        maxScrollPos = Math.max(0, scrollSize - offsetSize);
+
+        // make sure that the correct max scroll position is maintained.
+        setScrollPos(scrollPos);
+    }
+
+    /**
+     * This is a method that JSNI can call to synchronize the object state from
+     * the DOM.
+     */
+    @SuppressWarnings("unused")
+    private final void updateScrollPosFromDom() {
+        scrollPos = internalGetScrollPos();
+    }
 }