From 01751982e26917d071e1a81e88bc508d5ceb6b11 Mon Sep 17 00:00:00 2001 From: Kartikaya Gupta Date: Wed, 8 Jan 2014 17:55:33 -0500 Subject: [PATCH] Bug 907179 - Rewrite the displayport calculation to be simpler and more effective. r=botond --- gfx/layers/ipc/AsyncPanZoomController.cpp | 156 +++++++++------------- gfx/layers/ipc/AsyncPanZoomController.h | 16 --- 2 files changed, 61 insertions(+), 111 deletions(-) diff --git a/gfx/layers/ipc/AsyncPanZoomController.cpp b/gfx/layers/ipc/AsyncPanZoomController.cpp index fdc5e2937d50..58e5582aae72 100644 --- a/gfx/layers/ipc/AsyncPanZoomController.cpp +++ b/gfx/layers/ipc/AsyncPanZoomController.cpp @@ -168,10 +168,10 @@ static int32_t gPanRepaintInterval = 250; 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. @@ -207,22 +207,26 @@ static int gTouchListenerTimeout = 300; */ 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. @@ -1159,103 +1163,65 @@ void AsyncPanZoomController::ScaleWithFocus(float aScale, 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; - CSSIntRect compositionBounds = gfx::RoundedIn(aFrameMetrics.mCompositionBounds / aFrameMetrics.mZoom); + CSSRect compositionBounds = aFrameMetrics.CalculateCompositedRectInCssPixels(); + CSSPoint scrollOffset = aFrameMetrics.mScrollOffset; + CSSRect displayPort(scrollOffset, compositionBounds.Size()); + CSSPoint velocity = aVelocity / 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; + displayPort = displayPort.ForceInside(scrollableRect) - scrollOffset; - CSSRect displayPort = CSSRect(compositionBounds); - displayPort.MoveTo(0, 0); - displayPort.Scale(gXStationarySizeMultiplier, gYStationarySizeMultiplier); + 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 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); - - 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; - } - - // 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() { diff --git a/gfx/layers/ipc/AsyncPanZoomController.h b/gfx/layers/ipc/AsyncPanZoomController.h index dc59ae4b8207..3d67f5694035 100644 --- a/gfx/layers/ipc/AsyncPanZoomController.h +++ b/gfx/layers/ipc/AsyncPanZoomController.h @@ -439,22 +439,6 @@ protected: */ 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