summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorHenrik Paul <henrik@vaadin.com>2013-12-13 12:51:41 +0200
committerHenrik Paul <henrik@vaadin.com>2013-12-18 08:36:19 +0000
commit55e4f6b28c45915d3a63b37c383f78c7cfde44eb (patch)
tree1bfbe616f5b46e16ac217d366f36fb3a385157af
parent7e6322f9a8edb8332024fdb9b8cb77250ad756e2 (diff)
downloadvaadin-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.java135
-rw-r--r--client/src/com/vaadin/client/ui/grid/ScrollbarBundle.java65
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();
+ }
}