From 30bc34f191ca8a009af313fc751e5b4bff6e39a1 Mon Sep 17 00:00:00 2001 From: Jeff Brown Date: Tue, 25 Jan 2011 12:56:56 -0800 Subject: [PATCH] Fix surface frame size reporting. The SurfaceHolder provided by the wallpaper service was not reporting the correct size in getSurfaceFrame(). This broke an optimization in the ImageWallpaper. The old code happened to work because calling lockCanvas on the SurfaceHolder with a null dirty rectangle happened to have the side-effect of updating the SurfaceHolder's surface frame size field because it passed mSurfaceFrame as the dirty rect, causing mSurfaceFrame to be set to the size of the region to be drawn. However, relying on this side-effect is wrong. Among other things, the dirty region could actually be smaller than the surface frame. This patch fixes WallpaperService, SurfaceView and ViewRoot to ensure that the surface frame size is always set explicitly and is not modified by calls to lockCanvas. Change-Id: I10948f5ec269409ceaf0f7d32b3f6731e9499ebc --- .../service/wallpaper/WallpaperService.java | 3 +- core/java/android/view/SurfaceView.java | 12 +++- core/java/android/view/ViewRoot.java | 1 + .../service/wallpaper/ImageWallpaper.java | 58 ++++++++++++------- .../internal/view/BaseSurfaceHolder.java | 19 +++++- .../internal/policy/impl/PhoneWindow.java | 1 - 6 files changed, 66 insertions(+), 28 deletions(-) diff --git a/core/java/android/service/wallpaper/WallpaperService.java b/core/java/android/service/wallpaper/WallpaperService.java index 52b064348e55f..44887ede3c96b 100644 --- a/core/java/android/service/wallpaper/WallpaperService.java +++ b/core/java/android/service/wallpaper/WallpaperService.java @@ -546,7 +546,8 @@ public abstract class WallpaperService extends Service { sizeChanged = true; mCurHeight = h; } - + + mSurfaceHolder.setSurfaceFrameSize(w, h); mSurfaceHolder.mSurfaceLock.unlock(); if (!mSurfaceHolder.mSurface.isValid()) { diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java index 6451d47a189c0..8a95664adb388 100644 --- a/core/java/android/view/SurfaceView.java +++ b/core/java/android/view/SurfaceView.java @@ -158,6 +158,7 @@ public class SurfaceView extends View { int mHeight = -1; int mFormat = -1; final Rect mSurfaceFrame = new Rect(); + Rect mTmpDirty; int mLastSurfaceWidth = -1, mLastSurfaceHeight = -1; boolean mUpdateWindowNeeded; boolean mReportDrawNeeded; @@ -739,9 +740,16 @@ public class SurfaceView extends View { Canvas c = null; if (!mDrawingStopped && mWindow != null) { - Rect frame = dirty != null ? dirty : mSurfaceFrame; + if (dirty == null) { + if (mTmpDirty == null) { + mTmpDirty = new Rect(); + } + mTmpDirty.set(mSurfaceFrame); + dirty = mTmpDirty; + } + try { - c = mSurface.lockCanvas(frame); + c = mSurface.lockCanvas(dirty); } catch (Exception e) { Log.e(LOG_TAG, "Exception locking surface", e); } diff --git a/core/java/android/view/ViewRoot.java b/core/java/android/view/ViewRoot.java index 19d78111c1bf8..44aca7d9da3bc 100644 --- a/core/java/android/view/ViewRoot.java +++ b/core/java/android/view/ViewRoot.java @@ -1082,6 +1082,7 @@ public final class ViewRoot extends Handler implements ViewParent, //mSurfaceHolder.mSurface.copyFrom(mSurface); mSurfaceHolder.mSurface = mSurface; } + mSurfaceHolder.setSurfaceFrameSize(mWidth, mHeight); mSurfaceHolder.mSurfaceLock.unlock(); if (mSurface.isValid()) { if (!hadSurface) { diff --git a/core/java/com/android/internal/service/wallpaper/ImageWallpaper.java b/core/java/com/android/internal/service/wallpaper/ImageWallpaper.java index 78688ee203a82..595c6343b344b 100644 --- a/core/java/com/android/internal/service/wallpaper/ImageWallpaper.java +++ b/core/java/com/android/internal/service/wallpaper/ImageWallpaper.java @@ -88,9 +88,13 @@ public class ImageWallpaper extends WallpaperService { class WallpaperObserver extends BroadcastReceiver { public void onReceive(Context context, Intent intent) { + if (DEBUG) { + Log.d(TAG, "onReceive"); + } + synchronized (mLock) { updateWallpaperLocked(); - drawFrameLocked(true, false); + drawFrameLocked(); } // Assume we are the only one using the wallpaper in this @@ -101,6 +105,10 @@ public class ImageWallpaper extends WallpaperService { @Override public void onCreate(SurfaceHolder surfaceHolder) { + if (DEBUG) { + Log.d(TAG, "onCreate"); + } + super.onCreate(surfaceHolder); IntentFilter filter = new IntentFilter(Intent.ACTION_WALLPAPER_CHANGED); mReceiver = new WallpaperObserver(); @@ -120,9 +128,18 @@ public class ImageWallpaper extends WallpaperService { @Override public void onVisibilityChanged(boolean visible) { + if (DEBUG) { + Log.d(TAG, "onVisibilityChanged: visible=" + visible); + } + synchronized (mLock) { - mVisible = visible; - drawFrameLocked(false, false); + if (mVisible != visible) { + if (DEBUG) { + Log.d(TAG, "Visibility changed to visible=" + visible); + } + mVisible = visible; + drawFrameLocked(); + } } } @@ -135,6 +152,12 @@ public class ImageWallpaper extends WallpaperService { public void onOffsetsChanged(float xOffset, float yOffset, float xOffsetStep, float yOffsetStep, int xPixels, int yPixels) { + if (DEBUG) { + Log.d(TAG, "onOffsetsChanged: xOffset=" + xOffset + ", yOffset=" + yOffset + + ", xOffsetStep=" + xOffsetStep + ", yOffsetStep=" + yOffsetStep + + ", xPixels=" + xPixels + ", yPixels=" + yPixels); + } + synchronized (mLock) { if (mXOffset != xOffset || mYOffset != yOffset) { if (DEBUG) { @@ -142,36 +165,27 @@ public class ImageWallpaper extends WallpaperService { } mXOffset = xOffset; mYOffset = yOffset; - drawFrameLocked(false, true); - } else { - drawFrameLocked(false, false); + mOffsetsChanged = true; } + drawFrameLocked(); } } @Override public void onSurfaceChanged(SurfaceHolder holder, int format, int width, int height) { + if (DEBUG) { + Log.d(TAG, "onSurfaceChanged: width=" + width + ", height=" + height); + } + super.onSurfaceChanged(holder, format, width, height); - + synchronized (mLock) { - drawFrameLocked(true, false); + mRedrawNeeded = true; + drawFrameLocked(); } } - @Override - public void onSurfaceCreated(SurfaceHolder holder) { - super.onSurfaceCreated(holder); - } - - @Override - public void onSurfaceDestroyed(SurfaceHolder holder) { - super.onSurfaceDestroyed(holder); - } - - void drawFrameLocked(boolean redrawNeeded, boolean offsetsChanged) { - mRedrawNeeded |= redrawNeeded; - mOffsetsChanged |= offsetsChanged; - + void drawFrameLocked() { if (!mVisible) { if (DEBUG) { Log.d(TAG, "Suppressed drawFrame since wallpaper is not visible."); diff --git a/core/java/com/android/internal/view/BaseSurfaceHolder.java b/core/java/com/android/internal/view/BaseSurfaceHolder.java index 1e97cd687f40f..f9f94be9b6d9c 100644 --- a/core/java/com/android/internal/view/BaseSurfaceHolder.java +++ b/core/java/com/android/internal/view/BaseSurfaceHolder.java @@ -49,6 +49,7 @@ public abstract class BaseSurfaceHolder implements SurfaceHolder { int mType = -1; final Rect mSurfaceFrame = new Rect(); + Rect mTmpDirty; public abstract void onUpdateSurface(); public abstract void onRelayoutContainer(); @@ -171,9 +172,16 @@ public abstract class BaseSurfaceHolder implements SurfaceHolder { Canvas c = null; if (onAllowLockCanvas()) { - Rect frame = dirty != null ? dirty : mSurfaceFrame; + if (dirty == null) { + if (mTmpDirty == null) { + mTmpDirty = new Rect(); + } + mTmpDirty.set(mSurfaceFrame); + dirty = mTmpDirty; + } + try { - c = mSurface.lockCanvas(frame); + c = mSurface.lockCanvas(dirty); } catch (Exception e) { Log.e(TAG, "Exception locking surface", e); } @@ -215,4 +223,11 @@ public abstract class BaseSurfaceHolder implements SurfaceHolder { public Rect getSurfaceFrame() { return mSurfaceFrame; } + + public void setSurfaceFrameSize(int width, int height) { + mSurfaceFrame.top = 0; + mSurfaceFrame.left = 0; + mSurfaceFrame.right = width; + mSurfaceFrame.bottom = height; + } }; diff --git a/policy/src/com/android/internal/policy/impl/PhoneWindow.java b/policy/src/com/android/internal/policy/impl/PhoneWindow.java index 6a1d19900fcc6..88f30eda57e73 100644 --- a/policy/src/com/android/internal/policy/impl/PhoneWindow.java +++ b/policy/src/com/android/internal/policy/impl/PhoneWindow.java @@ -116,7 +116,6 @@ public class PhoneWindow extends Window implements MenuBuilder.Callback { private ViewGroup mContentParent; SurfaceHolder.Callback2 mTakeSurfaceCallback; - BaseSurfaceHolder mSurfaceHolder; InputQueue.Callback mTakeInputQueueCallback;