From a2f4e56fec0fb36c4a370eb23d6e9dc57f250b59 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Sun, 15 Apr 2012 23:34:59 -0700 Subject: [PATCH] get rid off preserve backbuffer optimization in SF this optimization didn't improve performance and in fact seemed to hurt more than anything else. it also made things a lot more complex as it introduced edges cases when switching to/from h/w composer. Change-Id: Iaafc235e175f5740cd98bff914d706e02ab88bb8 --- .../DisplayHardware/DisplayHardware.cpp | 6 - .../DisplayHardware/DisplayHardware.h | 1 - services/surfaceflinger/LayerBase.cpp | 10 +- services/surfaceflinger/LayerBase.h | 12 +- services/surfaceflinger/LayerScreenshot.cpp | 2 - services/surfaceflinger/SurfaceFlinger.cpp | 146 ++++-------------- services/surfaceflinger/SurfaceFlinger.h | 2 +- 7 files changed, 36 insertions(+), 143 deletions(-) diff --git a/services/surfaceflinger/DisplayHardware/DisplayHardware.cpp b/services/surfaceflinger/DisplayHardware/DisplayHardware.cpp index c46e630f4..48651b60b 100644 --- a/services/surfaceflinger/DisplayHardware/DisplayHardware.cpp +++ b/services/surfaceflinger/DisplayHardware/DisplayHardware.cpp @@ -280,12 +280,6 @@ void DisplayHardware::init(uint32_t dpy) EGL_SWAP_BEHAVIOR, EGL_BUFFER_DESTROYED); } - if (eglQuerySurface(display, surface, EGL_SWAP_BEHAVIOR, &dummy) == EGL_TRUE) { - if (dummy == EGL_BUFFER_PRESERVED) { - mFlags |= BUFFER_PRESERVED; - } - } - /* * Create our OpenGL ES context */ diff --git a/services/surfaceflinger/DisplayHardware/DisplayHardware.h b/services/surfaceflinger/DisplayHardware/DisplayHardware.h index 84a3effe7..f1dee91be 100644 --- a/services/surfaceflinger/DisplayHardware/DisplayHardware.h +++ b/services/surfaceflinger/DisplayHardware/DisplayHardware.h @@ -51,7 +51,6 @@ public: enum { COPY_BITS_EXTENSION = 0x00000008, - BUFFER_PRESERVED = 0x00010000, PARTIAL_UPDATES = 0x00020000, // video driver feature SLOW_CONFIG = 0x00040000, // software SWAP_RECTANGLE = 0x00080000, diff --git a/services/surfaceflinger/LayerBase.cpp b/services/surfaceflinger/LayerBase.cpp index e76400120..416fc93e1 100644 --- a/services/surfaceflinger/LayerBase.cpp +++ b/services/surfaceflinger/LayerBase.cpp @@ -43,7 +43,7 @@ LayerBase::LayerBase(SurfaceFlinger* flinger, DisplayID display) : dpy(display), contentDirty(false), sequence(uint32_t(android_atomic_inc(&sSequence))), mFlinger(flinger), mFiltering(false), - mNeedsFiltering(false), mInOverlay(false), + mNeedsFiltering(false), mOrientation(0), mPlaneOrientation(0), mTransactionFlags(0), @@ -333,14 +333,6 @@ void LayerBase::setPerFrameData(hwc_layer_t* hwcl) { hwcl->handle = NULL; } -void LayerBase::setOverlay(bool inOverlay) { - mInOverlay = inOverlay; -} - -bool LayerBase::isOverlay() const { - return mInOverlay; -} - void LayerBase::setFiltering(bool filtering) { mFiltering = filtering; diff --git a/services/surfaceflinger/LayerBase.h b/services/surfaceflinger/LayerBase.h index cd6efdd1f..cecc88546 100644 --- a/services/surfaceflinger/LayerBase.h +++ b/services/surfaceflinger/LayerBase.h @@ -109,8 +109,6 @@ public: virtual void setGeometry(hwc_layer_t* hwcl); virtual void setPerFrameData(hwc_layer_t* hwcl); - void setOverlay(bool inOverlay); - bool isOverlay() const; /** @@ -230,14 +228,15 @@ public: int32_t getOrientation() const { return mOrientation; } int32_t getPlaneOrientation() const { return mPlaneOrientation; } - + + void clearWithOpenGL(const Region& clip) const; + protected: const GraphicPlane& graphicPlane(int dpy) const; GraphicPlane& graphicPlane(int dpy); void clearWithOpenGL(const Region& clip, GLclampf r, GLclampf g, GLclampf b, GLclampf alpha) const; - void clearWithOpenGL(const Region& clip) const; void drawWithOpenGL(const Region& clip) const; void setFiltering(bool filtering); @@ -255,11 +254,6 @@ private: // Whether filtering is needed b/c of the drawingstate bool mNeedsFiltering; - // this layer is currently handled by the hwc. this is - // updated at composition time, always frmo the composition - // thread. - bool mInOverlay; - protected: // cached during validateVisibility() int32_t mOrientation; diff --git a/services/surfaceflinger/LayerScreenshot.cpp b/services/surfaceflinger/LayerScreenshot.cpp index c7cf46e53..c127fa621 100644 --- a/services/surfaceflinger/LayerScreenshot.cpp +++ b/services/surfaceflinger/LayerScreenshot.cpp @@ -70,8 +70,6 @@ void LayerScreenshot::initTexture(GLfloat u, GLfloat v) { glBindTexture(GL_TEXTURE_2D, mTextureName); glTexParameterx(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); glTexParameterx(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); - glTexParameterx(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE); - glTexParameterx(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE); mTexCoords[0] = 0; mTexCoords[1] = v; mTexCoords[2] = 0; mTexCoords[3] = 0; mTexCoords[4] = u; mTexCoords[5] = 0; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index fb0c30520..5d61460d3 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -834,22 +834,11 @@ void SurfaceFlinger::handleRepaint() glLoadIdentity(); uint32_t flags = hw.getFlags(); - if ((flags & DisplayHardware::SWAP_RECTANGLE) || - (flags & DisplayHardware::BUFFER_PRESERVED)) - { + if (flags & DisplayHardware::SWAP_RECTANGLE) { // we can redraw only what's dirty, but since SWAP_RECTANGLE only // takes a rectangle, we must make sure to update that whole // rectangle in that case - 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(mSwapRegion.bounds()); - } else { - // in the BUFFER_PRESERVED case, obviously, we can update only - // what's needed and nothing more. - // NOTE: this is NOT a common case, as preserving the backbuffer - // is costly and usually involves copying the whole update back. - } + mDirtyRegion.set(mSwapRegion.bounds()); } else { if (flags & DisplayHardware::PARTIAL_UPDATES) { // We need to redraw the rectangle that will be updated @@ -864,7 +853,7 @@ void SurfaceFlinger::handleRepaint() } } - setupHardwareComposer(mDirtyRegion); + setupHardwareComposer(); composeSurfaces(mDirtyRegion); // update the swap region and clear the dirty region @@ -872,7 +861,7 @@ void SurfaceFlinger::handleRepaint() mDirtyRegion.clear(); } -void SurfaceFlinger::setupHardwareComposer(Region& dirtyInOut) +void SurfaceFlinger::setupHardwareComposer() { const DisplayHardware& hw(graphicPlane(0).displayHardware()); HWComposer& hwc(hw.getHwComposer()); @@ -901,75 +890,8 @@ void SurfaceFlinger::setupHardwareComposer(Region& dirtyInOut) const sp& layer(layers[i]); layer->setPerFrameData(&cur[i]); } - const size_t fbLayerCount = hwc.getLayerCount(HWC_FRAMEBUFFER); status_t err = hwc.prepare(); ALOGE_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 be erasing them), - // that is, we can't erase anything outside the dirty region. - - Region transparent; - - 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); - } - // don't erase stuff outside the dirty region - transparent.andSelf(dirtyInOut); - } - - /* - * clear the area of the FB that need to be transparent - */ - if (!transparent.isEmpty()) { - glClearColor(0,0,0,0); - Region::const_iterator it = transparent.begin(); - Region::const_iterator const end = transparent.end(); - const int32_t height = hw.getHeight(); - while (it != end) { - const Rect& r(*it++); - const GLint sy = height - (r.top + r.height()); - glScissor(r.left, sy, r.width(), r.height()); - glClear(GL_COLOR_BUFFER_BIT); - } - } - } } void SurfaceFlinger::composeSurfaces(const Region& dirty) @@ -978,45 +900,38 @@ void SurfaceFlinger::composeSurfaces(const Region& dirty) HWComposer& hwc(hw.getHwComposer()); const size_t fbLayerCount = hwc.getLayerCount(HWC_FRAMEBUFFER); - if (CC_UNLIKELY(fbLayerCount && !mWormholeRegion.isEmpty())) { - // should never happen unless the window manager has a bug - // draw something... - drawWormhole(); - } + if (fbLayerCount) { + // Never touch the framebuffer if we don't have any framebuffer layers - /* - * and then, render the layers targeted at the framebuffer - */ + if (!mWormholeRegion.isEmpty()) { + // can happen with SurfaceView + drawWormhole(); + } - hwc_layer_t* const cur(hwc.getLayers()); - const Vector< sp >& layers(mVisibleLayersSortedByZ); - size_t count = layers.size(); + /* + * and then, render the layers targeted at the framebuffer + */ + hwc_layer_t* const cur(hwc.getLayers()); + const Vector< sp >& layers(mVisibleLayersSortedByZ); + const size_t count = layers.size(); - // FIXME: workaround for b/6020860 - if (hw.getFlags() & DisplayHardware::BUFFER_PRESERVED) { for (size_t i=0 ; i& layer(layers[i]); + const Region clip(dirty.intersect(layer->visibleRegionScreen)); + if (!clip.isEmpty()) { + if (cur && (cur[i].compositionType != HWC_FRAMEBUFFER)) { + if ((cur[i].hints & HWC_HINT_CLEAR_FB) + && layer->isOpaque()) { + layer->clearWithOpenGL(clip); + } + continue; + } + // render the layer + layer->draw(clip); } } } - // FIXME: bug6020860 for b/6020860 - - - for (size_t i=0 ; i& layer(layers[i]); - const Region clip(dirty.intersect(layer->visibleRegionScreen)); - if (!clip.isEmpty()) { - layer->draw(clip); - } - } } void SurfaceFlinger::debugFlashRegions() @@ -1028,8 +943,7 @@ void SurfaceFlinger::debugFlashRegions() return; } - if (!((flags & DisplayHardware::SWAP_RECTANGLE) || - (flags & DisplayHardware::BUFFER_PRESERVED))) { + if (!(flags & DisplayHardware::SWAP_RECTANGLE)) { const Region repaint((flags & DisplayHardware::PARTIAL_UPDATES) ? mDirtyRegion.bounds() : hw.bounds()); composeSurfaces(repaint); @@ -1903,6 +1817,8 @@ status_t SurfaceFlinger::renderScreenToTextureLocked(DisplayID dpy, GLuint name, tname; glGenTextures(1, &tname); glBindTexture(GL_TEXTURE_2D, tname); + glTexParameterx(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE); + glTexParameterx(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE); glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, hw_w, hw_h, 0, GL_RGB, GL_UNSIGNED_BYTE, 0); if (glGetError() != GL_NO_ERROR) { diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 46bbab215..1aea452b6 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -318,7 +318,7 @@ private: void handleWorkList(); void handleRepaint(); void postFramebuffer(); - void setupHardwareComposer(Region& dirtyInOut); + void setupHardwareComposer(); void composeSurfaces(const Region& dirty);