From 62feb3a0b4690144a067080ab17beae160ea6320 Mon Sep 17 00:00:00 2001 From: Matt Sarett Date: Tue, 20 Sep 2016 10:34:11 -0400 Subject: [PATCH] Use SkMakeBitmapShader, avoid bitmap copy CreateBitmapShader now forces a copy. This updates the call sites to use SkMakeBitmapShader (in SkImagePriv.h) with kNever_SkCopyPixelsMode. This maintains the behavior where apps can modify the bitmap in the shader after creating the shader. This also ensures that the texture cache will work (since it's based off of SkPixelRefs). BUG:31594626 Change-Id: Ic75cb6cdc05c750b7946208e48a8127838d9c2f8 --- core/jni/android/graphics/Shader.cpp | 14 ++++++---- libs/hwui/SkiaCanvas.cpp | 12 ++++++--- libs/hwui/tests/unit/RecordingCanvasTests.cpp | 26 ++++++++++++------- libs/hwui/tests/unit/SkiaBehaviorTests.cpp | 10 ++++--- 4 files changed, 40 insertions(+), 22 deletions(-) diff --git a/core/jni/android/graphics/Shader.cpp b/core/jni/android/graphics/Shader.cpp index de32dd92c6e45..513a0830a00b0 100644 --- a/core/jni/android/graphics/Shader.cpp +++ b/core/jni/android/graphics/Shader.cpp @@ -1,5 +1,6 @@ #include "GraphicsJNI.h" #include "SkGradientShader.h" +#include "SkImagePriv.h" #include "SkShader.h" #include "SkXfermode.h" #include "core_jni_helpers.h" @@ -94,12 +95,15 @@ static jlong BitmapShader_constructor(JNIEnv* env, jobject o, jobject jbitmap, // we'll pass an empty SkBitmap to avoid crashing/excepting for compatibility. GraphicsJNI::getSkBitmap(env, jbitmap, &bitmap); } - SkShader* s = SkShader::CreateBitmapShader(bitmap, - (SkShader::TileMode)tileModeX, - (SkShader::TileMode)tileModeY); + sk_sp s = SkMakeBitmapShader(bitmap, + (SkShader::TileMode)tileModeX, + (SkShader::TileMode)tileModeY, + nullptr, + kNever_SkCopyPixelsMode, + nullptr); - ThrowIAE_IfNull(env, s); - return reinterpret_cast(s); + ThrowIAE_IfNull(env, s.get()); + return reinterpret_cast(s.release()); } /////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/libs/hwui/SkiaCanvas.cpp b/libs/hwui/SkiaCanvas.cpp index 09775496af722..89af090fab38b 100644 --- a/libs/hwui/SkiaCanvas.cpp +++ b/libs/hwui/SkiaCanvas.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -593,10 +594,13 @@ void SkiaCanvas::drawBitmapMesh(const SkBitmap& bitmap, int meshWidth, int meshH if (paint) { tmpPaint = *paint; } - SkShader* shader = SkShader::CreateBitmapShader(bitmap, - SkShader::kClamp_TileMode, - SkShader::kClamp_TileMode); - SkSafeUnref(tmpPaint.setShader(shader)); + sk_sp shader = SkMakeBitmapShader(bitmap, + SkShader::kClamp_TileMode, + SkShader::kClamp_TileMode, + nullptr, + kNever_SkCopyPixelsMode, + nullptr); + tmpPaint.setShader(std::move(shader)); mCanvas->drawVertices(SkCanvas::kTriangles_VertexMode, ptCount, (SkPoint*)vertices, texs, (const SkColor*)colors, NULL, indices, diff --git a/libs/hwui/tests/unit/RecordingCanvasTests.cpp b/libs/hwui/tests/unit/RecordingCanvasTests.cpp index fdf0b547fb54c..edc7191459b7a 100644 --- a/libs/hwui/tests/unit/RecordingCanvasTests.cpp +++ b/libs/hwui/tests/unit/RecordingCanvasTests.cpp @@ -740,10 +740,13 @@ TEST(RecordingCanvas, refBitmapInShader_bitmapShader) { SkBitmap bitmap = TestUtils::createSkBitmap(100, 100); auto dl = TestUtils::createDisplayList(100, 100, [&bitmap](RecordingCanvas& canvas) { SkPaint paint; - SkAutoTUnref shader(SkShader::CreateBitmapShader(bitmap, + sk_sp shader = SkMakeBitmapShader(bitmap, SkShader::TileMode::kClamp_TileMode, - SkShader::TileMode::kClamp_TileMode)); - paint.setShader(shader); + SkShader::TileMode::kClamp_TileMode, + nullptr, + kNever_SkCopyPixelsMode, + nullptr); + paint.setShader(std::move(shader)); canvas.drawRoundRect(0, 0, 100, 100, 20.0f, 20.0f, paint); }); auto& bitmaps = dl->getBitmapResources(); @@ -754,21 +757,24 @@ TEST(RecordingCanvas, refBitmapInShader_composeShader) { SkBitmap bitmap = TestUtils::createSkBitmap(100, 100); auto dl = TestUtils::createDisplayList(100, 100, [&bitmap](RecordingCanvas& canvas) { SkPaint paint; - SkAutoTUnref shader1(SkShader::CreateBitmapShader(bitmap, + sk_sp shader1 = SkMakeBitmapShader(bitmap, SkShader::TileMode::kClamp_TileMode, - SkShader::TileMode::kClamp_TileMode)); + SkShader::TileMode::kClamp_TileMode, + nullptr, + kNever_SkCopyPixelsMode, + nullptr); SkPoint center; center.set(50, 50); SkColor colors[2]; colors[0] = Color::Black; colors[1] = Color::White; - SkAutoTUnref shader2(SkGradientShader::CreateRadial(center, 50, colors, nullptr, 2, - SkShader::TileMode::kRepeat_TileMode)); + sk_sp shader2 = SkGradientShader::MakeRadial(center, 50, colors, nullptr, 2, + SkShader::TileMode::kRepeat_TileMode); - SkAutoTUnref composeShader(SkShader::CreateComposeShader(shader1, shader2, - SkXfermode::Mode::kMultiply_Mode)); - paint.setShader(composeShader); + sk_sp composeShader = SkShader::MakeComposeShader(std::move(shader1), std::move(shader2), + SkXfermode::Mode::kMultiply_Mode); + paint.setShader(std::move(composeShader)); canvas.drawRoundRect(0, 0, 100, 100, 20.0f, 20.0f, paint); }); auto& bitmaps = dl->getBitmapResources(); diff --git a/libs/hwui/tests/unit/SkiaBehaviorTests.cpp b/libs/hwui/tests/unit/SkiaBehaviorTests.cpp index cd759cb3d3b6c..e6689a37fd31e 100644 --- a/libs/hwui/tests/unit/SkiaBehaviorTests.cpp +++ b/libs/hwui/tests/unit/SkiaBehaviorTests.cpp @@ -17,8 +17,9 @@ #include "tests/common/TestUtils.h" #include -#include #include +#include +#include using namespace android; using namespace android::uirenderer; @@ -29,10 +30,13 @@ using namespace android::uirenderer; */ TEST(SkiaBehavior, CreateBitmapShader1x1) { SkBitmap origBitmap = TestUtils::createSkBitmap(1, 1); - SkAutoTUnref s(SkShader::CreateBitmapShader( + sk_sp s = SkMakeBitmapShader( origBitmap, SkShader::kClamp_TileMode, - SkShader::kRepeat_TileMode)); + SkShader::kRepeat_TileMode, + nullptr, + kNever_SkCopyPixelsMode, + nullptr); SkBitmap bitmap; SkShader::TileMode xy[2];