GL: Make updateTexture invalidate texture on change of texture unit

Switching texture units means the value of lastTextureUsed could be
invalid, meaning we need to unconditionally re-bind and update parameters.

A future optimization would be to keep a per-texture-unit cache, so
that we wouldn't have to re-bind and set parameters when switching
units back and forth, but this complicates the current code somewhat,
so it's left for another patch.

Change-Id: Icb2a5d03457a907f3c25bbb437feeb7c5f155716
Reviewed-by: Laszlo Agocs <laszlo.agocs@theqtcompany.com>
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@theqtcompany.com>

2 files changed, 53 insertions, 4 deletions

diff --git a/src/gui/opengl/qopenglpaintengine.cpp b/src/gui/opengl/qopenglpaintengine.cpp
index 6570d49279..48a2ceaa55 100644
--- a/src/gui/opengl/qopenglpaintengine.cpp
+++ b/src/gui/opengl/qopenglpaintengine.cpp
@@ -153,12 +153,26 @@ void QOpenGL2PaintEngineExPrivate::useSimpleShader()
         updateMatrix();
 }
 
+/*
+    Single entry-point for activating, binding, and setting properties.
+
+    Allows keeping track of (caching) the latest texture unit and bound
+    texture in a central place, so that we can skip re-binding unless
+    needed.
+
+    \note Any code or Qt API that internally activates or binds will
+    not affect the cache used by this function, which means they will
+    lead to inconsisent state. QPainter::beginNativePainting() takes
+    care of resetting the cache, so for user–code this is fine, but
+    internally in the paint engine care must be taken to not call
+    functions that may activate or bind under our feet.
+*/
 template<typename T>
 void QOpenGL2PaintEngineExPrivate::updateTexture(GLenum textureUnit, const T &texture, GLenum wrapMode, GLenum filterMode, TextureUpdateMode updateMode)
 {
     static const GLenum target = GL_TEXTURE_2D;
 
-    funcs.glActiveTexture(GL_TEXTURE0 + textureUnit);
+    activateTextureUnit(textureUnit);
 
     GLuint textureId = bindTexture(texture);
 
@@ -174,6 +188,20 @@ void QOpenGL2PaintEngineExPrivate::updateTexture(GLenum textureUnit, const T &te
     funcs.glTexParameteri(target, GL_TEXTURE_MIN_FILTER, filterMode);
 }
 
+void QOpenGL2PaintEngineExPrivate::activateTextureUnit(GLenum textureUnit)
+{
+    if (textureUnit != lastTextureUnitUsed) {
+        funcs.glActiveTexture(GL_TEXTURE0 + textureUnit);
+        lastTextureUnitUsed = textureUnit;
+
+        // We simplify things by keeping a single cached value of the last
+        // texture that was bound, instead of one per texture unit. This
+        // means that switching texture units could potentially mean we
+        // need a re-bind and corresponding parameter updates.
+        lastTextureUsed = GLuint(-1);
+    }
+}
+
 template<>
 GLuint QOpenGL2PaintEngineExPrivate::bindTexture(const GLuint &textureId)
 {
@@ -597,9 +625,15 @@ void QOpenGL2PaintEngineEx::beginNativePainting()
     }
 #endif // QT_OPENGL_ES_2
 
+    d->resetGLState();
+
+    // We don't know what texture units and textures the native painting
+    // will activate and bind, so we can't assume anything when we return
+    // from the native painting.
+    d->lastTextureUnitUsed = QT_UNKNOWN_TEXTURE_UNIT;
     d->lastTextureUsed = GLuint(-1);
+
     d->dirtyStencilRegion = QRect(0, 0, d->width, d->height);
-    d->resetGLState();
 
     d->shaderManager->setDirty();
 
@@ -608,8 +642,9 @@ void QOpenGL2PaintEngineEx::beginNativePainting()
 
 void QOpenGL2PaintEngineExPrivate::resetGLState()
 {
+    activateTextureUnit(QT_DEFAULT_TEXTURE_UNIT);
+
     funcs.glDisable(GL_BLEND);
-    funcs.glActiveTexture(GL_TEXTURE0);
     funcs.glDisable(GL_STENCIL_TEST);
     funcs.glDisable(GL_DEPTH_TEST);
     funcs.glDisable(GL_SCISSOR_TEST);
@@ -1368,7 +1403,12 @@ void QOpenGL2PaintEngineEx::renderHintsChanged()
 #endif // QT_OPENGL_ES_2
 
     Q_D(QOpenGL2PaintEngineEx);
+
+    // This is a somewhat sneaky way of conceptually making the next call to
+    // updateTexture() use FoceUpdate for the TextureUpdateMode. We need this
+    // as new render hints may require updating the filter mode.
     d->lastTextureUsed = GLuint(-1);
+
     d->brushTextureDirty = true;
 //    qDebug("QOpenGL2PaintEngineEx::renderHintsChanged() not implemented!");
 }
@@ -1673,7 +1713,7 @@ void QOpenGL2PaintEngineExPrivate::drawCachedGlyphs(QFontEngine::GlyphFormat gly
             // uses the image texture unit for blitting to the cache, while
             // we switch between image and mask units when drawing.
             static const GLenum glypchCacheTextureUnit = QT_IMAGE_TEXTURE_UNIT;
-            funcs.glActiveTexture(GL_TEXTURE0 + glypchCacheTextureUnit);
+            activateTextureUnit(glypchCacheTextureUnit);
 
             cache->fillInPendingGlyphs();
 
@@ -1892,6 +1932,10 @@ void QOpenGL2PaintEngineExPrivate::drawCachedGlyphs(QFontEngine::GlyphFormat gly
         else
             funcs.glActiveTexture(GL_TEXTURE0 + QT_MASK_TEXTURE_UNIT);
 
+        // Need to reset the unit here, until we've made drawCachedGlyphs
+        // use the shared code-path for activating and binding.
+        lastTextureUnitUsed = QT_UNKNOWN_TEXTURE_UNIT;
+
         if (lastMaskTextureUsed != cache->texture()) {
             funcs.glBindTexture(GL_TEXTURE_2D, cache->texture());
             lastMaskTextureUsed = cache->texture();
diff --git a/src/gui/opengl/qopenglpaintengine_p.h b/src/gui/opengl/qopenglpaintengine_p.h
index 209cd3d7b5..297ac54640 100644
--- a/src/gui/opengl/qopenglpaintengine_p.h
+++ b/src/gui/opengl/qopenglpaintengine_p.h
@@ -69,6 +69,8 @@ enum EngineMode {
 QT_BEGIN_NAMESPACE
 
 #define GL_STENCIL_HIGH_BIT         GLuint(0x80)
+#define QT_UNKNOWN_TEXTURE_UNIT     GLuint(-1)
+#define QT_DEFAULT_TEXTURE_UNIT     GLuint(0)
 #define QT_BRUSH_TEXTURE_UNIT       GLuint(0)
 #define QT_IMAGE_TEXTURE_UNIT       GLuint(0) //Can be the same as brush texture unit
 #define QT_MASK_TEXTURE_UNIT        GLuint(1)
@@ -184,6 +186,7 @@ public:
             snapToPixelGrid(false),
             nativePaintingActive(false),
             inverseScale(1),
+            lastTextureUnitUsed(QT_UNKNOWN_TEXTURE_UNIT),
             lastMaskTextureUsed(0)
     { }
 
@@ -199,6 +202,7 @@ public:
     void updateTexture(GLenum textureUnit, const T &texture, GLenum wrapMode, GLenum filterMode, TextureUpdateMode updateMode = UpdateIfNeeded);
     template<typename T>
     GLuint bindTexture(const T &texture);
+    void activateTextureUnit(GLenum textureUnit);
 
     void resetGLState();
 
@@ -300,6 +304,7 @@ public:
     GLfloat pmvMatrix[3][3];
     GLfloat inverseScale;
 
+    GLenum lastTextureUnitUsed;
     GLuint lastTextureUsed;
     GLuint lastMaskTextureUsed;