Fix ordering of texture->upload arguments

Caught by scatter-shotting GL_CHECKPOINTS which
seem generally useful to have

Bug: 26609444

Change-Id: Ie31d9297d8dae56405126720f338b4256c8bae77

diff --git a/libs/hwui/BakedOpDispatcher.cpp b/libs/hwui/BakedOpDispatcher.cpp
index 7ecc743..00381ee 100644 (file)
--- a/libs/hwui/BakedOpDispatcher.cpp
+++ b/libs/hwui/BakedOpDispatcher.cpp
@@ -784,7 +784,9 @@ void BakedOpDispatcher::onCopyFromLayerOp(BakedOpRenderer& renderer, const CopyF
                 .build();
         renderer.renderGlop(state, glop);
     }
+    GL_CHECKPOINT();
     renderer.renderState().layerPool().putOrDelete(*op.layerHandle);
+    GL_CHECKPOINT();
 }
 
 } // namespace uirenderer

diff --git a/libs/hwui/BakedOpRenderer.cpp b/libs/hwui/BakedOpRenderer.cpp
index 4fbff0d..0931282 100644 (file)
--- a/libs/hwui/BakedOpRenderer.cpp
+++ b/libs/hwui/BakedOpRenderer.cpp
@@ -74,7 +74,8 @@ void BakedOpRenderer::endLayer() {
 
     // Detach the texture from the FBO
     glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, 0, 0);
-    LOG_ALWAYS_FATAL_IF(GLUtils::dumpGLErrors(), "endLayer FAILED");
+    LOG_ALWAYS_FATAL_IF(GLUtils::dumpGLErrors(), "endLayer FAILED, bound fbo = %u",
+            mRenderState.getFramebuffer());
     mRenderState.deleteFramebuffer(mRenderTarget.frameBufferId);
     mRenderTarget.frameBufferId = 0;
 }

diff --git a/libs/hwui/FrameBuilder.h b/libs/hwui/FrameBuilder.h
index 3ba73f0..bd01850 100644 (file)
--- a/libs/hwui/FrameBuilder.h
+++ b/libs/hwui/FrameBuilder.h
@@ -21,6 +21,7 @@
 #include "DisplayList.h"
 #include "LayerBuilder.h"
 #include "RecordedOp.h"
+#include "utils/GLUtils.h"
 
 #include <vector>
 #include <unordered_map>
@@ -99,22 +100,30 @@ public:
         // Relay through layers in reverse order, since layers
         // later in the list will be drawn by earlier ones
         for (int i = mLayerBuilders.size() - 1; i >= 1; i--) {
+            GL_CHECKPOINT();
             LayerBuilder& layer = *(mLayerBuilders[i]);
             if (layer.renderNode) {
                 // cached HW layer - can't skip layer if empty
                 renderer.startRepaintLayer(layer.offscreenBuffer, layer.repaintRect);
+                GL_CHECKPOINT();
                 layer.replayBakedOpsImpl((void*)&renderer, unmergedReceivers, mergedReceivers);
+                GL_CHECKPOINT();
                 renderer.endLayer();
             } else if (!layer.empty()) { // save layer - skip entire layer if empty
                 layer.offscreenBuffer = renderer.startTemporaryLayer(layer.width, layer.height);
+                GL_CHECKPOINT();
                 layer.replayBakedOpsImpl((void*)&renderer, unmergedReceivers, mergedReceivers);
+                GL_CHECKPOINT();
                 renderer.endLayer();
             }
         }
 
+        GL_CHECKPOINT();
         const LayerBuilder& fbo0 = *(mLayerBuilders[0]);
         renderer.startFrame(fbo0.width, fbo0.height, fbo0.repaintRect);
+        GL_CHECKPOINT();
         fbo0.replayBakedOpsImpl((void*)&renderer, unmergedReceivers, mergedReceivers);
+        GL_CHECKPOINT();
         renderer.endFrame(fbo0.repaintRect);
     }
 

diff --git a/libs/hwui/GradientCache.cpp b/libs/hwui/GradientCache.cpp
index 522aa96..1473bc8 100644 (file)
--- a/libs/hwui/GradientCache.cpp
+++ b/libs/hwui/GradientCache.cpp
@@ -277,9 +277,9 @@ void GradientCache::generateTexture(uint32_t* colors, float* positions,
 
     if (mUseFloatTexture) {
         // We have to use GL_RGBA16F because GL_RGBA32F does not support filtering
-        texture->upload(width, height, GL_RGBA16F, GL_RGBA, GL_FLOAT, pixels);
+        texture->upload(GL_RGBA16F, width, height, GL_RGBA, GL_FLOAT, pixels);
     } else {
-        texture->upload(width, height, GL_RGBA, GL_RGBA, GL_UNSIGNED_BYTE, pixels);
+        texture->upload(GL_RGBA, width, height, GL_RGBA, GL_UNSIGNED_BYTE, pixels);
     }
 
     texture->setFilter(GL_LINEAR);

diff --git a/libs/hwui/Texture.cpp b/libs/hwui/Texture.cpp
index 8a6b28d..771d004 100644 (file)
--- a/libs/hwui/Texture.cpp
+++ b/libs/hwui/Texture.cpp
@@ -16,6 +16,7 @@
 
 #include "Caches.h"
 #include "Texture.h"
+#include "utils/GLUtils.h"
 #include "utils/TraceUtils.h"
 
 #include <utils/Log.h>
@@ -93,22 +94,28 @@ bool Texture::updateSize(uint32_t width, uint32_t height, GLint format) {
 
 void Texture::upload(GLint internalformat, uint32_t width, uint32_t height,
         GLenum format, GLenum type, const void* pixels) {
+    GL_CHECKPOINT();
     bool needsAlloc = updateSize(width, height, internalformat);
     if (!needsAlloc && !pixels) {
         return;
     }
     mCaches.textureState().activateTexture(0);
+    GL_CHECKPOINT();
     if (!mId) {
         glGenTextures(1, &mId);
         needsAlloc = true;
     }
+    GL_CHECKPOINT();
     mCaches.textureState().bindTexture(GL_TEXTURE_2D, mId);
+    GL_CHECKPOINT();
     if (needsAlloc) {
         glTexImage2D(GL_TEXTURE_2D, 0, mFormat, mWidth, mHeight, 0,
                 format, type, pixels);
+        GL_CHECKPOINT();
     } else {
         glTexSubImage2D(GL_TEXTURE_2D, 0, mFormat, mWidth, mHeight, 0,
                 format, type, pixels);
+        GL_CHECKPOINT();
     }
 }
 

diff --git a/libs/hwui/renderstate/RenderState.cpp b/libs/hwui/renderstate/RenderState.cpp
index b6dba02..e71d6ee 100644 (file)
--- a/libs/hwui/renderstate/RenderState.cpp
+++ b/libs/hwui/renderstate/RenderState.cpp
@@ -241,6 +241,8 @@ void RenderState::render(const Glop& glop, const Matrix4& orthoMatrix) {
     const Glop::Mesh::Indices& indices = mesh.indices;
     const Glop::Fill& fill = glop.fill;
 
+    GL_CHECKPOINT();
+
     // ---------------------------------------------
     // ---------- Program + uniform setup ----------
     // ---------------------------------------------
@@ -284,6 +286,8 @@ void RenderState::render(const Glop& glop, const Matrix4& orthoMatrix) {
                 roundedOutRadius);
     }
 
+    GL_CHECKPOINT();
+
     // --------------------------------
     // ---------- Mesh setup ----------
     // --------------------------------
@@ -335,11 +339,15 @@ void RenderState::render(const Glop& glop, const Matrix4& orthoMatrix) {
     // Shader uniforms
     SkiaShader::apply(*mCaches, fill.skiaShaderData);
 
+    GL_CHECKPOINT();
+
     // ------------------------------------
     // ---------- GL state setup ----------
     // ------------------------------------
     blend().setFactors(glop.blend.src, glop.blend.dst);
 
+    GL_CHECKPOINT();
+
     // ------------------------------------
     // ---------- Actual drawing ----------
     // ------------------------------------
@@ -368,6 +376,8 @@ void RenderState::render(const Glop& glop, const Matrix4& orthoMatrix) {
         glDrawArrays(mesh.primitiveMode, 0, mesh.elementCount);
     }
 
+    GL_CHECKPOINT();
+
     // -----------------------------------
     // ---------- Mesh teardown ----------
     // -----------------------------------
@@ -377,6 +387,8 @@ void RenderState::render(const Glop& glop, const Matrix4& orthoMatrix) {
     if (vertices.attribFlags & VertexAttribFlags::Color) {
         glDisableVertexAttribArray(colorLocation);
     }
+
+    GL_CHECKPOINT();
 }
 
 void RenderState::dump() {

diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp
index dd48a83..6f8d627 100644 (file)
--- a/libs/hwui/renderthread/CanvasContext.cpp
+++ b/libs/hwui/renderthread/CanvasContext.cpp
@@ -29,6 +29,7 @@
 #include "renderstate/RenderState.h"
 #include "renderstate/Stencil.h"
 #include "protos/hwui.pb.h"
+#include "utils/GLUtils.h"
 #include "utils/TimeUtils.h"
 
 #if HWUI_NEW_OPS
@@ -213,10 +214,13 @@ void CanvasContext::prepareTree(TreeInfo& info, int64_t* uiFrameInfo,
         // node(s) are non client / filler nodes.
         info.mode = (node.get() == target ? TreeInfo::MODE_FULL : TreeInfo::MODE_RT_ONLY);
         node->prepareTree(info);
+        GL_CHECKPOINT();
     }
     mAnimationContext->runRemainingAnimations(info);
+    GL_CHECKPOINT();
 
     freePrefetechedLayers();
+    GL_CHECKPOINT();
 
     if (CC_UNLIKELY(!mNativeWindow.get())) {
         mCurrentFrameInfo->addFlag(FrameInfoFlags::SkippedFrame);
@@ -463,6 +467,9 @@ void CanvasContext::draw() {
 
     bool drew = mCanvas->finish();
 #endif
+
+    GL_CHECKPOINT();
+
     // Even if we decided to cancel the frame, from the perspective of jank
     // metrics the frame was swapped at this point
     mCurrentFrameInfo->markSwapBuffers();

diff --git a/libs/hwui/utils/GLUtils.h b/libs/hwui/utils/GLUtils.h
index 7020461..6c521e4 100644 (file)
--- a/libs/hwui/utils/GLUtils.h
+++ b/libs/hwui/utils/GLUtils.h
@@ -16,9 +16,18 @@
 #ifndef GLUTILS_H
 #define GLUTILS_H
 
+#include <cutils/log.h>
+
 namespace android {
 namespace uirenderer {
 
+#if 0
+#define GL_CHECKPOINT() LOG_ALWAYS_FATAL_IF(GLUtils::dumpGLErrors(),\
+        "GL errors! %s:%d", __FILE__, __LINE__)
+#else
+#define GL_CHECKPOINT()
+#endif
+
 class GLUtils {
 public:
     /**