Skip to content

Commit

Permalink
Bug 1884266 - Use a mutex to guard reading and writing to the axis lo…
Browse files Browse the repository at this point in the history
…ck. a=RyanVM

Before setting or reading from the variable indicating a scrolling axis
is locked, acquire a lock.

Original Revision: https://phabricator.services.mozilla.com/D210965

Differential Revision: https://phabricator.services.mozilla.com/D213973
  • Loading branch information
Ponchale committed Jun 23, 2024
1 parent be119b1 commit e1b2a93
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 13 deletions.
30 changes: 20 additions & 10 deletions gfx/layers/apz/src/Axis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ bool FuzzyEqualsCoordinate(CSSCoord aValue1, CSSCoord aValue2) {
Axis::Axis(AsyncPanZoomController* aAsyncPanZoomController)
: mPos(0),
mVelocity(0.0f, "Axis::mVelocity"),
mAxisLocked(false),
mAxisLocked(false, "Axis::mAxisLocked"),
mAsyncPanZoomController(aAsyncPanZoomController),
mOverscroll(0),
mMSDModel(0.0, 0.0, 0.0, StaticPrefs::apz_overscroll_spring_stiffness(),
Expand Down Expand Up @@ -68,25 +68,26 @@ void Axis::UpdateWithTouchAtDevicePoint(ParentLayerCoord aPos,
mPos.value);
if (Maybe<float> newVelocity =
mVelocityTracker->AddPosition(aPos, aTimestamp)) {
DoSetVelocity(mAxisLocked ? 0 : *newVelocity);
bool axisLocked = IsAxisLocked();
DoSetVelocity(axisLocked ? 0 : *newVelocity);
AXIS_LOG("%p|%s velocity from tracker is %f%s\n", mAsyncPanZoomController,
Name(), *newVelocity,
mAxisLocked ? ", but we are axis locked" : "");
axisLocked ? ", but we are axis locked" : "");
}
}

void Axis::StartTouch(ParentLayerCoord aPos, TimeStamp aTimestamp) {
mStartPos = aPos;
mPos = aPos;
mVelocityTracker->StartTracking(aPos, aTimestamp);
mAxisLocked = false;
SetAxisLocked(false);
}

bool Axis::AdjustDisplacement(ParentLayerCoord aDisplacement,
ParentLayerCoord& aDisplacementOut,
ParentLayerCoord& aOverscrollAmountOut,
bool aForceOverscroll /* = false */) {
if (mAxisLocked) {
if (IsAxisLocked()) {
aOverscrollAmountOut = 0;
aDisplacementOut = 0;
return false;
Expand Down Expand Up @@ -303,7 +304,8 @@ void Axis::EndTouch(TimeStamp aTimestamp, ClearAxisLock aClearAxisLock) {
// no-longer-relevant value of mVelocity. Also if the axis is locked then
// just reset the velocity to 0 since we don't need any velocity to carry
// into the fling.
if (mAxisLocked) {
auto axisLocked = mAxisLocked.Lock();
if (axisLocked.ref()) {
DoSetVelocity(0);
} else if (Maybe<float> velocity =
mVelocityTracker->ComputeVelocity(aTimestamp)) {
Expand All @@ -312,7 +314,7 @@ void Axis::EndTouch(TimeStamp aTimestamp, ClearAxisLock aClearAxisLock) {
DoSetVelocity(0);
}
if (aClearAxisLock == ClearAxisLock::Yes) {
mAxisLocked = false;
axisLocked.ref() = false;
}
AXIS_LOG("%p|%s ending touch, computed velocity %f\n",
mAsyncPanZoomController, Name(), DoGetVelocity());
Expand Down Expand Up @@ -373,7 +375,7 @@ CSSCoord Axis::ClampOriginToScrollableRect(CSSCoord aOrigin) const {
return result / zoom;
}

bool Axis::CanScrollNow() const { return !mAxisLocked && CanScroll(); }
bool Axis::CanScrollNow() const { return !IsAxisLocked() && CanScroll(); }

ParentLayerCoord Axis::DisplacementWillOverscrollAmount(
ParentLayerCoord aDisplacement) const {
Expand Down Expand Up @@ -429,9 +431,17 @@ CSSCoord Axis::ScaleWillOverscrollAmount(float aScale, CSSCoord aFocus) const {
return 0;
}

bool Axis::IsAxisLocked() const { return mAxisLocked; }
bool Axis::IsAxisLocked() const {
auto axisLocked = mAxisLocked.Lock();
return axisLocked.ref();
}

void Axis::SetAxisLocked(bool aAxisLocked) {
auto axisLocked = mAxisLocked.Lock();
axisLocked.ref() = aAxisLocked;
}

float Axis::GetVelocity() const { return mAxisLocked ? 0 : DoGetVelocity(); }
float Axis::GetVelocity() const { return IsAxisLocked() ? 0 : DoGetVelocity(); }

void Axis::SetVelocity(float aVelocity) {
AXIS_LOG("%p|%s direct-setting velocity to %f\n", mAsyncPanZoomController,
Expand Down
11 changes: 8 additions & 3 deletions gfx/layers/apz/src/Axis.h
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,6 @@ class Axis {
*/
CSSCoord ClampOriginToScrollableRect(CSSCoord aOrigin) const;

void SetAxisLocked(bool aAxisLocked) { mAxisLocked = aAxisLocked; }

/**
* Gets the raw velocity of this axis at this moment.
*/
Expand Down Expand Up @@ -297,6 +295,12 @@ class Axis {
*/
bool IsAxisLocked() const;

/**
* Set whether or not the axis is locked.
*/
void SetAxisLocked(bool aAxisLocked);


ParentLayerCoord GetOrigin() const;
ParentLayerCoord GetCompositionLength() const;
ParentLayerCoord GetPageStart() const;
Expand Down Expand Up @@ -359,7 +363,8 @@ class Axis {
// protected by a mutex.
// Units: ParentLayerCoords per millisecond
mutable DataMutex<float> mVelocity;
bool mAxisLocked; // Whether movement on this axis is locked.
// Whether movement on this axis is locked.
mutable DataMutex<bool> mAxisLocked;
AsyncPanZoomController* mAsyncPanZoomController;

// The amount by which we are overscrolled; see GetOverscroll().
Expand Down

0 comments on commit e1b2a93

Please sign in to comment.