From 9ef9d51ec9ded43d03d045f623edfa9ddd6d8620 Mon Sep 17 00:00:00 2001 From: Jamie Gennis Date: Mon, 25 Feb 2013 13:37:54 -0800 Subject: [PATCH 1/5] 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/5] 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/5] 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/5] 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/5] 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