Merge "Optimize Canvas::drawGlyphs"

7 files changed, 80 insertions, 78 deletions

diff --git a/libs/hwui/RecordingCanvas.cpp b/libs/hwui/RecordingCanvas.cpp
index 374c1b174640..d966372a7699 100644
--- a/libs/hwui/RecordingCanvas.cpp
+++ b/libs/hwui/RecordingCanvas.cpp
@@ -540,12 +540,13 @@ void RecordingCanvas::drawNinePatch(Bitmap& bitmap, const android::Res_png_9patc
 }
 
 // Text
-void RecordingCanvas::drawGlyphs(const uint16_t* glyphs, const float* positions, int glyphCount,
-            const SkPaint& paint, float x, float y, float boundsLeft, float boundsTop,
-            float boundsRight, float boundsBottom, float totalAdvance) {
-    if (!glyphs || !positions || glyphCount <= 0 || paint.nothingToDraw()) return;
-    glyphs = refBuffer<glyph_t>(glyphs, glyphCount);
-    positions = refBuffer<float>(positions, glyphCount * 2);
+void RecordingCanvas::drawGlyphs(ReadGlyphFunc glyphFunc, int glyphCount, const SkPaint& paint,
+        float x, float y, float boundsLeft, float boundsTop, float boundsRight, float boundsBottom,
+        float totalAdvance) {
+    if (glyphCount <= 0 || paint.nothingToDraw()) return;
+    uint16_t* glyphs = (glyph_t*)alloc().alloc<glyph_t>(glyphCount * sizeof(glyph_t));
+    float* positions = (float*)alloc().alloc<float>(2 * glyphCount * sizeof(float));
+    glyphFunc(glyphs, positions);
 
     // TODO: either must account for text shadow in bounds, or record separate ops for text shadows
     addOp(alloc().create_trivial<TextOp>(
diff --git a/libs/hwui/RecordingCanvas.h b/libs/hwui/RecordingCanvas.h
index 44181bd22397..682f244659e4 100644
--- a/libs/hwui/RecordingCanvas.h
+++ b/libs/hwui/RecordingCanvas.h
@@ -191,9 +191,8 @@ public:
     virtual bool drawTextAbsolutePos() const override { return false; }
 
 protected:
-    virtual void drawGlyphs(const uint16_t* text, const float* positions, int count,
-            const SkPaint& paint, float x, float y,
-            float boundsLeft, float boundsTop, float boundsRight, float boundsBottom,
+    virtual void drawGlyphs(ReadGlyphFunc glyphFunc, int count, const SkPaint& paint, float x,
+            float y, float boundsLeft, float boundsTop, float boundsRight, float boundsBottom,
             float totalAdvance) override;
     virtual void drawLayoutOnPath(const minikin::Layout& layout, float hOffset, float vOffset,
             const SkPaint& paint, const SkPath& path, size_t start, size_t end) override;
diff --git a/libs/hwui/SkiaCanvas.cpp b/libs/hwui/SkiaCanvas.cpp
index f91ea769ffc8..d344528c01ef 100644
--- a/libs/hwui/SkiaCanvas.cpp
+++ b/libs/hwui/SkiaCanvas.cpp
@@ -679,11 +679,10 @@ void SkiaCanvas::drawVectorDrawable(VectorDrawableRoot* vectorDrawable) {
 // Canvas draw operations: Text
 // ----------------------------------------------------------------------------
 
-void SkiaCanvas::drawGlyphs(const uint16_t* text, const float* positions, int count,
-        const SkPaint& paint, float x, float y,
-        float boundsLeft, float boundsTop, float boundsRight, float boundsBottom,
+void SkiaCanvas::drawGlyphs(ReadGlyphFunc glyphFunc, int count, const SkPaint& paint, float x,
+        float y, float boundsLeft, float boundsTop, float boundsRight, float boundsBottom,
         float totalAdvance) {
-     if (!text || !positions || count <= 0 || paint.nothingToDraw()) return;
+    if (count <= 0 || paint.nothingToDraw()) return;
     // Set align to left for drawing, as we don't want individual
     // glyphs centered or right-aligned; the offset above takes
     // care of all alignment.
@@ -695,10 +694,7 @@ void SkiaCanvas::drawGlyphs(const uint16_t* text, const float* positions, int co
 
     SkTextBlobBuilder builder;
     const SkTextBlobBuilder::RunBuffer& buffer = builder.allocRunPos(paintCopy, count, &bounds);
-    // TODO: we could reduce the number of memcpy's if the this were exposed further up
-    //       in the architecture.
-    memcpy(buffer.glyphs, text, count * sizeof(uint16_t));
-    memcpy(buffer.pos, positions, (count << 1) * sizeof(float));
+    glyphFunc(buffer.glyphs, buffer.pos);
 
     sk_sp<SkTextBlob> textBlob(builder.make());
     mCanvas->drawTextBlob(textBlob, 0, 0, paintCopy);
diff --git a/libs/hwui/SkiaCanvas.h b/libs/hwui/SkiaCanvas.h
index 34c3717557ff..c20c7dbad376 100644
--- a/libs/hwui/SkiaCanvas.h
+++ b/libs/hwui/SkiaCanvas.h
@@ -153,9 +153,8 @@ protected:
     void reset(SkCanvas* skiaCanvas);
     void drawDrawable(SkDrawable* drawable) { mCanvas->drawDrawable(drawable); }
 
-    virtual void drawGlyphs(const uint16_t* text, const float* positions, int count,
-            const SkPaint& paint, float x, float y,
-            float boundsLeft, float boundsTop, float boundsRight, float boundsBottom,
+    virtual void drawGlyphs(ReadGlyphFunc glyphFunc, int count, const SkPaint& paint, float x,
+            float y, float boundsLeft, float boundsTop, float boundsRight, float boundsBottom,
             float totalAdvance) override;
     virtual void drawLayoutOnPath(const minikin::Layout& layout, float hOffset, float vOffset,
             const SkPaint& paint, const SkPath& path, size_t start, size_t end) override;
diff --git a/libs/hwui/SkiaCanvasProxy.cpp b/libs/hwui/SkiaCanvasProxy.cpp
index 8630902b5f86..2e5a45ecbae9 100644
--- a/libs/hwui/SkiaCanvasProxy.cpp
+++ b/libs/hwui/SkiaCanvasProxy.cpp
@@ -288,7 +288,6 @@ void SkiaCanvasProxy::onDrawText(const void* text, size_t byteLength, SkScalar x
     GlyphIDConverter glyphs(text, byteLength, origPaint);
 
     // compute the glyph positions
-    std::unique_ptr<SkPoint[]> pointStorage(new SkPoint[glyphs.count]);
     std::unique_ptr<SkScalar[]> glyphWidths(new SkScalar[glyphs.count]);
     glyphs.paint.getTextWidths(glyphs.glyphIDs, glyphs.count << 1, glyphWidths.get());
 
@@ -322,22 +321,33 @@ void SkiaCanvasProxy::onDrawText(const void* text, size_t byteLength, SkScalar x
         xBaseline = x;
         yBaseline = y;
     }
-    pointStorage[0].set(xBaseline, yBaseline);
-
-    // setup the remaining glyph positions
-    if (glyphs.paint.isVerticalText()) {
-        for (int i = 1; i < glyphs.count; i++) {
-            pointStorage[i].set(xBaseline, glyphWidths[i-1] + pointStorage[i-1].fY);
-        }
-    } else {
-        for (int i = 1; i < glyphs.count; i++) {
-            pointStorage[i].set(glyphWidths[i-1] + pointStorage[i-1].fX, yBaseline);
-        }
-    }
 
     static_assert(sizeof(SkPoint) == sizeof(float)*2, "SkPoint is no longer two floats");
-    mCanvas->drawGlyphs(glyphs.glyphIDs, &pointStorage[0].fX, glyphs.count, glyphs.paint,
-                      x, y, bounds.fLeft, bounds.fTop, bounds.fRight, bounds.fBottom, 0);
+    auto glyphFunc = [&] (uint16_t* text, float* positions) {
+        memcpy(text, glyphs.glyphIDs, glyphs.count*sizeof(uint16_t));
+        size_t posIndex = 0;
+        // setup the first glyph position
+        positions[posIndex++] = xBaseline;
+        positions[posIndex++] = yBaseline;
+        // setup the remaining glyph positions
+        if (glyphs.paint.isVerticalText()) {
+            float yPosition = yBaseline;
+            for (int i = 1; i < glyphs.count; i++) {
+                positions[posIndex++] = xBaseline;
+                yPosition += glyphWidths[i-1];
+                positions[posIndex++] = yPosition;
+            }
+        } else {
+            float xPosition = xBaseline;
+            for (int i = 1; i < glyphs.count; i++) {
+                xPosition += glyphWidths[i-1];
+                positions[posIndex++] = xPosition;
+                positions[posIndex++] = yBaseline;
+            }
+        }
+    };
+    mCanvas->drawGlyphs(glyphFunc, glyphs.count, glyphs.paint, x, y, bounds.fLeft, bounds.fTop,
+            bounds.fRight, bounds.fBottom, 0);
 }
 
 void SkiaCanvasProxy::onDrawPosText(const void* text, size_t byteLength, const SkPoint pos[],
@@ -347,21 +357,12 @@ void SkiaCanvasProxy::onDrawPosText(const void* text, size_t byteLength, const S
 
     // convert to relative positions if necessary
     int x, y;
-    const SkPoint* posArray;
-    std::unique_ptr<SkPoint[]> pointStorage;
     if (mCanvas->drawTextAbsolutePos()) {
         x = 0;
         y = 0;
-        posArray = pos;
     } else {
         x = pos[0].fX;
         y = pos[0].fY;
-        pointStorage.reset(new SkPoint[glyphs.count]);
-        for (int i = 0; i < glyphs.count; i++) {
-            pointStorage[i].fX = pos[i].fX - x;
-            pointStorage[i].fY = pos[i].fY - y;
-        }
-        posArray = pointStorage.get();
     }
 
     // Compute conservative bounds.  If the content has already been processed
@@ -377,8 +378,19 @@ void SkiaCanvasProxy::onDrawPosText(const void* text, size_t byteLength, const S
     }
 
     static_assert(sizeof(SkPoint) == sizeof(float)*2, "SkPoint is no longer two floats");
-    mCanvas->drawGlyphs(glyphs.glyphIDs, &posArray[0].fX, glyphs.count, glyphs.paint, x, y,
-                      bounds.fLeft, bounds.fTop, bounds.fRight, bounds.fBottom, 0);
+    auto glyphFunc = [&] (uint16_t* text, float* positions) {
+        memcpy(text, glyphs.glyphIDs, glyphs.count*sizeof(uint16_t));
+        if (mCanvas->drawTextAbsolutePos()) {
+            memcpy(positions, pos, 2*glyphs.count*sizeof(float));
+        } else {
+            for (int i = 0, posIndex = 0; i < glyphs.count; i++) {
+                positions[posIndex++] = pos[i].fX - x;
+                positions[posIndex++] = pos[i].fY - y;
+            }
+        }
+    };
+    mCanvas->drawGlyphs(glyphFunc, glyphs.count, glyphs.paint, x, y, bounds.fLeft, bounds.fTop,
+            bounds.fRight, bounds.fBottom, 0);
 }
 
 void SkiaCanvasProxy::onDrawPosTextH(const void* text, size_t byteLength, const SkScalar xpos[],
diff --git a/libs/hwui/hwui/Canvas.cpp b/libs/hwui/hwui/Canvas.cpp
index c64a89de7296..679cb5021e27 100644
--- a/libs/hwui/hwui/Canvas.cpp
+++ b/libs/hwui/hwui/Canvas.cpp
@@ -80,13 +80,11 @@ static void simplifyPaint(int color, SkPaint* paint) {
 
 class DrawTextFunctor {
 public:
-    DrawTextFunctor(const minikin::Layout& layout, Canvas* canvas, uint16_t* glyphs, float* pos,
+    DrawTextFunctor(const minikin::Layout& layout, Canvas* canvas,
             const SkPaint& paint, float x, float y, minikin::MinikinRect& bounds,
             float totalAdvance)
         : layout(layout)
         , canvas(canvas)
-        , glyphs(glyphs)
-        , pos(pos)
         , paint(paint)
         , x(x)
         , y(y)
@@ -95,19 +93,21 @@ public:
     }
 
     void operator()(size_t start, size_t end) {
-        if (canvas->drawTextAbsolutePos()) {
-            for (size_t i = start; i < end; i++) {
-                glyphs[i] = layout.getGlyphId(i);
-                pos[2 * i] = x + layout.getX(i);
-                pos[2 * i + 1] = y + layout.getY(i);
+        auto glyphFunc = [&] (uint16_t* text, float* positions) {
+            if (canvas->drawTextAbsolutePos()) {
+                for (size_t i = start, textIndex = 0, posIndex = 0; i < end; i++) {
+                    text[textIndex++] = layout.getGlyphId(i);
+                    positions[posIndex++] = x + layout.getX(i);
+                    positions[posIndex++] = y + layout.getY(i);
+                }
+            } else {
+                for (size_t i = start, textIndex = 0, posIndex = 0; i < end; i++) {
+                    text[textIndex++] = layout.getGlyphId(i);
+                    positions[posIndex++] = layout.getX(i);
+                    positions[posIndex++] = layout.getY(i);
+                }
             }
-        } else {
-            for (size_t i = start; i < end; i++) {
-                glyphs[i] = layout.getGlyphId(i);
-                pos[2 * i] = layout.getX(i);
-                pos[2 * i + 1] = layout.getY(i);
-            }
-        }
+        };
 
         size_t glyphCount = end - start;
 
@@ -121,26 +121,24 @@ public:
             SkPaint outlinePaint(paint);
             simplifyPaint(darken ? SK_ColorWHITE : SK_ColorBLACK, &outlinePaint);
             outlinePaint.setStyle(SkPaint::kStrokeAndFill_Style);
-            canvas->drawGlyphs(glyphs + start, pos + (2 * start), glyphCount, outlinePaint, x, y,
-                    bounds.mLeft, bounds.mTop, bounds.mRight, bounds.mBottom, totalAdvance);
+            canvas->drawGlyphs(glyphFunc, glyphCount, outlinePaint, x, y, bounds.mLeft, bounds.mTop,
+                    bounds.mRight, bounds.mBottom, totalAdvance);
 
             // inner
             SkPaint innerPaint(paint);
             simplifyPaint(darken ? SK_ColorBLACK : SK_ColorWHITE, &innerPaint);
             innerPaint.setStyle(SkPaint::kFill_Style);
-            canvas->drawGlyphs(glyphs + start, pos + (2 * start), glyphCount, innerPaint, x, y,
-                    bounds.mLeft, bounds.mTop, bounds.mRight, bounds.mBottom, totalAdvance);
+            canvas->drawGlyphs(glyphFunc, glyphCount, innerPaint, x, y, bounds.mLeft, bounds.mTop,
+                    bounds.mRight, bounds.mBottom, totalAdvance);
         } else {
             // standard draw path
-            canvas->drawGlyphs(glyphs + start, pos + (2 * start), glyphCount, paint, x, y,
-                    bounds.mLeft, bounds.mTop, bounds.mRight, bounds.mBottom, totalAdvance);
+            canvas->drawGlyphs(glyphFunc, glyphCount, paint, x, y, bounds.mLeft, bounds.mTop,
+                    bounds.mRight, bounds.mBottom, totalAdvance);
         }
     }
 private:
     const minikin::Layout& layout;
     Canvas* canvas;
-    uint16_t* glyphs;
-    float* pos;
     const SkPaint& paint;
     float x;
     float y;
@@ -156,10 +154,6 @@ void Canvas::drawText(const uint16_t* text, int start, int count, int contextCou
     minikin::Layout layout = MinikinUtils::doLayout(
             &paint, bidiFlags, typeface, text, start, count, contextCount);
 
-    size_t nGlyphs = layout.nGlyphs();
-    std::unique_ptr<uint16_t[]> glyphs(new uint16_t[nGlyphs]);
-    std::unique_ptr<float[]> pos(new float[nGlyphs * 2]);
-
     x += MinikinUtils::xOffsetForTextAlign(&paint, layout);
 
     minikin::MinikinRect bounds;
@@ -173,8 +167,7 @@ void Canvas::drawText(const uint16_t* text, int start, int count, int contextCou
     // care of all alignment.
     paint.setTextAlign(Paint::kLeft_Align);
 
-    DrawTextFunctor f(layout, this, glyphs.get(), pos.get(),
-            paint, x, y, bounds, layout.getAdvance());
+    DrawTextFunctor f(layout, this, paint, x, y, bounds, layout.getAdvance());
     MinikinUtils::forFontRun(layout, &paint, f);
 }
 
diff --git a/libs/hwui/hwui/Canvas.h b/libs/hwui/hwui/Canvas.h
index 969d87716ce6..d6456006bd3e 100644
--- a/libs/hwui/hwui/Canvas.h
+++ b/libs/hwui/hwui/Canvas.h
@@ -65,6 +65,8 @@ class Tree;
 };
 typedef uirenderer::VectorDrawable::Tree VectorDrawableRoot;
 
+typedef std::function<void(uint16_t* text, float* positions)> ReadGlyphFunc;
+
 class Bitmap;
 class Paint;
 struct Typeface;
@@ -258,12 +260,12 @@ protected:
     void drawTextDecorations(float x, float y, float length, const SkPaint& paint);
 
     /**
+     * glyphFunc: valid only for the duration of the call and should not be cached.
      * drawText: count is of glyphs
      * totalAdvance: used to define width of text decorations (underlines, strikethroughs).
      */
-    virtual void drawGlyphs(const uint16_t* glyphs, const float* positions, int count,
-            const SkPaint& paint, float x, float y,
-            float boundsLeft, float boundsTop, float boundsRight, float boundsBottom,
+    virtual void drawGlyphs(ReadGlyphFunc glyphFunc, int count, const SkPaint& paint, float x,
+            float y, float boundsLeft, float boundsTop, float boundsRight, float boundsBottom,
             float totalAdvance) = 0;
     virtual void drawLayoutOnPath(const minikin::Layout& layout, float hOffset, float vOffset,
             const SkPaint& paint, const SkPath& path, size_t start, size_t end) = 0;