Clean up quick rejection, especially surrounding points + lines.
bug:4351353
quickReject and quickRejectNoScissor have been renamed and refactored:
- to make the scissor side effect clear and explicit
- dangerous methods no longer public
- to make the simple quick reject check logic const
- simple quick reject is now conservative
This CL also fixes several issues with line and point quickRejection -
sub-pixel and hairline lines are much less likely to be incorrectly
rejected, especially at small canvas scale.
Additionally, alpha modulation for AA points < 1px in size is now
correct, dumplicating SW behavior (similar to lines and stroked
shapes work).
Change-Id: Ibb0710c721b9fb415d05acf54dd3d2b4d602156a
diff --git a/core/jni/android_view_GLES20Canvas.cpp b/core/jni/android_view_GLES20Canvas.cpp
index b720e73..7a763ad 100644
--- a/core/jni/android_view_GLES20Canvas.cpp
+++ b/core/jni/android_view_GLES20Canvas.cpp
@@ -298,7 +298,7 @@
static bool android_view_GLES20Canvas_quickReject(JNIEnv* env, jobject clazz,
OpenGLRenderer* renderer, jfloat left, jfloat top, jfloat right, jfloat bottom) {
- return renderer->quickRejectNoScissor(left, top, right, bottom);
+ return renderer->quickRejectConservative(left, top, right, bottom);
}
static bool android_view_GLES20Canvas_clipRectF(JNIEnv* env, jobject clazz,
diff --git a/libs/hwui/DisplayList.cpp b/libs/hwui/DisplayList.cpp
index bb6526e..a3e4bb4 100644
--- a/libs/hwui/DisplayList.cpp
+++ b/libs/hwui/DisplayList.cpp
@@ -526,7 +526,7 @@
setViewProperties<T>(renderer, handler, level + 1);
- if (mClipToBounds && renderer.quickRejectNoScissor(0, 0, mWidth, mHeight)) {
+ if (mClipToBounds && renderer.quickRejectConservative(0, 0, mWidth, mHeight)) {
DISPLAY_LIST_LOGD("%*sRestoreToCount %d", (level + 1) * 2, "", restoreTo);
handler(mRestoreToCountOp->reinit(restoreTo), PROPERTY_SAVECOUNT, mClipToBounds);
renderer.restoreToCount(restoreTo);
diff --git a/libs/hwui/DisplayListOp.h b/libs/hwui/DisplayListOp.h
index 5816872..88077d4 100644
--- a/libs/hwui/DisplayListOp.h
+++ b/libs/hwui/DisplayListOp.h
@@ -182,9 +182,12 @@
}
inline float strokeWidthOutset() {
- float width = mPaint->getStrokeWidth();
- if (width == 0) return 0.5f; // account for hairline
- return width * 0.5f;
+ // since anything AA stroke with less than 1.0 pixel width is drawn with an alpha-reduced
+ // 1.0 stroke, treat 1.0 as minimum.
+
+ // TODO: it would be nice if this could take scale into account, but scale isn't stable
+ // since higher levels of the view hierarchy can change scale out from underneath it.
+ return fmaxf(mPaint->getStrokeWidth(), 1) * 0.5f;
}
protected:
diff --git a/libs/hwui/DisplayListRenderer.cpp b/libs/hwui/DisplayListRenderer.cpp
index 8866029..d024923 100644
--- a/libs/hwui/DisplayListRenderer.cpp
+++ b/libs/hwui/DisplayListRenderer.cpp
@@ -506,7 +506,7 @@
void DisplayListRenderer::addDrawOp(DrawOp* op) {
Rect localBounds;
if (op->getLocalBounds(mDrawModifiers, localBounds)) {
- bool rejected = quickRejectNoScissor(localBounds.left, localBounds.top,
+ bool rejected = quickRejectConservative(localBounds.left, localBounds.top,
localBounds.right, localBounds.bottom);
op->setQuickRejected(rejected);
}
diff --git a/libs/hwui/OpenGLRenderer.cpp b/libs/hwui/OpenGLRenderer.cpp
index 9b82013..e256ec2 100644
--- a/libs/hwui/OpenGLRenderer.cpp
+++ b/libs/hwui/OpenGLRenderer.cpp
@@ -1032,7 +1032,8 @@
const bool fboLayer = current->flags & Snapshot::kFlagIsFboLayer;
bool clipRequired = false;
- quickRejectNoScissor(rect, &clipRequired); // safely ignore return, should never be rejected
+ calculateQuickRejectForScissor(rect.left, rect.top, rect.right, rect.bottom,
+ &clipRequired, false); // safely ignore return, should never be rejected
mCaches.setScissorEnabled(mScissorOptimizationDisabled || clipRequired);
if (fboLayer) {
@@ -1629,8 +1630,18 @@
return mSnapshot->getLocalClip();
}
-bool OpenGLRenderer::quickRejectNoScissor(float left, float top, float right, float bottom,
- bool snapOut, bool* clipRequired) {
+/**
+ * Calculates whether content drawn within the passed bounds would be outside of, or intersect with
+ * the clipRect. Does not modify the scissor.
+ *
+ * @param clipRequired if not null, will be set to true if element intersects clip
+ * (and wasn't rejected)
+ *
+ * @param snapOut if set, the geometry will be treated as having an AA ramp.
+ * See Rect::snapGeometryToPixelBoundaries()
+ */
+bool OpenGLRenderer::calculateQuickRejectForScissor(float left, float top,
+ float right, float bottom, bool* clipRequired, bool snapOut) const {
if (mSnapshot->isIgnored() || bottom <= top || right <= left) {
return true;
}
@@ -1644,31 +1655,65 @@
if (!clipRect.intersects(r)) return true;
+ // clip is required if geometry intersects clip rect
if (clipRequired) *clipRequired = !clipRect.contains(r);
return false;
}
-bool OpenGLRenderer::quickRejectPreStroke(float left, float top, float right, float bottom,
- SkPaint* paint) {
- // AA geometry will likely have a ramp around it (not accounted for in local bounds). Snap out
- // the final mapped rect to ensure correct clipping behavior for the ramp.
- bool snapOut = paint->isAntiAlias();
-
- if (paint->getStyle() != SkPaint::kFill_Style) {
- float outset = paint->getStrokeWidth() * 0.5f;
- return quickReject(left - outset, top - outset, right + outset, bottom + outset, snapOut);
- } else {
- return quickReject(left, top, right, bottom, snapOut);
+/**
+ * Returns false if drawing won't be clipped out.
+ *
+ * Makes the decision conservatively, by rounding out the mapped rect before comparing with the
+ * clipRect. To be used when perfect, pixel accuracy is not possible (esp. with tessellation) but
+ * rejection is still desired.
+ *
+ * This function, unlike quickRejectSetupScissor, should be used where precise geometry information
+ * isn't known (esp. when geometry adjusts based on scale). Generally, this will be first pass
+ * rejection where precise rejection isn't important, or precise information isn't available.
+ */
+bool OpenGLRenderer::quickRejectConservative(float left, float top,
+ float right, float bottom) const {
+ if (mSnapshot->isIgnored() || bottom <= top || right <= left) {
+ return true;
}
+
+ Rect r(left, top, right, bottom);
+ currentTransform().mapRect(r);
+ r.roundOut(); // rounded out to be conservative
+
+ Rect clipRect(*mSnapshot->clipRect);
+ clipRect.snapToPixelBoundaries();
+
+ if (!clipRect.intersects(r)) return true;
+
+ return false;
}
-bool OpenGLRenderer::quickReject(float left, float top, float right, float bottom, bool snapOut) {
+/**
+ * Returns false and sets scissor enable based upon bounds if drawing won't be clipped out.
+ *
+ * @param paint if not null, the bounds will be expanded to account for stroke depending on paint
+ * style, and tessellated AA ramp
+ */
+bool OpenGLRenderer::quickRejectSetupScissor(float left, float top, float right, float bottom,
+ SkPaint* paint) {
bool clipRequired = false;
- if (quickRejectNoScissor(left, top, right, bottom, snapOut, &clipRequired)) {
+ bool snapOut = paint && paint->isAntiAlias();
+
+ if (paint && paint->getStyle() != SkPaint::kFill_Style) {
+ float outset = paint->getStrokeWidth() * 0.5f;
+ left -= outset;
+ top -= outset;
+ right += outset;
+ bottom += outset;
+ }
+
+ if (calculateQuickRejectForScissor(left, top, right, bottom, &clipRequired, snapOut)) {
return true;
}
if (!isDeferred()) {
+ // not quick rejected, so enable the scissor if clipRequired
mCaches.setScissorEnabled(mScissorOptimizationDisabled || clipRequired);
}
return false;
@@ -1743,7 +1788,7 @@
///////////////////////////////////////////////////////////////////////////////
void OpenGLRenderer::setupDraw(bool clear) {
- // TODO: It would be best if we could do this before quickReject()
+ // TODO: It would be best if we could do this before quickRejectSetupScissor()
// changes the scissor test state
if (clear) clearLayerRegions();
// Make sure setScissor & setStencil happen at the beginning of
@@ -2123,7 +2168,7 @@
const float right = left + bitmap->width();
const float bottom = top + bitmap->height();
- if (quickReject(left, top, right, bottom)) {
+ if (quickRejectSetupScissor(left, top, right, bottom)) {
return DrawGlInfo::kStatusDone;
}
@@ -2146,7 +2191,7 @@
const mat4 transform(*matrix);
transform.mapRect(r);
- if (quickReject(r.left, r.top, r.right, r.bottom)) {
+ if (quickRejectSetupScissor(r.left, r.top, r.right, r.bottom)) {
return DrawGlInfo::kStatusDone;
}
@@ -2173,7 +2218,7 @@
const float right = left + bitmap->width();
const float bottom = top + bitmap->height();
- if (quickReject(left, top, right, bottom)) {
+ if (quickRejectSetupScissor(left, top, right, bottom)) {
return DrawGlInfo::kStatusDone;
}
@@ -2256,7 +2301,7 @@
}
}
- if (quickReject(left, top, right, bottom)) {
+ if (quickRejectSetupScissor(left, top, right, bottom)) {
if (cleanupColors) delete[] colors;
return DrawGlInfo::kStatusDone;
}
@@ -2312,7 +2357,7 @@
float srcLeft, float srcTop, float srcRight, float srcBottom,
float dstLeft, float dstTop, float dstRight, float dstBottom,
SkPaint* paint) {
- if (quickReject(dstLeft, dstTop, dstRight, dstBottom)) {
+ if (quickRejectSetupScissor(dstLeft, dstTop, dstRight, dstBottom)) {
return DrawGlInfo::kStatusDone;
}
@@ -2402,7 +2447,7 @@
status_t OpenGLRenderer::drawPatch(SkBitmap* bitmap, Res_png_9patch* patch,
float left, float top, float right, float bottom, SkPaint* paint) {
- if (quickReject(left, top, right, bottom)) {
+ if (quickRejectSetupScissor(left, top, right, bottom)) {
return DrawGlInfo::kStatusDone;
}
@@ -2415,7 +2460,7 @@
status_t OpenGLRenderer::drawPatch(SkBitmap* bitmap, const Patch* mesh, AssetAtlas::Entry* entry,
float left, float top, float right, float bottom, SkPaint* paint) {
- if (quickReject(left, top, right, bottom)) {
+ if (quickRejectSetupScissor(left, top, right, bottom)) {
return DrawGlInfo::kStatusDone;
}
@@ -2564,7 +2609,7 @@
if (hasLayer()) {
SkRect bounds = path.getBounds();
- PathTessellator::expandBoundsForStroke(bounds, paint, false);
+ PathTessellator::expandBoundsForStroke(bounds, paint);
dirtyLayer(bounds.fLeft, bounds.fTop, bounds.fRight, bounds.fBottom, currentTransform());
}
@@ -2591,7 +2636,8 @@
SkRect bounds;
PathTessellator::tessellateLines(points, count, paint, mSnapshot->transform, bounds, buffer);
- if (quickReject(bounds.fLeft, bounds.fTop, bounds.fRight, bounds.fBottom)) {
+ // can't pass paint, since style would be checked for outset. outset done by tessellation.
+ if (quickRejectSetupScissor(bounds.fLeft, bounds.fTop, bounds.fRight, bounds.fBottom)) {
return DrawGlInfo::kStatusDone;
}
@@ -2610,7 +2656,8 @@
SkRect bounds;
PathTessellator::tessellatePoints(points, count, paint, mSnapshot->transform, bounds, buffer);
- if (quickReject(bounds.fLeft, bounds.fTop, bounds.fRight, bounds.fBottom)) {
+ // can't pass paint, since style would be checked for outset. outset done by tessellation.
+ if (quickRejectSetupScissor(bounds.fLeft, bounds.fTop, bounds.fRight, bounds.fBottom)) {
return DrawGlInfo::kStatusDone;
}
@@ -2647,7 +2694,7 @@
status_t OpenGLRenderer::drawRoundRect(float left, float top, float right, float bottom,
float rx, float ry, SkPaint* p) {
- if (mSnapshot->isIgnored() || quickRejectPreStroke(left, top, right, bottom, p) ||
+ if (mSnapshot->isIgnored() || quickRejectSetupScissor(left, top, right, bottom, p) ||
(p->getAlpha() == 0 && getXfermode(p->getXfermode()) != SkXfermode::kClear_Mode)) {
return DrawGlInfo::kStatusDone;
}
@@ -2672,7 +2719,7 @@
}
status_t OpenGLRenderer::drawCircle(float x, float y, float radius, SkPaint* p) {
- if (mSnapshot->isIgnored() || quickRejectPreStroke(x - radius, y - radius,
+ if (mSnapshot->isIgnored() || quickRejectSetupScissor(x - radius, y - radius,
x + radius, y + radius, p) ||
(p->getAlpha() == 0 && getXfermode(p->getXfermode()) != SkXfermode::kClear_Mode)) {
return DrawGlInfo::kStatusDone;
@@ -2694,7 +2741,7 @@
status_t OpenGLRenderer::drawOval(float left, float top, float right, float bottom,
SkPaint* p) {
- if (mSnapshot->isIgnored() || quickRejectPreStroke(left, top, right, bottom, p) ||
+ if (mSnapshot->isIgnored() || quickRejectSetupScissor(left, top, right, bottom, p) ||
(p->getAlpha() == 0 && getXfermode(p->getXfermode()) != SkXfermode::kClear_Mode)) {
return DrawGlInfo::kStatusDone;
}
@@ -2716,7 +2763,7 @@
status_t OpenGLRenderer::drawArc(float left, float top, float right, float bottom,
float startAngle, float sweepAngle, bool useCenter, SkPaint* p) {
- if (mSnapshot->isIgnored() || quickRejectPreStroke(left, top, right, bottom, p) ||
+ if (mSnapshot->isIgnored() || quickRejectSetupScissor(left, top, right, bottom, p) ||
(p->getAlpha() == 0 && getXfermode(p->getXfermode()) != SkXfermode::kClear_Mode)) {
return DrawGlInfo::kStatusDone;
}
@@ -2753,7 +2800,7 @@
#define SkPaintDefaults_MiterLimit SkIntToScalar(4)
status_t OpenGLRenderer::drawRect(float left, float top, float right, float bottom, SkPaint* p) {
- if (mSnapshot->isIgnored() || quickRejectPreStroke(left, top, right, bottom, p) ||
+ if (mSnapshot->isIgnored() || quickRejectSetupScissor(left, top, right, bottom, p) ||
(p->getAlpha() == 0 && getXfermode(p->getXfermode()) != SkXfermode::kClear_Mode)) {
return DrawGlInfo::kStatusDone;
}
@@ -2917,7 +2964,7 @@
// The checks for corner-case ignorable text and quick rejection is only done for immediate
// drawing as ops from DeferredDisplayList are already filtered for these
if (text == NULL || count == 0 || mSnapshot->isIgnored() || canSkipText(paint) ||
- quickReject(bounds)) {
+ quickRejectSetupScissor(bounds)) {
return DrawGlInfo::kStatusDone;
}
}
@@ -3062,8 +3109,8 @@
}
bool clipRequired = false;
- const bool rejected = quickRejectNoScissor(x, y,
- x + layer->layer.getWidth(), y + layer->layer.getHeight(), false, &clipRequired);
+ const bool rejected = calculateQuickRejectForScissor(x, y,
+ x + layer->layer.getWidth(), y + layer->layer.getHeight(), &clipRequired, false);
if (rejected) {
if (transform && !transform->isIdentity()) {
@@ -3235,7 +3282,7 @@
void OpenGLRenderer::drawPathTexture(const PathTexture* texture,
float x, float y, SkPaint* paint) {
- if (quickReject(x, y, x + texture->width, y + texture->height)) {
+ if (quickRejectSetupScissor(x, y, x + texture->width, y + texture->height)) {
return;
}
@@ -3356,7 +3403,7 @@
bottom = fmaxf(bottom, b);
}
- if (clip && quickReject(left, top, right, bottom)) {
+ if (clip && quickRejectSetupScissor(left, top, right, bottom)) {
return DrawGlInfo::kStatusDone;
}
diff --git a/libs/hwui/OpenGLRenderer.h b/libs/hwui/OpenGLRenderer.h
index febf14a..cfc5931 100644
--- a/libs/hwui/OpenGLRenderer.h
+++ b/libs/hwui/OpenGLRenderer.h
@@ -268,30 +268,8 @@
ANDROID_API const Rect& getClipBounds();
- /**
- * Performs a quick reject but adjust the bounds to account for stroke width if necessary,
- * and handling snapOut for AA geometry.
- */
- bool quickRejectPreStroke(float left, float top, float right, float bottom, SkPaint* paint);
-
- /**
- * Returns false and sets scissor based upon bounds if drawing won't be clipped out
- */
- bool quickReject(float left, float top, float right, float bottom, bool snapOut = false);
- bool quickReject(const Rect& bounds) {
- return quickReject(bounds.left, bounds.top, bounds.right, bounds.bottom);
- }
-
- /**
- * Same as quickReject, without the scissor, instead returning clipRequired through pointer.
- * clipRequired will be only set if not rejected
- */
- ANDROID_API bool quickRejectNoScissor(float left, float top, float right, float bottom,
- bool snapOut = false, bool* clipRequired = NULL);
- bool quickRejectNoScissor(const Rect& bounds, bool* clipRequired = NULL) {
- return quickRejectNoScissor(bounds.left, bounds.top, bounds.right, bounds.bottom,
- clipRequired);
- }
+ ANDROID_API bool quickRejectConservative(float left, float top,
+ float right, float bottom) const;
virtual bool clipRect(float left, float top, float right, float bottom, SkRegion::Op op);
virtual bool clipPath(SkPath* path, SkRegion::Op op);
@@ -495,6 +473,16 @@
*/
void attachStencilBufferToLayer(Layer* layer);
+ bool calculateQuickRejectForScissor(float left, float top, float right, float bottom,
+ bool* clipRequired, bool snapOut) const;
+
+ bool quickRejectSetupScissor(float left, float top, float right, float bottom,
+ SkPaint* paint = NULL);
+ bool quickRejectSetupScissor(const Rect& bounds, SkPaint* paint = NULL) {
+ return quickRejectSetupScissor(bounds.left, bounds.top,
+ bounds.right, bounds.bottom, paint);
+ }
+
/**
* Compose the layer defined in the current snapshot with the layer
* defined by the previous snapshot.
diff --git a/libs/hwui/PathTessellator.cpp b/libs/hwui/PathTessellator.cpp
index 03b2099..7e8a45b 100644
--- a/libs/hwui/PathTessellator.cpp
+++ b/libs/hwui/PathTessellator.cpp
@@ -14,7 +14,7 @@
* limitations under the License.
*/
-#define LOG_TAG "PathTessellator"
+#define LOG_TAG "OpenGLRenderer"
#define LOG_NDEBUG 1
#define ATRACE_TAG ATRACE_TAG_GRAPHICS
@@ -57,9 +57,12 @@
#define ROUND_CAP_THRESH 0.25f
#define PI 3.1415926535897932f
-void PathTessellator::expandBoundsForStroke(SkRect& bounds, const SkPaint* paint,
- bool forceExpand) {
- if (forceExpand || paint->getStyle() != SkPaint::kFill_Style) {
+/**
+ * Note: this function doesn't account for the AA case with sub-pixel line thickness (not just 0 <
+ * width < 1.0, canvas scale factors in as well) so this can't be used for points/lines
+ */
+void PathTessellator::expandBoundsForStroke(SkRect& bounds, const SkPaint* paint) {
+ if (paint->getStyle() != SkPaint::kFill_Style) {
float outset = paint->getStrokeWidth() * 0.5f;
if (outset == 0) outset = 0.5f; // account for hairline
bounds.outset(outset, outset);
@@ -159,6 +162,17 @@
}
return 0;
}
+
+ /**
+ * Outset the bounds of point data (for line endpoints or points) to account for AA stroke
+ * geometry.
+ */
+ void expandBoundsForStrokeAA(SkRect& bounds) const {
+ float outset = halfStrokeWidth;
+ if (outset == 0) outset = 0.5f;
+ bounds.outset(outset * inverseScaleX + Vertex::gGeometryFudgeFactor,
+ outset * inverseScaleY + Vertex::gGeometryFudgeFactor);
+ }
};
void getFillVerticesFromPerimeter(const Vector<Vertex>& perimeter, VertexBuffer& vertexBuffer) {
@@ -329,7 +343,7 @@
* 3 - zig zag back and forth inside the shape to fill it (using perimeter.size() vertices)
*/
void getFillVerticesFromPerimeterAA(const PaintInfo& paintInfo, const Vector<Vertex>& perimeter,
- VertexBuffer& vertexBuffer) {
+ VertexBuffer& vertexBuffer, float maxAlpha = 1.0f) {
AlphaVertex* buffer = vertexBuffer.alloc<AlphaVertex>(perimeter.size() * 3 + 2);
// generate alpha points - fill Alpha vertex gaps in between each point with
@@ -357,7 +371,7 @@
AlphaVertex::set(&buffer[currentIndex++],
current->x - totalOffset.x,
current->y - totalOffset.y,
- 1.0f);
+ maxAlpha);
last = current;
current = next;
@@ -829,11 +843,14 @@
getFillVerticesFromPerimeter(outlineVertices, tempBuffer);
instanceVertices<Vertex>(tempBuffer, vertexBuffer, points, count, bounds);
} else {
- getFillVerticesFromPerimeterAA(paintInfo, outlineVertices, tempBuffer);
+ // note: pass maxAlpha directly, since we want fill to be alpha modulated
+ getFillVerticesFromPerimeterAA(paintInfo, outlineVertices, tempBuffer, paintInfo.maxAlpha);
instanceVertices<AlphaVertex>(tempBuffer, vertexBuffer, points, count, bounds);
}
- expandBoundsForStroke(bounds, paint, true); // force-expand bounds to incorporate stroke
+ // expand bounds from vertex coords to pixel data
+ paintInfo.expandBoundsForStrokeAA(bounds);
+
}
void PathTessellator::tessellateLines(const float* points, int count, SkPaint* paint,
@@ -873,14 +890,15 @@
expandRectToCoverVertex(bounds, tempVerticesData[1]);
}
- expandBoundsForStroke(bounds, paint, true); // force-expand bounds to incorporate stroke
-
// since multiple objects tessellated into buffer, separate them with degen tris
if (paintInfo.isAA) {
vertexBuffer.createDegenerateSeparators<AlphaVertex>(lineAllocSize);
} else {
vertexBuffer.createDegenerateSeparators<Vertex>(lineAllocSize);
}
+
+ // expand bounds from vertex coords to pixel data
+ paintInfo.expandBoundsForStrokeAA(bounds);
}
///////////////////////////////////////////////////////////////////////////////
diff --git a/libs/hwui/PathTessellator.h b/libs/hwui/PathTessellator.h
index 85797fc..e0044e8 100644
--- a/libs/hwui/PathTessellator.h
+++ b/libs/hwui/PathTessellator.h
@@ -102,7 +102,7 @@
class PathTessellator {
public:
- static void expandBoundsForStroke(SkRect& bounds, const SkPaint* paint, bool forceExpand);
+ static void expandBoundsForStroke(SkRect& bounds, const SkPaint* paint);
static void tessellatePath(const SkPath& path, const SkPaint* paint,
const mat4 *transform, VertexBuffer& vertexBuffer);
diff --git a/libs/hwui/Rect.h b/libs/hwui/Rect.h
index dabd8d4..83b3436 100644
--- a/libs/hwui/Rect.h
+++ b/libs/hwui/Rect.h
@@ -213,6 +213,13 @@
bottom = floorf(bottom + 0.5f);
}
+ void roundOut() {
+ left = floorf(left);
+ top = floorf(top);
+ right = ceilf(right);
+ bottom = ceilf(bottom);
+ }
+
void dump() const {
ALOGD("Rect[l=%f t=%f r=%f b=%f]", left, top, right, bottom);
}