Fix various hw-accelerated line/point bugs

All accelerated lines are now rendered as quads. Hairlines used to
be rendered as GL_LINES, but these lines don't render the same as our
non-accelerated lines, so we're using quads for everything. Also, fixed
a bug in the way that we were offsetting quads (and not offseting points)
to ensure that our lines/points actuall start on the same pixels as
Skia's.

Change-Id: I51b923cc08a9858444c430ba07bc8aa0c83cbe6a

7 files changed, 213 insertions, 107 deletions

diff --git a/libs/hwui/OpenGLRenderer.cpp b/libs/hwui/OpenGLRenderer.cpp
index dd0cca2..e1d7f6b 100644
--- a/libs/hwui/OpenGLRenderer.cpp
+++ b/libs/hwui/OpenGLRenderer.cpp
@@ -979,8 +979,8 @@ void OpenGLRenderer::setupDrawModelViewTranslate(float left, float top, float ri
     }
 }
 
-void OpenGLRenderer::setupDrawModelViewIdentity() {
-    mCaches.currentProgram->set(mOrthoMatrix, mIdentity, *mSnapshot->transform);
+void OpenGLRenderer::setupDrawModelViewIdentity(bool offset) {
+    mCaches.currentProgram->set(mOrthoMatrix, mIdentity, *mSnapshot->transform, offset);
 }
 
 void OpenGLRenderer::setupDrawModelView(float left, float top, float right, float bottom,
@@ -1389,13 +1389,46 @@ void OpenGLRenderer::drawPatch(SkBitmap* bitmap, const int32_t* xDivs, const int
     }
 }
 
-void OpenGLRenderer::drawLinesAsQuads(float *points, int count, bool isAA, bool isHairline,
-        float strokeWidth) {
+void OpenGLRenderer::drawLines(float* points, int count, SkPaint* paint) {
+    if (mSnapshot->isIgnored()) return;
+
+    const bool isAA = paint->isAntiAlias();
+    float strokeWidth = paint->getStrokeWidth() * 0.5f;
+    // A stroke width of 0 has a special meaning in Skia:
+    // it draws a line 1 px wide regardless of current transform
+    bool isHairLine = paint->getStrokeWidth() == 0.0f;
+    int alpha;
+    SkXfermode::Mode mode;
+    int generatedVerticesCount = 0;
     int verticesCount = count;
     if (count > 4) {
         // Polyline: account for extra vertices needed for continous tri-strip
         verticesCount += (count -4);
     }
+
+    getAlphaAndMode(paint, &alpha, &mode);
+    setupDraw();
+    if (isAA) {
+        setupDrawAALine();
+    }
+    setupDrawColor(paint->getColor(), alpha);
+    setupDrawColorFilter();
+    setupDrawShader();
+    if (isAA) {
+        setupDrawBlending(true, mode);
+    } else {
+        setupDrawBlending(mode);
+    }
+    setupDrawProgram();
+    setupDrawModelViewIdentity(true);
+    setupDrawColorUniforms();
+    setupDrawColorFilterUniforms();
+    setupDrawShaderIdentityUniforms();
+
+    if (isHairLine) {
+        // Set a real stroke width to be used in quad construction
+        strokeWidth = .5;
+    }
     if (isAA) {
         // Expand boundary to enable AA calculations on the quad border
         strokeWidth += .5f;
@@ -1407,25 +1440,22 @@ void OpenGLRenderer::drawLinesAsQuads(float *points, int count, bool isAA, bool
     if (!isAA) {
         setupDrawVertices(vertices);
     } else {
-        AlphaVertex* alphaCoords = aaVertices + gVertexAlphaOffset;
+        void* alphaCoords = ((GLbyte*) aaVertices) + gVertexAlphaOffset;
         // innerProportion is the ratio of the inner (non-AA) port of the line to the total
         // AA stroke width (the base stroke width expanded by a half pixel on either side).
         // This value is used in the fragment shader to determine how to fill fragments.
         float innerProportion = fmax(strokeWidth - 1.0f, 0) / (strokeWidth + .5f);
-        setupDrawAALine((void*) aaVertices, (void*) alphaCoords, innerProportion);
+        setupDrawAALine((void*) aaVertices, alphaCoords, innerProportion);
     }
 
-    int generatedVerticesCount = 0;
     AlphaVertex *prevAAVertex = NULL;
     Vertex *prevVertex = NULL;
     float inverseScaleX = 1.0f;
     float inverseScaleY = 1.0f;
 
-    if (isHairline) {
+    if (isHairLine) {
         // The quad that we use for AA hairlines needs to account for scaling because the line
         // should always be one pixel wide regardless of scale.
-        inverseScaleX = 1.0f;
-        inverseScaleY = 1.0f;
         if (!mSnapshot->transform->isPureTranslate()) {
             Matrix4 *mat = mSnapshot->transform;
             float m00 = mat->data[Matrix4::kScaleX];
@@ -1446,22 +1476,19 @@ void OpenGLRenderer::drawLinesAsQuads(float *points, int count, bool isAA, bool
         vec2 a(points[i], points[i + 1]);
         vec2 b(points[i + 2], points[i + 3]);
 
-        // Bias to snap to the same pixels as Skia
-        a += 0.375;
-        b += 0.375;
-
         // Find the normal to the line
         vec2 n = (b - a).copyNormalized() * strokeWidth;
-        if (isHairline) {
-            float wideningFactor;
-            if (fabs(n.x) >= fabs(n.y)) {
-                wideningFactor = fabs(1.0f / n.x);
-            } else {
-                wideningFactor = fabs(1.0f / n.y);
+        if (isHairLine) {
+            n *= inverseScaleX;
+            if (isAA) {
+                float wideningFactor;
+                if (fabs(n.x) >= fabs(n.y)) {
+                    wideningFactor = fabs(1.0f / n.x);
+                } else {
+                    wideningFactor = fabs(1.0f / n.y);
+                }
+                n *= wideningFactor;
             }
-            n.x *= inverseScaleX;
-            n.y *= inverseScaleY;
-            n *= wideningFactor;
         }
         float x = n.x;
         n.x = -n.y;
@@ -1525,69 +1552,6 @@ void OpenGLRenderer::drawLinesAsQuads(float *points, int count, bool isAA, bool
     }
 }
 
-void OpenGLRenderer::drawLines(float* points, int count, SkPaint* paint) {
-    if (mSnapshot->isIgnored()) return;
-
-    const bool isAA = paint->isAntiAlias();
-    const float strokeWidth = paint->getStrokeWidth() * 0.5f;
-    // A stroke width of 0 has a special meaning in Skia:
-    // it draws a line 1 px wide regardless of current transform
-    bool isHairLine = paint->getStrokeWidth() == 0.0f;
-
-    int alpha;
-    SkXfermode::Mode mode;
-    getAlphaAndMode(paint, &alpha, &mode);
-    int generatedVerticesCount = 0;
-
-    setupDraw();
-    if (isAA) {
-        setupDrawAALine();
-    }
-    setupDrawColor(paint->getColor(), alpha);
-    setupDrawColorFilter();
-    setupDrawShader();
-    if (isAA) {
-        setupDrawBlending(true, mode);
-    } else {
-        setupDrawBlending(mode);
-    }
-    setupDrawProgram();
-    setupDrawModelViewIdentity();
-    setupDrawColorUniforms();
-    setupDrawColorFilterUniforms();
-    setupDrawShaderIdentityUniforms();
-
-    if (!isHairLine) {
-        drawLinesAsQuads(points, count, isAA, isHairLine, strokeWidth);
-    } else {
-        if (isAA) {
-            drawLinesAsQuads(points, count, isAA, isHairLine, .5f);
-        } else {
-            int verticesCount = count >> 1;
-            Vertex lines[verticesCount];
-            Vertex* vertices = &lines[0];
-            setupDrawVertices(vertices);
-            for (int i = 0; i < count; i += 4) {
-
-                const float left = fmin(points[i], points[i + 2]);
-                const float right = fmax(points[i], points[i + 2]);
-                const float top = fmin(points[i + 1], points[i + 3]);
-                const float bottom = fmax(points[i + 1], points[i + 3]);
-
-                Vertex::set(vertices++, points[i], points[i + 1]);
-                Vertex::set(vertices++, points[i + 2], points[i + 3]);
-                generatedVerticesCount += 2;
-                dirtyLayer(left, top,
-                        right == left ? left + 1 : right, bottom == top ? top + 1 : bottom,
-                        *mSnapshot->transform);
-            }
-
-            glLineWidth(1.0f);
-            glDrawArrays(GL_LINES, 0, generatedVerticesCount);
-        }
-    }
-}
-
 void OpenGLRenderer::drawPoints(float* points, int count, SkPaint* paint) {
     if (mSnapshot->isIgnored()) return;
 
@@ -1596,8 +1560,13 @@ void OpenGLRenderer::drawPoints(float* points, int count, SkPaint* paint) {
 
     // A stroke width of 0 has a special meaning in Skia:
     // it draws an unscaled 1px point
+    float strokeWidth = paint->getStrokeWidth();
     const bool isHairLine = paint->getStrokeWidth() == 0.0f;
-
+    if (isHairLine) {
+        // Now that we know it's hairline, we can set the effective width, to be used later
+        strokeWidth = 1.0f;
+    }
+    const float halfWidth = strokeWidth / 2;
     int alpha;
     SkXfermode::Mode mode;
     getAlphaAndMode(paint, &alpha, &mode);
@@ -1609,13 +1578,13 @@ void OpenGLRenderer::drawPoints(float* points, int count, SkPaint* paint) {
     TextureVertex* vertex = &pointsData[0];
 
     setupDraw();
-    setupDrawPoint(isHairLine ? 1.0f : paint->getStrokeWidth());
+    setupDrawPoint(strokeWidth);
     setupDrawColor(paint->getColor(), alpha);
     setupDrawColorFilter();
     setupDrawShader();
     setupDrawBlending(mode);
     setupDrawProgram();
-    setupDrawModelViewIdentity();
+    setupDrawModelViewIdentity(true);
     setupDrawColorUniforms();
     setupDrawColorFilterUniforms();
     setupDrawPointUniforms();
@@ -1625,6 +1594,11 @@ void OpenGLRenderer::drawPoints(float* points, int count, SkPaint* paint) {
     for (int i = 0; i < count; i += 2) {
         TextureVertex::set(vertex++, points[i], points[i + 1], 0.0f, 0.0f);
         generatedVerticesCount++;
+        float left = points[i] - halfWidth;
+        float right = points[i] + halfWidth;
+        float top = points[i + 1] - halfWidth;
+        float bottom = points [i + 1] + halfWidth;
+        dirtyLayer(left, top, right, bottom, *mSnapshot->transform);
     }
 
     glDrawArrays(GL_POINTS, 0, generatedVerticesCount);
diff --git a/libs/hwui/OpenGLRenderer.h b/libs/hwui/OpenGLRenderer.h
index 0276095..918e1fb 100644
--- a/libs/hwui/OpenGLRenderer.h
+++ b/libs/hwui/OpenGLRenderer.h
@@ -283,19 +283,6 @@ private:
     void drawAlphaBitmap(Texture* texture, float left, float top, SkPaint* paint);
 
     /**
-     * Draws a line as a quad. Called by drawLines() for all cases except hairline without AA.
-     *
-     * @param points The vertices of the lines. Every four entries specifies the x/y points
-     * of a single line segment.
-     * @param count The number of entries in the points array.
-     * @param isAA Whether the line is anti-aliased
-     * @param isHairline Whether the line has strokeWidth==0, which results in the line being
-     * one pixel wide on the display regardless of scale.
-     */
-    void drawLinesAsQuads(float *points, int count, bool isAA, bool isHairline,
-            float strokeWidth);
-
-    /**
      * Draws a textured rectangle with the specified texture. The specified coordinates
      * are transformed by the current snapshot's transform matrix.
      *
@@ -453,7 +440,7 @@ private:
             bool swapSrcDst = false);
     void setupDrawProgram();
     void setupDrawDirtyRegionsDisabled();
-    void setupDrawModelViewIdentity();
+    void setupDrawModelViewIdentity(bool offset = false);
     void setupDrawModelView(float left, float top, float right, float bottom,
             bool ignoreTransform = false, bool ignoreModelView = false);
     void setupDrawModelViewTranslate(float left, float top, float right, float bottom,
diff --git a/libs/hwui/Program.cpp b/libs/hwui/Program.cpp
index 2187f24..972dd87 100644
--- a/libs/hwui/Program.cpp
+++ b/libs/hwui/Program.cpp
@@ -124,8 +124,15 @@ GLuint Program::buildShader(const char* source, GLenum type) {
 }
 
 void Program::set(const mat4& projectionMatrix, const mat4& modelViewMatrix,
-        const mat4& transformMatrix) {
+        const mat4& transformMatrix, bool offset) {
     mat4 t(projectionMatrix);
+    if (offset) {
+        // offset screenspace xy by an amount that compensates for typical precision issues
+        // in GPU hardware that tends to paint hor/vert lines in pixels shifted up and to the left.
+        // This offset value is based on an assumption that some hardware may use as little
+        // as 12.4 precision, so we offset by slightly more than 1/16.
+        t.translate(.375, .375, 0);
+    }
     t.multiply(transformMatrix);
     t.multiply(modelViewMatrix);
 
diff --git a/libs/hwui/Program.h b/libs/hwui/Program.h
index afc6f3d..764cb05 100644
--- a/libs/hwui/Program.h
+++ b/libs/hwui/Program.h
@@ -81,7 +81,7 @@ public:
      * transform matrices.
      */
     void set(const mat4& projectionMatrix, const mat4& modelViewMatrix,
-             const mat4& transformMatrix);
+             const mat4& transformMatrix, bool offset = false);
 
     /**
      * Sets the color associated with this shader.
diff --git a/tests/HwAccelerationTest/AndroidManifest.xml b/tests/HwAccelerationTest/AndroidManifest.xml
index c763b1d..31b2ddd 100644
--- a/tests/HwAccelerationTest/AndroidManifest.xml
+++ b/tests/HwAccelerationTest/AndroidManifest.xml
@@ -406,6 +406,15 @@
         </activity>
 
         <activity
+                android:name="PointsActivity"
+                android:label="_Points">
+            <intent-filter>
+                <action android:name="android.intent.action.MAIN" />
+                <category android:name="android.intent.category.LAUNCHER" />
+            </intent-filter>
+        </activity>
+
+        <activity
                 android:name="Transform3dActivity"
                 android:label="_3d">
             <intent-filter>
diff --git a/tests/HwAccelerationTest/src/com/android/test/hwui/Lines2Activity.java b/tests/HwAccelerationTest/src/com/android/test/hwui/Lines2Activity.java
index ccf0631..55fab3f 100644
--- a/tests/HwAccelerationTest/src/com/android/test/hwui/Lines2Activity.java
+++ b/tests/HwAccelerationTest/src/com/android/test/hwui/Lines2Activity.java
@@ -42,8 +42,9 @@ public class Lines2Activity extends Activity {
         swView.setLayerType(View.LAYER_TYPE_SOFTWARE, null);
         frame.addView(swView);
         final LinesView hwBothView = new LinesView(this, 850, Color.GREEN);
-        // BUG: some lines not drawn or drawn with alpha when enabling hw layers
-//        hwBothView.setLayerType(View.LAYER_TYPE_HARDWARE, null);
+        // Don't actually need to render to a hw layer, but it's a good sanity-check that
+        // we're rendering to/from layers correctly
+        hwBothView.setLayerType(View.LAYER_TYPE_HARDWARE, null);
         frame.addView(hwBothView);
         final LinesView swBothView = new LinesView(this, 854, Color.RED);
         swBothView.setLayerType(View.LAYER_TYPE_SOFTWARE, null);
diff --git a/tests/HwAccelerationTest/src/com/android/test/hwui/PointsActivity.java b/tests/HwAccelerationTest/src/com/android/test/hwui/PointsActivity.java
new file mode 100644
index 0000000..b3fb7a1
--- /dev/null
+++ b/tests/HwAccelerationTest/src/com/android/test/hwui/PointsActivity.java
@@ -0,0 +1,128 @@
+/*
+ * Copyright (C) 2011 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.
+ */
+
+package com.android.test.hwui;
+
+import android.animation.ObjectAnimator;
+import android.app.Activity;
+import android.content.Context;
+import android.graphics.Canvas;
+import android.graphics.Color;
+import android.graphics.Paint;
+import android.graphics.drawable.ColorDrawable;
+import android.os.Bundle;
+import android.view.View;
+import android.widget.FrameLayout;
+import android.widget.LinearLayout;
+import android.widget.ProgressBar;
+import android.widget.SeekBar;
+
+@SuppressWarnings({"UnusedDeclaration"})
+public class PointsActivity extends Activity {
+
+    float mSeekValue = .5f;
+
+    @Override
+    protected void onCreate(Bundle savedInstanceState) {
+        super.onCreate(savedInstanceState);
+        getWindow().setBackgroundDrawable(new ColorDrawable(0xff000000));
+        SeekBar slider = new SeekBar(this);
+        LinearLayout container = new LinearLayout(this);
+        container.setOrientation(LinearLayout.VERTICAL);
+        setContentView(container);
+
+        container.addView(slider);
+        slider.setMax(100);
+        slider.setProgress(50);
+        FrameLayout frame = new FrameLayout(this);
+        final RenderingView gpuView = new RenderingView(this, Color.GREEN);
+        frame.addView(gpuView);
+        final RenderingView swView = new RenderingView(this, Color.RED);
+        swView.setLayerType(View.LAYER_TYPE_SOFTWARE, null);
+        frame.addView(swView);
+        container.addView(frame);
+
+        slider.setOnSeekBarChangeListener(new SeekBar.OnSeekBarChangeListener() {
+            @Override
+            public void onProgressChanged(SeekBar seekBar, int progress, boolean fromUser) {
+                mSeekValue = (float)progress / 100.0f;
+                float gpuAlpha = Math.min(2.0f * mSeekValue, 1f);
+                gpuView.setAlpha(gpuAlpha);
+                float swAlpha = Math.min((1 - mSeekValue) * 2.0f, 1f);
+                System.out.println("(gpuAlpha, swAlpha = " + gpuAlpha + ", " + swAlpha);
+                swView.setAlpha(swAlpha);
+            }
+
+            @Override
+            public void onStartTrackingTouch(SeekBar seekBar) {
+            }
+
+            @Override
+            public void onStopTrackingTouch(SeekBar seekBar) {
+            }
+        });
+    }
+
+    @Override
+    protected void onDestroy() {
+        super.onDestroy();
+    }
+
+    public static class RenderingView extends View {
+
+        private int mColor;
+
+        public RenderingView(Context c, int color) {
+            super(c);
+            mColor = color;
+        }
+
+        private void drawPoints(Canvas canvas, Paint p, float xOffset, float yOffset) {
+        }
+
+        @Override
+        protected void onDraw(Canvas canvas) {
+            super.onDraw(canvas);
+            Paint p = new Paint();
+            p.setColor(mColor);
+
+            float yOffset = 0;
+            for (int i = 0; i < 2; ++i) {
+                float xOffset = 0;
+
+                p.setStrokeWidth(0f);
+                p.setStrokeCap(Paint.Cap.SQUARE);
+                canvas.drawPoint(100 + xOffset, 100 + yOffset, p);
+                xOffset += 5;
+
+                p.setStrokeWidth(1f);
+                canvas.drawPoint(100 + xOffset, 100 + yOffset, p);
+                xOffset += 15;
+
+                p.setStrokeWidth(20);
+                canvas.drawPoint(100 + xOffset, 100 + yOffset, p);
+                xOffset += 30;
+
+                p.setStrokeCap(Paint.Cap.ROUND);
+                canvas.drawPoint(100 + xOffset, 100 + yOffset, p);
+
+                p.setAntiAlias(true);
+                yOffset += 30;
+            }
+
+        }
+    }
+}