Fix rendering artifact in edge fades.
Bug #4092053
The problem always existed but was made visible by partial invalidation.
When saving a layer, the renderer would try to postpone glClear()
operations until the next drawing command. This however does not work
since the clip might have changed. The fix is rather simple and
simply gets rid of this "optimization" (that turned out to be
usless anyway given how View issues saveLayer() calls.)
This change also fixes an issue with gradients (color stops where
not properly computed when using a null stops array) and optimizes
display lists rendering (quickly rejects larger portions of the
tree to avoid executing unnecessary code.)
Change-Id: I0f5b5f6e1220d41a09cc2fa84c212b0b4afd9c46
diff --git a/core/java/android/view/GLES20Canvas.java b/core/java/android/view/GLES20Canvas.java
index fa5479b..14f2e9d 100644
--- a/core/java/android/view/GLES20Canvas.java
+++ b/core/java/android/view/GLES20Canvas.java
@@ -245,12 +245,13 @@
private static native void nDestroyDisplayList(int displayList);
@Override
- public boolean drawDisplayList(DisplayList displayList, Rect dirty) {
+ public boolean drawDisplayList(DisplayList displayList, int width, int height, Rect dirty) {
return nDrawDisplayList(mRenderer,
- ((GLES20DisplayList) displayList).mNativeDisplayList, dirty);
+ ((GLES20DisplayList) displayList).mNativeDisplayList, width, height, dirty);
}
- private static native boolean nDrawDisplayList(int renderer, int displayList, Rect dirty);
+ private static native boolean nDrawDisplayList(int renderer, int displayList,
+ int width, int height, Rect dirty);
///////////////////////////////////////////////////////////////////////////
// Hardware layer
diff --git a/core/java/android/view/HardwareCanvas.java b/core/java/android/view/HardwareCanvas.java
index cb1003a..caa7b74 100644
--- a/core/java/android/view/HardwareCanvas.java
+++ b/core/java/android/view/HardwareCanvas.java
@@ -53,13 +53,15 @@
* Draws the specified display list onto this canvas.
*
* @param displayList The display list to replay.
+ * @param width The width of the display list.
+ * @param height The height of the display list.
* @param dirty The dirty region to redraw in the next pass, matters only
* if this method returns true, can be null.
*
* @return True if the content of the display list requires another
* drawing pass (invalidate()), false otherwise
*/
- abstract boolean drawDisplayList(DisplayList displayList, Rect dirty);
+ abstract boolean drawDisplayList(DisplayList displayList, int width, int height, Rect dirty);
/**
* Draws the specified layer onto this canvas.
diff --git a/core/java/android/view/HardwareRenderer.java b/core/java/android/view/HardwareRenderer.java
index 0cf7ae6..8584bf2 100644
--- a/core/java/android/view/HardwareRenderer.java
+++ b/core/java/android/view/HardwareRenderer.java
@@ -608,7 +608,8 @@
DisplayList displayList = view.getDisplayList();
if (displayList != null) {
- if (canvas.drawDisplayList(displayList, mRedrawClip)) {
+ if (canvas.drawDisplayList(displayList, view.getWidth(),
+ view.getHeight(), mRedrawClip)) {
if (mRedrawClip.isEmpty() || view.getParent() == null) {
view.invalidate();
} else {
diff --git a/core/java/android/view/View.java b/core/java/android/view/View.java
index 32c9e27..5a96efd 100644
--- a/core/java/android/view/View.java
+++ b/core/java/android/view/View.java
@@ -59,7 +59,6 @@
import android.util.SparseArray;
import android.util.TypedValue;
import android.view.ContextMenu.ContextMenuInfo;
-import android.view.View.MeasureSpec;
import android.view.accessibility.AccessibilityEvent;
import android.view.accessibility.AccessibilityEventSource;
import android.view.accessibility.AccessibilityManager;
@@ -4651,6 +4650,7 @@
* @return True if the event was handled by the view, false otherwise.
*/
public boolean dispatchGenericMotionEvent(MotionEvent event) {
+ //noinspection SimplifiableIfStatement
if (mOnGenericMotionListener != null && (mViewFlags & ENABLED_MASK) == ENABLED
&& mOnGenericMotionListener.onGenericMotion(this, event)) {
return true;
@@ -9326,7 +9326,8 @@
}
final ScrollabilityCache scrollabilityCache = mScrollCache;
- int length = scrollabilityCache.fadingEdgeLength;
+ final float fadeHeight = scrollabilityCache.fadingEdgeLength;
+ int length = (int) fadeHeight;
// clip the fade length if top and bottom fades overlap
// overlapping fades produce odd-looking artifacts
@@ -9341,16 +9342,16 @@
if (verticalEdges) {
topFadeStrength = Math.max(0.0f, Math.min(1.0f, getTopFadingEdgeStrength()));
- drawTop = topFadeStrength > 0.0f;
+ drawTop = topFadeStrength * fadeHeight > 1.0f;
bottomFadeStrength = Math.max(0.0f, Math.min(1.0f, getBottomFadingEdgeStrength()));
- drawBottom = bottomFadeStrength > 0.0f;
+ drawBottom = bottomFadeStrength * fadeHeight > 1.0f;
}
if (horizontalEdges) {
leftFadeStrength = Math.max(0.0f, Math.min(1.0f, getLeftFadingEdgeStrength()));
- drawLeft = leftFadeStrength > 0.0f;
+ drawLeft = leftFadeStrength * fadeHeight > 1.0f;
rightFadeStrength = Math.max(0.0f, Math.min(1.0f, getRightFadingEdgeStrength()));
- drawRight = rightFadeStrength > 0.0f;
+ drawRight = rightFadeStrength * fadeHeight > 1.0f;
}
saveCount = canvas.getSaveCount();
@@ -9388,7 +9389,6 @@
final Paint p = scrollabilityCache.paint;
final Matrix matrix = scrollabilityCache.matrix;
final Shader fade = scrollabilityCache.shader;
- final float fadeHeight = scrollabilityCache.fadingEdgeLength;
if (drawTop) {
matrix.setScale(1, fadeHeight * topFadeStrength);
@@ -9438,6 +9438,7 @@
*
* @return The known solid color background for this view, or 0 if the color may vary
*/
+ @ViewDebug.ExportedProperty(category = "drawing")
public int getSolidColor() {
return 0;
}
@@ -11644,6 +11645,7 @@
* @return true if scrolling was clamped to an over-scroll boundary along either
* axis, false otherwise.
*/
+ @SuppressWarnings({"UnusedParameters"})
protected boolean overScrollBy(int deltaX, int deltaY,
int scrollX, int scrollY,
int scrollRangeX, int scrollRangeY,
diff --git a/core/java/android/view/ViewGroup.java b/core/java/android/view/ViewGroup.java
index f9692da..8dc86ac 100644
--- a/core/java/android/view/ViewGroup.java
+++ b/core/java/android/view/ViewGroup.java
@@ -2585,7 +2585,7 @@
}
} else {
child.mPrivateFlags &= ~DIRTY_MASK;
- ((HardwareCanvas) canvas).drawDisplayList(displayList, null);
+ ((HardwareCanvas) canvas).drawDisplayList(displayList, cr - cl, cb - ct, null);
}
}
} else if (cache != null) {
diff --git a/core/java/android/widget/AbsListView.java b/core/java/android/widget/AbsListView.java
index eca39fc..d39271e 100644
--- a/core/java/android/widget/AbsListView.java
+++ b/core/java/android/widget/AbsListView.java
@@ -5226,6 +5226,7 @@
*
* @return The cache color hint
*/
+ @ViewDebug.ExportedProperty(category = "drawing")
public int getCacheColorHint() {
return mCacheColorHint;
}
diff --git a/core/jni/android/graphics/Shader.cpp b/core/jni/android/graphics/Shader.cpp
index 0ea8225..7fdad10 100644
--- a/core/jni/android/graphics/Shader.cpp
+++ b/core/jni/android/graphics/Shader.cpp
@@ -119,7 +119,7 @@
const jint* colorValues = env->GetIntArrayElements(colorArray, NULL);
SkAutoSTMalloc<8, SkScalar> storage(posArray ? count : 0);
- SkScalar* pos = NULL;
+ SkScalar* pos = NULL;
if (posArray) {
AutoJavaFloatArray autoPos(env, posArray, count);
@@ -164,7 +164,11 @@
}
} else {
storedPositions[0] = 0.0f;
- storedPositions[1] = 1.0f;
+ const jfloat step = 1.0f / (count - 1);
+ for (size_t i = 1; i < count - 1; i++) {
+ storedPositions[i] = step * i;
+ }
+ storedPositions[count - 1] = 1.0f;
}
SkiaShader* skiaShader = new SkiaLinearGradientShader(storedBounds, storedColors,
@@ -289,7 +293,11 @@
}
} else {
storedPositions[0] = 0.0f;
- storedPositions[1] = 1.0f;
+ const jfloat step = 1.0f / (count - 1);
+ for (size_t i = 1; i < count - 1; i++) {
+ storedPositions[i] = step * i;
+ }
+ storedPositions[count - 1] = 1.0f;
}
SkiaShader* skiaShader = new SkiaCircularGradientShader(x, y, radius, storedColors,
@@ -384,7 +392,11 @@
}
} else {
storedPositions[0] = 0.0f;
- storedPositions[1] = 1.0f;
+ const jfloat step = 1.0f / (count - 1);
+ for (size_t i = 1; i < count - 1; i++) {
+ storedPositions[i] = step * i;
+ }
+ storedPositions[count - 1] = 1.0f;
}
SkiaShader* skiaShader = new SkiaSweepGradientShader(x, y, storedColors, storedPositions, count,
diff --git a/core/jni/android_view_GLES20Canvas.cpp b/core/jni/android_view_GLES20Canvas.cpp
index a78f660..5116f09 100644
--- a/core/jni/android_view_GLES20Canvas.cpp
+++ b/core/jni/android_view_GLES20Canvas.cpp
@@ -498,9 +498,10 @@
}
static bool android_view_GLES20Canvas_drawDisplayList(JNIEnv* env,
- jobject clazz, OpenGLRenderer* renderer, DisplayList* displayList, jobject dirty) {
+ jobject clazz, OpenGLRenderer* renderer, DisplayList* displayList,
+ jint width, jint height, jobject dirty) {
android::uirenderer::Rect bounds;
- bool redraw = renderer->drawDisplayList(displayList, bounds);
+ bool redraw = renderer->drawDisplayList(displayList, width, height, bounds);
if (redraw && dirty != NULL) {
env->CallVoidMethod(dirty, gRectClassInfo.set,
int(bounds.left), int(bounds.top), int(bounds.right), int(bounds.bottom));
@@ -664,7 +665,7 @@
{ "nGetDisplayList", "(I)I", (void*) android_view_GLES20Canvas_getDisplayList },
{ "nDestroyDisplayList", "(I)V", (void*) android_view_GLES20Canvas_destroyDisplayList },
{ "nGetDisplayListRenderer", "(I)I", (void*) android_view_GLES20Canvas_getDisplayListRenderer },
- { "nDrawDisplayList", "(IILandroid/graphics/Rect;)Z",
+ { "nDrawDisplayList", "(IIIILandroid/graphics/Rect;)Z",
(void*) android_view_GLES20Canvas_drawDisplayList },
{ "nInterrupt", "(I)V", (void*) android_view_GLES20Canvas_interrupt },
diff --git a/libs/hwui/DisplayListRenderer.cpp b/libs/hwui/DisplayListRenderer.cpp
index 737fa02..868290b 100644
--- a/libs/hwui/DisplayListRenderer.cpp
+++ b/libs/hwui/DisplayListRenderer.cpp
@@ -285,9 +285,12 @@
break;
case DrawDisplayList: {
DisplayList* displayList = getDisplayList();
- DISPLAY_LIST_LOGD("%s%s %p, %d", (char*) indent, OP_NAMES[op],
- displayList, level + 1);
- needsInvalidate |= renderer.drawDisplayList(displayList, dirty, level + 1);
+ uint32_t width = getUInt();
+ uint32_t height = getUInt();
+ DISPLAY_LIST_LOGD("%s%s %p, %dx%d, %d", (char*) indent, OP_NAMES[op],
+ displayList, width, height, level + 1);
+ needsInvalidate |= renderer.drawDisplayList(displayList, width, height,
+ dirty, level + 1);
}
break;
case DrawLayer: {
@@ -674,11 +677,13 @@
return OpenGLRenderer::clipRect(left, top, right, bottom, op);
}
-bool DisplayListRenderer::drawDisplayList(DisplayList* displayList, Rect& dirty, uint32_t level) {
+bool DisplayListRenderer::drawDisplayList(DisplayList* displayList,
+ uint32_t width, uint32_t height, Rect& dirty, uint32_t level) {
// dirty is an out parameter and should not be recorded,
// it matters only when replaying the display list
addOp(DisplayList::DrawDisplayList);
addDisplayList(displayList);
+ addSize(width, height);
return false;
}
diff --git a/libs/hwui/DisplayListRenderer.h b/libs/hwui/DisplayListRenderer.h
index f24545d..6fc315c 100644
--- a/libs/hwui/DisplayListRenderer.h
+++ b/libs/hwui/DisplayListRenderer.h
@@ -144,6 +144,10 @@
return mReader.readInt();
}
+ inline uint32_t getUInt() {
+ return mReader.readU32();
+ }
+
SkMatrix* getMatrix() {
return (SkMatrix*) getInt();
}
@@ -238,7 +242,8 @@
bool clipRect(float left, float top, float right, float bottom, SkRegion::Op op);
- bool drawDisplayList(DisplayList* displayList, Rect& dirty, uint32_t level = 0);
+ bool drawDisplayList(DisplayList* displayList, uint32_t width, uint32_t height,
+ Rect& dirty, uint32_t level = 0);
void drawLayer(Layer* layer, float x, float y, SkPaint* paint);
void drawBitmap(SkBitmap* bitmap, float left, float top, SkPaint* paint);
void drawBitmap(SkBitmap* bitmap, SkMatrix* matrix, SkPaint* paint);
@@ -323,6 +328,11 @@
mWriter.writeInt(value);
}
+ inline void addSize(uint32_t w, uint32_t h) {
+ mWriter.writeInt(w);
+ mWriter.writeInt(h);
+ }
+
void addInts(const int32_t* values, uint32_t count) {
mWriter.writeInt(count);
for (uint32_t i = 0; i < count; i++) {
diff --git a/libs/hwui/OpenGLRenderer.cpp b/libs/hwui/OpenGLRenderer.cpp
index 1f65201..5d9522e 100644
--- a/libs/hwui/OpenGLRenderer.cpp
+++ b/libs/hwui/OpenGLRenderer.cpp
@@ -403,10 +403,7 @@
// Window coordinates of the layer
Rect bounds(left, top, right, bottom);
- if (fboLayer) {
- // Clear the previous layer regions before we change the viewport
- clearLayerRegions();
- } else {
+ if (!fboLayer) {
mSnapshot->transform->mapRect(bounds);
// Layers only make sense if they are in the framebuffer's bounds
@@ -464,8 +461,14 @@
glCopyTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, bounds.left,
snapshot->height - bounds.bottom, bounds.getWidth(), bounds.getHeight());
}
- // Enqueue the buffer coordinates to clear the corresponding region later
- mLayers.push(new Rect(bounds));
+
+ // Clear the framebuffer where the layer will draw
+ glScissor(bounds.left, mSnapshot->height - bounds.bottom,
+ bounds.getWidth(), bounds.getHeight());
+ glClearColor(0.0f, 0.0f, 0.0f, 0.0f);
+ glClear(GL_COLOR_BUFFER_BIT);
+
+ dirtyClip();
}
}
@@ -760,31 +763,6 @@
#endif
}
-void OpenGLRenderer::clearLayerRegions() {
- if (mLayers.size() == 0 || mSnapshot->isIgnored()) return;
-
- Rect clipRect(*mSnapshot->clipRect);
- clipRect.snapToPixelBoundaries();
-
- for (uint32_t i = 0; i < mLayers.size(); i++) {
- Rect* bounds = mLayers.itemAt(i);
- if (clipRect.intersects(*bounds)) {
- // Clear the framebuffer where the layer will draw
- glScissor(bounds->left, mSnapshot->height - bounds->bottom,
- bounds->getWidth(), bounds->getHeight());
- glClearColor(0.0f, 0.0f, 0.0f, 0.0f);
- glClear(GL_COLOR_BUFFER_BIT);
-
- // Restore the clip
- dirtyClip();
- }
-
- delete bounds;
- }
-
- mLayers.clear();
-}
-
///////////////////////////////////////////////////////////////////////////////
// Transforms
///////////////////////////////////////////////////////////////////////////////
@@ -870,7 +848,6 @@
///////////////////////////////////////////////////////////////////////////////
void OpenGLRenderer::setupDraw() {
- clearLayerRegions();
if (mDirtyClip) {
setScissorFromClip();
}
@@ -1064,12 +1041,18 @@
// Drawing
///////////////////////////////////////////////////////////////////////////////
-bool OpenGLRenderer::drawDisplayList(DisplayList* displayList, Rect& dirty, uint32_t level) {
+bool OpenGLRenderer::drawDisplayList(DisplayList* displayList, uint32_t width, uint32_t height,
+ Rect& dirty, uint32_t level) {
+ if (quickReject(0.0f, 0.0f, width, height)) {
+ return false;
+ }
+
// All the usual checks and setup operations (quickReject, setupDraw, etc.)
// will be performed by the display list itself
if (displayList) {
return displayList->replay(*this, dirty, level);
}
+
return false;
}
diff --git a/libs/hwui/OpenGLRenderer.h b/libs/hwui/OpenGLRenderer.h
index 9d86388..7362473 100644
--- a/libs/hwui/OpenGLRenderer.h
+++ b/libs/hwui/OpenGLRenderer.h
@@ -96,7 +96,8 @@
bool quickReject(float left, float top, float right, float bottom);
virtual bool clipRect(float left, float top, float right, float bottom, SkRegion::Op op);
- virtual bool drawDisplayList(DisplayList* displayList, Rect& dirty, uint32_t level = 0);
+ virtual bool drawDisplayList(DisplayList* displayList, uint32_t width, uint32_t height,
+ Rect& dirty, uint32_t level = 0);
virtual void drawLayer(Layer* layer, float x, float y, SkPaint* paint);
virtual void drawBitmap(SkBitmap* bitmap, float left, float top, SkPaint* paint);
virtual void drawBitmap(SkBitmap* bitmap, SkMatrix* matrix, SkPaint* paint);
@@ -247,12 +248,6 @@
void composeLayerRect(Layer* layer, const Rect& rect, bool swap = false);
/**
- * Clears all the regions corresponding to the current list of layers.
- * This method MUST be invoked before any drawing operation.
- */
- void clearLayerRegions();
-
- /**
* Mark the layer as dirty at the specified coordinates. The coordinates
* are transformed with the supplied matrix.
*/
@@ -499,9 +494,6 @@
// Various caches
Caches& mCaches;
- // List of rectangles to clear due to calls to saveLayer()
- Vector<Rect*> mLayers;
-
// Indentity matrix
const mat4 mIdentity;
diff --git a/libs/hwui/ProgramCache.h b/libs/hwui/ProgramCache.h
index 3acd18a..ead5b92 100644
--- a/libs/hwui/ProgramCache.h
+++ b/libs/hwui/ProgramCache.h
@@ -231,9 +231,11 @@
* Logs the specified message followed by the key identifying this program.
*/
void log(const char* message) const {
+#if DEBUG_PROGRAMS
programid k = key();
PROGRAM_LOGD("%s (key = 0x%.8x%.8x)", message, uint32_t(k >> 32),
uint32_t(k & 0xffffffff));
+#endif
}
private:
diff --git a/tests/HwAccelerationTest/res/drawable/gradient.xml b/tests/HwAccelerationTest/res/drawable/gradient.xml
new file mode 100644
index 0000000..756db0b
--- /dev/null
+++ b/tests/HwAccelerationTest/res/drawable/gradient.xml
@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright (C) 2010 The Android Open Source Project
+
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<shape xmlns:android="http://schemas.android.com/apk/res/android">
+ <gradient
+ android:startColor="#FF707070"
+ android:endColor="#FF0C0C0C"
+ android:angle="270" />
+</shape>
diff --git a/tests/HwAccelerationTest/src/com/android/test/hwui/GradientsActivity.java b/tests/HwAccelerationTest/src/com/android/test/hwui/GradientsActivity.java
index f8422f4..90db818 100644
--- a/tests/HwAccelerationTest/src/com/android/test/hwui/GradientsActivity.java
+++ b/tests/HwAccelerationTest/src/com/android/test/hwui/GradientsActivity.java
@@ -193,6 +193,7 @@
private final float mDrawWidth;
private final float mDrawHeight;
private final LinearGradient mGradient;
+ private final LinearGradient mGradientStops;
private final Matrix mMatrix;
ShadersView(Context c) {
@@ -202,6 +203,9 @@
mDrawHeight = 200;
mGradient = new LinearGradient(0, 0, 0, 1, 0xFF000000, 0, Shader.TileMode.CLAMP);
+ mGradientStops = new LinearGradient(0, 0, 0, 1,
+ new int[] { 0xFFFF0000, 0xFF00FF00, 0xFF0000FF }, null, Shader.TileMode.CLAMP);
+
mMatrix = new Matrix();
mPaint = new Paint();
@@ -255,6 +259,19 @@
mGradient.setLocalMatrix(mMatrix);
canvas.drawRect(left, top, left + mDrawWidth, bottom, mPaint);
+ right = left + mDrawWidth;
+ left = 40.0f;
+ top = bottom + 20.0f;
+ bottom = top + 50.0f;
+
+ mPaint.setShader(mGradientStops);
+
+ mMatrix.setScale(1, mDrawWidth);
+ mMatrix.postRotate(90);
+ mMatrix.postTranslate(right, top);
+ mGradientStops.setLocalMatrix(mMatrix);
+ canvas.drawRect(left, top, right, bottom, mPaint);
+
canvas.restore();
}
}
diff --git a/tests/HwAccelerationTest/src/com/android/test/hwui/TransparentListActivity.java b/tests/HwAccelerationTest/src/com/android/test/hwui/TransparentListActivity.java
index 763169a..535f865 100644
--- a/tests/HwAccelerationTest/src/com/android/test/hwui/TransparentListActivity.java
+++ b/tests/HwAccelerationTest/src/com/android/test/hwui/TransparentListActivity.java
@@ -79,7 +79,7 @@
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
- getWindow().setBackgroundDrawable(getResources().getDrawable(R.drawable.default_wallpaper));
+ getWindow().setBackgroundDrawable(getResources().getDrawable(R.drawable.gradient));
setContentView(R.layout.list_activity);
ListAdapter adapter = new SimpleListAdapter(this);