From dd671599bed9d3ca28e2c744e8c224e1e15bc914 Mon Sep 17 00:00:00 2001 From: Chet Haase Date: Fri, 19 Apr 2013 14:54:34 -0700 Subject: [PATCH] Fix quickReject logic to account for setClipChildren() setting The rendering code optimizes by rejecting drawing operations that lie outside of the bounds of their views. This works in most situations, but breaks down when containers have called setClipChildren(false), because we reject drawing that is outside of that container, but which should be drawn anyway. Fix is to pass in the value of that flag to the DisplayList drawing routines which take that flag into account when deciding whether to quickReject any particular operation. Issue #8659277 animation clipping Change-Id: Ief568e4db01b533a97b3c5ea5ad777c03c0eea71 --- core/java/android/view/DisplayList.java | 4 +- core/java/android/view/GLES20DisplayList.java | 6 +-- core/java/android/view/View.java | 4 +- core/java/android/view/ViewGroup.java | 2 +- core/java/android/widget/Editor.java | 2 +- core/jni/android_view_GLES20DisplayList.cpp | 8 ++-- libs/hwui/DisplayList.cpp | 33 ++++++++------- libs/hwui/DisplayList.h | 6 +-- libs/hwui/DisplayListOp.h | 42 ++++++++++++------- 9 files changed, 59 insertions(+), 48 deletions(-) diff --git a/core/java/android/view/DisplayList.java b/core/java/android/view/DisplayList.java index 3bad98ef67701..2d24c1ea42fa6 100644 --- a/core/java/android/view/DisplayList.java +++ b/core/java/android/view/DisplayList.java @@ -285,9 +285,9 @@ public abstract class DisplayList { * Set whether the display list should clip itself to its bounds. This property is controlled by * the view's parent. * - * @param clipChildren true if the display list should clip to its bounds + * @param clipToBounds true if the display list should clip to its bounds */ - public abstract void setClipChildren(boolean clipChildren); + public abstract void setClipToBounds(boolean clipToBounds); /** * Set the static matrix on the display list. The specified matrix is combined with other diff --git a/core/java/android/view/GLES20DisplayList.java b/core/java/android/view/GLES20DisplayList.java index 9c5cdb2bd2c40..3272504fbfa68 100644 --- a/core/java/android/view/GLES20DisplayList.java +++ b/core/java/android/view/GLES20DisplayList.java @@ -137,9 +137,9 @@ class GLES20DisplayList extends DisplayList { } @Override - public void setClipChildren(boolean clipChildren) { + public void setClipToBounds(boolean clipToBounds) { if (hasNativeDisplayList()) { - nSetClipChildren(mFinalizer.mNativeDisplayList, clipChildren); + nSetClipToBounds(mFinalizer.mNativeDisplayList, clipToBounds); } } @@ -450,7 +450,7 @@ class GLES20DisplayList extends DisplayList { private static native void nSetPivotY(int displayList, float pivotY); private static native void nSetPivotX(int displayList, float pivotX); private static native void nSetCaching(int displayList, boolean caching); - private static native void nSetClipChildren(int displayList, boolean clipChildren); + private static native void nSetClipToBounds(int displayList, boolean clipToBounds); private static native void nSetAlpha(int displayList, float alpha); private static native void nSetHasOverlappingRendering(int displayList, boolean hasOverlappingRendering); diff --git a/core/java/android/view/View.java b/core/java/android/view/View.java index be26d20d53501..c5054d63a2b9b 100644 --- a/core/java/android/view/View.java +++ b/core/java/android/view/View.java @@ -13489,8 +13489,8 @@ public class View implements Drawable.Callback, KeyEvent.Callback, displayList.setLeftTopRightBottom(mLeft, mTop, mRight, mBottom); displayList.setHasOverlappingRendering(hasOverlappingRendering()); if (mParent instanceof ViewGroup) { - displayList.setClipChildren( - (((ViewGroup)mParent).mGroupFlags & ViewGroup.FLAG_CLIP_CHILDREN) != 0); + displayList.setClipToBounds( + (((ViewGroup) mParent).mGroupFlags & ViewGroup.FLAG_CLIP_CHILDREN) != 0); } float alpha = 1; if (mParent instanceof ViewGroup && (((ViewGroup) mParent).mGroupFlags & diff --git a/core/java/android/view/ViewGroup.java b/core/java/android/view/ViewGroup.java index bf502dda9f2fe..39bff689fa72f 100644 --- a/core/java/android/view/ViewGroup.java +++ b/core/java/android/view/ViewGroup.java @@ -3109,7 +3109,7 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager for (int i = 0; i < mChildrenCount; ++i) { View child = getChildAt(i); if (child.mDisplayList != null) { - child.mDisplayList.setClipChildren(clipChildren); + child.mDisplayList.setClipToBounds(clipChildren); } } } diff --git a/core/java/android/widget/Editor.java b/core/java/android/widget/Editor.java index 0aeef63020c73..f57f333817a88 100644 --- a/core/java/android/widget/Editor.java +++ b/core/java/android/widget/Editor.java @@ -1351,7 +1351,7 @@ public class Editor { } finally { blockDisplayList.end(); // Same as drawDisplayList below, handled by our TextView's parent - blockDisplayList.setClipChildren(false); + blockDisplayList.setClipToBounds(false); } } diff --git a/core/jni/android_view_GLES20DisplayList.cpp b/core/jni/android_view_GLES20DisplayList.cpp index f7a5302ea2b0a..4ce2e24730c83 100644 --- a/core/jni/android_view_GLES20DisplayList.cpp +++ b/core/jni/android_view_GLES20DisplayList.cpp @@ -84,9 +84,9 @@ static void android_view_GLES20DisplayList_setAnimationMatrix(JNIEnv* env, displayList->setAnimationMatrix(matrix); } -static void android_view_GLES20DisplayList_setClipChildren(JNIEnv* env, - jobject clazz, DisplayList* displayList, jboolean clipChildren) { - displayList->setClipChildren(clipChildren); +static void android_view_GLES20DisplayList_setClipToBounds(JNIEnv* env, + jobject clazz, DisplayList* displayList, jboolean clipToBounds) { + displayList->setClipToBounds(clipToBounds); } static void android_view_GLES20DisplayList_setAlpha(JNIEnv* env, @@ -308,7 +308,7 @@ static JNINativeMethod gMethods[] = { { "nSetCaching", "(IZ)V", (void*) android_view_GLES20DisplayList_setCaching }, { "nSetStaticMatrix", "(II)V", (void*) android_view_GLES20DisplayList_setStaticMatrix }, { "nSetAnimationMatrix", "(II)V", (void*) android_view_GLES20DisplayList_setAnimationMatrix }, - { "nSetClipChildren", "(IZ)V", (void*) android_view_GLES20DisplayList_setClipChildren }, + { "nSetClipToBounds", "(IZ)V", (void*) android_view_GLES20DisplayList_setClipToBounds }, { "nSetAlpha", "(IF)V", (void*) android_view_GLES20DisplayList_setAlpha }, { "nSetHasOverlappingRendering", "(IZ)V", (void*) android_view_GLES20DisplayList_setHasOverlappingRendering }, diff --git a/libs/hwui/DisplayList.cpp b/libs/hwui/DisplayList.cpp index 26abec27bc9e3..b7d3d6fad1d56 100644 --- a/libs/hwui/DisplayList.cpp +++ b/libs/hwui/DisplayList.cpp @@ -231,7 +231,7 @@ void DisplayList::init() { mTop = 0; mRight = 0; mBottom = 0; - mClipChildren = true; + mClipToBounds = true; mAlpha = 1; mHasOverlappingRendering = true; mTranslationX = 0; @@ -358,7 +358,7 @@ void DisplayList::outputViewProperties(const int level) { ALOGD("%*sScaleAlpha %.2f", level * 2, "", mAlpha); } else { int flags = SkCanvas::kHasAlphaLayer_SaveFlag; - if (mClipChildren) { + if (mClipToBounds) { flags |= SkCanvas::kClipToLayer_SaveFlag; } ALOGD("%*sSaveLayerAlpha %.2f, %.2f, %.2f, %.2f, %d, 0x%x", level * 2, "", @@ -366,7 +366,7 @@ void DisplayList::outputViewProperties(const int level) { (int)(mAlpha * 255), flags); } } - if (mClipChildren && !mCaching) { + if (mClipToBounds && !mCaching) { ALOGD("%*sClipRect %.2f, %.2f, %.2f, %.2f", level * 2, "", 0.0f, 0.0f, (float) mRight - mLeft, (float) mBottom - mTop); } @@ -411,16 +411,17 @@ void DisplayList::setViewProperties(OpenGLRenderer& renderer, T& handler, // have to pass it into this call. In fact, this information might be in the // location/size info that we store with the new native transform data. int saveFlags = SkCanvas::kHasAlphaLayer_SaveFlag; - if (mClipChildren) { + if (mClipToBounds) { saveFlags |= SkCanvas::kClipToLayer_SaveFlag; } handler(mSaveLayerOp->reinit(0, 0, mRight - mLeft, mBottom - mTop, - mAlpha * 255, SkXfermode::kSrcOver_Mode, saveFlags), PROPERTY_SAVECOUNT); + mAlpha * 255, SkXfermode::kSrcOver_Mode, saveFlags), PROPERTY_SAVECOUNT, + mClipToBounds); } } - if (mClipChildren && !mCaching) { + if (mClipToBounds && !mCaching) { handler(mClipRectOp->reinit(0, 0, mRight - mLeft, mBottom - mTop, SkRegion::kIntersect_Op), - PROPERTY_SAVECOUNT); + PROPERTY_SAVECOUNT, mClipToBounds); } } @@ -428,8 +429,8 @@ class DeferOperationHandler { public: DeferOperationHandler(DeferStateStruct& deferStruct, int level) : mDeferStruct(deferStruct), mLevel(level) {} - inline void operator()(DisplayListOp* operation, int saveCount) { - operation->defer(mDeferStruct, saveCount, mLevel); + inline void operator()(DisplayListOp* operation, int saveCount, bool clipToBounds) { + operation->defer(mDeferStruct, saveCount, mLevel, clipToBounds); } private: DeferStateStruct& mDeferStruct; @@ -445,11 +446,11 @@ class ReplayOperationHandler { public: ReplayOperationHandler(ReplayStateStruct& replayStruct, int level) : mReplayStruct(replayStruct), mLevel(level) {} - inline void operator()(DisplayListOp* operation, int saveCount) { + inline void operator()(DisplayListOp* operation, int saveCount, bool clipToBounds) { #if DEBUG_DISPLAY_LIST_OPS_AS_EVENTS mReplayStruct.mRenderer.eventMark(operation->name()); #endif - operation->replay(mReplayStruct, saveCount, mLevel); + operation->replay(mReplayStruct, saveCount, mLevel, clipToBounds); } private: ReplayStateStruct& mReplayStruct; @@ -492,16 +493,16 @@ void DisplayList::iterate(OpenGLRenderer& renderer, T& handler, const int level) int restoreTo = renderer.getSaveCount(); handler(mSaveOp->reinit(SkCanvas::kMatrix_SaveFlag | SkCanvas::kClip_SaveFlag), - PROPERTY_SAVECOUNT); + PROPERTY_SAVECOUNT, mClipToBounds); DISPLAY_LIST_LOGD("%*sSave %d %d", (level + 1) * 2, "", SkCanvas::kMatrix_SaveFlag | SkCanvas::kClip_SaveFlag, restoreTo); setViewProperties(renderer, handler, level + 1); - if (renderer.quickRejectNoScissor(0, 0, mWidth, mHeight)) { + if (mClipToBounds && renderer.quickRejectNoScissor(0, 0, mWidth, mHeight)) { DISPLAY_LIST_LOGD("%*sRestoreToCount %d", level * 2, "", restoreTo); - handler(mRestoreToCountOp->reinit(restoreTo), PROPERTY_SAVECOUNT); + handler(mRestoreToCountOp->reinit(restoreTo), PROPERTY_SAVECOUNT, mClipToBounds); return; } @@ -510,12 +511,12 @@ void DisplayList::iterate(OpenGLRenderer& renderer, T& handler, const int level) for (unsigned int i = 0; i < mDisplayListData->displayListOps.size(); i++) { DisplayListOp *op = mDisplayListData->displayListOps[i]; - handler(op, saveCount); + handler(op, saveCount, mClipToBounds); logBuffer.writeCommand(level, op->name()); } DISPLAY_LIST_LOGD("%*sRestoreToCount %d", (level + 1) * 2, "", restoreTo); - handler(mRestoreToCountOp->reinit(restoreTo), PROPERTY_SAVECOUNT); + handler(mRestoreToCountOp->reinit(restoreTo), PROPERTY_SAVECOUNT, mClipToBounds); renderer.restoreToCount(restoreTo); renderer.setOverrideLayerAlpha(1.0f); } diff --git a/libs/hwui/DisplayList.h b/libs/hwui/DisplayList.h index 84f20abe02494..5f843291f8479 100644 --- a/libs/hwui/DisplayList.h +++ b/libs/hwui/DisplayList.h @@ -137,8 +137,8 @@ public: return mName.string(); } - void setClipChildren(bool clipChildren) { - mClipChildren = clipChildren; + void setClipToBounds(bool clipToBounds) { + mClipToBounds = clipToBounds; } void setStaticMatrix(SkMatrix* matrix) { @@ -498,7 +498,7 @@ private: String8 mName; // View properties - bool mClipChildren; + bool mClipToBounds; float mAlpha; bool mHasOverlappingRendering; float mTranslationX, mTranslationY; diff --git a/libs/hwui/DisplayListOp.h b/libs/hwui/DisplayListOp.h index c277c247dc326..a0290e37012fe 100644 --- a/libs/hwui/DisplayListOp.h +++ b/libs/hwui/DisplayListOp.h @@ -78,9 +78,11 @@ public: kOpLogFlag_JSON = 0x2 // TODO: add? }; - virtual void defer(DeferStateStruct& deferStruct, int saveCount, int level) = 0; + virtual void defer(DeferStateStruct& deferStruct, int saveCount, int level, + bool useQuickReject) = 0; - virtual void replay(ReplayStateStruct& replayStruct, int saveCount, int level) = 0; + virtual void replay(ReplayStateStruct& replayStruct, int saveCount, int level, + bool useQuickReject) = 0; virtual void output(int level, uint32_t logFlags = 0) = 0; @@ -104,7 +106,8 @@ public: virtual ~StateOp() {} - virtual void defer(DeferStateStruct& deferStruct, int saveCount, int level) { + virtual void defer(DeferStateStruct& deferStruct, int saveCount, int level, + bool useQuickReject) { // default behavior only affects immediate, deferrable state, issue directly to renderer applyState(deferStruct.mRenderer, saveCount); } @@ -113,7 +116,8 @@ public: * State operations are applied directly to the renderer, but can cause the deferred drawing op * list to flush */ - virtual void replay(ReplayStateStruct& replayStruct, int saveCount, int level) { + virtual void replay(ReplayStateStruct& replayStruct, int saveCount, int level, + bool useQuickReject) { applyState(replayStruct.mRenderer, saveCount); } @@ -126,9 +130,9 @@ public: DrawOp(SkPaint* paint) : mPaint(paint), mQuickRejected(false) {} - virtual void defer(DeferStateStruct& deferStruct, int saveCount, int level) { - if (mQuickRejected && - CC_LIKELY(deferStruct.mReplayFlags & DisplayList::kReplayFlag_ClipChildren)) { + virtual void defer(DeferStateStruct& deferStruct, int saveCount, int level, + bool useQuickReject) { + if (mQuickRejected && CC_LIKELY(useQuickReject)) { return; } @@ -140,9 +144,9 @@ public: deferStruct.mDeferredList.addDrawOp(deferStruct.mRenderer, this); } - virtual void replay(ReplayStateStruct& replayStruct, int saveCount, int level) { - if (mQuickRejected && - CC_LIKELY(replayStruct.mReplayFlags & DisplayList::kReplayFlag_ClipChildren)) { + virtual void replay(ReplayStateStruct& replayStruct, int saveCount, int level, + bool useQuickReject) { + if (mQuickRejected && CC_LIKELY(useQuickReject)) { return; } @@ -261,7 +265,8 @@ public: SaveOp(int flags) : mFlags(flags) {} - virtual void defer(DeferStateStruct& deferStruct, int saveCount, int level) { + virtual void defer(DeferStateStruct& deferStruct, int saveCount, int level, + bool useQuickReject) { int newSaveCount = deferStruct.mRenderer.save(mFlags); deferStruct.mDeferredList.addSave(deferStruct.mRenderer, this, newSaveCount); } @@ -293,7 +298,8 @@ public: RestoreToCountOp(int count) : mCount(count) {} - virtual void defer(DeferStateStruct& deferStruct, int saveCount, int level) { + virtual void defer(DeferStateStruct& deferStruct, int saveCount, int level, + bool useQuickReject) { deferStruct.mDeferredList.addRestoreToCount(deferStruct.mRenderer, this, saveCount + mCount); deferStruct.mRenderer.restoreToCount(saveCount + mCount); @@ -326,7 +332,8 @@ public: int alpha, SkXfermode::Mode mode, int flags) : mArea(left, top, right, bottom), mAlpha(alpha), mMode(mode), mFlags(flags) {} - virtual void defer(DeferStateStruct& deferStruct, int saveCount, int level) { + virtual void defer(DeferStateStruct& deferStruct, int saveCount, int level, + bool useQuickReject) { // NOTE: don't bother with actual saveLayer, instead issuing it at flush time int newSaveCount = deferStruct.mRenderer.getSaveCount(); deferStruct.mDeferredList.addSaveLayer(deferStruct.mRenderer, this, newSaveCount); @@ -490,7 +497,8 @@ class ClipOp : public StateOp { public: ClipOp(SkRegion::Op op) : mOp(op) {} - virtual void defer(DeferStateStruct& deferStruct, int saveCount, int level) { + virtual void defer(DeferStateStruct& deferStruct, int saveCount, int level, + bool useQuickReject) { // NOTE: must defer op BEFORE applying state, since it may read clip deferStruct.mDeferredList.addClip(deferStruct.mRenderer, this); @@ -1371,12 +1379,14 @@ public: : DrawBoundedOp(0, 0, displayList->getWidth(), displayList->getHeight(), 0), mDisplayList(displayList), mFlags(flags) {} - virtual void defer(DeferStateStruct& deferStruct, int saveCount, int level) { + virtual void defer(DeferStateStruct& deferStruct, int saveCount, int level, + bool useQuickReject) { if (mDisplayList && mDisplayList->isRenderable()) { mDisplayList->defer(deferStruct, level + 1); } } - virtual void replay(ReplayStateStruct& replayStruct, int saveCount, int level) { + virtual void replay(ReplayStateStruct& replayStruct, int saveCount, int level, + bool useQuickReject) { if (mDisplayList && mDisplayList->isRenderable()) { mDisplayList->replay(replayStruct, level + 1); }