Make comparisonKey() a pure virtual in QSGTexture

In Qt 5.14 this was a virtual in QSGTexturePrivate due to having had
to maintain binary compatibility. In 6.0 it becomes a pure virtual in
QSGTexture.

Task-number: QTBUG-82997
Change-Id: I26a42169b071fe8d413f5c89ef45850fa7222006
Reviewed-by: Andy Nichols <andy.nichols@qt.io>

19 files changed, 53 insertions, 83 deletions

diff --git a/src/quick/scenegraph/adaptations/software/qsgsoftwarelayer.cpp b/src/quick/scenegraph/adaptations/software/qsgsoftwarelayer.cpp
index 683ccb60c2..ffeb55c7ef 100644
--- a/src/quick/scenegraph/adaptations/software/qsgsoftwarelayer.cpp
+++ b/src/quick/scenegraph/adaptations/software/qsgsoftwarelayer.cpp
@@ -45,7 +45,7 @@
 QT_BEGIN_NAMESPACE
 
 QSGSoftwareLayer::QSGSoftwareLayer(QSGRenderContext *renderContext)
-    : QSGLayer(*(new QSGSoftwareLayerPrivate))
+    : QSGLayer(*(new QSGTexturePrivate))
     , m_item(nullptr)
     , m_context(renderContext)
     , m_renderer(nullptr)
@@ -70,7 +70,7 @@ int QSGSoftwareLayer::textureId() const
     return 0;
 }
 
-int QSGSoftwareLayerPrivate::comparisonKey() const
+int QSGSoftwareLayer::comparisonKey() const
 {
     return 0;
 }
diff --git a/src/quick/scenegraph/adaptations/software/qsgsoftwarelayer_p.h b/src/quick/scenegraph/adaptations/software/qsgsoftwarelayer_p.h
index 1859e14514..4961c91c1c 100644
--- a/src/quick/scenegraph/adaptations/software/qsgsoftwarelayer_p.h
+++ b/src/quick/scenegraph/adaptations/software/qsgsoftwarelayer_p.h
@@ -58,11 +58,9 @@
 QT_BEGIN_NAMESPACE
 
 class QSGSoftwarePixmapRenderer;
-class QSGSoftwareLayerPrivate;
 
 class QSGSoftwareLayer : public QSGLayer
 {
-    Q_DECLARE_PRIVATE(QSGSoftwareLayer)
     Q_OBJECT
 public:
     QSGSoftwareLayer(QSGRenderContext *renderContext);
@@ -72,6 +70,7 @@ public:
 
     // QSGTexture interface
 public:
+    int comparisonKey() const override;
     int textureId() const override;
     QSize textureSize() const override;
     bool hasAlphaChannel() const override;
@@ -120,13 +119,6 @@ private:
     bool m_dirtyTexture;
 };
 
-class QSGSoftwareLayerPrivate : public QSGTexturePrivate
-{
-    Q_DECLARE_PUBLIC(QSGSoftwareLayer)
-public:
-    int comparisonKey() const override;
-};
-
 QT_END_NAMESPACE
 
 #endif // QSGSOFTWARELAYER_H
diff --git a/src/quick/scenegraph/adaptations/software/qsgsoftwarepixmaptexture.cpp b/src/quick/scenegraph/adaptations/software/qsgsoftwarepixmaptexture.cpp
index 82a48d80ca..7a3be61cc0 100644
--- a/src/quick/scenegraph/adaptations/software/qsgsoftwarepixmaptexture.cpp
+++ b/src/quick/scenegraph/adaptations/software/qsgsoftwarepixmaptexture.cpp
@@ -43,7 +43,6 @@
 QT_BEGIN_NAMESPACE
 
 QSGSoftwarePixmapTexture::QSGSoftwarePixmapTexture(const QImage &image, uint flags)
-    : QSGTexture(*(new QSGSoftwarePixmapTexturePrivate))
 {
     // Prevent pixmap format conversion to reduce memory consumption
     // and surprises in calling code. (See QTBUG-47328)
@@ -57,8 +56,7 @@ QSGSoftwarePixmapTexture::QSGSoftwarePixmapTexture(const QImage &image, uint fla
 }
 
 QSGSoftwarePixmapTexture::QSGSoftwarePixmapTexture(const QPixmap &pixmap)
-    : QSGTexture(*(new QSGSoftwarePixmapTexturePrivate)),
-      m_pixmap(pixmap)
+    : m_pixmap(pixmap)
 {
 }
 
@@ -87,7 +85,7 @@ void QSGSoftwarePixmapTexture::bind()
     Q_UNREACHABLE();
 }
 
-int QSGSoftwarePixmapTexturePrivate::comparisonKey() const
+int QSGSoftwarePixmapTexture::comparisonKey() const
 {
     return 0;
 }
diff --git a/src/quick/scenegraph/adaptations/software/qsgsoftwarepixmaptexture_p.h b/src/quick/scenegraph/adaptations/software/qsgsoftwarepixmaptexture_p.h
index baa62d93de..dda1358124 100644
--- a/src/quick/scenegraph/adaptations/software/qsgsoftwarepixmaptexture_p.h
+++ b/src/quick/scenegraph/adaptations/software/qsgsoftwarepixmaptexture_p.h
@@ -56,16 +56,15 @@
 
 QT_BEGIN_NAMESPACE
 
-class QSGSoftwarePixmapTexturePrivate;
-
 class QSGSoftwarePixmapTexture : public QSGTexture
 {
     Q_OBJECT
-    Q_DECLARE_PRIVATE(QSGSoftwarePixmapTexture)
+
 public:
     QSGSoftwarePixmapTexture(const QImage &image, uint flags);
     QSGSoftwarePixmapTexture(const QPixmap &pixmap);
 
+    int comparisonKey() const override;
     int textureId() const override;
     QSize textureSize() const override;
     bool hasAlphaChannel() const override;
@@ -78,13 +77,6 @@ private:
     QPixmap m_pixmap;
 };
 
-class QSGSoftwarePixmapTexturePrivate : public QSGTexturePrivate
-{
-    Q_DECLARE_PUBLIC(QSGSoftwarePixmapTexture)
-public:
-    int comparisonKey() const override;
-};
-
 QT_END_NAMESPACE
 
 #endif // QSGSOFTWAREPIXMAPTEXTURE_H
diff --git a/src/quick/scenegraph/compressedtexture/qsgcompressedtexture.cpp b/src/quick/scenegraph/compressedtexture/qsgcompressedtexture.cpp
index b30960af1c..44ddb805d0 100644
--- a/src/quick/scenegraph/compressedtexture/qsgcompressedtexture.cpp
+++ b/src/quick/scenegraph/compressedtexture/qsgcompressedtexture.cpp
@@ -90,18 +90,17 @@ int QSGCompressedTexture::textureId() const
     return m_textureId;
 }
 
-int QSGCompressedTexturePrivate::comparisonKey() const
+int QSGCompressedTexture::comparisonKey() const
 {
-    Q_Q(const QSGCompressedTexture);
     // not textureId() as that would create an id when not yet done - that's not wanted here
-    if (q->m_textureId)
-        return q->m_textureId;
+    if (m_textureId)
+        return m_textureId;
 
-    if (q->m_texture)
-        return int(qintptr(q->m_texture));
+    if (m_texture)
+        return int(qintptr(m_texture));
 
     // two textures (and so materials) with not-yet-created texture underneath are never equal
-    return int(qintptr(q));
+    return int(qintptr(this));
 }
 
 QSize QSGCompressedTexture::textureSize() const
diff --git a/src/quick/scenegraph/compressedtexture/qsgcompressedtexture_p.h b/src/quick/scenegraph/compressedtexture/qsgcompressedtexture_p.h
index b858d8ddee..084205c5b5 100644
--- a/src/quick/scenegraph/compressedtexture/qsgcompressedtexture_p.h
+++ b/src/quick/scenegraph/compressedtexture/qsgcompressedtexture_p.h
@@ -73,6 +73,7 @@ public:
     bool hasAlphaChannel() const override;
     bool hasMipmaps() const override;
 
+    int comparisonKey() const override;
     int textureId() const override;
     void bind() override;
 
@@ -97,7 +98,6 @@ class QSGCompressedTexturePrivate : public QSGTexturePrivate
 {
     Q_DECLARE_PUBLIC(QSGCompressedTexture)
 public:
-    int comparisonKey() const override;
     QRhiTexture *rhiTexture() const override;
     void updateRhiTexture(QRhi *rhi, QRhiResourceUpdateBatch *resourceUpdates) override;
 };
diff --git a/src/quick/scenegraph/coreapi/qsgtexture.cpp b/src/quick/scenegraph/coreapi/qsgtexture.cpp
index df6f5cffe6..1dde0a31f2 100644
--- a/src/quick/scenegraph/coreapi/qsgtexture.cpp
+++ b/src/quick/scenegraph/coreapi/qsgtexture.cpp
@@ -485,6 +485,8 @@ bool QSGTexture::isAtlasTexture() const
  */
 
 /*!
+    \fn int QSGTexture::comparisonKey() const
+
     Returns a key suitable for comparing textures. Typically used in
     QSGMaterial::compare() implementations.
 
@@ -506,11 +508,6 @@ bool QSGTexture::isAtlasTexture() const
 
     \since 5.14
  */
-int QSGTexture::comparisonKey() const
-{
-    Q_D(const QSGTexture);
-    return d->comparisonKey();
-}
 
 /*!
     \fn QSize QSGTexture::textureSize() const
@@ -789,14 +786,6 @@ void QSGTexturePrivate::resetDirtySamplerOptions()
     wrapChanged = filteringChanged = anisotropyChanged = false;
 }
 
-int QSGTexturePrivate::comparisonKey() const
-{
-    // Must be overridden in subclasses but we cannot make this pure virtual
-    // before Qt 6 because the simple QSGTexture ctor must be kept working.
-    Q_Q(const QSGTexture);
-    return q->textureId(); // this is semantically wrong but at least compatible with existing, non-RHI-aware subclasses
-}
-
 /*!
     \internal
 
diff --git a/src/quick/scenegraph/coreapi/qsgtexture.h b/src/quick/scenegraph/coreapi/qsgtexture.h
index 207ef52f4e..f394e1977b 100644
--- a/src/quick/scenegraph/coreapi/qsgtexture.h
+++ b/src/quick/scenegraph/coreapi/qsgtexture.h
@@ -85,6 +85,7 @@ public:
         int layout;
     };
 
+    virtual int comparisonKey() const = 0;
     virtual int textureId() const = 0; // ### Qt 6: remove
     NativeTexture nativeTexture() const;
     virtual QSize textureSize() const = 0;
@@ -97,8 +98,8 @@ public:
 
     virtual QSGTexture *removedFromAtlas() const;
 
-    virtual void bind() = 0;
-    void updateBindOptions(bool force = false);
+    virtual void bind() = 0; // ### Qt 6: remove
+    void updateBindOptions(bool force = false); // ### Qt 6: remove
 
     void setMipmapFiltering(Filtering filter);
     QSGTexture::Filtering mipmapFiltering() const;
@@ -117,8 +118,6 @@ public:
 
     inline QRectF convertToNormalizedSourceRect(const QRectF &rect) const;
 
-    // ### Qt 6: make these virtual
-    int comparisonKey() const;
     void updateRhiTexture(QRhi *rhi, QRhiResourceUpdateBatch *resourceUpdates);
 
     // ### Qt 6: make this an argument for removedFromAtlas()
diff --git a/src/quick/scenegraph/coreapi/qsgtexture_p.h b/src/quick/scenegraph/coreapi/qsgtexture_p.h
index cb59d32012..b98642b206 100644
--- a/src/quick/scenegraph/coreapi/qsgtexture_p.h
+++ b/src/quick/scenegraph/coreapi/qsgtexture_p.h
@@ -85,8 +85,7 @@ public:
 
     virtual QRhiTexture *rhiTexture() const;
 
-    // ### Qt 6: these should be virtuals in the public class instead
-    virtual int comparisonKey() const; // ### Qt 6: pure virtual
+    // ### Qt 6: should be virtual in the public class instead
     virtual void updateRhiTexture(QRhi *rhi, QRhiResourceUpdateBatch *resourceUpdates);
 
     QRhiResourceUpdateBatch *workResourceUpdateBatch = nullptr; // ### Qt 6: remove
diff --git a/src/quick/scenegraph/qsgopengllayer.cpp b/src/quick/scenegraph/qsgopengllayer.cpp
index be289aeace..c585cd05f8 100644
--- a/src/quick/scenegraph/qsgopengllayer.cpp
+++ b/src/quick/scenegraph/qsgopengllayer.cpp
@@ -89,7 +89,7 @@ namespace
 }
 
 QSGOpenGLLayer::QSGOpenGLLayer(QSGRenderContext *context)
-    : QSGLayer(*(new QSGOpenGLLayerPrivate))
+    : QSGLayer(*(new QSGTexturePrivate))
     , m_item(nullptr)
     , m_device_pixel_ratio(1)
     , m_format(GL_RGBA)
@@ -141,10 +141,9 @@ int QSGOpenGLLayer::textureId() const
     return m_fbo ? m_fbo->texture() : 0;
 }
 
-int QSGOpenGLLayerPrivate::comparisonKey() const
+int QSGOpenGLLayer::comparisonKey() const
 {
-    Q_Q(const QSGOpenGLLayer);
-    return q->m_fbo ? q->m_fbo->texture() : 0;
+    return m_fbo ? m_fbo->texture() : 0;
 }
 
 bool QSGOpenGLLayer::hasAlphaChannel() const
diff --git a/src/quick/scenegraph/qsgopengllayer_p.h b/src/quick/scenegraph/qsgopengllayer_p.h
index c6246843e2..e54571e311 100644
--- a/src/quick/scenegraph/qsgopengllayer_p.h
+++ b/src/quick/scenegraph/qsgopengllayer_p.h
@@ -62,11 +62,9 @@ QT_BEGIN_NAMESPACE
 class QOpenGLFramebufferObject;
 class QSGDepthStencilBuffer;
 class QSGDefaultRenderContext;
-class QSGOpenGLLayerPrivate;
 
 class Q_QUICK_PRIVATE_EXPORT QSGOpenGLLayer : public QSGLayer
 {
-    Q_DECLARE_PRIVATE(QSGOpenGLLayer)
     Q_OBJECT
 public:
     QSGOpenGLLayer(QSGRenderContext *context);
@@ -92,6 +90,7 @@ public:
     bool hasMipmaps() const override;
     int textureId() const override;
     QSize textureSize() const override { return m_size; }
+    int comparisonKey() const override;
 
     GLenum format() const { return m_format; }
     void setFormat(GLenum format) override;
@@ -157,13 +156,6 @@ private:
     uint m_mirrorVertical : 1;
 };
 
-class QSGOpenGLLayerPrivate : public QSGTexturePrivate
-{
-    Q_DECLARE_PUBLIC(QSGOpenGLLayer)
-public:
-    int comparisonKey() const override;
-};
-
 QT_END_NAMESPACE
 
 #endif // QSGOPENGLLAYER_P_H
diff --git a/src/quick/scenegraph/qsgrhilayer.cpp b/src/quick/scenegraph/qsgrhilayer.cpp
index 952279a3ff..924c807989 100644
--- a/src/quick/scenegraph/qsgrhilayer.cpp
+++ b/src/quick/scenegraph/qsgrhilayer.cpp
@@ -72,10 +72,9 @@ void QSGRhiLayer::invalidated()
     m_renderer = nullptr;
 }
 
-int QSGRhiLayerPrivate::comparisonKey() const
+int QSGRhiLayer::comparisonKey() const
 {
-    Q_Q(const QSGRhiLayer);
-    return int(qintptr(q->m_texture));
+    return int(qintptr(m_texture));
 }
 
 bool QSGRhiLayer::hasAlphaChannel() const
diff --git a/src/quick/scenegraph/qsgrhilayer_p.h b/src/quick/scenegraph/qsgrhilayer_p.h
index 6c4953ce17..f3a9ec5e9d 100644
--- a/src/quick/scenegraph/qsgrhilayer_p.h
+++ b/src/quick/scenegraph/qsgrhilayer_p.h
@@ -76,6 +76,7 @@ public:
 
     void bind() override;
     int textureId() const override;
+    int comparisonKey() const override;
 
     void setItem(QSGNode *item) override;
     void setRect(const QRectF &rect) override;
@@ -133,7 +134,6 @@ class QSGRhiLayerPrivate : public QSGTexturePrivate
 {
     Q_DECLARE_PUBLIC(QSGRhiLayer)
 public:
-    int comparisonKey() const override;
     QRhiTexture *rhiTexture() const override;
     void updateRhiTexture(QRhi *rhi, QRhiResourceUpdateBatch *resourceUpdates) override;
 };
diff --git a/src/quick/scenegraph/util/qsgopenglatlastexture.cpp b/src/quick/scenegraph/util/qsgopenglatlastexture.cpp
index b445a61940..a3f9286cfb 100644
--- a/src/quick/scenegraph/util/qsgopenglatlastexture.cpp
+++ b/src/quick/scenegraph/util/qsgopenglatlastexture.cpp
@@ -516,6 +516,19 @@ TextureBase::~TextureBase()
     m_atlas->remove(this);
 }
 
+int TextureBase::comparisonKey() const
+{
+    // We need special care here: a typical comparisonKey() implementation
+    // returns a unique result when there is no underlying texture yet. This is
+    // not quite ideal for atlasing however since textures with the same atlas
+    // should be considered equal regardless of the state of the underlying
+    // graphics resources.
+
+    // base the comparison on the atlas ptr; this way textures for the same
+    // atlas are considered equal
+    return int(qintptr(m_atlas));
+}
+
 void TextureBase::bind()
 {
     m_atlas->bind(filtering());
diff --git a/src/quick/scenegraph/util/qsgopenglatlastexture_p.h b/src/quick/scenegraph/util/qsgopenglatlastexture_p.h
index 379c4c7fff..8860872ae1 100644
--- a/src/quick/scenegraph/util/qsgopenglatlastexture_p.h
+++ b/src/quick/scenegraph/util/qsgopenglatlastexture_p.h
@@ -103,6 +103,7 @@ public:
 
     void invalidate();
 
+    int comparisonKey() const { return m_texture_id; }
     int textureId() const;
     void bind(QSGTexture::Filtering filtering);
 
@@ -158,6 +159,7 @@ public:
     TextureBase(AtlasBase *atlas, const QRect &textureRect);
     ~TextureBase();
 
+    int comparisonKey() const override;
     int textureId() const override { return m_atlas->textureId(); }
     bool isAtlasTexture() const override { return true; }
 
diff --git a/src/quick/scenegraph/util/qsgplaintexture.cpp b/src/quick/scenegraph/util/qsgplaintexture.cpp
index 37dc0f9138..45018de872 100644
--- a/src/quick/scenegraph/util/qsgplaintexture.cpp
+++ b/src/quick/scenegraph/util/qsgplaintexture.cpp
@@ -304,19 +304,17 @@ void QSGPlainTexture::setTextureFromNativeObject(QRhi *rhi, QQuickWindow::Native
     setTexture(t);
 }
 
-int QSGPlainTexturePrivate::comparisonKey() const
+int QSGPlainTexture::comparisonKey() const
 {
-    Q_Q(const QSGPlainTexture);
-
     // not textureId() as that would create an id when not yet done - that's not wanted here
-    if (q->m_texture_id)
-        return q->m_texture_id;
+    if (m_texture_id)
+        return m_texture_id;
 
-    if (q->m_texture)
-        return int(qintptr(q->m_texture));
+    if (m_texture)
+        return int(qintptr(m_texture));
 
     // two textures (and so materials) with not-yet-created texture underneath are never equal
-    return int(qintptr(q));
+    return int(qintptr(this));
 }
 
 QRhiTexture *QSGPlainTexturePrivate::rhiTexture() const
diff --git a/src/quick/scenegraph/util/qsgplaintexture_p.h b/src/quick/scenegraph/util/qsgplaintexture_p.h
index 1eb0b59d2e..4ef9f0ce0a 100644
--- a/src/quick/scenegraph/util/qsgplaintexture_p.h
+++ b/src/quick/scenegraph/util/qsgplaintexture_p.h
@@ -83,6 +83,8 @@ public:
     void setImage(const QImage &image);
     const QImage &image() { return m_image; }
 
+    int comparisonKey() const override;
+
     void bind() override;
 
     void setTexture(QRhiTexture *texture);
@@ -119,7 +121,6 @@ class QSGPlainTexturePrivate : public QSGTexturePrivate
 {
     Q_DECLARE_PUBLIC(QSGPlainTexture)
 public:
-    int comparisonKey() const override;
     QRhiTexture *rhiTexture() const override;
     void updateRhiTexture(QRhi *rhi, QRhiResourceUpdateBatch *resourceUpdates) override;
 
diff --git a/src/quick/scenegraph/util/qsgrhiatlastexture.cpp b/src/quick/scenegraph/util/qsgrhiatlastexture.cpp
index 6b5f1aec4f..ec92666f0d 100644
--- a/src/quick/scenegraph/util/qsgrhiatlastexture.cpp
+++ b/src/quick/scenegraph/util/qsgrhiatlastexture.cpp
@@ -398,10 +398,8 @@ QRhiResourceUpdateBatch *TextureBase::workResourceUpdateBatch() const
     return d->workResourceUpdateBatch;
 }
 
-int TextureBasePrivate::comparisonKey() const
+int TextureBase::comparisonKey() const
 {
-    Q_Q(const TextureBase);
-
     // We need special care here: a typical comparisonKey() implementation
     // returns a unique result when there is no underlying texture yet. This is
     // not quite ideal for atlasing however since textures with the same atlas
@@ -410,7 +408,7 @@ int TextureBasePrivate::comparisonKey() const
 
     // base the comparison on the atlas ptr; this way textures for the same
     // atlas are considered equal
-    return int(qintptr(q->m_atlas));
+    return int(qintptr(m_atlas));
 }
 
 QRhiTexture *TextureBasePrivate::rhiTexture() const
diff --git a/src/quick/scenegraph/util/qsgrhiatlastexture_p.h b/src/quick/scenegraph/util/qsgrhiatlastexture_p.h
index 50d7b2a53f..85d07ae24b 100644
--- a/src/quick/scenegraph/util/qsgrhiatlastexture_p.h
+++ b/src/quick/scenegraph/util/qsgrhiatlastexture_p.h
@@ -159,6 +159,7 @@ public:
     TextureBase(AtlasBase *atlas, const QRect &textureRect);
     ~TextureBase();
 
+    int comparisonKey() const override;
     int textureId() const override { return 0; } // not used
     void bind() override { } // not used
 
@@ -176,7 +177,6 @@ class TextureBasePrivate : public QSGTexturePrivate
 {
     Q_DECLARE_PUBLIC(TextureBase)
 public:
-    int comparisonKey() const override;
     QRhiTexture *rhiTexture() const override;
     void updateRhiTexture(QRhi *rhi, QRhiResourceUpdateBatch *resourceUpdates) override;
 };