SurfaceFlinger: setGeometryAppliesWithResize crop latching fixes.
The same sort of thing we had with setPosition...not sure why I didn't
realize we would need the fixes here too! In particular we need to ensure
the following scenarios work:
1. Additional calls to set(Final)Crop while in the setGeometryAppliesWithResize
state are eventually applied.
2. Additional calls to set(Final)Crop while in the setGeometryAppliesWithResize
state are not immediately applied.
3. In LayerRejector.cpp we have to be sure we are not just latching a buffer
at the old size, which we still allow. This is the correct time to latch
the transparentRegion as it is content dependent, but doesn't represent
a size changing.
The difference between this and the original CL which was reverted has to do with
point 3. The original CL tried to solve point 3 by moving the latching logic from
the LayerRejecter in to Layer::doTransaction. However, in general doTransaction
will not be called in between Latching the buffer and drawing the frame, so this
introduced errors. The new test "FinalCropLatchingBufferOldSize" encapsulates this.
Bug: 37621737
Bug: 37531386
Test: Included in Transaction_test.cpp
Change-Id: I14bd09d01ac6b85895caa1b707d6fa7dac962074
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index fe16b76..d5498ed 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -103,7 +103,7 @@
mLastFrameNumberReceived(0),
mUpdateTexImageFailed(false),
mAutoRefresh(false),
- mFreezePositionUpdates(false)
+ mFreezeGeometryUpdates(false)
{
#ifdef USE_HWC2
ALOGV("Creating Layer %s", name.string());
@@ -131,6 +131,8 @@
mCurrentState.active.transform.set(0, 0);
mCurrentState.crop.makeInvalid();
mCurrentState.finalCrop.makeInvalid();
+ mCurrentState.requestedFinalCrop = mCurrentState.finalCrop;
+ mCurrentState.requestedCrop = mCurrentState.crop;
mCurrentState.z = 0;
#ifdef USE_HWC2
mCurrentState.alpha = 1.0f;
@@ -1641,12 +1643,25 @@
}
}
- // always set active to requested, unless we're asked not to
- // this is used by Layer, which special cases resizes.
- if (flags & eDontUpdateGeometryState) {
- } else {
+ // Here we apply various requested geometry states, depending on our
+ // latching configuration. See Layer.h for a detailed discussion of
+ // how geometry latching is controlled.
+ if (!(flags & eDontUpdateGeometryState)) {
Layer::State& editCurrentState(getCurrentState());
- if (mFreezePositionUpdates) {
+
+ // If mFreezeGeometryUpdates is true we are in the setGeometryAppliesWithResize
+ // mode, which causes attributes which normally latch regardless of scaling mode,
+ // to be delayed. We copy the requested state to the active state making sure
+ // to respect these rules (again see Layer.h for a detailed discussion).
+ //
+ // There is an awkward asymmetry in the handling of the crop states in the position
+ // states, as can be seen below. Largely this arises from position and transform
+ // being stored in the same data structure while having different latching rules.
+ // b/38182305
+ //
+ // Careful that "c" and editCurrentState may not begin as equivalent due to
+ // applyPendingStates in the presence of deferred transactions.
+ if (mFreezeGeometryUpdates) {
float tx = c.active.transform.tx();
float ty = c.active.transform.ty();
c.active = c.requested;
@@ -1707,10 +1722,14 @@
// we want to apply the position portion of the transform matrix immediately,
// but still delay scaling when resizing a SCALING_MODE_FREEZE layer.
mCurrentState.requested.transform.set(x, y);
- if (immediate && !mFreezePositionUpdates) {
+ if (immediate && !mFreezeGeometryUpdates) {
+ // Here we directly update the active state
+ // unlike other setters, because we store it within
+ // the transform, but use different latching rules.
+ // b/38182305
mCurrentState.active.transform.set(x, y);
}
- mFreezePositionUpdates = mFreezePositionUpdates || !immediate;
+ mFreezeGeometryUpdates = mFreezeGeometryUpdates || !immediate;
mCurrentState.modified = true;
setTransactionFlags(eTransactionNeeded);
@@ -1833,26 +1852,30 @@
}
bool Layer::setCrop(const Rect& crop, bool immediate) {
- if (mCurrentState.crop == crop)
+ if (mCurrentState.requestedCrop == crop)
return false;
mCurrentState.sequence++;
mCurrentState.requestedCrop = crop;
- if (immediate) {
+ if (immediate && !mFreezeGeometryUpdates) {
mCurrentState.crop = crop;
}
+ mFreezeGeometryUpdates = mFreezeGeometryUpdates || !immediate;
+
mCurrentState.modified = true;
setTransactionFlags(eTransactionNeeded);
return true;
}
bool Layer::setFinalCrop(const Rect& crop, bool immediate) {
- if (mCurrentState.finalCrop == crop)
+ if (mCurrentState.requestedFinalCrop == crop)
return false;
mCurrentState.sequence++;
mCurrentState.requestedFinalCrop = crop;
- if (immediate) {
+ if (immediate && !mFreezeGeometryUpdates) {
mCurrentState.finalCrop = crop;
}
+ mFreezeGeometryUpdates = mFreezeGeometryUpdates || !immediate;
+
mCurrentState.modified = true;
setTransactionFlags(eTransactionNeeded);
return true;
@@ -2142,7 +2165,7 @@
bool queuedBuffer = false;
LayerRejecter r(mDrawingState, getCurrentState(), recomputeVisibleRegions,
getProducerStickyTransform() != 0, mName.string(),
- mOverrideScalingMode, mFreezePositionUpdates);
+ mOverrideScalingMode, mFreezeGeometryUpdates);
status_t updateResult = mSurfaceFlingerConsumer->updateTexImage(&r,
mFlinger->mPrimaryDispSync, &mAutoRefresh, &queuedBuffer,
mLastFrameNumberReceived);