From 670a8990e822785da2a684f71f0035725022ced4 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 20 Sep 2011 15:13:14 -0700 Subject: [PATCH 1/3] improve hwc dumpsys we now log the buffer's format Change-Id: I9d3ad8018e884240a153de3baefb6331cb014d0f --- .../DisplayHardware/HWComposer.cpp | 21 ++++++++++++++----- services/surfaceflinger/Layer.h | 2 ++ services/surfaceflinger/LayerBase.h | 2 ++ services/surfaceflinger/SurfaceFlinger.cpp | 2 +- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index c9567d527..daefd5e32 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -143,18 +143,29 @@ void HWComposer::dump(String8& result, char* buffer, size_t SIZE, snprintf(buffer, SIZE, " numHwLayers=%u, flags=%08x\n", mList->numHwLayers, mList->flags); result.append(buffer); - + result.append( + " type | hints | flags | tr | blend | format | source rectangle | crop rectangle name \n" + "-----------+----------+----------+----+-------+----------+---------------------------+--------------------------------\n"); + // " ________ | ________ | ________ | __ | _____ | ________ | [_____,_____,_____,_____] | [_____,_____,_____,_____] for (size_t i=0 ; inumHwLayers ; i++) { const hwc_layer_t& l(mList->hwLayers[i]); - snprintf(buffer, SIZE, " %8s | %08x | %08x | %02x | %04x | [%5d,%5d,%5d,%5d] | [%5d,%5d,%5d,%5d] %s\n", + const sp layer(visibleLayersSortedByZ[i]); + int32_t format = -1; + if (layer->getLayer() != NULL) { + const sp& buffer(layer->getLayer()->getActiveBuffer()); + if (buffer != NULL) { + format = buffer->getPixelFormat(); + } + } + snprintf(buffer, SIZE, + " %8s | %08x | %08x | %02x | %05x | %08x | [%5d,%5d,%5d,%5d] | [%5d,%5d,%5d,%5d] %s\n", l.compositionType ? "OVERLAY" : "FB", - l.hints, l.flags, l.transform, l.blending, + l.hints, l.flags, l.transform, l.blending, format, l.sourceCrop.left, l.sourceCrop.top, l.sourceCrop.right, l.sourceCrop.bottom, l.displayFrame.left, l.displayFrame.top, l.displayFrame.right, l.displayFrame.bottom, - visibleLayersSortedByZ[i]->getName().string()); + layer->getName().string()); result.append(buffer); } - } if (mHwc && mHwc->common.version >= 1 && mHwc->dump) { mHwc->dump(mHwc, buffer, SIZE); diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index d06a35f01..ff389aece 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -73,12 +73,14 @@ public: virtual bool isSecure() const { return mSecure; } virtual bool isProtected() const; virtual void onRemoved(); + virtual sp getLayer() const { return const_cast(this); } // LayerBaseClient interface virtual wp getSurfaceTextureBinder() const; // only for debugging inline const sp& getFreezeLock() const { return mFreezeLock; } + inline const sp& getActiveBuffer() const { return mActiveBuffer; } protected: virtual void onFirstRef(); diff --git a/services/surfaceflinger/LayerBase.h b/services/surfaceflinger/LayerBase.h index ee5042838..a14b397a8 100644 --- a/services/surfaceflinger/LayerBase.h +++ b/services/surfaceflinger/LayerBase.h @@ -46,6 +46,7 @@ class Client; class DisplayHardware; class GraphicBuffer; class GraphicPlane; +class Layer; class LayerBaseClient; class SurfaceFlinger; @@ -105,6 +106,7 @@ public: void invalidate(); virtual sp getLayerBaseClient() const { return 0; } + virtual sp getLayer() const { return 0; } virtual const char* getTypeId() const { return "LayerBase"; } diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index df13640b6..431cd1a1e 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -925,7 +925,7 @@ Region SurfaceFlinger::setupHardwareComposer(const Region& dirty) // what's happening here is tricky. // we want to clear all the layers with the CLEAR_FB flags // that are opaque. - // however, since some GPU have are efficient at preserving + // however, since some GPU are efficient at preserving // the backbuffer, we want to take advantage of that so we do the // clear only in the dirty region (other areas will be preserved // on those GPUs). From 56eb3fc695869c05ef3b35dfe9dfb25d2f826825 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 20 Sep 2011 17:22:44 -0700 Subject: [PATCH 2/3] rename mInvalidRegion to mSwapRegion Change-Id: I946cbc782c0c84645843ea44c3d8b04a0a2fe658 --- services/surfaceflinger/SurfaceFlinger.cpp | 20 ++++++++++---------- services/surfaceflinger/SurfaceFlinger.h | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 431cd1a1e..1703448cf 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -469,14 +469,14 @@ bool SurfaceFlinger::threadLoop() void SurfaceFlinger::postFramebuffer() { - if (!mInvalidRegion.isEmpty()) { + if (!mSwapRegion.isEmpty()) { const DisplayHardware& hw(graphicPlane(0).displayHardware()); const nsecs_t now = systemTime(); mDebugInSwapBuffers = now; - hw.flip(mInvalidRegion); + hw.flip(mSwapRegion); mLastSwapBufferTime = systemTime() - now; mDebugInSwapBuffers = 0; - mInvalidRegion.clear(); + mSwapRegion.clear(); } } @@ -834,7 +834,7 @@ void SurfaceFlinger::handleWorkList() void SurfaceFlinger::handleRepaint() { // compute the invalid region - mInvalidRegion.orSelf(mDirtyRegion); + mSwapRegion.orSelf(mDirtyRegion); if (UNLIKELY(mDebugRegion)) { debugFlashRegions(); @@ -855,7 +855,7 @@ void SurfaceFlinger::handleRepaint() if (flags & DisplayHardware::SWAP_RECTANGLE) { // TODO: we really should be able to pass a region to // SWAP_RECTANGLE so that we don't have to redraw all this. - mDirtyRegion.set(mInvalidRegion.bounds()); + mDirtyRegion.set(mSwapRegion.bounds()); } else { // in the BUFFER_PRESERVED case, obviously, we can update only // what's needed and nothing more. @@ -868,17 +868,17 @@ void SurfaceFlinger::handleRepaint() // (pushed to the framebuffer). // This is needed because PARTIAL_UPDATES only takes one // rectangle instead of a region (see DisplayHardware::flip()) - mDirtyRegion.set(mInvalidRegion.bounds()); + mDirtyRegion.set(mSwapRegion.bounds()); } else { // we need to redraw everything (the whole screen) mDirtyRegion.set(hw.bounds()); - mInvalidRegion = mDirtyRegion; + mSwapRegion = mDirtyRegion; } } Region expandDirty = setupHardwareComposer(mDirtyRegion); mDirtyRegion.orSelf(expandDirty); - mInvalidRegion.orSelf(mDirtyRegion); + mSwapRegion.orSelf(mDirtyRegion); composeSurfaces(mDirtyRegion); // clear the dirty regions @@ -1014,7 +1014,7 @@ void SurfaceFlinger::debugFlashRegions() const DisplayHardware& hw(graphicPlane(0).displayHardware()); const uint32_t flags = hw.getFlags(); const int32_t height = hw.getHeight(); - if (mInvalidRegion.isEmpty()) { + if (mSwapRegion.isEmpty()) { return; } @@ -1051,7 +1051,7 @@ void SurfaceFlinger::debugFlashRegions() glDrawArrays(GL_TRIANGLE_FAN, 0, 4); } - hw.flip(mInvalidRegion); + hw.flip(mSwapRegion); if (mDebugRegion > 1) usleep(mDebugRegion * 1000); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 126ca39d8..a8036a68f 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -358,7 +358,7 @@ private: State mDrawingState; Region mDirtyRegion; Region mDirtyRegionRemovedLayer; - Region mInvalidRegion; + Region mSwapRegion; Region mWormholeRegion; bool mVisibleRegionsDirty; bool mHwWorkListDirty; From 8bfdda9ab34466f97d024c1dcd63019de880b806 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 20 Sep 2011 17:21:56 -0700 Subject: [PATCH 3/3] fix transition from full overlays to fb we need to clear the whole framebuffer in that situation because we can't trust the content of the FB when partial (fb preserving) updates are used. Bug: 5318492 Change-Id: I3f0e01b0fb665a34e44d88ad9f0f54a5d990060b --- .../DisplayHardware/DisplayHardware.h | 3 +- .../DisplayHardware/HWComposer.cpp | 32 +++++++ .../DisplayHardware/HWComposer.h | 5 + services/surfaceflinger/SurfaceFlinger.cpp | 96 ++++++++++--------- services/surfaceflinger/SurfaceFlinger.h | 2 +- 5 files changed, 90 insertions(+), 48 deletions(-) diff --git a/services/surfaceflinger/DisplayHardware/DisplayHardware.h b/services/surfaceflinger/DisplayHardware/DisplayHardware.h index 40a6f1e8c..f02c95414 100644 --- a/services/surfaceflinger/DisplayHardware/DisplayHardware.h +++ b/services/surfaceflinger/DisplayHardware/DisplayHardware.h @@ -84,9 +84,10 @@ public: status_t compositionComplete() const; - Rect bounds() const { + Rect getBounds() const { return Rect(mWidth, mHeight); } + inline Rect bounds() const { return getBounds(); } // only for debugging int getCurrentBufferIndex() const; diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index daefd5e32..e707bdce7 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -40,6 +40,7 @@ namespace android { HWComposer::HWComposer(const sp& flinger) : mFlinger(flinger), mModule(0), mHwc(0), mList(0), mCapacity(0), + mNumOVLayers(0), mNumFBLayers(0), mDpy(EGL_NO_DISPLAY), mSur(EGL_NO_SURFACE) { int err = hw_get_module(HWC_HARDWARE_MODULE_ID, &mModule); @@ -98,9 +99,40 @@ status_t HWComposer::createWorkList(size_t numLayers) { status_t HWComposer::prepare() const { int err = mHwc->prepare(mHwc, mList); + if (err == NO_ERROR) { + size_t numOVLayers = 0; + size_t numFBLayers = 0; + size_t count = mList->numHwLayers; + for (size_t i=0 ; ihwLayers[i]); + if (l.flags & HWC_SKIP_LAYER) { + l.compositionType = HWC_FRAMEBUFFER; + } + switch (l.compositionType) { + case HWC_OVERLAY: + numOVLayers++; + break; + case HWC_FRAMEBUFFER: + numFBLayers++; + break; + } + } + mNumOVLayers = numOVLayers; + mNumFBLayers = numFBLayers; + } return (status_t)err; } +size_t HWComposer::getLayerCount(int type) const { + switch (type) { + case HWC_OVERLAY: + return mNumOVLayers; + case HWC_FRAMEBUFFER: + return mNumFBLayers; + } + return 0; +} + status_t HWComposer::commit() const { int err = mHwc->set(mHwc, mDpy, mSur, mList); if (mList) { diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h index 8758a801b..aa8ebe1fd 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.h +++ b/services/surfaceflinger/DisplayHardware/HWComposer.h @@ -64,6 +64,9 @@ public: size_t getNumLayers() const; hwc_layer_t* getLayers() const; + // updated in preapre() + size_t getLayerCount(int type) const; + // for debugging void dump(String8& out, char* scratch, size_t SIZE, const Vector< sp >& visibleLayersSortedByZ) const; @@ -81,6 +84,8 @@ private: hwc_composer_device_t* mHwc; hwc_layer_list_t* mList; size_t mCapacity; + mutable size_t mNumOVLayers; + mutable size_t mNumFBLayers; hwc_display_t mDpy; hwc_surface_t mSur; cb_context mCBContext; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 1703448cf..0ef03bb7d 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -876,24 +876,21 @@ void SurfaceFlinger::handleRepaint() } } - Region expandDirty = setupHardwareComposer(mDirtyRegion); - mDirtyRegion.orSelf(expandDirty); - mSwapRegion.orSelf(mDirtyRegion); + setupHardwareComposer(mDirtyRegion); composeSurfaces(mDirtyRegion); - // clear the dirty regions + // update the swap region and clear the dirty region + mSwapRegion.orSelf(mDirtyRegion); mDirtyRegion.clear(); } -Region SurfaceFlinger::setupHardwareComposer(const Region& dirty) +void SurfaceFlinger::setupHardwareComposer(Region& dirtyInOut) { - Region dirtyOut(dirty); - const DisplayHardware& hw(graphicPlane(0).displayHardware()); HWComposer& hwc(hw.getHwComposer()); hwc_layer_t* const cur(hwc.getLayers()); if (!cur) { - return dirtyOut; + return; } const Vector< sp >& layers(mVisibleLayersSortedByZ); @@ -916,53 +913,62 @@ Region SurfaceFlinger::setupHardwareComposer(const Region& dirty) const sp& layer(layers[i]); layer->setPerFrameData(&cur[i]); } + const size_t fbLayerCount = hwc.getLayerCount(HWC_FRAMEBUFFER); status_t err = hwc.prepare(); LOGE_IF(err, "HWComposer::prepare failed (%s)", strerror(-err)); if (err == NO_ERROR) { + // what's happening here is tricky. + // we want to clear all the layers with the CLEAR_FB flags + // that are opaque. + // however, since some GPU are efficient at preserving + // the backbuffer, we want to take advantage of that so we do the + // clear only in the dirty region (other areas will be preserved + // on those GPUs). + // NOTE: on non backbuffer preserving GPU, the dirty region + // has already been expanded as needed, so the code is correct + // there too. + // + // However, the content of the framebuffer cannot be trusted when + // we switch to/from FB/OVERLAY, in which case we need to + // expand the dirty region to those areas too. + // + // Note also that there is a special case when switching from + // "no layers in FB" to "some layers in FB", where we need to redraw + // the entire FB, since some areas might contain uninitialized + // data. + // + // Also we want to make sure to not clear areas that belong to + // layers above that won't redraw (we would just erasing them), + // that is, we can't erase anything outside the dirty region. + Region transparent; - for (size_t i=0 ; i& layer(layers[i]); - if ((cur[i].hints & HWC_HINT_CLEAR_FB) && layer->isOpaque()) { - transparent.orSelf(layer->visibleRegionScreen); + if (!fbLayerCount && hwc.getLayerCount(HWC_FRAMEBUFFER)) { + transparent.set(hw.getBounds()); + dirtyInOut = transparent; + } else { + for (size_t i=0 ; i& layer(layers[i]); + if ((cur[i].hints & HWC_HINT_CLEAR_FB) && layer->isOpaque()) { + transparent.orSelf(layer->visibleRegionScreen); + } + bool isOverlay = (cur[i].compositionType != HWC_FRAMEBUFFER); + if (isOverlay != layer->isOverlay()) { + // we transitioned to/from overlay, so add this layer + // to the dirty region so the framebuffer can be either + // cleared or redrawn. + dirtyInOut.orSelf(layer->visibleRegionScreen); + } + layer->setOverlay(isOverlay); } - - bool isOverlay = (cur[i].compositionType != HWC_FRAMEBUFFER) && - !(cur[i].flags & HWC_SKIP_LAYER); - - if (isOverlay != layer->isOverlay()) { - // we transitioned to/from overlay, so add this layer - // to the dirty region so the framebuffer can be either - // cleared or redrawn. - dirtyOut.orSelf(layer->visibleRegionScreen); - } - layer->setOverlay(isOverlay); + // don't erase stuff outside the dirty region + transparent.andSelf(dirtyInOut); } - /* * clear the area of the FB that need to be transparent */ - // don't erase stuff outside the dirty region - transparent.andSelf(dirtyOut); if (!transparent.isEmpty()) { glClearColor(0,0,0,0); Region::const_iterator it = transparent.begin(); @@ -976,7 +982,6 @@ Region SurfaceFlinger::setupHardwareComposer(const Region& dirty) } } } - return dirtyOut; } void SurfaceFlinger::composeSurfaces(const Region& dirty) @@ -997,8 +1002,7 @@ void SurfaceFlinger::composeSurfaces(const Region& dirty) const Vector< sp >& layers(mVisibleLayersSortedByZ); size_t count = layers.size(); for (size_t i=0 ; i& layer(layers[i]); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index a8036a68f..d7f005f7c 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -277,7 +277,7 @@ private: void handleWorkList(); void handleRepaint(); void postFramebuffer(); - Region setupHardwareComposer(const Region& dirty); + void setupHardwareComposer(Region& dirtyInOut); void composeSurfaces(const Region& dirty); void repaintEverything();