From 9ef9d51ec9ded43d03d045f623edfa9ddd6d8620 Mon Sep 17 00:00:00 2001 From: Jamie Gennis Date: Mon, 25 Feb 2013 13:37:54 -0800 Subject: [PATCH 1/7] SurfaceFlinger: fix a couple NULL fence checks This change replaces checks for a NULL fence pointer with calls to Fence::isValid. There should no longer be NULL fences. Change-Id: If17c9c132fcb1801531bf7588f8ba53476c57dad --- services/surfaceflinger/Layer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 1401154d3..439acb52a 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -526,7 +526,7 @@ void Layer::onPostComposition() { mFrameTracker.setDesiredPresentTime(desiredPresentTime); sp frameReadyFence = mSurfaceFlingerConsumer->getCurrentFence(); - if (frameReadyFence != NULL) { + if (frameReadyFence->isValid()) { mFrameTracker.setFrameReadyFence(frameReadyFence); } else { // There was no fence for this frame, so assume that it was ready @@ -536,7 +536,7 @@ void Layer::onPostComposition() { const HWComposer& hwc = mFlinger->getHwComposer(); sp presentFence = hwc.getDisplayFence(HWC_DISPLAY_PRIMARY); - if (presentFence != NULL) { + if (presentFence->isValid()) { mFrameTracker.setActualPresentFence(presentFence); } else { // The HWC doesn't support present fences, so use the refresh From 0d69ad5d651ac3438cfdd527f6f26901e459d53e Mon Sep 17 00:00:00 2001 From: Dave Burke Date: Fri, 1 Mar 2013 20:39:03 +0000 Subject: [PATCH 2/7] Revert "Change SurfaceControl setPosition to take floats" Temporary, to fix weekend build, until we get Nvidia code drop. This reverts commit 9a867a8798fa6ea21f6341db31e38ea64fde6c83 DO NOT MERGE Change-Id: I7b5dbc4db46ef3d97dc8598057d5487d6971178b --- include/gui/SurfaceControl.h | 2 +- libs/gui/SurfaceControl.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/gui/SurfaceControl.h b/include/gui/SurfaceControl.h index 9268e7dc8..f70888ddf 100644 --- a/include/gui/SurfaceControl.h +++ b/include/gui/SurfaceControl.h @@ -57,7 +57,7 @@ public: status_t setLayerStack(int32_t layerStack); status_t setLayer(int32_t layer); - status_t setPosition(float x, float y); + status_t setPosition(int32_t x, int32_t y); status_t setSize(uint32_t w, uint32_t h); status_t hide(); status_t show(); diff --git a/libs/gui/SurfaceControl.cpp b/libs/gui/SurfaceControl.cpp index e621b22a0..ef52269b0 100644 --- a/libs/gui/SurfaceControl.cpp +++ b/libs/gui/SurfaceControl.cpp @@ -106,7 +106,7 @@ status_t SurfaceControl::setLayer(int32_t layer) { const sp& client(mClient); return client->setLayer(mSurface, layer); } -status_t SurfaceControl::setPosition(float x, float y) { +status_t SurfaceControl::setPosition(int32_t x, int32_t y) { status_t err = validate(); if (err < 0) return err; const sp& client(mClient); From 70b52f5ce2bbcb09cd4551982d984e0b47b4477f Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Thu, 7 Mar 2013 09:56:26 -0800 Subject: [PATCH 3/7] Defer destroying surfaces until not current Bug: 8320762 Change-Id: I1320cf87923bcc5b795a86a13193363a49e29653 --- opengl/libagl/egl.cpp | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/opengl/libagl/egl.cpp b/opengl/libagl/egl.cpp index 172ef9571..bc1cf3636 100644 --- a/opengl/libagl/egl.cpp +++ b/opengl/libagl/egl.cpp @@ -147,6 +147,7 @@ struct egl_surface_t EGLDisplay dpy; EGLConfig config; EGLContext ctx; + bool zombie; egl_surface_t(EGLDisplay dpy, EGLConfig config, int32_t depthFormat); virtual ~egl_surface_t(); @@ -173,7 +174,7 @@ protected: egl_surface_t::egl_surface_t(EGLDisplay dpy, EGLConfig config, int32_t depthFormat) - : magic(MAGIC), dpy(dpy), config(config), ctx(0) + : magic(MAGIC), dpy(dpy), config(config), ctx(0), zombie(false) { depth.version = sizeof(GGLSurface); depth.data = 0; @@ -1580,11 +1581,12 @@ EGLBoolean eglDestroySurface(EGLDisplay dpy, EGLSurface eglSurface) if (surface->dpy != dpy) return setError(EGL_BAD_DISPLAY, EGL_FALSE); if (surface->ctx) { - // FIXME: this surface is current check what the spec says + // defer disconnect/delete until no longer current + surface->zombie = true; + } else { surface->disconnect(); - surface->ctx = 0; + delete surface; } - delete surface; } return EGL_TRUE; } @@ -1736,6 +1738,9 @@ EGLBoolean eglMakeCurrent( EGLDisplay dpy, EGLSurface draw, if (c->draw) { egl_surface_t* s = reinterpret_cast(c->draw); s->disconnect(); + s->ctx = EGL_NO_CONTEXT; + if (s->zombie) + delete s; } if (c->read) { // FIXME: unlock/disconnect the read surface too @@ -1777,8 +1782,10 @@ EGLBoolean eglMakeCurrent( EGLDisplay dpy, EGLSurface draw, egl_surface_t* r = (egl_surface_t*)c->read; if (d) { c->draw = 0; - d->ctx = EGL_NO_CONTEXT; d->disconnect(); + d->ctx = EGL_NO_CONTEXT; + if (d->zombie) + delete d; } if (r) { c->read = 0; From f8cbf2c06c8a5b49059d56ae3cea8115d4ff52e0 Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Thu, 7 Mar 2013 15:14:18 -0800 Subject: [PATCH 4/7] When disconnecting a surface, cancel don't queue the buffer This isn't really right either, but avoids having an extra buffer that the consumer has to drain which it might not be expecting. To be correct, disconnecting a surface from a context should retain the current buffer and continue using it when reconnected. The buffer should only be canceled when the surface is destroyed. That will wait for a later change. Bug: 8320762 Change-Id: I5efa39c741193ca4f5612ea9de001ccbb683b345 --- opengl/libagl/egl.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/opengl/libagl/egl.cpp b/opengl/libagl/egl.cpp index bc1cf3636..0ed572717 100644 --- a/opengl/libagl/egl.cpp +++ b/opengl/libagl/egl.cpp @@ -420,9 +420,8 @@ void egl_window_surface_v2_t::disconnect() bits = NULL; unlock(buffer); } - // enqueue the last frame - nativeWindow->queueBuffer(nativeWindow, buffer, -1); if (buffer) { + nativeWindow->cancelBuffer(nativeWindow, buffer, -1); buffer->common.decRef(&buffer->common); buffer = 0; } From 397ff875b29ac6305d16e7225a646454ff0b5a07 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 13 Mar 2013 15:22:11 -0700 Subject: [PATCH 5/7] size IMemoryHeap properly for screenshots since we're using glReadPixels(), we only need to use the width (as opposed to the stride) of the source screenshot. Bug: 8374664 Change-Id: I145c80f4fff5444df7c77c4f52e70a7203caddbd --- services/surfaceflinger/SurfaceFlinger.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 184f47e91..884d7edbe 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -2783,7 +2783,7 @@ status_t SurfaceFlinger::captureScreenImplLocked( sp buf(consumer->getCurrentBuffer()); sw = buf->getWidth(); sh = buf->getHeight(); - size_t size = buf->getStride() * sh * 4; + size_t size = sw * sh * 4; // allocate shared memory large enough to hold the // screen capture From 284414bf4f1a6a649b2177286d473e4d3626631f Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Thu, 14 Mar 2013 10:31:38 -0700 Subject: [PATCH 6/7] am 7212ff29: am 20e154f1: Merge "Second try at adding a compatibility symbol for the MemoryBase constructor." * commit '7212ff29c6f4e4cd192fee6f072e80b36d8a728b': Second try at adding a compatibility symbol for the MemoryBase constructor. --- libs/binder/MemoryBase.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/libs/binder/MemoryBase.cpp b/libs/binder/MemoryBase.cpp index 033066bea..5c823302f 100644 --- a/libs/binder/MemoryBase.cpp +++ b/libs/binder/MemoryBase.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#define LOG_TAG "MemoryBase" #include #include @@ -44,3 +45,11 @@ MemoryBase::~MemoryBase() // --------------------------------------------------------------------------- }; // namespace android + +// Backwards compatibility for libdatabase_sqlcipher (http://b/8253769). +extern "C" void _ZN7android10MemoryBaseC1ERKNS_2spINS_11IMemoryHeapEEEij(void*, void*, ssize_t, size_t); +extern "C" void _ZN7android10MemoryBaseC1ERKNS_2spINS_11IMemoryHeapEEElj(void* obj, void* h, long o, unsigned int size) { + _ZN7android10MemoryBaseC1ERKNS_2spINS_11IMemoryHeapEEEij(obj, h, o, size); + ALOGW("Using temporary compatibility workaround for usage of MemoryBase " + "private API. Please fix your application!"); +} From b90d8dc527f02687b36f0ab766855024b7bdb4a8 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 10 Apr 2013 22:55:41 -0700 Subject: [PATCH 7/7] Improve screenshot performance on some devices (DO NOT MERGE) this affects devices that need a glReadPixels(). We use a FBO instead of a GlConsumer as an intermediate render target, this saves 2 calls to eglMakeCurrent(). On Galaxy Nexus this allows us to go from ~135ms to ~35ms for recent's screenshots. Bug: 8582615 Change-Id: I6b25291ecc235f1927579bbb2db3c731e985c6e8 --- services/surfaceflinger/SurfaceFlinger.cpp | 261 +++++++++++---------- services/surfaceflinger/SurfaceFlinger.h | 6 + 2 files changed, 139 insertions(+), 128 deletions(-) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 32b97ebac..4bc0abdf4 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -2646,6 +2646,61 @@ status_t SurfaceFlinger::captureScreen(const sp& display, return res; } + +void SurfaceFlinger::renderScreenImplLocked( + const sp& hw, + uint32_t reqWidth, uint32_t reqHeight, + uint32_t minLayerZ, uint32_t maxLayerZ, + bool yswap) +{ + ATRACE_CALL(); + + // get screen geometry + const uint32_t hw_w = hw->getWidth(); + const uint32_t hw_h = hw->getHeight(); + + const bool filtering = reqWidth != hw_w || reqWidth != hw_h; + + // make sure to clear all GL error flags + while ( glGetError() != GL_NO_ERROR ) ; + + // set-up our viewport + glViewport(0, 0, reqWidth, reqHeight); + glMatrixMode(GL_PROJECTION); + glLoadIdentity(); + if (yswap) glOrthof(0, hw_w, hw_h, 0, 0, 1); + else glOrthof(0, hw_w, 0, hw_h, 0, 1); + glMatrixMode(GL_MODELVIEW); + glLoadIdentity(); + + // redraw the screen entirely... + glDisable(GL_SCISSOR_TEST); + glClearColor(0,0,0,1); + glClear(GL_COLOR_BUFFER_BIT); + glDisable(GL_TEXTURE_EXTERNAL_OES); + glDisable(GL_TEXTURE_2D); + + const LayerVector& layers( mDrawingState.layersSortedByZ ); + const size_t count = layers.size(); + for (size_t i=0 ; i& layer(layers[i]); + const Layer::State& state(layer->drawingState()); + if (state.layerStack == hw->getLayerStack()) { + if (state.z >= minLayerZ && state.z <= maxLayerZ) { + if (layer->isVisible()) { + if (filtering) layer->setFiltering(true); + layer->draw(hw); + if (filtering) layer->setFiltering(false); + } + } + } + } + + // compositionComplete is needed for older driver + hw->compositionComplete(); +} + + status_t SurfaceFlinger::captureScreenImplLocked( const sp& hw, const sp& producer, @@ -2672,7 +2727,6 @@ status_t SurfaceFlinger::captureScreenImplLocked( reqWidth = (!reqWidth) ? hw_w : reqWidth; reqHeight = (!reqHeight) ? hw_h : reqHeight; - const bool filtering = reqWidth != hw_w || reqWidth != hw_h; // Create a surface to render into sp surface = new Surface(producer); @@ -2697,41 +2751,7 @@ status_t SurfaceFlinger::captureScreenImplLocked( return BAD_VALUE; } - // make sure to clear all GL error flags - while ( glGetError() != GL_NO_ERROR ) ; - - // set-up our viewport - glViewport(0, 0, reqWidth, reqHeight); - glMatrixMode(GL_PROJECTION); - glLoadIdentity(); - glOrthof(0, hw_w, 0, hw_h, 0, 1); - glMatrixMode(GL_MODELVIEW); - glLoadIdentity(); - - // redraw the screen entirely... - glDisable(GL_TEXTURE_EXTERNAL_OES); - glDisable(GL_TEXTURE_2D); - glClearColor(0,0,0,1); - glClear(GL_COLOR_BUFFER_BIT); - - const LayerVector& layers( mDrawingState.layersSortedByZ ); - const size_t count = layers.size(); - for (size_t i=0 ; i& layer(layers[i]); - const Layer::State& state(layer->drawingState()); - if (state.layerStack == hw->getLayerStack()) { - if (state.z >= minLayerZ && state.z <= maxLayerZ) { - if (layer->isVisible()) { - if (filtering) layer->setFiltering(true); - layer->draw(hw); - if (filtering) layer->setFiltering(false); - } - } - } - } - - // compositionComplete is needed for older driver - hw->compositionComplete(); + renderScreenImplLocked(hw, reqWidth, reqHeight, minLayerZ, maxLayerZ, false); // and finishing things up... if (eglSwapBuffers(mEGLDisplay, eglSurface) != EGL_TRUE) { @@ -2759,102 +2779,87 @@ status_t SurfaceFlinger::captureScreenImplCpuConsumerLocked( return INVALID_OPERATION; } - // create the texture that will receive the screenshot, later we'll - // attach a FBO to it so we can call glReadPixels(). - GLuint 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); + // get screen geometry + const uint32_t hw_w = hw->getWidth(); + const uint32_t hw_h = hw->getHeight(); - // the GLConsumer will provide the BufferQueue - sp consumer = new GLConsumer(tname, true, GL_TEXTURE_2D); - consumer->getBufferQueue()->setDefaultBufferFormat(HAL_PIXEL_FORMAT_RGBA_8888); - - // call the new screenshot taking code, passing a BufferQueue to it - status_t result = captureScreenImplLocked(hw, - consumer->getBufferQueue(), reqWidth, reqHeight, minLayerZ, maxLayerZ); - - if (result == NO_ERROR) { - result = consumer->updateTexImage(); - if (result == NO_ERROR) { - // create a FBO - GLuint name; - glGenFramebuffersOES(1, &name); - glBindFramebufferOES(GL_FRAMEBUFFER_OES, name); - glFramebufferTexture2DOES(GL_FRAMEBUFFER_OES, - GL_COLOR_ATTACHMENT0_OES, GL_TEXTURE_2D, tname, 0); - - reqWidth = consumer->getCurrentBuffer()->getWidth(); - reqHeight = consumer->getCurrentBuffer()->getHeight(); - - { - // in this block we render the screenshot into the - // CpuConsumer using glReadPixels from our GLConsumer, - // Some older drivers don't support the GL->CPU path so - // have to wrap it with a CPU->CPU path, which is what - // glReadPixels essentially is - - sp sur = new Surface(producer); - ANativeWindow* window = sur.get(); - ANativeWindowBuffer* buffer; - void* vaddr; - - if (native_window_api_connect(window, - NATIVE_WINDOW_API_CPU) == NO_ERROR) { - int err = 0; - err = native_window_set_buffers_dimensions(window, - reqWidth, reqHeight); - err |= native_window_set_buffers_format(window, - HAL_PIXEL_FORMAT_RGBA_8888); - err |= native_window_set_usage(window, - GRALLOC_USAGE_SW_READ_OFTEN | - GRALLOC_USAGE_SW_WRITE_OFTEN); - - if (err == NO_ERROR) { - if (native_window_dequeue_buffer_and_wait(window, - &buffer) == NO_ERROR) { - sp buf = - static_cast(buffer); - if (buf->lock(GRALLOC_USAGE_SW_WRITE_OFTEN, - &vaddr) == NO_ERROR) { - if (buffer->stride != int(reqWidth)) { - // we're unlucky here, glReadPixels is - // not able to deal with a stride not - // equal to the width. - uint32_t* tmp = new uint32_t[reqWidth*reqHeight]; - if (tmp != NULL) { - glReadPixels(0, 0, reqWidth, reqHeight, - GL_RGBA, GL_UNSIGNED_BYTE, tmp); - for (size_t y=0 ; ystride, - tmp + y*reqWidth, reqWidth*4); - } - delete [] tmp; - } - } else { - glReadPixels(0, 0, reqWidth, reqHeight, - GL_RGBA, GL_UNSIGNED_BYTE, vaddr); - } - buf->unlock(); - } - window->queueBuffer(window, buffer, -1); - } - } - native_window_api_disconnect(window, NATIVE_WINDOW_API_CPU); - } - } - - // back to main framebuffer - glBindFramebufferOES(GL_FRAMEBUFFER_OES, 0); - glDeleteFramebuffersOES(1, &name); - } + // if we have secure windows on this display, never allow the screen capture + if (hw->getSecureLayerVisible()) { + ALOGW("FB is protected: PERMISSION_DENIED"); + return PERMISSION_DENIED; } - glDeleteTextures(1, &tname); + if ((reqWidth > hw_w) || (reqHeight > hw_h)) { + ALOGE("size mismatch (%d, %d) > (%d, %d)", + reqWidth, reqHeight, hw_w, hw_h); + return BAD_VALUE; + } - DisplayDevice::makeCurrent(mEGLDisplay, - getDefaultDisplayDevice(), mEGLContext); + reqWidth = (!reqWidth) ? hw_w : reqWidth; + reqHeight = (!reqHeight) ? hw_h : reqHeight; + + GLuint tname; + glGenRenderbuffersOES(1, &tname); + glBindRenderbufferOES(GL_RENDERBUFFER_OES, tname); + glRenderbufferStorageOES(GL_RENDERBUFFER_OES, GL_RGBA8_OES, reqWidth, reqHeight); + + // create a FBO + GLuint name; + glGenFramebuffersOES(1, &name); + glBindFramebufferOES(GL_FRAMEBUFFER_OES, name); + glFramebufferRenderbufferOES(GL_FRAMEBUFFER_OES, + GL_COLOR_ATTACHMENT0_OES, GL_RENDERBUFFER_OES, tname); + + GLenum status = glCheckFramebufferStatusOES(GL_FRAMEBUFFER_OES); + + status_t result = NO_ERROR; + if (status == GL_FRAMEBUFFER_COMPLETE_OES) { + + renderScreenImplLocked(hw, reqWidth, reqHeight, minLayerZ, maxLayerZ, true); + + // Below we render the screenshot into the + // CpuConsumer using glReadPixels from our FBO. + // Some older drivers don't support the GL->CPU path so we + // have to wrap it with a CPU->CPU path, which is what + // glReadPixels essentially is. + + sp sur = new Surface(producer); + ANativeWindow* window = sur.get(); + + if (native_window_api_connect(window, NATIVE_WINDOW_API_CPU) == NO_ERROR) { + int err = 0; + err = native_window_set_buffers_dimensions(window, reqWidth, reqHeight); + err |= native_window_set_buffers_format(window, HAL_PIXEL_FORMAT_RGBA_8888); + err |= native_window_set_usage(window, + GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_SW_WRITE_OFTEN); + + if (err == NO_ERROR) { + ANativeWindowBuffer* buffer; + if (native_window_dequeue_buffer_and_wait(window, &buffer) == NO_ERROR) { + sp buf = static_cast(buffer); + void* vaddr; + if (buf->lock(GRALLOC_USAGE_SW_WRITE_OFTEN, &vaddr) == NO_ERROR) { + glReadPixels(0, 0, buffer->stride, reqHeight, + GL_RGBA, GL_UNSIGNED_BYTE, vaddr); + buf->unlock(); + } + window->queueBuffer(window, buffer, -1); + } + } + native_window_api_disconnect(window, NATIVE_WINDOW_API_CPU); + } + + } else { + ALOGE("got GL_FRAMEBUFFER_COMPLETE_OES while taking screenshot"); + result = INVALID_OPERATION; + } + + // back to main framebuffer + glBindFramebufferOES(GL_FRAMEBUFFER_OES, 0); + glDeleteRenderbuffersOES(1, &tname); + glDeleteFramebuffersOES(1, &name); + + DisplayDevice::setViewportAndProjection(hw); return result; } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 57ee8b92e..739099c75 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -288,6 +288,12 @@ private: void startBootAnim(); + void renderScreenImplLocked( + const sp& hw, + uint32_t reqWidth, uint32_t reqHeight, + uint32_t minLayerZ, uint32_t maxLayerZ, + bool yswap); + status_t captureScreenImplLocked( const sp& hw, const sp& producer,