Bug 907179 - Rewrite the displayport calculation to be simpler and more effective. r=botond
authorKartikaya Gupta <kgupta@mozilla.com>
Wed, 08 Jan 2014 17:55:33 -0500
changeset 178664 6f522af059d30165bd84b35ae0e475f73303dad3
parent 178663 49c6393d37f3dc43e24fd147843fbbb1075e2f04
child 178665 4a3e51bf999b74d532c5edd237d2f3f3a8a7d43c
push id3343
push userffxbld
push dateMon, 17 Mar 2014 21:55:32 +0000
treeherdermozilla-beta@2f7d3415f79f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond
bugs907179
milestone29.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 907179 - Rewrite the displayport calculation to be simpler and more effective. r=botond
gfx/layers/ipc/AsyncPanZoomController.cpp
gfx/layers/ipc/AsyncPanZoomController.h
--- a/gfx/layers/ipc/AsyncPanZoomController.cpp
+++ b/gfx/layers/ipc/AsyncPanZoomController.cpp
@@ -163,20 +163,20 @@ static int32_t gPanRepaintInterval = 250
 
 /**
  * Maximum amount of time flinging before sending a viewport change. This will
  * asynchronously repaint the page.
  */
 static int32_t gFlingRepaintInterval = 75;
 
 /**
- * Minimum amount of speed along an axis before we begin painting far ahead by
- * adjusting the displayport.
+ * Minimum amount of speed along an axis before we switch to "skate" multipliers
+ * rather than using the "stationary" multipliers.
  */
-static float gMinSkateSpeed = 0.7f;
+static float gMinSkateSpeed = 1.0f;
 
 /**
  * Duration of a zoom to animation.
  */
 static const TimeDuration ZOOM_TO_DURATION = TimeDuration::FromSeconds(0.25);
 
 /**
  * Computed time function used for sampling frames of a zoom to animation.
@@ -202,32 +202,36 @@ static const CSSToScreenScale MIN_ZOOM(0
 static int gTouchListenerTimeout = 300;
 
 /**
  * Number of samples to store of how long it took to paint after the previous
  * requests.
  */
 static int gNumPaintDurationSamples = 3;
 
-/** The multiplier we apply to a dimension's length if it is skating. That is,
- * if it's going above sMinSkateSpeed. We prefer to increase the size of the
+/**
+ * The multiplier we apply to the displayport size if it is skating (current
+ * velocity is above gMinSkateSpeed). We prefer to increase the size of the
  * Y axis because it is more natural in the case that a user is reading a page
  * that scrolls up/down. Note that one, both or neither of these may be used
  * at any instant.
+ * In general we want g[XY]SkateSizeMultiplier to be smaller than the corresponding
+ * stationary size multiplier because when panning fast we would like to paint
+ * less and get faster, more predictable paint times. When panning slowly we
+ * can afford to paint more even though it's slower.
  */
-static float gXSkateSizeMultiplier = 3.0f;
-static float gYSkateSizeMultiplier = 3.5f;
+static float gXSkateSizeMultiplier = 1.5f;
+static float gYSkateSizeMultiplier = 2.5f;
 
-/** The multiplier we apply to a dimension's length if it is stationary. We
- * prefer to increase the size of the Y axis because it is more natural in the
- * case that a user is reading a page that scrolls up/down. Note that one,
- * both or neither of these may be used at any instant.
+/**
+ * The multiplier we apply to the displayport size if it is not skating (see
+ * documentation for gXSkateSizeMultiplier).
  */
-static float gXStationarySizeMultiplier = 1.5f;
-static float gYStationarySizeMultiplier = 2.5f;
+static float gXStationarySizeMultiplier = 3.0f;
+static float gYStationarySizeMultiplier = 3.5f;
 
 /**
  * The time period in ms that throttles mozbrowserasyncscroll event.
  * Default is 100ms if there is no "apz.asyncscroll.throttle" in preference.
  */
 
 static int gAsyncScrollThrottleTime = 100;
 
@@ -1154,113 +1158,75 @@ void AsyncPanZoomController::ScaleWithFo
   mFrameMetrics.mZoom.scale *= aScale;
   // We want to adjust the scroll offset such that the CSS point represented by aFocus remains
   // at the same position on the screen before and after the change in zoom. The below code
   // accomplishes this; see https://bugzilla.mozilla.org/show_bug.cgi?id=923431#c6 for an
   // in-depth explanation of how.
   mFrameMetrics.mScrollOffset = (mFrameMetrics.mScrollOffset + aFocus) - (aFocus / aScale);
 }
 
-bool AsyncPanZoomController::EnlargeDisplayPortAlongAxis(float aSkateSizeMultiplier,
-                                                         double aEstimatedPaintDuration,
-                                                         float aCompositionBounds,
-                                                         float aVelocity,
-                                                         float aAcceleration,
-                                                         float* aDisplayPortOffset,
-                                                         float* aDisplayPortLength)
+/**
+ * Attempts to enlarge the displayport along a single axis based on the
+ * velocity. aOffset and aLength are in/out parameters; they are initially set
+ * to the currently visible area and will be transformed to the area we should
+ * be drawing to minimize checkerboarding.
+ */
+static void
+EnlargeDisplayPortAlongAxis(float* aOutOffset, float* aOutLength,
+                            double aEstimatedPaintDurationMillis, float aVelocity,
+                            float aStationarySizeMultiplier, float aSkateSizeMultiplier)
 {
-  if (fabsf(aVelocity) > gMinSkateSpeed) {
-    // Enlarge the area we paint.
-    *aDisplayPortLength = aCompositionBounds * aSkateSizeMultiplier;
-    // Position the area we paint such that all of the excess that extends past
-    // the screen is on the side towards the velocity.
-    *aDisplayPortOffset = aVelocity > 0 ? 0 : aCompositionBounds - *aDisplayPortLength;
+  // Scale up the length using the appropriate multiplier and center the
+  // displayport around the visible area.
+  float multiplier = (fabsf(aVelocity) < gMinSkateSpeed
+                        ? aStationarySizeMultiplier
+                        : aSkateSizeMultiplier);
+  float newLength = (*aOutLength) * multiplier;
+  *aOutOffset -= (newLength - (*aOutLength)) / 2;
+  *aOutLength = newLength;
 
-    // Only compensate for acceleration when we actually have any. Otherwise
-    // we'll overcompensate when a user is just panning around without flinging.
-    if (aAcceleration > 1.01f) {
-      // Compensate for acceleration and how long we expect a paint to take. We
-      // try to predict where the viewport will be when painting has finished.
-      *aDisplayPortOffset +=
-        fabsf(aAcceleration) * aVelocity * aCompositionBounds * aEstimatedPaintDuration;
-      // If our velocity is in the negative direction of the axis, we have to
-      // compensate for the fact that our scroll offset is the top-left position
-      // of the viewport. In this case, let's make it relative to the
-      // bottom-right. That way, we'll always be growing the displayport upwards
-      // and to the left when skating negatively.
-      *aDisplayPortOffset -= aVelocity < 0 ? aCompositionBounds : 0;
-    }
-    return true;
-  }
-  return false;
+  // Project the displayport out based on the estimated time it will take to paint
+  *aOutOffset += (aVelocity * aEstimatedPaintDurationMillis);
 }
 
+/* static */
 const CSSRect AsyncPanZoomController::CalculatePendingDisplayPort(
   const FrameMetrics& aFrameMetrics,
   const ScreenPoint& aVelocity,
   const gfx::Point& aAcceleration,
   double aEstimatedPaintDuration)
 {
-  // If we don't get an estimated paint duration, we probably don't have any
-  // data. In this case, we're dealing with either a stationary frame or a first
-  // paint. In either of these cases, we can just assume it'll take 1 second to
-  // paint. Getting this correct is not important anyways since it's only really
-  // useful when accelerating, which can't be happening at this point.
-  double estimatedPaintDuration =
-    aEstimatedPaintDuration > EPSILON ? aEstimatedPaintDuration : 1.0;
+  // convert to milliseconds
+  double estimatedPaintDurationMillis = aEstimatedPaintDuration * 1000;
+
+  CSSRect compositionBounds = aFrameMetrics.CalculateCompositedRectInCssPixels();
+  CSSPoint scrollOffset = aFrameMetrics.mScrollOffset;
+  CSSRect displayPort(scrollOffset, compositionBounds.Size());
+  CSSPoint velocity = aVelocity / aFrameMetrics.mZoom;
 
-  CSSIntRect compositionBounds = gfx::RoundedIn(aFrameMetrics.mCompositionBounds / aFrameMetrics.mZoom);
+  // Enlarge the displayport along both axes depending on how fast we're moving
+  // on that axis and how long it takes to paint. Apply some heuristics to try
+  // to minimize checkerboarding.
+  EnlargeDisplayPortAlongAxis(&(displayPort.x), &(displayPort.width),
+    estimatedPaintDurationMillis, velocity.x,
+    gXStationarySizeMultiplier, gXSkateSizeMultiplier);
+  EnlargeDisplayPortAlongAxis(&(displayPort.y), &(displayPort.height),
+    estimatedPaintDurationMillis, velocity.y,
+    gYStationarySizeMultiplier, gYSkateSizeMultiplier);
 
   CSSRect scrollableRect = aFrameMetrics.GetExpandedScrollableRect();
-  CSSPoint scrollOffset = aFrameMetrics.mScrollOffset;
-
-  CSSRect displayPort = CSSRect(compositionBounds);
-  displayPort.MoveTo(0, 0);
-  displayPort.Scale(gXStationarySizeMultiplier, gYStationarySizeMultiplier);
-
-  // If there's motion along an axis of movement, and it's above a threshold,
-  // then we want to paint a larger area in the direction of that motion so that
-  // it's less likely to checkerboard.
-  bool enlargedX = EnlargeDisplayPortAlongAxis(
-    gXSkateSizeMultiplier, estimatedPaintDuration,
-    compositionBounds.width, aVelocity.x, aAcceleration.x,
-    &displayPort.x, &displayPort.width);
-  bool enlargedY = EnlargeDisplayPortAlongAxis(
-    gYSkateSizeMultiplier, estimatedPaintDuration,
-    compositionBounds.height, aVelocity.y, aAcceleration.y,
-    &displayPort.y, &displayPort.height);
+  displayPort = displayPort.ForceInside(scrollableRect) - scrollOffset;
 
-  if (!enlargedX && !enlargedY) {
-    // Position the x and y such that the screen falls in the middle of the displayport.
-    displayPort.x = -(displayPort.width - compositionBounds.width) / 2;
-    displayPort.y = -(displayPort.height - compositionBounds.height) / 2;
-  } else if (!enlargedX) {
-    displayPort.width = compositionBounds.width;
-  } else if (!enlargedY) {
-    displayPort.height = compositionBounds.height;
-  }
+  APZC_LOG_FM(aFrameMetrics,
+    "Calculated displayport as (%f %f %f %f) from velocity (%f %f) acceleration (%f %f) paint time %f metrics",
+    displayPort.x, displayPort.y, displayPort.width, displayPort.height,
+    aVelocity.x, aVelocity.y, aAcceleration.x, aAcceleration.y,
+    (float)estimatedPaintDurationMillis);
 
-  // If we go over the bounds when trying to predict where we will be when this
-  // paint finishes, move it back into the range of the CSS content rect.
-  // FIXME/bug 780395: Generalize this. This code is pretty hacky as it will
-  // probably not work at all for RTL content. This is not intended to be
-  // incredibly accurate; it'll just prevent the entire displayport from being
-  // outside the content rect (which causes bad things to happen).
-  if (scrollOffset.x + compositionBounds.width > scrollableRect.width) {
-    scrollOffset.x -= compositionBounds.width + scrollOffset.x - scrollableRect.width;
-  } else if (scrollOffset.x < scrollableRect.x) {
-    scrollOffset.x = scrollableRect.x;
-  }
-  if (scrollOffset.y + compositionBounds.height > scrollableRect.height) {
-    scrollOffset.y -= compositionBounds.height + scrollOffset.y - scrollableRect.height;
-  } else if (scrollOffset.y < scrollableRect.y) {
-    scrollOffset.y = scrollableRect.y;
-  }
-
-  return displayPort.ForceInside(scrollableRect - scrollOffset);
+  return displayPort;
 }
 
 void AsyncPanZoomController::ScheduleComposite() {
   if (mCompositorParent) {
     mCompositorParent->ScheduleRenderOnCompositorThread();
   }
 }
 
--- a/gfx/layers/ipc/AsyncPanZoomController.h
+++ b/gfx/layers/ipc/AsyncPanZoomController.h
@@ -435,32 +435,16 @@ protected:
   void UpdateWithTouchAtDevicePoint(const MultiTouchInput& aEvent);
 
   /**
    * Does any panning required due to a new touch event.
    */
   void TrackTouch(const MultiTouchInput& aEvent);
 
   /**
-   * Attempts to enlarge the displayport along a single axis. Returns whether or
-   * not the displayport was enlarged. This will fail in circumstances where the
-   * velocity along that axis is not high enough to need any changes. The
-   * displayport metrics are expected to be passed into |aDisplayPortOffset| and
-   * |aDisplayPortLength|. If enlarged, these will be updated with the new
-   * metrics.
-   */
-  static bool EnlargeDisplayPortAlongAxis(float aSkateSizeMultiplier,
-                                          double aEstimatedPaintDuration,
-                                          float aCompositionBounds,
-                                          float aVelocity,
-                                          float aAcceleration,
-                                          float* aDisplayPortOffset,
-                                          float* aDisplayPortLength);
-
-  /**
    * Utility function to send updated FrameMetrics to Gecko so that it can paint
    * the displayport area. Calls into GeckoContentController to do the actual
    * work. Note that only one paint request can be active at a time. If a paint
    * request is made while a paint is currently happening, it gets queued up. If
    * a new paint request arrives before a paint is completed, the old request
    * gets discarded.
    */
   void RequestContentRepaint();