Bug 923431 - Fix the code that adjusts scroll offset while pinching; add tests to go with. r=botond

This commit is contained in:
Kartikaya Gupta 2013-10-10 12:21:50 -04:00
parent 687253b614
commit 4d36be887f
5 changed files with 114 additions and 48 deletions

View File

@ -602,7 +602,7 @@ nsEventStatus AsyncPanZoomController::OnScaleBegin(const PinchGestureInput& aEve
}
SetState(PINCHING);
mLastZoomFocus = aEvent.mFocusPoint;
mLastZoomFocus = aEvent.mFocusPoint - mFrameMetrics.mCompositionBounds.TopLeft();
return nsEventStatus_eConsumeNoDefault;
}
@ -619,13 +619,14 @@ nsEventStatus AsyncPanZoomController::OnScale(const PinchGestureInput& aEvent) {
return nsEventStatus_eConsumeNoDefault;
}
ScreenToScreenScale spanRatio(aEvent.mCurrentSpan / aEvent.mPreviousSpan);
float spanRatio = aEvent.mCurrentSpan / aEvent.mPreviousSpan;
{
ReentrantMonitorAutoEnter lock(mMonitor);
CSSToScreenScale userZoom = mFrameMetrics.mZoom;
ScreenPoint focusPoint = aEvent.mFocusPoint;
ScreenPoint focusPoint = aEvent.mFocusPoint - mFrameMetrics.mCompositionBounds.TopLeft();
CSSPoint cssFocusPoint = focusPoint / userZoom;
CSSPoint focusChange = (mLastZoomFocus - focusPoint) / userZoom;
// If displacing by the change in focus point will take us off page bounds,
@ -641,23 +642,23 @@ nsEventStatus AsyncPanZoomController::OnScale(const PinchGestureInput& aEvent) {
// When we zoom in with focus, we can zoom too much towards the boundaries
// that we actually go over them. These are the needed displacements along
// either axis such that we don't overscroll the boundaries when zooming.
gfx::Point neededDisplacement;
CSSPoint neededDisplacement;
bool doScale = (spanRatio > ScreenToScreenScale(1.0) && userZoom < mMaxZoom) ||
(spanRatio < ScreenToScreenScale(1.0) && userZoom > mMinZoom);
bool doScale = (spanRatio > 1.0 && userZoom < mMaxZoom) ||
(spanRatio < 1.0 && userZoom > mMinZoom);
if (doScale) {
spanRatio.scale = clamped(spanRatio.scale,
mMinZoom.scale / userZoom.scale,
mMaxZoom.scale / userZoom.scale);
spanRatio = clamped(spanRatio,
mMinZoom.scale / userZoom.scale,
mMaxZoom.scale / userZoom.scale);
switch (mX.ScaleWillOverscroll(spanRatio, focusPoint.x))
switch (mX.ScaleWillOverscroll(spanRatio, cssFocusPoint.x))
{
case Axis::OVERSCROLL_NONE:
break;
case Axis::OVERSCROLL_MINUS:
case Axis::OVERSCROLL_PLUS:
neededDisplacement.x = -mX.ScaleWillOverscrollAmount(spanRatio, focusPoint.x);
neededDisplacement.x = -mX.ScaleWillOverscrollAmount(spanRatio, cssFocusPoint.x);
break;
case Axis::OVERSCROLL_BOTH:
// If scaling this way will make us overscroll in both directions, then
@ -670,13 +671,13 @@ nsEventStatus AsyncPanZoomController::OnScale(const PinchGestureInput& aEvent) {
}
if (doScale) {
switch (mY.ScaleWillOverscroll(spanRatio, focusPoint.y))
switch (mY.ScaleWillOverscroll(spanRatio, cssFocusPoint.y))
{
case Axis::OVERSCROLL_NONE:
break;
case Axis::OVERSCROLL_MINUS:
case Axis::OVERSCROLL_PLUS:
neededDisplacement.y = -mY.ScaleWillOverscrollAmount(spanRatio, focusPoint.y);
neededDisplacement.y = -mY.ScaleWillOverscrollAmount(spanRatio, cssFocusPoint.y);
break;
case Axis::OVERSCROLL_BOTH:
doScale = false;
@ -685,10 +686,10 @@ nsEventStatus AsyncPanZoomController::OnScale(const PinchGestureInput& aEvent) {
}
if (doScale) {
ScaleWithFocus(userZoom * spanRatio, focusPoint);
ScaleWithFocus(spanRatio, cssFocusPoint);
if (neededDisplacement != gfx::Point()) {
ScrollBy(CSSPoint::FromUnknownPoint(neededDisplacement));
if (neededDisplacement != CSSPoint()) {
ScrollBy(neededDisplacement);
}
ScheduleComposite();
@ -977,19 +978,14 @@ void AsyncPanZoomController::ScrollBy(const CSSPoint& aOffset) {
mFrameMetrics.mScrollOffset += aOffset;
}
void AsyncPanZoomController::ScaleWithFocus(const CSSToScreenScale& aZoom,
const ScreenPoint& aFocus) {
ScreenToScreenScale zoomFactor(aZoom.scale / mFrameMetrics.mZoom.scale);
CSSToScreenScale resolution = mFrameMetrics.mZoom;
SetZoomAndResolution(aZoom);
// If the new scale is very small, we risk multiplying in huge rounding
// errors, so don't bother adjusting the scroll offset.
if (resolution.scale >= 0.01f) {
zoomFactor.scale -= 1.0;
mFrameMetrics.mScrollOffset += aFocus * zoomFactor / resolution;
}
void AsyncPanZoomController::ScaleWithFocus(float aScale,
const CSSPoint& aFocus) {
SetZoomAndResolution(CSSToScreenScale(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,

View File

@ -359,12 +359,10 @@ protected:
/**
* Scales the viewport by an amount (note that it multiplies this scale in to
* the current scale, it doesn't set it to |aScale|). Also considers a focus
* point so that the page zooms outward from that point.
*
* XXX: Fix focus point calculations.
* point so that the page zooms inward/outward from that point.
*/
void ScaleWithFocus(const mozilla::CSSToScreenScale& aScale,
const ScreenPoint& aFocus);
void ScaleWithFocus(float aScale,
const CSSPoint& aFocus);
/**
* Schedules a composite on the compositor thread. Wrapper for

View File

@ -271,12 +271,12 @@ float Axis::DisplacementWillOverscrollAmount(float aDisplacement) {
}
}
Axis::Overscroll Axis::ScaleWillOverscroll(ScreenToScreenScale aScale, float aFocus) {
float originAfterScale = (GetOrigin() + aFocus) * aScale.scale - aFocus;
Axis::Overscroll Axis::ScaleWillOverscroll(float aScale, float aFocus) {
float originAfterScale = (GetOrigin() + aFocus) - (aFocus / aScale);
bool both = ScaleWillOverscrollBothSides(aScale);
bool minus = originAfterScale < GetPageStart() * aScale.scale;
bool plus = (originAfterScale + GetCompositionLength()) > GetPageEnd() * aScale.scale;
bool minus = originAfterScale < GetPageStart();
bool plus = (originAfterScale + (GetCompositionLength() / aScale)) > GetPageEnd();
if ((minus && plus) || both) {
return OVERSCROLL_BOTH;
@ -290,12 +290,11 @@ Axis::Overscroll Axis::ScaleWillOverscroll(ScreenToScreenScale aScale, float aFo
return OVERSCROLL_NONE;
}
float Axis::ScaleWillOverscrollAmount(ScreenToScreenScale aScale, float aFocus) {
float originAfterScale = (GetOrigin() + aFocus) * aScale.scale - aFocus;
float Axis::ScaleWillOverscrollAmount(float aScale, float aFocus) {
float originAfterScale = (GetOrigin() + aFocus) - (aFocus / aScale);
switch (ScaleWillOverscroll(aScale, aFocus)) {
case OVERSCROLL_MINUS: return originAfterScale - GetPageStart() * aScale.scale;
case OVERSCROLL_PLUS: return (originAfterScale + GetCompositionLength()) -
NS_lround(GetPageEnd() * aScale.scale);
case OVERSCROLL_MINUS: return originAfterScale - GetPageStart();
case OVERSCROLL_PLUS: return originAfterScale + (GetCompositionLength() / aScale) - GetPageEnd();
// Don't handle OVERSCROLL_BOTH. Client code is expected to deal with it.
default: return 0;
}
@ -338,12 +337,12 @@ float Axis::GetPageLength() {
return GetRectLength(pageRect);
}
bool Axis::ScaleWillOverscrollBothSides(ScreenToScreenScale aScale) {
bool Axis::ScaleWillOverscrollBothSides(float aScale) {
const FrameMetrics& metrics = mAsyncPanZoomController->GetFrameMetrics();
CSSRect cssContentRect = metrics.mScrollableRect;
CSSToScreenScale scale = metrics.mZoom * aScale;
CSSToScreenScale scale(metrics.mZoom.scale * aScale);
CSSIntRect cssCompositionBounds = RoundedIn(metrics.mCompositionBounds / scale);
return GetRectLength(cssContentRect) < GetRectLength(CSSRect(cssCompositionBounds));

View File

@ -155,7 +155,7 @@ public:
* scroll offset in such a way that it remains in the same place on the page
* relative.
*/
Overscroll ScaleWillOverscroll(ScreenToScreenScale aScale, float aFocus);
Overscroll ScaleWillOverscroll(float aScale, float aFocus);
/**
* If a scale will overscroll the axis, this returns the amount and in what
@ -165,7 +165,7 @@ public:
* scroll offset in such a way that it remains in the same place on the page
* relative.
*/
float ScaleWillOverscrollAmount(ScreenToScreenScale aScale, float aFocus);
float ScaleWillOverscrollAmount(float aScale, float aFocus);
/**
* Checks if an axis will overscroll in both directions by computing the
@ -174,7 +174,7 @@ public:
*
* This gets called by ScaleWillOverscroll().
*/
bool ScaleWillOverscrollBothSides(ScreenToScreenScale aScale);
bool ScaleWillOverscrollBothSides(float aScale);
float GetOrigin();
float GetCompositionLength();

View File

@ -53,6 +53,11 @@ public:
ReentrantMonitorAutoEnter lock(mMonitor);
mFrameMetrics = metrics;
}
FrameMetrics GetFrameMetrics() {
ReentrantMonitorAutoEnter lock(mMonitor);
return mFrameMetrics;
}
};
class TestAPZCTreeManager : public APZCTreeManager {
@ -115,6 +120,74 @@ TEST(AsyncPanZoomController, Constructor) {
apzc->SetFrameMetrics(TestFrameMetrics());
}
TEST(AsyncPanZoomController, Pinch) {
nsRefPtr<MockContentController> mcc = new MockContentController();
nsRefPtr<TestAsyncPanZoomController> apzc = new TestAsyncPanZoomController(0, mcc);
FrameMetrics fm;
fm.mViewport = CSSRect(0, 0, 980, 480);
fm.mCompositionBounds = ScreenIntRect(200, 200, 100, 200);
fm.mScrollableRect = CSSRect(0, 0, 980, 1000);
fm.mScrollOffset = CSSPoint(300, 300);
fm.mZoom = CSSToScreenScale(2.0);
apzc->SetFrameMetrics(fm);
// the visible area of the document in CSS pixels is x=300 y=300 w=50 h=100
EXPECT_CALL(*mcc, SendAsyncScrollDOMEvent(_,_,_)).Times(2);
EXPECT_CALL(*mcc, RequestContentRepaint(_)).Times(1);
apzc->HandleInputEvent(PinchGestureInput(PinchGestureInput::PINCHGESTURE_START,
0,
ScreenPoint(250, 300),
10.0,
10.0));
apzc->HandleInputEvent(PinchGestureInput(PinchGestureInput::PINCHGESTURE_SCALE,
0,
ScreenPoint(250, 300),
12.5,
10.0));
apzc->HandleInputEvent(PinchGestureInput(PinchGestureInput::PINCHGESTURE_END,
0,
ScreenPoint(250, 300),
12.5,
12.5));
// the visible area of the document in CSS pixels is now x=305 y=310 w=40 h=80
fm = apzc->GetFrameMetrics();
EXPECT_EQ(fm.mZoom.scale, 2.5f);
EXPECT_EQ(fm.mScrollOffset.x, 305);
EXPECT_EQ(fm.mScrollOffset.y, 310);
// part 2 of the test, move to the top-right corner of the page and pinch and
// make sure we stay in the correct spot
fm.mZoom = CSSToScreenScale(2.0);
fm.mScrollOffset = CSSPoint(930, 5);
apzc->SetFrameMetrics(fm);
// the visible area of the document in CSS pixels is x=930 y=5 w=50 h=100
apzc->HandleInputEvent(PinchGestureInput(PinchGestureInput::PINCHGESTURE_START,
0,
ScreenPoint(250, 300),
10.0,
10.0));
apzc->HandleInputEvent(PinchGestureInput(PinchGestureInput::PINCHGESTURE_SCALE,
0,
ScreenPoint(250, 300),
5.0,
10.0));
apzc->HandleInputEvent(PinchGestureInput(PinchGestureInput::PINCHGESTURE_END,
0,
ScreenPoint(250, 300),
5.0,
5.0));
// the visible area of the document in CSS pixels is now x=880 y=0 w=100 h=200
fm = apzc->GetFrameMetrics();
EXPECT_EQ(fm.mZoom.scale, 1.0f);
EXPECT_EQ(fm.mScrollOffset.x, 880);
EXPECT_EQ(fm.mScrollOffset.y, 0);
}
TEST(AsyncPanZoomController, SimpleTransform) {
TimeStamp testStartTime = TimeStamp::Now();
// RefCounted class can't live in the stack