Avoid incomplete textures in GL paint engine

...which can happen if the OpenGL implementation reuses IDs
so that a glGenTextures results in the same ID that was used
by a texture destroyed by glDeleteTextures just before.

In this case the lastTextureUsed tracking needs to be aware
and invalidate, otherwise the newly created texture is
assumed to be already existing (the ID is the same as before
after all), and so operations such as setting the minification
filter is skipped (which is fatal with OpenGL given that we
do not use mipmaps).

This is exercised in practice by the drawPixmapFragments test
introduced in the previous patch. On Intel graphics it would
work, but it does not render correctly with NVIDIA due to how
the driver's internal ID handling works.

Pick-to: 6.5.0 6.5 6.4 6.2
Change-Id: Ic2771fe9a2dec7da4aa3804d8498dd89e3c18415
Reviewed-by: Eirik Aavitsland <eirik.aavitsland@qt.io>
This commit is contained in:
Laszlo Agocs 2023-03-01 13:51:58 +01:00
parent d6c5a2f917
commit 7cf6c57648
4 changed files with 67 additions and 30 deletions

View File

@ -160,10 +160,14 @@ template<typename T>
void QOpenGL2PaintEngineExPrivate::updateTexture(GLenum textureUnit, const T &texture, GLenum wrapMode, GLenum filterMode, TextureUpdateMode updateMode)
{
static const GLenum target = GL_TEXTURE_2D;
bool newTextureCreated = false;
activateTextureUnit(textureUnit);
GLuint textureId = bindTexture(texture);
GLuint textureId = bindTexture(texture, &newTextureCreated);
if (newTextureCreated)
lastTextureUsed = GLuint(-1);
if (updateMode == UpdateIfNeeded && textureId == lastTextureUsed)
return;
@ -192,8 +196,11 @@ void QOpenGL2PaintEngineExPrivate::activateTextureUnit(GLenum textureUnit)
}
template<>
GLuint QOpenGL2PaintEngineExPrivate::bindTexture(const GLuint &textureId)
GLuint QOpenGL2PaintEngineExPrivate::bindTexture(const GLuint &textureId, bool *newTextureCreated)
{
if (newTextureCreated)
*newTextureCreated = false;
if (textureId != lastTextureUsed)
funcs.glBindTexture(GL_TEXTURE_2D, textureId);
@ -201,19 +208,25 @@ GLuint QOpenGL2PaintEngineExPrivate::bindTexture(const GLuint &textureId)
}
template<>
GLuint QOpenGL2PaintEngineExPrivate::bindTexture(const QImage &image)
GLuint QOpenGL2PaintEngineExPrivate::bindTexture(const QImage &image, bool *newTextureCreated)
{
return QOpenGLTextureCache::cacheForContext(ctx)->bindTexture(ctx, image);
QOpenGLTextureCache::BindResult result = QOpenGLTextureCache::cacheForContext(ctx)->bindTexture(ctx, image);
if (newTextureCreated)
*newTextureCreated = result.flags.testFlag(QOpenGLTextureCache::BindResultFlag::NewTexture);
return result.id;
}
template<>
GLuint QOpenGL2PaintEngineExPrivate::bindTexture(const QPixmap &pixmap)
GLuint QOpenGL2PaintEngineExPrivate::bindTexture(const QPixmap &pixmap, bool *newTextureCreated)
{
return QOpenGLTextureCache::cacheForContext(ctx)->bindTexture(ctx, pixmap);
QOpenGLTextureCache::BindResult result = QOpenGLTextureCache::cacheForContext(ctx)->bindTexture(ctx, pixmap);
if (newTextureCreated)
*newTextureCreated = result.flags.testFlag(QOpenGLTextureCache::BindResultFlag::NewTexture);
return result.id;
}
template<>
GLuint QOpenGL2PaintEngineExPrivate::bindTexture(const QGradient &gradient)
GLuint QOpenGL2PaintEngineExPrivate::bindTexture(const QGradient &gradient, bool *newTextureCreated)
{
// We apply global opacity in the fragment shaders, so we always pass 1.0
// for opacity to the cache.
@ -223,7 +236,7 @@ GLuint QOpenGL2PaintEngineExPrivate::bindTexture(const QGradient &gradient)
// hasn't been cached yet, but will otherwise return an unbound texture id. To
// be sure that the texture is bound, we unfortunately have to bind again,
// which results in the initial generation of the texture doing two binds.
return bindTexture(textureId);
return bindTexture(textureId, newTextureCreated);
}
struct ImageWithBindOptions
@ -233,9 +246,14 @@ struct ImageWithBindOptions
};
template<>
GLuint QOpenGL2PaintEngineExPrivate::bindTexture(const ImageWithBindOptions &imageWithOptions)
GLuint QOpenGL2PaintEngineExPrivate::bindTexture(const ImageWithBindOptions &imageWithOptions, bool *newTextureCreated)
{
return QOpenGLTextureCache::cacheForContext(ctx)->bindTexture(ctx, imageWithOptions.image, imageWithOptions.options);
QOpenGLTextureCache::BindResult result = QOpenGLTextureCache::cacheForContext(ctx)->bindTexture(ctx,
imageWithOptions.image,
imageWithOptions.options);
if (newTextureCreated)
*newTextureCreated = result.flags.testFlag(QOpenGLTextureCache::BindResultFlag::NewTexture);
return result.id;
}
inline static bool isPowerOfTwo(int x)

View File

@ -177,7 +177,7 @@ public:
template<typename T>
void updateTexture(GLenum textureUnit, const T &texture, GLenum wrapMode, GLenum filterMode, TextureUpdateMode updateMode = UpdateIfNeeded);
template<typename T>
GLuint bindTexture(const T &texture);
GLuint bindTexture(const T &texture, bool *newTextureCreated);
void activateTextureUnit(GLenum textureUnit);
void resetGLState();

View File

@ -81,10 +81,12 @@ QOpenGLTextureCache::~QOpenGLTextureCache()
{
}
GLuint QOpenGLTextureCache::bindTexture(QOpenGLContext *context, const QPixmap &pixmap, QOpenGLTextureUploader::BindOptions options)
QOpenGLTextureCache::BindResult QOpenGLTextureCache::bindTexture(QOpenGLContext *context,
const QPixmap &pixmap,
QOpenGLTextureUploader::BindOptions options)
{
if (pixmap.isNull())
return 0;
return { 0, {} };
QMutexLocker locker(&m_mutex);
qint64 key = pixmap.cacheKey();
@ -93,21 +95,23 @@ GLuint QOpenGLTextureCache::bindTexture(QOpenGLContext *context, const QPixmap &
QOpenGLCachedTexture *entry = m_cache.object(key);
if (entry && entry->options() == options) {
context->functions()->glBindTexture(GL_TEXTURE_2D, entry->id());
return entry->id();
return { entry->id(), {} };
}
}
GLuint id = bindTexture(context, key, pixmap.toImage(), options);
if (id > 0)
BindResult result = bindTexture(context, key, pixmap.toImage(), options);
if (result.id > 0)
QImagePixmapCleanupHooks::enableCleanupHooks(pixmap);
return id;
return result;
}
GLuint QOpenGLTextureCache::bindTexture(QOpenGLContext *context, const QImage &image, QOpenGLTextureUploader::BindOptions options)
QOpenGLTextureCache::BindResult QOpenGLTextureCache::bindTexture(QOpenGLContext *context,
const QImage &image,
QOpenGLTextureUploader::BindOptions options)
{
if (image.isNull())
return 0;
return { 0, {} };
QMutexLocker locker(&m_mutex);
qint64 key = image.cacheKey();
@ -116,7 +120,7 @@ GLuint QOpenGLTextureCache::bindTexture(QOpenGLContext *context, const QImage &i
QOpenGLCachedTexture *entry = m_cache.object(key);
if (entry && entry->options() == options) {
context->functions()->glBindTexture(GL_TEXTURE_2D, entry->id());
return entry->id();
return { entry->id(), {} };
}
}
@ -124,17 +128,20 @@ GLuint QOpenGLTextureCache::bindTexture(QOpenGLContext *context, const QImage &i
if (!context->functions()->hasOpenGLFeature(QOpenGLFunctions::NPOTTextures))
options |= QOpenGLTextureUploader::PowerOfTwoBindOption;
GLuint id = bindTexture(context, key, img, options);
if (id > 0)
BindResult result = bindTexture(context, key, img, options);
if (result.id > 0)
QImagePixmapCleanupHooks::enableCleanupHooks(image);
return id;
return result;
}
Q_TRACE_POINT(qtopengl, QOpenGLTextureCache_bindTexture_entry, QOpenGLContext *context, qint64 key, const unsigned char *image, int options);
Q_TRACE_POINT(qtopengl, QOpenGLTextureCache_bindTexture_exit);
GLuint QOpenGLTextureCache::bindTexture(QOpenGLContext *context, qint64 key, const QImage &image, QOpenGLTextureUploader::BindOptions options)
QOpenGLTextureCache::BindResult QOpenGLTextureCache::bindTexture(QOpenGLContext *context,
qint64 key,
const QImage &image,
QOpenGLTextureUploader::BindOptions options)
{
Q_TRACE_SCOPE(QOpenGLTextureCache_bindTexture, context, key, image.bits(), options);
@ -147,7 +154,7 @@ GLuint QOpenGLTextureCache::bindTexture(QOpenGLContext *context, qint64 key, con
m_cache.insert(key, new QOpenGLCachedTexture(id, options, context), cost / 1024);
return id;
return { id, BindResultFlag::NewTexture };
}
void QOpenGLTextureCache::invalidate(qint64 key)

View File

@ -35,10 +35,20 @@ public:
QOpenGLTextureCache(QOpenGLContext *);
~QOpenGLTextureCache();
GLuint bindTexture(QOpenGLContext *context, const QPixmap &pixmap,
QOpenGLTextureUploader::BindOptions options = QOpenGLTextureUploader::PremultipliedAlphaBindOption);
GLuint bindTexture(QOpenGLContext *context, const QImage &image,
QOpenGLTextureUploader::BindOptions options = QOpenGLTextureUploader::PremultipliedAlphaBindOption);
enum class BindResultFlag : quint8 {
NewTexture = 0x01
};
Q_DECLARE_FLAGS(BindResultFlags, BindResultFlag)
struct BindResult {
GLuint id;
BindResultFlags flags;
};
BindResult bindTexture(QOpenGLContext *context, const QPixmap &pixmap,
QOpenGLTextureUploader::BindOptions options = QOpenGLTextureUploader::PremultipliedAlphaBindOption);
BindResult bindTexture(QOpenGLContext *context, const QImage &image,
QOpenGLTextureUploader::BindOptions options = QOpenGLTextureUploader::PremultipliedAlphaBindOption);
void invalidate(qint64 key);
@ -46,12 +56,14 @@ public:
void freeResource(QOpenGLContext *ctx) override;
private:
GLuint bindTexture(QOpenGLContext *context, qint64 key, const QImage &image, QOpenGLTextureUploader::BindOptions options);
BindResult bindTexture(QOpenGLContext *context, qint64 key, const QImage &image, QOpenGLTextureUploader::BindOptions options);
QMutex m_mutex;
QCache<quint64, QOpenGLCachedTexture> m_cache;
};
Q_DECLARE_OPERATORS_FOR_FLAGS(QOpenGLTextureCache::BindResultFlags)
class QOpenGLCachedTexture
{
public: