From 08f56c9830c12f6b443649e3e4f51b3cea69d79b Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Tue, 26 May 2020 15:31:42 +0200 Subject: Adjust QSGTexture comparisonKey type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original choice was int, simply following textureId(). This was later deemed insufficient: instead of a GLuint, the value is now often a 64-bit value (on 64 bit systems), based on a pointer, since the identity of a texture in the RHI world is the QRhiTexture* itself. In a custom texture implementation it is likely that the value here is the value of a native object handle, either a pointer or some 32 or 64 bit integer. Inspired by the recent QSGTexture::NativeTexture struct change (void* -> quint64), switch to a qint64 which is big enough to hold all these without truncation. We choose a signed value here, in order to allow for the following pattern that is widespread in material compare() implementations: if (qint64 diff = m_texture->comparisonKey() - other->texture()->comparisonKey()) return diff; Fixes: QTBUG-83769 Change-Id: I8bdae8cd835282358ded53b3703142b8f26e4400 Reviewed-by: Christian Strømme --- examples/quick/scenegraph/graph/noisynode.cpp | 2 +- examples/quick/scenegraph/twotextureproviders/xorblender.cpp | 4 ++-- src/plugins/scenegraph/openvg/qsgopenvglayer.cpp | 2 +- src/plugins/scenegraph/openvg/qsgopenvglayer.h | 2 +- .../scenegraph/adaptations/software/qsgsoftwarelayer.cpp | 2 +- .../scenegraph/adaptations/software/qsgsoftwarelayer_p.h | 2 +- .../adaptations/software/qsgsoftwarepixmaptexture.cpp | 2 +- .../adaptations/software/qsgsoftwarepixmaptexture_p.h | 2 +- .../scenegraph/compressedtexture/qsgcompressedtexture.cpp | 6 +++--- .../scenegraph/compressedtexture/qsgcompressedtexture_p.h | 2 +- src/quick/scenegraph/coreapi/qsgtexture.cpp | 12 +++++------- src/quick/scenegraph/coreapi/qsgtexture.h | 2 +- src/quick/scenegraph/qsgopengllayer.cpp | 2 +- src/quick/scenegraph/qsgopengllayer_p.h | 2 +- src/quick/scenegraph/qsgrhilayer.cpp | 4 ++-- src/quick/scenegraph/qsgrhilayer_p.h | 2 +- src/quick/scenegraph/qsgrhishadereffectnode.cpp | 2 +- src/quick/scenegraph/util/qsgopenglatlastexture.cpp | 4 ++-- src/quick/scenegraph/util/qsgopenglatlastexture_p.h | 4 ++-- src/quick/scenegraph/util/qsgplaintexture.cpp | 6 +++--- src/quick/scenegraph/util/qsgplaintexture_p.h | 2 +- src/quick/scenegraph/util/qsgrhiatlastexture.cpp | 4 ++-- src/quick/scenegraph/util/qsgrhiatlastexture_p.h | 2 +- src/quick/scenegraph/util/qsgtexturematerial.cpp | 2 +- 24 files changed, 37 insertions(+), 39 deletions(-) diff --git a/examples/quick/scenegraph/graph/noisynode.cpp b/examples/quick/scenegraph/graph/noisynode.cpp index d67110f48a..e85757d426 100644 --- a/examples/quick/scenegraph/graph/noisynode.cpp +++ b/examples/quick/scenegraph/graph/noisynode.cpp @@ -107,7 +107,7 @@ public: if (!state.texture || !other->state.texture) return state.texture ? 1 : -1; - if (int diff = state.texture->comparisonKey() - other->state.texture->comparisonKey()) + if (qint64 diff = state.texture->comparisonKey() - other->state.texture->comparisonKey()) return diff; return 0; diff --git a/examples/quick/scenegraph/twotextureproviders/xorblender.cpp b/examples/quick/scenegraph/twotextureproviders/xorblender.cpp index ad959b723d..276625d908 100644 --- a/examples/quick/scenegraph/twotextureproviders/xorblender.cpp +++ b/examples/quick/scenegraph/twotextureproviders/xorblender.cpp @@ -117,10 +117,10 @@ int XorBlendMaterial::compare(const QSGMaterial *o) const if (!state.texture2 || !other->state.texture2) return state.texture2 ? -1 : 1; - if (int diff = state.texture1->comparisonKey() - other->state.texture1->comparisonKey()) + if (qint64 diff = state.texture1->comparisonKey() - other->state.texture1->comparisonKey()) return diff; - if (int diff = state.texture2->comparisonKey() - other->state.texture2->comparisonKey()) + if (qint64 diff = state.texture2->comparisonKey() - other->state.texture2->comparisonKey()) return diff; return 0; diff --git a/src/plugins/scenegraph/openvg/qsgopenvglayer.cpp b/src/plugins/scenegraph/openvg/qsgopenvglayer.cpp index b03168b334..3ecf0dcb31 100644 --- a/src/plugins/scenegraph/openvg/qsgopenvglayer.cpp +++ b/src/plugins/scenegraph/openvg/qsgopenvglayer.cpp @@ -313,7 +313,7 @@ void QSGOpenVGLayer::grab() markDirtyTexture(); // Continuously update if 'live' and 'recursive'. } -int QSGOpenVGLayerPrivate::comparisonKey() const +qint64 QSGOpenVGLayerPrivate::comparisonKey() const { return 0; } diff --git a/src/plugins/scenegraph/openvg/qsgopenvglayer.h b/src/plugins/scenegraph/openvg/qsgopenvglayer.h index f2763463cd..1d52310f5c 100644 --- a/src/plugins/scenegraph/openvg/qsgopenvglayer.h +++ b/src/plugins/scenegraph/openvg/qsgopenvglayer.h @@ -116,7 +116,7 @@ class QSGOpenVGLayerPrivate : public QSGTexturePrivate { Q_DECLARE_PUBLIC(QSGOpenVGLayer) public: - int comparisonKey() const override; + qint64 comparisonKey() const override; }; QT_END_NAMESPACE diff --git a/src/quick/scenegraph/adaptations/software/qsgsoftwarelayer.cpp b/src/quick/scenegraph/adaptations/software/qsgsoftwarelayer.cpp index ffeb55c7ef..0d599209b7 100644 --- a/src/quick/scenegraph/adaptations/software/qsgsoftwarelayer.cpp +++ b/src/quick/scenegraph/adaptations/software/qsgsoftwarelayer.cpp @@ -70,7 +70,7 @@ int QSGSoftwareLayer::textureId() const return 0; } -int QSGSoftwareLayer::comparisonKey() const +qint64 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 4961c91c1c..c1b5686d0a 100644 --- a/src/quick/scenegraph/adaptations/software/qsgsoftwarelayer_p.h +++ b/src/quick/scenegraph/adaptations/software/qsgsoftwarelayer_p.h @@ -70,7 +70,7 @@ public: // QSGTexture interface public: - int comparisonKey() const override; + qint64 comparisonKey() const override; int textureId() const override; QSize textureSize() const override; bool hasAlphaChannel() const override; diff --git a/src/quick/scenegraph/adaptations/software/qsgsoftwarepixmaptexture.cpp b/src/quick/scenegraph/adaptations/software/qsgsoftwarepixmaptexture.cpp index 7a3be61cc0..0812290def 100644 --- a/src/quick/scenegraph/adaptations/software/qsgsoftwarepixmaptexture.cpp +++ b/src/quick/scenegraph/adaptations/software/qsgsoftwarepixmaptexture.cpp @@ -85,7 +85,7 @@ void QSGSoftwarePixmapTexture::bind() Q_UNREACHABLE(); } -int QSGSoftwarePixmapTexture::comparisonKey() const +qint64 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 dda1358124..801d33c091 100644 --- a/src/quick/scenegraph/adaptations/software/qsgsoftwarepixmaptexture_p.h +++ b/src/quick/scenegraph/adaptations/software/qsgsoftwarepixmaptexture_p.h @@ -64,7 +64,7 @@ public: QSGSoftwarePixmapTexture(const QImage &image, uint flags); QSGSoftwarePixmapTexture(const QPixmap &pixmap); - int comparisonKey() const override; + qint64 comparisonKey() const override; int textureId() const override; QSize textureSize() const override; bool hasAlphaChannel() const override; diff --git a/src/quick/scenegraph/compressedtexture/qsgcompressedtexture.cpp b/src/quick/scenegraph/compressedtexture/qsgcompressedtexture.cpp index 9e3cc83433..5f00da48f2 100644 --- a/src/quick/scenegraph/compressedtexture/qsgcompressedtexture.cpp +++ b/src/quick/scenegraph/compressedtexture/qsgcompressedtexture.cpp @@ -90,17 +90,17 @@ int QSGCompressedTexture::textureId() const return m_textureId; } -int QSGCompressedTexture::comparisonKey() const +qint64 QSGCompressedTexture::comparisonKey() const { // not textureId() as that would create an id when not yet done - that's not wanted here if (m_textureId) return m_textureId; if (m_texture) - return int(qintptr(m_texture)); + return qint64(m_texture); // two textures (and so materials) with not-yet-created texture underneath are never equal - return int(qintptr(this)); + return qint64(this); } QSize QSGCompressedTexture::textureSize() const diff --git a/src/quick/scenegraph/compressedtexture/qsgcompressedtexture_p.h b/src/quick/scenegraph/compressedtexture/qsgcompressedtexture_p.h index 00e37098b1..228f8dd7b7 100644 --- a/src/quick/scenegraph/compressedtexture/qsgcompressedtexture_p.h +++ b/src/quick/scenegraph/compressedtexture/qsgcompressedtexture_p.h @@ -73,7 +73,7 @@ public: bool hasAlphaChannel() const override; bool hasMipmaps() const override; - int comparisonKey() const override; + qint64 comparisonKey() const override; int textureId() const override; void bind() override; QRhiTexture *rhiTexture() const override; diff --git a/src/quick/scenegraph/coreapi/qsgtexture.cpp b/src/quick/scenegraph/coreapi/qsgtexture.cpp index 2cf60f317e..65a43785aa 100644 --- a/src/quick/scenegraph/coreapi/qsgtexture.cpp +++ b/src/quick/scenegraph/coreapi/qsgtexture.cpp @@ -489,21 +489,19 @@ bool QSGTexture::isAtlasTexture() const */ /*! - \fn int QSGTexture::comparisonKey() const + \fn qint64 QSGTexture::comparisonKey() const Returns a key suitable for comparing textures. Typically used in QSGMaterial::compare() implementations. Just comparing QSGTexture pointers is not always sufficient because two QSGTexture instances that refer to the same native texture object - underneath should also be considered equal. Hence this function. - - \note Unlike textureId(), implementations of this function are not expected - to and should not create any graphics resources (so texture objects) in - case there is none yet. + underneath should also be considered equal. Hence the need for this function. + Implementations of this function are not expected to, and should not create + any graphics resources (native texture objects) in case there are none yet. A QSGTexture that does not have a native texture object underneath is - typically not equal to any other QSGTexture. There are exceptions to this, + typically \b not equal to any other QSGTexture. There are exceptions to this, in particular when atlasing is used (where multiple textures share the same atlas texture under the hood), that is then up to the subclass implementations to deal with as appropriate. diff --git a/src/quick/scenegraph/coreapi/qsgtexture.h b/src/quick/scenegraph/coreapi/qsgtexture.h index d9176f58f1..067a5151fd 100644 --- a/src/quick/scenegraph/coreapi/qsgtexture.h +++ b/src/quick/scenegraph/coreapi/qsgtexture.h @@ -85,7 +85,7 @@ public: int layout; }; - virtual int comparisonKey() const = 0; + virtual qint64 comparisonKey() const = 0; virtual int textureId() const = 0; // ### Qt 6: remove virtual QRhiTexture *rhiTexture() const; NativeTexture nativeTexture() const; diff --git a/src/quick/scenegraph/qsgopengllayer.cpp b/src/quick/scenegraph/qsgopengllayer.cpp index c585cd05f8..6f06c27459 100644 --- a/src/quick/scenegraph/qsgopengllayer.cpp +++ b/src/quick/scenegraph/qsgopengllayer.cpp @@ -141,7 +141,7 @@ int QSGOpenGLLayer::textureId() const return m_fbo ? m_fbo->texture() : 0; } -int QSGOpenGLLayer::comparisonKey() const +qint64 QSGOpenGLLayer::comparisonKey() const { return m_fbo ? m_fbo->texture() : 0; } diff --git a/src/quick/scenegraph/qsgopengllayer_p.h b/src/quick/scenegraph/qsgopengllayer_p.h index e54571e311..2d33a6e5be 100644 --- a/src/quick/scenegraph/qsgopengllayer_p.h +++ b/src/quick/scenegraph/qsgopengllayer_p.h @@ -90,7 +90,7 @@ public: bool hasMipmaps() const override; int textureId() const override; QSize textureSize() const override { return m_size; } - int comparisonKey() const override; + qint64 comparisonKey() const override; GLenum format() const { return m_format; } void setFormat(GLenum format) override; diff --git a/src/quick/scenegraph/qsgrhilayer.cpp b/src/quick/scenegraph/qsgrhilayer.cpp index 003ffaf5ea..4abd54fe94 100644 --- a/src/quick/scenegraph/qsgrhilayer.cpp +++ b/src/quick/scenegraph/qsgrhilayer.cpp @@ -72,9 +72,9 @@ void QSGRhiLayer::invalidated() m_renderer = nullptr; } -int QSGRhiLayer::comparisonKey() const +qint64 QSGRhiLayer::comparisonKey() const { - return int(qintptr(m_texture)); + return qint64(m_texture); } bool QSGRhiLayer::hasAlphaChannel() const diff --git a/src/quick/scenegraph/qsgrhilayer_p.h b/src/quick/scenegraph/qsgrhilayer_p.h index ba0567c737..eaa2c6d7b0 100644 --- a/src/quick/scenegraph/qsgrhilayer_p.h +++ b/src/quick/scenegraph/qsgrhilayer_p.h @@ -75,7 +75,7 @@ public: void bind() override; int textureId() const override; - int comparisonKey() const override; + qint64 comparisonKey() const override; QRhiTexture *rhiTexture() const override; void commitTextureOperations(QRhi *rhi, QRhiResourceUpdateBatch *resourceUpdates) override; diff --git a/src/quick/scenegraph/qsgrhishadereffectnode.cpp b/src/quick/scenegraph/qsgrhishadereffectnode.cpp index 0508ae9c7a..2afb40a61e 100644 --- a/src/quick/scenegraph/qsgrhishadereffectnode.cpp +++ b/src/quick/scenegraph/qsgrhishadereffectnode.cpp @@ -494,7 +494,7 @@ int QSGRhiShaderEffectMaterial::compare(const QSGMaterial *other) const QSGTexture *t1 = tp1->texture(); QSGTexture *t2 = tp2->texture(); if (t1 && t2) { - if (int diff = t1->comparisonKey() - t2->comparisonKey()) + if (qint64 diff = t1->comparisonKey() - t2->comparisonKey()) return diff; } else { if (!t1 && t2) diff --git a/src/quick/scenegraph/util/qsgopenglatlastexture.cpp b/src/quick/scenegraph/util/qsgopenglatlastexture.cpp index 94f77c8a08..f9bc0a552c 100644 --- a/src/quick/scenegraph/util/qsgopenglatlastexture.cpp +++ b/src/quick/scenegraph/util/qsgopenglatlastexture.cpp @@ -522,7 +522,7 @@ TextureBase::~TextureBase() m_atlas->remove(this); } -int TextureBase::comparisonKey() const +qint64 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 @@ -532,7 +532,7 @@ int TextureBase::comparisonKey() const // base the comparison on the atlas ptr; this way textures for the same // atlas are considered equal - return int(qintptr(m_atlas)); + return qint64(m_atlas); } void TextureBase::bind() diff --git a/src/quick/scenegraph/util/qsgopenglatlastexture_p.h b/src/quick/scenegraph/util/qsgopenglatlastexture_p.h index a81c62ad63..d9d9d3175e 100644 --- a/src/quick/scenegraph/util/qsgopenglatlastexture_p.h +++ b/src/quick/scenegraph/util/qsgopenglatlastexture_p.h @@ -103,7 +103,7 @@ public: void invalidate(); - int comparisonKey() const { return m_texture_id; } + qint64 comparisonKey() const { return m_texture_id; } int textureId() const; void bind(QSGTexture::Filtering filtering); @@ -159,7 +159,7 @@ public: TextureBase(AtlasBase *atlas, const QRect &textureRect); ~TextureBase(); - int comparisonKey() const override; + qint64 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 93448d7738..35d204fed2 100644 --- a/src/quick/scenegraph/util/qsgplaintexture.cpp +++ b/src/quick/scenegraph/util/qsgplaintexture.cpp @@ -304,17 +304,17 @@ void QSGPlainTexture::setTextureFromNativeObject(QRhi *rhi, QQuickWindow::Native setTexture(t); } -int QSGPlainTexture::comparisonKey() const +qint64 QSGPlainTexture::comparisonKey() const { // not textureId() as that would create an id when not yet done - that's not wanted here if (m_texture_id) return m_texture_id; if (m_texture) - return int(qintptr(m_texture)); + return qint64(m_texture); // two textures (and so materials) with not-yet-created texture underneath are never equal - return int(qintptr(this)); + return qint64(this); } QRhiTexture *QSGPlainTexture::rhiTexture() const diff --git a/src/quick/scenegraph/util/qsgplaintexture_p.h b/src/quick/scenegraph/util/qsgplaintexture_p.h index fed464a0b4..3589bdbcf7 100644 --- a/src/quick/scenegraph/util/qsgplaintexture_p.h +++ b/src/quick/scenegraph/util/qsgplaintexture_p.h @@ -83,7 +83,7 @@ public: void setImage(const QImage &image); const QImage &image() { return m_image; } - int comparisonKey() const override; + qint64 comparisonKey() const override; void bind() override; diff --git a/src/quick/scenegraph/util/qsgrhiatlastexture.cpp b/src/quick/scenegraph/util/qsgrhiatlastexture.cpp index 2935c61e7e..e7430a47d4 100644 --- a/src/quick/scenegraph/util/qsgrhiatlastexture.cpp +++ b/src/quick/scenegraph/util/qsgrhiatlastexture.cpp @@ -381,7 +381,7 @@ TextureBase::~TextureBase() m_atlas->remove(this); } -int TextureBase::comparisonKey() const +qint64 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 @@ -391,7 +391,7 @@ int TextureBase::comparisonKey() const // base the comparison on the atlas ptr; this way textures for the same // atlas are considered equal - return int(qintptr(m_atlas)); + return qint64(m_atlas); } QRhiTexture *TextureBase::rhiTexture() const diff --git a/src/quick/scenegraph/util/qsgrhiatlastexture_p.h b/src/quick/scenegraph/util/qsgrhiatlastexture_p.h index 739498f137..0ef14c1ce0 100644 --- a/src/quick/scenegraph/util/qsgrhiatlastexture_p.h +++ b/src/quick/scenegraph/util/qsgrhiatlastexture_p.h @@ -157,7 +157,7 @@ public: TextureBase(AtlasBase *atlas, const QRect &textureRect); ~TextureBase(); - int comparisonKey() const override; + qint64 comparisonKey() const override; int textureId() const override { return 0; } // not used void bind() override { } // not used QRhiTexture *rhiTexture() const override; diff --git a/src/quick/scenegraph/util/qsgtexturematerial.cpp b/src/quick/scenegraph/util/qsgtexturematerial.cpp index e6311fc652..c2fe0364b3 100644 --- a/src/quick/scenegraph/util/qsgtexturematerial.cpp +++ b/src/quick/scenegraph/util/qsgtexturematerial.cpp @@ -378,7 +378,7 @@ int QSGOpaqueTextureMaterial::compare(const QSGMaterial *o) const { Q_ASSERT(o && type() == o->type()); const QSGOpaqueTextureMaterial *other = static_cast(o); - if (int diff = m_texture->comparisonKey() - other->texture()->comparisonKey()) + if (qint64 diff = m_texture->comparisonKey() - other->texture()->comparisonKey()) return diff; return int(m_filtering) - int(other->m_filtering); } -- cgit v1.2.3