From 93ffb86b909005bbee4993fc9053f017466311c7 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 16 May 2012 17:07:49 -0700 Subject: [PATCH 1/2] minor refactoring in praparation of crop fix Bug: 6498869 Change-Id: I12a6f9a9fdfd2ea1db3fbe5fc8cb443aeaedb328 --- services/surfaceflinger/Layer.cpp | 57 +++++++++++++------------ services/surfaceflinger/LayerBase.cpp | 61 ++++++++++++++------------- services/surfaceflinger/LayerBase.h | 12 +++--- 3 files changed, 67 insertions(+), 63 deletions(-) diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 7eba89b7b..5988ed24a 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -232,27 +232,28 @@ Rect Layer::computeBufferCrop() const { // ... then reduce that in the same proportions as the window crop reduces // the window size. const State& s(drawingState()); - if (!s.crop.isEmpty()) { + if (!s.active.crop.isEmpty()) { // Transform the window crop to match the buffer coordinate system, // which means using the inverse of the current transform set on the // SurfaceTexture. uint32_t invTransform = mCurrentTransform; - int winWidth = s.w; - int winHeight = s.h; + int winWidth = s.active.w; + int winHeight = s.active.h; if (invTransform & NATIVE_WINDOW_TRANSFORM_ROT_90) { invTransform ^= NATIVE_WINDOW_TRANSFORM_FLIP_V | NATIVE_WINDOW_TRANSFORM_FLIP_H; - winWidth = s.h; - winHeight = s.w; + winWidth = s.active.h; + winHeight = s.active.w; } - Rect winCrop = s.crop.transform(invTransform, s.w, s.h); + Rect winCrop = s.active.crop.transform(invTransform, + s.active.w, s.active.h); float xScale = float(crop.width()) / float(winWidth); float yScale = float(crop.height()) / float(winHeight); - crop.left += int(ceil(float(winCrop.left) * xScale)); - crop.top += int(ceil(float(winCrop.top) * yScale)); - crop.right -= int(ceil(float(winWidth - winCrop.right) * xScale)); - crop.bottom -= int(ceil(float(winHeight - winCrop.bottom) * yScale)); + crop.left += int(ceilf(float(winCrop.left) * xScale)); + crop.top += int(ceilf(float(winCrop.top) * yScale)); + crop.right -= int(ceilf(float(winWidth - winCrop.right) * xScale)); + crop.bottom -= int(ceilf(float(winHeight - winCrop.bottom) * yScale)); } return crop; @@ -427,8 +428,8 @@ uint32_t Layer::doTransaction(uint32_t flags) const Layer::State& front(drawingState()); const Layer::State& temp(currentState()); - const bool sizeChanged = (front.requested_w != temp.requested_w) || - (front.requested_h != temp.requested_h); + const bool sizeChanged = (front.requested.w != temp.requested.w) || + (front.requested.h != temp.requested.h); if (sizeChanged) { // the size changed, we need to ask our client to request a new buffer @@ -437,22 +438,22 @@ uint32_t Layer::doTransaction(uint32_t flags) "resize (layer=%p), requested (%dx%d), drawing (%d,%d), " "scalingMode=%d", this, - int(temp.requested_w), int(temp.requested_h), - int(front.requested_w), int(front.requested_h), + int(temp.requested.w), int(temp.requested.h), + int(front.requested.w), int(front.requested.h), mCurrentScalingMode); if (!isFixedSize()) { // this will make sure LayerBase::doTransaction doesn't update // the drawing state's size Layer::State& editDraw(mDrawingState); - editDraw.requested_w = temp.requested_w; - editDraw.requested_h = temp.requested_h; + editDraw.requested.w = temp.requested.w; + editDraw.requested.h = temp.requested.h; } // record the new size, form this point on, when the client request // a buffer, it'll get the new size. - mSurfaceTexture->setDefaultBufferSize(temp.requested_w, - temp.requested_h); + mSurfaceTexture->setDefaultBufferSize(temp.requested.w, + temp.requested.h); } return LayerBase::doTransaction(flags); @@ -551,10 +552,10 @@ void Layer::lockPageFlip(bool& recomputeVisibleRegions) const Layer::State& front(drawingState()); // FIXME: mPostedDirtyRegion = dirty & bounds - mPostedDirtyRegion.set(front.w, front.h); + mPostedDirtyRegion.set(front.active.w, front.active.h); - if ((front.w != front.requested_w) || - (front.h != front.requested_h)) + if ((front.active.w != front.requested.w) || + (front.active.h != front.requested.h)) { // check that we received a buffer of the right size // (Take the buffer's orientation into account) @@ -563,15 +564,15 @@ void Layer::lockPageFlip(bool& recomputeVisibleRegions) } if (isFixedSize() || - (bufWidth == front.requested_w && - bufHeight == front.requested_h)) + (bufWidth == front.requested.w && + bufHeight == front.requested.h)) { // Here we pretend the transaction happened by updating the // current and drawing states. Drawing state is only accessed // in this thread, no need to have it locked Layer::State& editDraw(mDrawingState); - editDraw.w = editDraw.requested_w; - editDraw.h = editDraw.requested_h; + editDraw.active.w = editDraw.requested.w; + editDraw.active.h = editDraw.requested.h; // We also need to update the current state so that we don't // end-up doing too much work during the next transaction. @@ -579,8 +580,8 @@ void Layer::lockPageFlip(bool& recomputeVisibleRegions) // because State::w and State::h are only accessed from // this thread Layer::State& editTemp(currentState()); - editTemp.w = editDraw.w; - editTemp.h = editDraw.h; + editTemp.active.w = editDraw.active.w; + editTemp.active.h = editDraw.active.h; // recompute visible region recomputeVisibleRegions = true; @@ -592,7 +593,7 @@ void Layer::lockPageFlip(bool& recomputeVisibleRegions) "requested (%dx%d)", this, bufWidth, bufHeight, mCurrentTransform, - front.requested_w, front.requested_h); + front.requested.w, front.requested.h); } } } diff --git a/services/surfaceflinger/LayerBase.cpp b/services/surfaceflinger/LayerBase.cpp index 81031b14f..c52b49ce8 100644 --- a/services/surfaceflinger/LayerBase.cpp +++ b/services/surfaceflinger/LayerBase.cpp @@ -84,16 +84,15 @@ void LayerBase::initStates(uint32_t w, uint32_t h, uint32_t flags) if (flags & ISurfaceComposer::eNonPremultiplied) mPremultipliedAlpha = false; - mCurrentState.z = 0; - mCurrentState.w = w; - mCurrentState.h = h; - mCurrentState.requested_w = w; - mCurrentState.requested_h = h; - mCurrentState.alpha = 0xFF; - mCurrentState.flags = layerFlags; - mCurrentState.sequence = 0; + mCurrentState.active.w = w; + mCurrentState.active.h = h; + mCurrentState.active.crop.makeInvalid(); + mCurrentState.z = 0; + mCurrentState.alpha = 0xFF; + mCurrentState.flags = layerFlags; + mCurrentState.sequence = 0; mCurrentState.transform.set(0, 0); - mCurrentState.crop.makeInvalid(); + mCurrentState.requested = mCurrentState.active; // drawing state & current state are identical mDrawingState = mCurrentState; @@ -136,10 +135,10 @@ bool LayerBase::setLayer(uint32_t z) { return true; } bool LayerBase::setSize(uint32_t w, uint32_t h) { - if (mCurrentState.requested_w == w && mCurrentState.requested_h == h) + if (mCurrentState.requested.w == w && mCurrentState.requested.h == h) return false; - mCurrentState.requested_w = w; - mCurrentState.requested_h = h; + mCurrentState.requested.w = w; + mCurrentState.requested.h = h; requestTransaction(); return true; } @@ -174,10 +173,10 @@ bool LayerBase::setFlags(uint8_t flags, uint8_t mask) { return true; } bool LayerBase::setCrop(const Rect& crop) { - if (mCurrentState.crop == crop) + if (mCurrentState.active.crop == crop) return false; mCurrentState.sequence++; - mCurrentState.crop = crop; + mCurrentState.active.crop = crop; requestTransaction(); return true; } @@ -202,15 +201,15 @@ uint32_t LayerBase::doTransaction(uint32_t flags) const Layer::State& front(drawingState()); const Layer::State& temp(currentState()); - if ((front.requested_w != temp.requested_w) || - (front.requested_h != temp.requested_h)) { + if ((front.requested.w != temp.requested.w) || + (front.requested.h != temp.requested.h)) { // resize the layer, set the physical size to the requested size Layer::State& editTemp(currentState()); - editTemp.w = temp.requested_w; - editTemp.h = temp.requested_h; + editTemp.active.w = temp.requested.w; + editTemp.active.h = temp.requested.h; } - if ((front.w != temp.w) || (front.h != temp.h)) { + if ((front.active.w != temp.active.w) || (front.active.h != temp.active.h)) { // invalidate and recompute the visible regions if needed flags |= Layer::eVisibleRegion; } @@ -238,9 +237,9 @@ void LayerBase::validateVisibility(const Transform& planeTransform) const bool transformed = tr.transformed(); const DisplayHardware& hw(graphicPlane(0).displayHardware()); const uint32_t hw_h = hw.getHeight(); - const Rect& crop(s.crop); + const Rect& crop(s.active.crop); - Rect win(s.w, s.h); + Rect win(s.active.w, s.active.h); if (!crop.isEmpty()) { win.intersect(crop, &win); } @@ -403,14 +402,14 @@ void LayerBase::drawWithOpenGL(const Region& clip) const GLfloat v; }; - Rect crop(s.w, s.h); - if (!s.crop.isEmpty()) { - crop = s.crop; + Rect crop(s.active.w, s.active.h); + if (!s.active.crop.isEmpty()) { + crop = s.active.crop; } - GLfloat left = GLfloat(crop.left) / GLfloat(s.w); - GLfloat top = GLfloat(crop.top) / GLfloat(s.h); - GLfloat right = GLfloat(crop.right) / GLfloat(s.w); - GLfloat bottom = GLfloat(crop.bottom) / GLfloat(s.h); + GLfloat left = GLfloat(crop.left) / GLfloat(s.active.w); + GLfloat top = GLfloat(crop.top) / GLfloat(s.active.h); + GLfloat right = GLfloat(crop.right) / GLfloat(s.active.w); + GLfloat bottom = GLfloat(crop.bottom) / GLfloat(s.active.h); TexCoords texCoords[4]; texCoords[0].u = left; @@ -449,10 +448,12 @@ void LayerBase::dump(String8& result, char* buffer, size_t SIZE) const snprintf(buffer, SIZE, " " - "z=%9d, pos=(%g,%g), size=(%4d,%4d), " + "z=%9d, pos=(%g,%g), size=(%4d,%4d), crop=(%4d,%4d,%4d,%4d), " "isOpaque=%1d, needsDithering=%1d, invalidate=%1d, " "alpha=0x%02x, flags=0x%08x, tr=[%.2f, %.2f][%.2f, %.2f]\n", - s.z, s.transform.tx(), s.transform.ty(), s.w, s.h, + s.z, s.transform.tx(), s.transform.ty(), s.active.w, s.active.h, + s.active.crop.left, s.active.crop.top, + s.active.crop.right, s.active.crop.bottom, isOpaque(), needsDithering(), contentDirty, s.alpha, s.flags, s.transform[0][0], s.transform[0][1], diff --git a/services/surfaceflinger/LayerBase.h b/services/surfaceflinger/LayerBase.h index 31f6dfdc6..d0b2a7442 100644 --- a/services/surfaceflinger/LayerBase.h +++ b/services/surfaceflinger/LayerBase.h @@ -65,20 +65,22 @@ public: Region coveredRegionScreen; int32_t sequence; - struct State { + struct Geometry { uint32_t w; uint32_t h; - uint32_t requested_w; - uint32_t requested_h; + Rect crop; + }; + + struct State { + Geometry active; + Geometry requested; uint32_t z; uint8_t alpha; uint8_t flags; uint8_t reserved[2]; int32_t sequence; // changes when visible regions can change - uint32_t tint; Transform transform; Region transparentRegion; - Rect crop; }; virtual void setName(const String8& name); From b30c415539813b96a831b75d07f3d12aef1aeab7 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 16 May 2012 18:21:32 -0700 Subject: [PATCH 2/2] Fix "Battery/Status/Clock status bar area flickers when dragging down" The crop is now handled like a resize, it's latched only when we receive a new buffer in the case we have a resize in the same transaction. Bug: 6498869 Change-Id: I9f3cbbe08fb19443899461ec441c714748a4fd1a --- services/surfaceflinger/Layer.cpp | 34 +++++++++++++++++---------- services/surfaceflinger/LayerBase.cpp | 16 ++++++------- services/surfaceflinger/LayerBase.h | 12 +++++++--- 3 files changed, 37 insertions(+), 25 deletions(-) diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 5988ed24a..6ec4f4991 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -435,25 +435,37 @@ uint32_t Layer::doTransaction(uint32_t flags) // the size changed, we need to ask our client to request a new buffer ALOGD_IF(DEBUG_RESIZE, "doTransaction: " - "resize (layer=%p), requested (%dx%d), drawing (%d,%d), " + "geometry (layer=%p), size: current (%dx%d), drawing (%dx%d), " + "crop: current (%d,%d,%d,%d [%dx%d]), drawing (%d,%d,%d,%d [%dx%d]), " "scalingMode=%d", this, int(temp.requested.w), int(temp.requested.h), int(front.requested.w), int(front.requested.h), + temp.requested.crop.left, + temp.requested.crop.top, + temp.requested.crop.right, + temp.requested.crop.bottom, + temp.requested.crop.getWidth(), + temp.requested.crop.getHeight(), + front.requested.crop.left, + front.requested.crop.top, + front.requested.crop.right, + front.requested.crop.bottom, + front.requested.crop.getWidth(), + front.requested.crop.getHeight(), mCurrentScalingMode); if (!isFixedSize()) { // this will make sure LayerBase::doTransaction doesn't update - // the drawing state's size + // the drawing state's geometry Layer::State& editDraw(mDrawingState); - editDraw.requested.w = temp.requested.w; - editDraw.requested.h = temp.requested.h; + editDraw.requested = temp.requested; } // record the new size, form this point on, when the client request // a buffer, it'll get the new size. - mSurfaceTexture->setDefaultBufferSize(temp.requested.w, - temp.requested.h); + mSurfaceTexture->setDefaultBufferSize( + temp.requested.w, temp.requested.h); } return LayerBase::doTransaction(flags); @@ -554,9 +566,7 @@ void Layer::lockPageFlip(bool& recomputeVisibleRegions) // FIXME: mPostedDirtyRegion = dirty & bounds mPostedDirtyRegion.set(front.active.w, front.active.h); - if ((front.active.w != front.requested.w) || - (front.active.h != front.requested.h)) - { + if (front.active != front.requested) { // check that we received a buffer of the right size // (Take the buffer's orientation into account) if (mCurrentTransform & Transform::ROT_90) { @@ -571,8 +581,7 @@ void Layer::lockPageFlip(bool& recomputeVisibleRegions) // current and drawing states. Drawing state is only accessed // in this thread, no need to have it locked Layer::State& editDraw(mDrawingState); - editDraw.active.w = editDraw.requested.w; - editDraw.active.h = editDraw.requested.h; + editDraw.active = editDraw.requested; // We also need to update the current state so that we don't // end-up doing too much work during the next transaction. @@ -580,8 +589,7 @@ void Layer::lockPageFlip(bool& recomputeVisibleRegions) // because State::w and State::h are only accessed from // this thread Layer::State& editTemp(currentState()); - editTemp.active.w = editDraw.active.w; - editTemp.active.h = editDraw.active.h; + editTemp.active = editDraw.active; // recompute visible region recomputeVisibleRegions = true; diff --git a/services/surfaceflinger/LayerBase.cpp b/services/surfaceflinger/LayerBase.cpp index c52b49ce8..7c6a28afb 100644 --- a/services/surfaceflinger/LayerBase.cpp +++ b/services/surfaceflinger/LayerBase.cpp @@ -173,10 +173,10 @@ bool LayerBase::setFlags(uint8_t flags, uint8_t mask) { return true; } bool LayerBase::setCrop(const Rect& crop) { - if (mCurrentState.active.crop == crop) + if (mCurrentState.requested.crop == crop) return false; mCurrentState.sequence++; - mCurrentState.active.crop = crop; + mCurrentState.requested.crop = crop; requestTransaction(); return true; } @@ -201,15 +201,13 @@ uint32_t LayerBase::doTransaction(uint32_t flags) const Layer::State& front(drawingState()); const Layer::State& temp(currentState()); - if ((front.requested.w != temp.requested.w) || - (front.requested.h != temp.requested.h)) { - // resize the layer, set the physical size to the requested size + if (front.requested != temp.requested) { + // geometry of the layer has changed, set the active geometry + // to the requested geometry. Layer::State& editTemp(currentState()); - editTemp.active.w = temp.requested.w; - editTemp.active.h = temp.requested.h; + editTemp.active = temp.requested; } - - if ((front.active.w != temp.active.w) || (front.active.h != temp.active.h)) { + if (front.active != temp.active) { // invalidate and recompute the visible regions if needed flags |= Layer::eVisibleRegion; } diff --git a/services/surfaceflinger/LayerBase.h b/services/surfaceflinger/LayerBase.h index d0b2a7442..954242442 100644 --- a/services/surfaceflinger/LayerBase.h +++ b/services/surfaceflinger/LayerBase.h @@ -66,9 +66,15 @@ public: int32_t sequence; struct Geometry { - uint32_t w; - uint32_t h; - Rect crop; + uint32_t w; + uint32_t h; + Rect crop; + inline bool operator == (const Geometry& rhs) const { + return (w==rhs.w && h==rhs.h && crop==rhs.crop); + } + inline bool operator != (const Geometry& rhs) const { + return !operator == (rhs); + } }; struct State {