From abbedfc84ff28ba2d47c110c9322f2c4133411e7 Mon Sep 17 00:00:00 2001 From: John Reck Date: Thu, 6 Jul 2017 15:27:23 -0700 Subject: Add missing notifyPixelsChanged Fixes: 63400947 Test: CTS test PixelCopyTest#testReuseBitmap Change-Id: Iad6fe331f84415528c1858a1fdbf26bce784cd53 --- libs/hwui/OpenGLReadback.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'libs/hwui/OpenGLReadback.cpp') diff --git a/libs/hwui/OpenGLReadback.cpp b/libs/hwui/OpenGLReadback.cpp index 19d5d9d2250e..e798edfc095a 100644 --- a/libs/hwui/OpenGLReadback.cpp +++ b/libs/hwui/OpenGLReadback.cpp @@ -238,6 +238,7 @@ inline CopyResult copyTextureInto(Caches& caches, RenderState& renderState, // TODO: We should convert to linear space when the target is RGBA16F glReadPixels(0, 0, bitmap->width(), bitmap->height(), format, type, bitmap->getPixels()); + bitmap->notifyPixelsChanged(); } // Cleanup -- cgit v1.2.3-59-g8ed1b From df6520efd78e5c4a06b1b50149280d03e828326d Mon Sep 17 00:00:00 2001 From: Stan Iliev Date: Mon, 17 Jul 2017 18:50:16 -0400 Subject: Fix Skia pipeline readback for non-portrait mode Apply correctly the image transform matrix passed to SkiaOpenGLReadback::copyImageInto. This is fixing 13 of 14 failing android.view.cts.PixelCopyTest tests (the last failing test is caused by missing wide gamut support). Test: Ran CtsViewTestCases CTS tests for Skia and HWUI pipelines. Bug: 63629735 Change-Id: I67fb06968e21f9aa88973c076870b5557f15d483 --- libs/hwui/OpenGLReadback.cpp | 11 +++--- libs/hwui/pipeline/skia/SkiaOpenGLReadback.cpp | 46 +++++++++++++++++--------- 2 files changed, 36 insertions(+), 21 deletions(-) (limited to 'libs/hwui/OpenGLReadback.cpp') diff --git a/libs/hwui/OpenGLReadback.cpp b/libs/hwui/OpenGLReadback.cpp index e798edfc095a..025503b15975 100644 --- a/libs/hwui/OpenGLReadback.cpp +++ b/libs/hwui/OpenGLReadback.cpp @@ -85,11 +85,6 @@ CopyResult OpenGLReadback::copyGraphicBufferInto(GraphicBuffer* graphicBuffer, uint32_t width = graphicBuffer->getWidth(); uint32_t height = graphicBuffer->getHeight(); - // If this is a 90 or 270 degree rotation we need to swap width/height - // This is a fuzzy way of checking that. - if (texTransform[Matrix4::kSkewX] >= 0.5f || texTransform[Matrix4::kSkewX] <= -0.5f) { - std::swap(width, height); - } CopyResult copyResult = copyImageInto(sourceImage, texTransform, width, height, srcRect, bitmap); @@ -254,6 +249,12 @@ CopyResult OpenGLReadbackImpl::copyImageInto(EGLImageKHR eglImage, const Matrix4& imgTransform, int imgWidth, int imgHeight, const Rect& srcRect, SkBitmap* bitmap) { + // If this is a 90 or 270 degree rotation we need to swap width/height + // This is a fuzzy way of checking that. + if (imgTransform[Matrix4::kSkewX] >= 0.5f || imgTransform[Matrix4::kSkewX] <= -0.5f) { + std::swap(imgWidth, imgHeight); + } + Caches& caches = Caches::getInstance(); GLuint sourceTexId; // Create a 2D texture to sample from the EGLImage diff --git a/libs/hwui/pipeline/skia/SkiaOpenGLReadback.cpp b/libs/hwui/pipeline/skia/SkiaOpenGLReadback.cpp index 69b9c01fe023..311419dd2b65 100644 --- a/libs/hwui/pipeline/skia/SkiaOpenGLReadback.cpp +++ b/libs/hwui/pipeline/skia/SkiaOpenGLReadback.cpp @@ -60,37 +60,51 @@ CopyResult SkiaOpenGLReadback::copyImageInto(EGLImageKHR eglImage, const Matrix4 sk_sp image(SkImage::MakeFromAdoptedTexture(grContext.get(), backendTexture, kTopLeft_GrSurfaceOrigin)); if (image) { - // convert to Skia data structures - const SkRect bufferRect = SkRect::MakeIWH(imgWidth, imgHeight); - SkRect skiaSrcRect = srcRect.toSkRect(); SkMatrix textureMatrix; imgTransform.copyTo(textureMatrix); - // remove the y-flip applied to the matrix so that we can scale the srcRect. - // This flip is not needed as we specify the origin of the texture when we - // wrap it as an SkImage. + // remove the y-flip applied to the matrix SkMatrix yFlip = SkMatrix::MakeScale(1, -1); yFlip.postTranslate(0,1); textureMatrix.preConcat(yFlip); - // copy the entire src if the rect is empty - if (skiaSrcRect.isEmpty()) { - skiaSrcRect = bufferRect; - } + // multiply by image size, because textureMatrix maps to [0..1] range + textureMatrix[SkMatrix::kMTransX] *= imgWidth; + textureMatrix[SkMatrix::kMTransY] *= imgHeight; - // since the y-flip has been removed we can simply scale & translate - // the source rectangle - textureMatrix.mapRect(&skiaSrcRect); + // swap rotation and translation part of the matrix, because we convert from + // right-handed Cartesian to left-handed coordinate system. + std::swap(textureMatrix[SkMatrix::kMTransX], textureMatrix[SkMatrix::kMTransY]); + std::swap(textureMatrix[SkMatrix::kMSkewX], textureMatrix[SkMatrix::kMSkewY]); - if (skiaSrcRect.intersect(bufferRect)) { + // convert to Skia data structures + SkRect skiaSrcRect = srcRect.toSkRect(); + SkMatrix textureMatrixInv; + SkRect skiaDestRect = SkRect::MakeWH(bitmap->width(), bitmap->height()); + bool srcNotEmpty = false; + if (textureMatrix.invert(&textureMatrixInv)) { + if (skiaSrcRect.isEmpty()) { + skiaSrcRect = SkRect::MakeIWH(imgWidth, imgHeight); + srcNotEmpty = !skiaSrcRect.isEmpty(); + } else { + // src and dest rectangles need to be converted into texture coordinates before the + // rotation matrix is applied (because drawImageRect preconcat its matrix). + textureMatrixInv.mapRect(&skiaSrcRect); + srcNotEmpty = skiaSrcRect.intersect(SkRect::MakeIWH(imgWidth, imgHeight)); + } + textureMatrixInv.mapRect(&skiaDestRect); + } + + if (srcNotEmpty) { // we render in an offscreen buffer to scale and to avoid an issue b/62262733 // with reading incorrect data from EGLImage backed SkImage (likely a driver bug) sk_sp scaledSurface = SkSurface::MakeRenderTarget( grContext.get(), SkBudgeted::kYes, bitmap->info()); SkPaint paint; paint.setBlendMode(SkBlendMode::kSrc); - scaledSurface->getCanvas()->drawImageRect(image, skiaSrcRect, - SkRect::MakeWH(bitmap->width(), bitmap->height()), &paint); + scaledSurface->getCanvas()->concat(textureMatrix); + scaledSurface->getCanvas()->drawImageRect(image, skiaSrcRect, skiaDestRect, &paint); + image = scaledSurface->makeImageSnapshot(); if (image->readPixels(bitmap->info(), bitmap->getPixels(), bitmap->rowBytes(), 0, 0)) { -- cgit v1.2.3-59-g8ed1b From 530a2b44d9a4b40d028c912ade858da73081ed85 Mon Sep 17 00:00:00 2001 From: Arun Date: Mon, 23 Jan 2017 12:47:57 +0000 Subject: Disable hwui blending for first draw to main FBO bug:34809371 In some applications, the first draw is not opaque - either because the application is misbehaved, or because hwui is not able to reliably tell whether the layer is opaque or translucent. This is undefined behaviour in OpenGL ES and has a significant performance and bandwidth impact on some tiler GPUs as it requires loading the previous frame's color data. This change disables blending in that case and also for effectively opaque blend modes (SRC=GL_ONE, DST=GL_ZERO). It increases performance by ~10% for Leanback CTS on some low-end GPUs (gradient layer that hwui incorrectly believes to be translucent). Test: manual - visual inspection on fugu (nexus player) Change-Id: I2cbf1c76678acae1a36923e72fd18ed55cd89dc2 --- libs/hwui/BakedOpRenderer.cpp | 14 ++++++++++---- libs/hwui/OpenGLReadback.cpp | 2 +- libs/hwui/renderstate/Blend.cpp | 2 +- libs/hwui/renderstate/RenderState.cpp | 9 +++++++-- libs/hwui/renderstate/RenderState.h | 2 +- 5 files changed, 20 insertions(+), 9 deletions(-) (limited to 'libs/hwui/OpenGLReadback.cpp') diff --git a/libs/hwui/BakedOpRenderer.cpp b/libs/hwui/BakedOpRenderer.cpp index df2b35b39aa8..4e59baa48983 100644 --- a/libs/hwui/BakedOpRenderer.cpp +++ b/libs/hwui/BakedOpRenderer.cpp @@ -208,7 +208,6 @@ void BakedOpRenderer::drawRects(const float* rects, int count, const SkPaint* pa // TODO: Currently assume full FBO damage, due to FrameInfoVisualizer::unionDirty. // Should should scissor/set mHasDrawn safely. mRenderState.scissor().setEnabled(false); - mHasDrawn = true; Glop glop; GlopBuilder(mRenderState, mCaches, &glop) .setRoundRectClipState(nullptr) @@ -217,7 +216,11 @@ void BakedOpRenderer::drawRects(const float* rects, int count, const SkPaint* pa .setTransform(Matrix4::identity(), TransformFlags::None) .setModelViewIdentityEmptyBounds() .build(); - mRenderState.render(glop, mRenderTarget.orthoMatrix); + // Disable blending if this is the first draw to the main framebuffer, in case app has defined + // transparency where it doesn't make sense - as first draw in opaque window. + bool overrideDisableBlending = !mHasDrawn && mOpaque && !mRenderTarget.frameBufferId; + mRenderState.render(glop, mRenderTarget.orthoMatrix, overrideDisableBlending); + mHasDrawn = true; } // clears and re-fills stencil with provided rendertarget space quads, @@ -234,7 +237,7 @@ void BakedOpRenderer::setupStencilQuads(std::vector& quadVertices, .setTransform(Matrix4::identity(), TransformFlags::None) .setModelViewIdentityEmptyBounds() .build(); - mRenderState.render(glop, mRenderTarget.orthoMatrix); + mRenderState.render(glop, mRenderTarget.orthoMatrix, false); mRenderState.stencil().enableTest(incrementThreshold); } @@ -346,7 +349,10 @@ void BakedOpRenderer::prepareRender(const Rect* dirtyBounds, const ClipBase* cli void BakedOpRenderer::renderGlopImpl(const Rect* dirtyBounds, const ClipBase* clip, const Glop& glop) { prepareRender(dirtyBounds, clip); - mRenderState.render(glop, mRenderTarget.orthoMatrix); + // Disable blending if this is the first draw to the main framebuffer, in case app has defined + // transparency where it doesn't make sense - as first draw in opaque window. + bool overrideDisableBlending = !mHasDrawn && mOpaque && !mRenderTarget.frameBufferId; + mRenderState.render(glop, mRenderTarget.orthoMatrix, overrideDisableBlending); if (!mRenderTarget.frameBufferId) mHasDrawn = true; } diff --git a/libs/hwui/OpenGLReadback.cpp b/libs/hwui/OpenGLReadback.cpp index 025503b15975..f9a1cc5296d5 100644 --- a/libs/hwui/OpenGLReadback.cpp +++ b/libs/hwui/OpenGLReadback.cpp @@ -228,7 +228,7 @@ inline CopyResult copyTextureInto(Caches& caches, RenderState& renderState, .build(); Matrix4 ortho; ortho.loadOrtho(destWidth, destHeight); - renderState.render(glop, ortho); + renderState.render(glop, ortho, false); // TODO: We should convert to linear space when the target is RGBA16F glReadPixels(0, 0, bitmap->width(), bitmap->height(), format, diff --git a/libs/hwui/renderstate/Blend.cpp b/libs/hwui/renderstate/Blend.cpp index 8865c6efce8c..b1ca4a248a80 100644 --- a/libs/hwui/renderstate/Blend.cpp +++ b/libs/hwui/renderstate/Blend.cpp @@ -118,7 +118,7 @@ void Blend::getFactors(SkBlendMode mode, ModeOrderSwap modeUsage, GLenum* outSrc } void Blend::setFactors(GLenum srcMode, GLenum dstMode) { - if (srcMode == GL_ZERO && dstMode == GL_ZERO) { + if ((srcMode == GL_ZERO || srcMode == GL_ONE) && dstMode == GL_ZERO) { // disable blending if (mEnabled) { glDisable(GL_BLEND); diff --git a/libs/hwui/renderstate/RenderState.cpp b/libs/hwui/renderstate/RenderState.cpp index ededffb0f4bb..5fc5cb275741 100644 --- a/libs/hwui/renderstate/RenderState.cpp +++ b/libs/hwui/renderstate/RenderState.cpp @@ -262,7 +262,8 @@ void RenderState::postDecStrong(VirtualLightRefBase* object) { // Render /////////////////////////////////////////////////////////////////////////////// -void RenderState::render(const Glop& glop, const Matrix4& orthoMatrix) { +void RenderState::render(const Glop& glop, const Matrix4& orthoMatrix, + bool overrideDisableBlending) { const Glop::Mesh& mesh = glop.mesh; const Glop::Mesh::Vertices& vertices = mesh.vertices; const Glop::Mesh::Indices& indices = mesh.indices; @@ -417,7 +418,11 @@ void RenderState::render(const Glop& glop, const Matrix4& orthoMatrix) { // ------------------------------------ // ---------- GL state setup ---------- // ------------------------------------ - blend().setFactors(glop.blend.src, glop.blend.dst); + if (CC_UNLIKELY(overrideDisableBlending)) { + blend().setFactors(GL_ZERO, GL_ZERO); + } else { + blend().setFactors(glop.blend.src, glop.blend.dst); + } GL_CHECKPOINT(MODERATE); diff --git a/libs/hwui/renderstate/RenderState.h b/libs/hwui/renderstate/RenderState.h index df81e864a0b5..315fa2db6878 100644 --- a/libs/hwui/renderstate/RenderState.h +++ b/libs/hwui/renderstate/RenderState.h @@ -106,7 +106,7 @@ public: // more thinking... void postDecStrong(VirtualLightRefBase* object); - void render(const Glop& glop, const Matrix4& orthoMatrix); + void render(const Glop& glop, const Matrix4& orthoMatrix, bool overrideDisableBlending); Blend& blend() { return *mBlend; } MeshState& meshState() { return *mMeshState; } -- cgit v1.2.3-59-g8ed1b From 89cd62c8f31d2029263f41d050cf806bfb9935f5 Mon Sep 17 00:00:00 2001 From: Chris Craik Date: Fri, 22 Sep 2017 09:31:05 -0700 Subject: Speculative fix for missing target crash Bug: 66451158 Test: manual app usage Change-Id: I2db3aa73edcb80da25ead64205011705f0beef91 --- libs/hwui/OpenGLReadback.cpp | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'libs/hwui/OpenGLReadback.cpp') diff --git a/libs/hwui/OpenGLReadback.cpp b/libs/hwui/OpenGLReadback.cpp index 025503b15975..5c4263e03abc 100644 --- a/libs/hwui/OpenGLReadback.cpp +++ b/libs/hwui/OpenGLReadback.cpp @@ -280,6 +280,11 @@ CopyResult OpenGLReadbackImpl::copyImageInto(EGLImageKHR eglImage, bool OpenGLReadbackImpl::copyLayerInto(renderthread::RenderThread& renderThread, GlLayer& layer, SkBitmap* bitmap) { + if (!layer.isRenderable()) { + // layer has never been updated by DeferredLayerUpdater, abort copy + return false; + } + return CopyResult::Success == copyTextureInto(Caches::getInstance(), renderThread.renderState(), layer.getTexture(), layer.getTexTransform(), Rect(), bitmap); -- cgit v1.2.3-59-g8ed1b