Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 42a5407f authored by Chris Craik's avatar Chris Craik
Browse files

Fix RecordingCanvas::refPaint, add tests

Also add text align support to TestUtils::drawTextToCanvas

Change-Id: I105adb0d15e697c03adfd00a56e8ec9265953ff1
parent 95cdbd6f
Loading
Loading
Loading
Loading
+4 −16
Original line number Diff line number Diff line
@@ -205,10 +205,6 @@ private:
        return dstBuffer;
    }

    inline char* refText(const char* text, size_t byteLength) {
        return (char*) refBuffer<uint8_t>((uint8_t*)text, byteLength);
    }

    inline const SkPath* refPath(const SkPath* path) {
        if (!path) return nullptr;

@@ -220,13 +216,8 @@ private:
    }

    /**
     * Returns a RenderThread-safe, const copy of the SkPaint parameter passed in (with deduping
     * based on paint generation ID)
     *
     * Note that this forces Left_Align, since drawText glyph rendering expects left alignment,
     * since alignment offsetting has been done at a higher level. This is done to essentially all
     * copied paints, since the deduping can mean a paint is shared by drawText commands and other
     * types (which wouldn't care about alignment).
     * Returns a RenderThread-safe, const copy of the SkPaint parameter passed in
     * (with deduping based on paint hash / equality check)
     */
    inline const SkPaint* refPaint(const SkPaint* paint) {
        if (!paint) return nullptr;
@@ -246,11 +237,8 @@ private:
        // In the unlikely event that 2 unique paints have the same hash we do a
        // object equality check to ensure we don't erroneously dedup them.
        if (cachedPaint == nullptr || *cachedPaint != *paint) {
            SkPaint* copy = new SkPaint(*paint);
            copy->setTextAlign(SkPaint::kLeft_Align);

            cachedPaint = copy;
            mDisplayList->paints.emplace_back(copy);
            cachedPaint = new SkPaint(*paint);
            mDisplayList->paints.emplace_back(cachedPaint);
            // replaceValueFor() performs an add if the entry doesn't exist
            mPaintMap.replaceValueFor(key, cachedPaint);
            refBitmapsInShader(cachedPaint->getShader());
+1 −0
Original line number Diff line number Diff line
@@ -184,6 +184,7 @@ TEST(OpReorderer, textStrikethroughBatching) {
        textPaint.setAntiAlias(true);
        textPaint.setTextSize(20);
        textPaint.setStrikeThruText(true);
        textPaint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
        for (int i = 0; i < LOOPS; i++) {
            TestUtils::drawTextToCanvas(&canvas, "test text", textPaint, 10, 100 * (i + 1));
        }
+44 −2
Original line number Diff line number Diff line
@@ -75,6 +75,7 @@ TEST(RecordingCanvas, drawText) {
        SkPaint paint;
        paint.setAntiAlias(true);
        paint.setTextSize(20);
        paint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
        TestUtils::drawTextToCanvas(&canvas, "test text", paint, 25, 25);
    });

@@ -95,6 +96,7 @@ TEST(RecordingCanvas, drawText_strikeThruAndUnderline) {
        SkPaint paint;
        paint.setAntiAlias(true);
        paint.setTextSize(20);
        paint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
        for (int i = 0; i < 2; i++) {
            for (int j = 0; j < 2; j++) {
                paint.setUnderlineText(i != 0);
@@ -126,6 +128,7 @@ TEST(RecordingCanvas, drawText_forceAlignLeft) {
        SkPaint paint;
        paint.setAntiAlias(true);
        paint.setTextSize(20);
        paint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
        paint.setTextAlign(SkPaint::kLeft_Align);
        TestUtils::drawTextToCanvas(&canvas, "test text", paint, 25, 25);
        paint.setTextAlign(SkPaint::kCenter_Align);
@@ -135,12 +138,17 @@ TEST(RecordingCanvas, drawText_forceAlignLeft) {
    });

    int count = 0;
    playbackOps(*dl, [&count](const RecordedOp& op) {
    float lastX = FLT_MAX;
    playbackOps(*dl, [&count, &lastX](const RecordedOp& op) {
        count++;
        ASSERT_EQ(RecordedOpId::TextOp, op.opId);
        EXPECT_EQ(SkPaint::kLeft_Align, op.paint->getTextAlign())
                << "recorded drawText commands must force kLeft_Align on their paint";
        EXPECT_EQ(SkPaint::kGlyphID_TextEncoding, op.paint->getTextEncoding()); // verify TestUtils

        // verify TestUtils alignment offsetting (TODO: move asserts to Canvas base class)
        EXPECT_GT(lastX, ((const TextOp&)op).x)
                << "x coordinate should reduce across each of the draw commands, from alignment";
        lastX = ((const TextOp&)op).x;
    });
    ASSERT_EQ(3, count);
}
@@ -334,5 +342,39 @@ TEST(RecordingCanvas, insertReorderBarrier) {
    EXPECT_TRUE(chunks[1].reorderChildren);
}

TEST(RecordingCanvas, refPaint) {
    SkPaint paint;
    paint.setAntiAlias(true);
    paint.setTextSize(20);
    paint.setTextAlign(SkPaint::kLeft_Align);
    paint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);

    auto dl = TestUtils::createDisplayList<RecordingCanvas>(200, 200, [&paint](RecordingCanvas& canvas) {
        paint.setColor(SK_ColorBLUE);
        // first three should use same paint
        canvas.drawRect(0, 0, 200, 10, paint);
        SkPaint paintCopy(paint);
        canvas.drawRect(0, 10, 200, 20, paintCopy);
        TestUtils::drawTextToCanvas(&canvas, "helloworld", paint, 50, 25);

        // only here do we use different paint ptr
        paint.setColor(SK_ColorRED);
        canvas.drawRect(0, 20, 200, 30, paint);
    });
    auto ops = dl->getOps();
    ASSERT_EQ(4u, ops.size());

    // first three are the same
    EXPECT_NE(nullptr, ops[0]->paint);
    EXPECT_NE(&paint, ops[0]->paint);
    EXPECT_EQ(ops[0]->paint, ops[1]->paint);
    EXPECT_EQ(ops[0]->paint, ops[2]->paint);

    // last is different, but still copied / non-null
    EXPECT_NE(nullptr, ops[3]->paint);
    EXPECT_NE(ops[0]->paint, ops[3]->paint);
    EXPECT_NE(&paint, ops[3]->paint);
}

} // namespace uirenderer
} // namespace android
+45 −33
Original line number Diff line number Diff line
@@ -37,10 +37,10 @@ SkColor TestUtils::interpolateColor(float fraction, SkColor start, SkColor end)
}

void TestUtils::drawTextToCanvas(TestCanvas* canvas, const char* text,
        const SkPaint& inPaint, float x, float y) {
   // copy to force TextEncoding (which JNI layer would have done)
   SkPaint paint(inPaint);
   paint.setTextEncoding(SkPaint::kGlyphID_TextEncoding);
        const SkPaint& paint, float x, float y) {
    // drawing text requires GlyphID TextEncoding (which JNI layer would have done)
    LOG_ALWAYS_FATAL_IF(paint.getTextEncoding() != SkPaint::kGlyphID_TextEncoding,
            "must use glyph encoding");

    SkMatrix identity;
    identity.setIdentity();
@@ -72,8 +72,20 @@ void TestUtils::drawTextToCanvas(TestCanvas* canvas, const char* text,
        paint.getTextWidths(&glyph, sizeof(glyph), &skWidth, NULL);
        totalAdvance += skWidth;
    }

    // apply alignment via x parameter (which JNI layer would have done)
    if (paint.getTextAlign() == SkPaint::kCenter_Align) {
        x -= totalAdvance / 2;
    } else if (paint.getTextAlign() == SkPaint::kRight_Align) {
        x -= totalAdvance;
    }

    bounds.translate(x, y);
   canvas->drawText(glyphs.data(), positions.data(), glyphs.size(), paint, x, y,

    // Force left alignment, since alignment offset is already baked in
    SkPaint alignPaintCopy(paint);
    alignPaintCopy.setTextAlign(SkPaint::kLeft_Align);
    canvas->drawText(glyphs.data(), positions.data(), glyphs.size(), alignPaintCopy, x, y,
                bounds.left, bounds.top, bounds.right, bounds.bottom, totalAdvance);
}

+1 −1
Original line number Diff line number Diff line
@@ -197,7 +197,7 @@ public:
    static SkColor interpolateColor(float fraction, SkColor start, SkColor end);

    static void drawTextToCanvas(TestCanvas* canvas, const char* text,
            const SkPaint& inPaint, float x, float y);
            const SkPaint& paint, float x, float y);

private:
    static void syncHierarchyPropertiesAndDisplayListImpl(RenderNode* node) {