diff options
author | Henrik Paul <henrik@vaadin.com> | 2013-12-13 12:51:41 +0200 |
---|---|---|
committer | Henrik Paul <henrik@vaadin.com> | 2013-12-18 08:36:19 +0000 |
commit | 55e4f6b28c45915d3a63b37c383f78c7cfde44eb (patch) | |
tree | 1bfbe616f5b46e16ac217d366f36fb3a385157af | |
parent | 7e6322f9a8edb8332024fdb9b8cb77250ad756e2 (diff) | |
download | vaadin-framework-55e4f6b28c45915d3a63b37c383f78c7cfde44eb.tar.gz vaadin-framework-55e4f6b28c45915d3a63b37c383f78c7cfde44eb.zip |
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
-rw-r--r-- | client/src/com/vaadin/client/ui/grid/Escalator.java | 135 | ||||
-rw-r--r-- | client/src/com/vaadin/client/ui/grid/ScrollbarBundle.java | 65 |
2 files changed, 145 insertions, 55 deletions
diff --git a/client/src/com/vaadin/client/ui/grid/Escalator.java b/client/src/com/vaadin/client/ui/grid/Escalator.java index a395038890..97c7df2fea 100644 --- a/client/src/com/vaadin/client/ui/grid/Escalator.java +++ b/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(); diff --git a/client/src/com/vaadin/client/ui/grid/ScrollbarBundle.java b/client/src/com/vaadin/client/ui/grid/ScrollbarBundle.java index f0e966176c..922daefa5f 100644 --- a/client/src/com/vaadin/client/ui/grid/ScrollbarBundle.java +++ b/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(); + } } |