From 576b6a8a7994f649c0dbacfc34611d1580e16bd6 Mon Sep 17 00:00:00 2001 From: John Reck Date: Tue, 16 May 2017 22:02:36 +0000 Subject: [PATCH] Revert "Fix recent apps in system UI for Skia pipeline" This reverts commit 625dd56a45bfe95c5f1baa1891529503ff3374a9. Reason for revert: Caused a memory leak, b/38330767 Bug: 38136140 Bug: 38330767 Test: manual, verified memory isn't leaking doing the steps in b/38330767 Change-Id: I998bea04788d58ba6bad71c1691d5a3b33190c1b Merged-In: I98b2dfd750be57a15785808e2d5723616e2ce20a --- core/jni/android/graphics/Shader.cpp | 9 +- libs/hwui/hwui/Bitmap.cpp | 41 +-------- libs/hwui/hwui/Bitmap.h | 12 --- .../hwui/pipeline/skia/SkiaOpenGLPipeline.cpp | 83 ------------------- libs/hwui/pipeline/skia/SkiaOpenGLPipeline.h | 2 - .../hwui/pipeline/skia/SkiaOpenGLReadback.cpp | 1 - libs/hwui/pipeline/skia/SkiaPipeline.cpp | 10 +-- .../pipeline/skia/SkiaRecordingCanvas.cpp | 28 +++++-- libs/hwui/renderthread/RenderProxy.cpp | 12 --- libs/hwui/renderthread/RenderProxy.h | 2 - libs/hwui/renderthread/RenderThread.cpp | 19 ----- libs/hwui/renderthread/RenderThread.h | 3 - 12 files changed, 30 insertions(+), 192 deletions(-) diff --git a/core/jni/android/graphics/Shader.cpp b/core/jni/android/graphics/Shader.cpp index b529e37456478..214d97c213e49 100644 --- a/core/jni/android/graphics/Shader.cpp +++ b/core/jni/android/graphics/Shader.cpp @@ -60,17 +60,14 @@ static void Shader_safeUnref(JNIEnv* env, jobject o, jlong shaderHandle) { static jlong BitmapShader_constructor(JNIEnv* env, jobject o, jlong matrixPtr, jobject jbitmap, jint tileModeX, jint tileModeY) { const SkMatrix* matrix = reinterpret_cast(matrixPtr); - sk_sp image; + SkBitmap bitmap; if (jbitmap) { // Only pass a valid SkBitmap object to the constructor if the Bitmap exists. Otherwise, // we'll pass an empty SkBitmap to avoid crashing/excepting for compatibility. - image = android::bitmap::toBitmap(env, jbitmap).makeImage(nullptr); + android::bitmap::toBitmap(env, jbitmap).getSkBitmapForShaders(&bitmap); } - if (!image.get()) { - SkBitmap bitmap; - image = SkMakeImageFromRasterBitmap(bitmap, kNever_SkCopyPixelsMode); - } + sk_sp image = SkMakeImageFromRasterBitmap(bitmap, kNever_SkCopyPixelsMode); sk_sp baseShader = image->makeShader( (SkShader::TileMode)tileModeX, (SkShader::TileMode)tileModeY); diff --git a/libs/hwui/hwui/Bitmap.cpp b/libs/hwui/hwui/Bitmap.cpp index b0e486d874caf..d765584a7530c 100644 --- a/libs/hwui/hwui/Bitmap.cpp +++ b/libs/hwui/hwui/Bitmap.cpp @@ -16,7 +16,6 @@ #include "Bitmap.h" #include "Caches.h" -#include "pipeline/skia/SkiaOpenGLPipeline.h" #include "renderthread/EglManager.h" #include "renderthread/RenderThread.h" #include "renderthread/RenderProxy.h" @@ -35,15 +34,11 @@ #include #include #include -#include #include -#include namespace android { -Mutex Bitmap::gLock; - static bool computeAllocationSize(size_t rowBytes, int height, size_t* size) { int32_t rowBytes32 = SkToS32(rowBytes); int64_t bigSize = (int64_t) height * rowBytes32; @@ -322,7 +317,8 @@ sk_sp Bitmap::createFrom(sp graphicBuffer) { return nullptr; } SkImageInfo info = SkImageInfo::Make(graphicBuffer->getWidth(), graphicBuffer->getHeight(), - kRGBA_8888_SkColorType, kPremul_SkAlphaType, SkColorSpace::MakeSRGB()); + kRGBA_8888_SkColorType, kPremul_SkAlphaType, + SkColorSpace::MakeSRGB()); return sk_sp(new Bitmap(graphicBuffer.get(), info)); } @@ -397,7 +393,6 @@ Bitmap::Bitmap(GraphicBuffer* buffer, const SkImageInfo& info) mPixelStorage.hardware.buffer = buffer; buffer->incStrong(buffer); mRowBytes = bytesPerPixel(buffer->getPixelFormat()) * buffer->getStride(); - setImmutable(); // HW bitmaps are always immutable } Bitmap::~Bitmap() { @@ -491,13 +486,7 @@ void Bitmap::setAlphaType(SkAlphaType alphaType) { void Bitmap::getSkBitmap(SkBitmap* outBitmap) { outBitmap->setHasHardwareMipMap(mHasHardwareMipMap); if (isHardware()) { - if (uirenderer::Properties::isSkiaEnabled()) { - // TODO: add color correctness for Skia pipeline - pass null color space for now - outBitmap->allocPixels(SkImageInfo::Make(info().width(), info().height(), - info().colorType(), info().alphaType(), nullptr)); - } else { - outBitmap->allocPixels(info()); - } + outBitmap->allocPixels(info()); uirenderer::renderthread::RenderProxy::copyGraphicBufferInto(graphicBuffer(), outBitmap); return; } @@ -523,28 +512,4 @@ GraphicBuffer* Bitmap::graphicBuffer() { return nullptr; } -sk_sp Bitmap::makeImage(const uirenderer::renderthread::RenderThread* renderThread) { - AutoMutex _lock(gLock); //TODO: implement lock free solution - auto image = mImage; - //TODO: use new API SkImage::isValid() instead of SkImage::getTexture()->getContext() - if (!image.get() || (image->getTexture() && nullptr == image->getTexture()->getContext())) { - if (isHardware() && uirenderer::RenderPipelineType::SkiaGL - == uirenderer::Properties::getRenderPipelineType()) { - //TODO: add Vulkan support - if (renderThread) { - image = uirenderer::skiapipeline::SkiaOpenGLPipeline::makeTextureImage( - *renderThread, this); - } else { - image = uirenderer::renderthread::RenderProxy::makeTextureImage(this); - } - } else { - SkBitmap skiaBitmap; - getSkBitmapForShaders(&skiaBitmap); - image = SkMakeImageFromRasterBitmap(skiaBitmap, kNever_SkCopyPixelsMode); - } - mImage = image; - } - return image; -} - } // namespace android diff --git a/libs/hwui/hwui/Bitmap.h b/libs/hwui/hwui/Bitmap.h index 621e43979969c..da45f7697f56f 100644 --- a/libs/hwui/hwui/Bitmap.h +++ b/libs/hwui/hwui/Bitmap.h @@ -22,8 +22,6 @@ #include #include #include -#include -#include namespace android { @@ -113,13 +111,6 @@ public: } GraphicBuffer* graphicBuffer(); - - // makeImage creates or returns a cached SkImage. Can be invoked from UI or render thread. - // If invoked on the render thread, then RenderThread* argument is required. - // If not invoked on the render thread, then RenderThread* must be nullptr. - // makeImage is wrapping a gralloc buffer with an EGLImage and is passing a texture to Skia. - // This is a temporary implementation until Skia can wrap the gralloc buffer in a SkImage. - sk_sp makeImage(const uirenderer::renderthread::RenderThread*); protected: virtual bool onNewLockPixels(LockRec* rec) override; virtual void onUnlockPixels() override { }; @@ -154,9 +145,6 @@ private: GraphicBuffer* buffer; } hardware; } mPixelStorage; - - sk_sp mImage; - static Mutex gLock; }; } //namespace android \ No newline at end of file diff --git a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp index 4885873e232d6..ae1313101f3c0 100644 --- a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp +++ b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp @@ -28,8 +28,6 @@ #include #include -#include -#include using namespace android::uirenderer::renderthread; @@ -199,87 +197,6 @@ void SkiaOpenGLPipeline::invokeFunctor(const RenderThread& thread, Functor* func } } -static void deleteImageTexture(void* context) { - EGLImageKHR EGLimage = reinterpret_cast(context); - if (EGLimage != EGL_NO_IMAGE_KHR) { - EGLDisplay display = eglGetCurrentDisplay(); - if (EGL_NO_DISPLAY == display) { - display = eglGetDisplay(EGL_DEFAULT_DISPLAY); - } - eglDestroyImageKHR(display, EGLimage); - } -} - -sk_sp SkiaOpenGLPipeline::makeTextureImage( - const uirenderer::renderthread::RenderThread& renderThread, Bitmap* bitmap) { - renderThread.eglManager().initialize(); - - GraphicBuffer* buffer = bitmap->graphicBuffer(); - EGLDisplay display = eglGetCurrentDisplay(); - if (EGL_NO_DISPLAY == display) { - display = eglGetDisplay(EGL_DEFAULT_DISPLAY); - } - LOG_ALWAYS_FATAL_IF(!bitmap->isHardware(), - "Texture image requires a HW bitmap."); - // We use an EGLImage to access the content of the GraphicBuffer - // The EGL image is later bound to a 2D texture - EGLClientBuffer clientBuffer = (EGLClientBuffer) buffer->getNativeBuffer(); - EGLint imageAttrs[] = { EGL_IMAGE_PRESERVED_KHR, EGL_TRUE, EGL_NONE }; - EGLImageKHR EGLimage = eglCreateImageKHR(display, EGL_NO_CONTEXT, EGL_NATIVE_BUFFER_ANDROID, - clientBuffer, imageAttrs); - if (EGLimage == EGL_NO_IMAGE_KHR) { - ALOGW("Could not create EGL image, err =%s", - uirenderer::renderthread::EglManager::eglErrorString()); - return nullptr; - } - - GLuint textureId = 0; - glGenTextures(1, &textureId); - glBindTexture(GL_TEXTURE_EXTERNAL_OES, textureId); - glEGLImageTargetTexture2DOES(GL_TEXTURE_EXTERNAL_OES, EGLimage); - - GLenum status = GL_NO_ERROR; - while ((status = glGetError()) != GL_NO_ERROR) { - ALOGW("glEGLImageTargetTexture2DOES failed (%#x)", status); - eglDestroyImageKHR(display, EGLimage); - return nullptr; - } - - sk_sp grContext = sk_ref_sp(renderThread.getGrContext()); - grContext->resetContext(); - - GrGLTextureInfo textureInfo; - textureInfo.fTarget = GL_TEXTURE_EXTERNAL_OES; - textureInfo.fID = textureId; - - GrBackendTextureDesc textureDescription; - textureDescription.fWidth = bitmap->info().width(); - textureDescription.fHeight = bitmap->info().height(); - textureDescription.fOrigin = kTopLeft_GrSurfaceOrigin; - textureDescription.fTextureHandle = reinterpret_cast(&textureInfo); - PixelFormat format = buffer->getPixelFormat(); - switch (format) { - case PIXEL_FORMAT_RGBA_8888: - textureDescription.fConfig = kRGBA_8888_GrPixelConfig; - break; - case PIXEL_FORMAT_RGBA_FP16: - textureDescription.fConfig = kRGBA_half_GrPixelConfig; - break; - default: - eglDestroyImageKHR(display, EGLimage); - return nullptr; - } - - // TODO: add color correctness - pass null color space for now - sk_sp image = SkImage::MakeFromTexture(grContext.get(), textureDescription, - bitmap->info().alphaType(), nullptr, deleteImageTexture, EGLimage); - if (!image.get()) { - eglDestroyImageKHR(display, EGLimage); - return nullptr; - } - return image; -} - } /* namespace skiapipeline */ } /* namespace uirenderer */ } /* namespace android */ diff --git a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.h b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.h index f3ce189df5743..36685ddb17a78 100644 --- a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.h +++ b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.h @@ -46,8 +46,6 @@ public: bool isContextReady() override; static void invokeFunctor(const renderthread::RenderThread& thread, Functor* functor); - static sk_sp makeTextureImage( - const uirenderer::renderthread::RenderThread& renderThread, Bitmap* bitmap); private: renderthread::EglManager& mEglManager; diff --git a/libs/hwui/pipeline/skia/SkiaOpenGLReadback.cpp b/libs/hwui/pipeline/skia/SkiaOpenGLReadback.cpp index 1a6e709499924..a18d26471a297 100644 --- a/libs/hwui/pipeline/skia/SkiaOpenGLReadback.cpp +++ b/libs/hwui/pipeline/skia/SkiaOpenGLReadback.cpp @@ -61,7 +61,6 @@ CopyResult SkiaOpenGLReadback::copyImageInto(EGLImageKHR eglImage, const Matrix4 textureDescription.fTextureHandle = reinterpret_cast(&externalTexture); CopyResult copyResult = CopyResult::UnknownError; - // TODO: add color correctness - pass null color space for now sk_sp image(SkImage::MakeFromAdoptedTexture(grContext.get(), textureDescription)); if (image) { SkAutoLockPixels alp(*bitmap); diff --git a/libs/hwui/pipeline/skia/SkiaPipeline.cpp b/libs/hwui/pipeline/skia/SkiaPipeline.cpp index 349d4ccfa04db..10c1865ac50c7 100644 --- a/libs/hwui/pipeline/skia/SkiaPipeline.cpp +++ b/libs/hwui/pipeline/skia/SkiaPipeline.cpp @@ -155,11 +155,11 @@ void SkiaPipeline::prepareToDraw(const RenderThread& thread, Bitmap* bitmap) { GrContext* context = thread.getGrContext(); if (context) { ATRACE_FORMAT("Bitmap#prepareToDraw %dx%d", bitmap->width(), bitmap->height()); - auto image = bitmap->makeImage(&thread); - if (image.get() && !bitmap->isHardware()) { - SkImage_pinAsTexture(image.get(), context); - SkImage_unpinAsTexture(image.get(), context); - } + SkBitmap skiaBitmap; + bitmap->getSkBitmap(&skiaBitmap); + sk_sp image = SkMakeImageFromRasterBitmap(skiaBitmap, kNever_SkCopyPixelsMode); + SkImage_pinAsTexture(image.get(), context); + SkImage_unpinAsTexture(image.get(), context); } } diff --git a/libs/hwui/pipeline/skia/SkiaRecordingCanvas.cpp b/libs/hwui/pipeline/skia/SkiaRecordingCanvas.cpp index 5d7bbfa11c7d3..559d268b71f75 100644 --- a/libs/hwui/pipeline/skia/SkiaRecordingCanvas.cpp +++ b/libs/hwui/pipeline/skia/SkiaRecordingCanvas.cpp @@ -168,8 +168,11 @@ inline static const SkPaint* nonAAPaint(const SkPaint* origPaint, SkPaint* tmpPa } void SkiaRecordingCanvas::drawBitmap(Bitmap& bitmap, float left, float top, const SkPaint* paint) { - sk_sp image = bitmap.makeImage(nullptr); - if (!bitmap.isImmutable()) { + SkBitmap skBitmap; + bitmap.getSkBitmap(&skBitmap); + + sk_sp image = SkMakeImageFromRasterBitmap(skBitmap, kNever_SkCopyPixelsMode); + if (!skBitmap.isImmutable()) { mDisplayList->mMutableImages.push_back(image.get()); } SkPaint tmpPaint; @@ -178,10 +181,12 @@ void SkiaRecordingCanvas::drawBitmap(Bitmap& bitmap, float left, float top, cons void SkiaRecordingCanvas::drawBitmap(Bitmap& hwuiBitmap, const SkMatrix& matrix, const SkPaint* paint) { + SkBitmap bitmap; + hwuiBitmap.getSkBitmap(&bitmap); SkAutoCanvasRestore acr(&mRecorder, true); concat(matrix); - sk_sp image = hwuiBitmap.makeImage(nullptr); - if (!hwuiBitmap.isImmutable()) { + sk_sp image = SkMakeImageFromRasterBitmap(bitmap, kNever_SkCopyPixelsMode); + if (!bitmap.isImmutable()) { mDisplayList->mMutableImages.push_back(image.get()); } SkPaint tmpPaint; @@ -191,10 +196,12 @@ void SkiaRecordingCanvas::drawBitmap(Bitmap& hwuiBitmap, const SkMatrix& matrix, void SkiaRecordingCanvas::drawBitmap(Bitmap& hwuiBitmap, float srcLeft, float srcTop, float srcRight, float srcBottom, float dstLeft, float dstTop, float dstRight, float dstBottom, const SkPaint* paint) { + SkBitmap bitmap; + hwuiBitmap.getSkBitmap(&bitmap); SkRect srcRect = SkRect::MakeLTRB(srcLeft, srcTop, srcRight, srcBottom); SkRect dstRect = SkRect::MakeLTRB(dstLeft, dstTop, dstRight, dstBottom); - sk_sp image = hwuiBitmap.makeImage(nullptr); - if (!hwuiBitmap.isImmutable()) { + sk_sp image = SkMakeImageFromRasterBitmap(bitmap, kNever_SkCopyPixelsMode); + if (!bitmap.isImmutable()) { mDisplayList->mMutableImages.push_back(image.get()); } SkPaint tmpPaint; @@ -203,8 +210,11 @@ void SkiaRecordingCanvas::drawBitmap(Bitmap& hwuiBitmap, float srcLeft, float sr void SkiaRecordingCanvas::drawNinePatch(Bitmap& hwuiBitmap, const Res_png_9patch& chunk, float dstLeft, float dstTop, float dstRight, float dstBottom, const SkPaint* paint) { + SkBitmap bitmap; + hwuiBitmap.getSkBitmap(&bitmap); + SkCanvas::Lattice lattice; - NinePatchUtils::SetLatticeDivs(&lattice, chunk, hwuiBitmap.width(), hwuiBitmap.height()); + NinePatchUtils::SetLatticeDivs(&lattice, chunk, bitmap.width(), bitmap.height()); lattice.fFlags = nullptr; int numFlags = 0; @@ -221,8 +231,8 @@ void SkiaRecordingCanvas::drawNinePatch(Bitmap& hwuiBitmap, const Res_png_9patch lattice.fBounds = nullptr; SkRect dst = SkRect::MakeLTRB(dstLeft, dstTop, dstRight, dstBottom); - sk_sp image = hwuiBitmap.makeImage(nullptr); - if (!hwuiBitmap.isImmutable()) { + sk_sp image = SkMakeImageFromRasterBitmap(bitmap, kNever_SkCopyPixelsMode); + if (!bitmap.isImmutable()) { mDisplayList->mMutableImages.push_back(image.get()); } diff --git a/libs/hwui/renderthread/RenderProxy.cpp b/libs/hwui/renderthread/RenderProxy.cpp index 5c2ec0ea3c88b..a1f1717e6b962 100644 --- a/libs/hwui/renderthread/RenderProxy.cpp +++ b/libs/hwui/renderthread/RenderProxy.cpp @@ -693,18 +693,6 @@ int RenderProxy::copyGraphicBufferInto(GraphicBuffer* buffer, SkBitmap* bitmap) } } -CREATE_BRIDGE2(makeTextureImage, RenderThread* thread, Bitmap* bitmap) { - return args->thread->makeTextureImage(args->bitmap).release(); -} - -sk_sp RenderProxy::makeTextureImage(Bitmap* bitmap) { - SETUP_TASK(makeTextureImage); - args->bitmap = bitmap; - args->thread = &RenderThread::getInstance(); - sk_sp hardwareImage(reinterpret_cast(staticPostAndWait(task))); - return hardwareImage; -} - void RenderProxy::post(RenderTask* task) { mRenderThread.queue(task); } diff --git a/libs/hwui/renderthread/RenderProxy.h b/libs/hwui/renderthread/RenderProxy.h index 97ad796b6065f..a60ed55c70d25 100644 --- a/libs/hwui/renderthread/RenderProxy.h +++ b/libs/hwui/renderthread/RenderProxy.h @@ -135,8 +135,6 @@ public: static sk_sp allocateHardwareBitmap(SkBitmap& bitmap); static int copyGraphicBufferInto(GraphicBuffer* buffer, SkBitmap* bitmap); - - static sk_sp makeTextureImage(Bitmap* bitmap); private: RenderThread& mRenderThread; CanvasContext* mContext; diff --git a/libs/hwui/renderthread/RenderThread.cpp b/libs/hwui/renderthread/RenderThread.cpp index d62b55633e2da..1450ec98dabf0 100644 --- a/libs/hwui/renderthread/RenderThread.cpp +++ b/libs/hwui/renderthread/RenderThread.cpp @@ -17,7 +17,6 @@ #include "RenderThread.h" #include "../renderstate/RenderState.h" -#include "../pipeline/skia/SkiaOpenGLPipeline.h" #include "../pipeline/skia/SkiaOpenGLReadback.h" #include "CanvasContext.h" #include "EglManager.h" @@ -434,24 +433,6 @@ RenderTask* RenderThread::nextTask(nsecs_t* nextWakeup) { return next; } -sk_sp RenderThread::makeTextureImage(Bitmap* bitmap) { - auto renderType = Properties::getRenderPipelineType(); - sk_sp hardwareImage; - switch (renderType) { - case RenderPipelineType::SkiaGL: - hardwareImage = skiapipeline::SkiaOpenGLPipeline::makeTextureImage(*this, bitmap); - break; - case RenderPipelineType::SkiaVulkan: - //TODO: add Vulkan support - break; - default: - LOG_ALWAYS_FATAL("makeTextureImage: canvas context type %d not supported", - (int32_t) renderType); - break; - } - return hardwareImage; -} - } /* namespace renderthread */ } /* namespace uirenderer */ } /* namespace android */ diff --git a/libs/hwui/renderthread/RenderThread.h b/libs/hwui/renderthread/RenderThread.h index 34542c6021ec6..9bc5985e5b162 100644 --- a/libs/hwui/renderthread/RenderThread.h +++ b/libs/hwui/renderthread/RenderThread.h @@ -33,7 +33,6 @@ namespace android { -class Bitmap; class DisplayEventReceiver; namespace uirenderer { @@ -105,8 +104,6 @@ public: VulkanManager& vulkanManager() { return *mVkManager; } - sk_sp makeTextureImage(Bitmap* bitmap); - protected: virtual bool threadLoop() override;