From 6db96dcd4acccbc13161f85adf3164907b7b5cae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20R=C3=B8dal?= Date: Thu, 10 Dec 2009 12:52:10 +0100 Subject: Fix redraw bugs when using graphics effects in device coordinate mode. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QGraphicsEffect::boundingRectFor() needs the source bounding rect in device coordinates. This patch fixes the documentation to reflect this, and fixes some internal usage of boundingRectFor() to ensure it always gets the device rect of the source. Task-number: QTBUG-5918 Reviewed-by: Bjørn Erik Nilsen --- src/gui/effects/qgraphicseffect.cpp | 14 +++++-- src/gui/graphicsview/qgraphicsitem.cpp | 54 +++++++++++++++++++++----- src/gui/graphicsview/qgraphicsitem_p.h | 2 + src/gui/graphicsview/qgraphicsview.cpp | 26 +++++++++++++ src/gui/graphicsview/qgraphicsview.h | 1 + src/gui/graphicsview/qgraphicsview_p.h | 3 ++ tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp | 9 ++++- 7 files changed, 95 insertions(+), 14 deletions(-) diff --git a/src/gui/effects/qgraphicseffect.cpp b/src/gui/effects/qgraphicseffect.cpp index 239e29c..d6cabaa 100644 --- a/src/gui/effects/qgraphicseffect.cpp +++ b/src/gui/effects/qgraphicseffect.cpp @@ -160,6 +160,10 @@ QRectF QGraphicsEffectSource::boundingRect(Qt::CoordinateSystem system) const /*! Returns the bounding rectangle of the source mapped to the given \a system. + Calling this function with Qt::DeviceCoordinates outside of + QGraphicsEffect::draw() will give undefined results, as there is no device + context available. + \sa draw() */ QRectF QGraphicsEffect::sourceBoundingRect(Qt::CoordinateSystem system) const @@ -348,6 +352,10 @@ QPixmap QGraphicsEffectSource::pixmap(Qt::CoordinateSystem system, QPoint *offse The returned pixmap is clipped to the current painter's device rectangle when \a system is Qt::DeviceCoordinates. + Calling this function with Qt::DeviceCoordinates outside of + QGraphicsEffect::draw() will give undefined results, as there is no device + context available. + \sa draw(), boundingRect() */ QPixmap QGraphicsEffect::sourcePixmap(Qt::CoordinateSystem system, QPoint *offset, QGraphicsEffect::PixmapPadMode mode) const @@ -398,8 +406,8 @@ QGraphicsEffect::~QGraphicsEffect() /*! Returns the effective bounding rectangle for this effect, i.e., the - bounding rectangle of the source, adjusted by any margins applied by - the effect itself. + bounding rectangle of the source in device coordinates, adjusted by + any margins applied by the effect itself. \sa boundingRectFor(), updateBoundingRect() */ @@ -413,7 +421,7 @@ QRectF QGraphicsEffect::boundingRect() const /*! Returns the effective bounding rectangle for this effect, given the - provided \a rect in the source's coordinate space. When writing + provided \a rect in the device coordinates. When writing you own custom effect, you must call updateBoundingRect() whenever any parameters are changed that may cause this this function to return a different value. diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp index 90cc132..8bbe929 100644 --- a/src/gui/graphicsview/qgraphicsitem.cpp +++ b/src/gui/graphicsview/qgraphicsitem.cpp @@ -2584,6 +2584,35 @@ void QGraphicsItem::setGraphicsEffect(QGraphicsEffect *effect) /*! \internal \since 4.6 + Returns the effective bounding rect of the given item space rect. + If the item has no effect, the rect is returned unmodified. + If the item has an effect, the effective rect can be extend beyond the + item's bounding rect, depending on the effect. + + \sa boundingRect() +*/ +QRectF QGraphicsItemPrivate::effectiveBoundingRect(const QRectF &rect) const +{ +#ifndef QT_NO_GRAPHICSEFFECT + Q_Q(const QGraphicsItem); + QGraphicsEffect *effect = graphicsEffect; + if (scene && effect && effect->isEnabled()) { + QRectF sceneRect = q->mapRectToScene(rect); + QRectF sceneEffectRect; + foreach (QGraphicsView *view, scene->views()) { + QRectF deviceRect = view->d_func()->mapRectFromScene(sceneRect); + QRect deviceEffectRect = effect->boundingRectFor(deviceRect).toAlignedRect(); + sceneEffectRect |= view->d_func()->mapRectToScene(deviceEffectRect); + } + return q->mapRectFromScene(sceneEffectRect); + } +#endif //QT_NO_GRAPHICSEFFECT + return rect; +} + +/*! + \internal + \since 4.6 Returns the effective bounding rect of the item. If the item has no effect, this is the same as the item's bounding rect. If the item has an effect, the effective rect can be larger than the item's @@ -2594,16 +2623,19 @@ void QGraphicsItem::setGraphicsEffect(QGraphicsEffect *effect) QRectF QGraphicsItemPrivate::effectiveBoundingRect() const { #ifndef QT_NO_GRAPHICSEFFECT - QGraphicsEffect *effect = graphicsEffect; - QRectF brect = effect && effect->isEnabled() ? effect->boundingRect() : q_ptr->boundingRect(); + Q_Q(const QGraphicsItem); + QRectF brect = effectiveBoundingRect(q_ptr->boundingRect()); if (ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren) return brect; const QGraphicsItem *effectParent = parent; while (effectParent) { - effect = effectParent->d_ptr->graphicsEffect; - if (effect && effect->isEnabled()) - brect = effect->boundingRectFor(brect); + QGraphicsEffect *effect = effectParent->d_ptr->graphicsEffect; + if (scene && effect && effect->isEnabled()) { + const QRectF brectInParentSpace = q->mapRectToItem(effectParent, brect); + const QRectF effectRectInParentSpace = effectParent->d_ptr->effectiveBoundingRect(brectInParentSpace); + brect = effectParent->mapRectToItem(q, effectRectInParentSpace); + } if (effectParent->d_ptr->ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren) return brect; effectParent = effectParent->d_ptr->parent; @@ -10649,17 +10681,21 @@ QPixmap QGraphicsItemEffectSourcePrivate::pixmap(Qt::CoordinateSystem system, QP QGraphicsScenePrivate *scened = item->d_ptr->scene->d_func(); const QRectF sourceRect = boundingRect(system); - QRect effectRect; + QRectF effectRectF; if (mode == QGraphicsEffect::PadToEffectiveBoundingRect) { - effectRect = item->graphicsEffect()->boundingRectFor(sourceRect).toAlignedRect(); + effectRectF = item->graphicsEffect()->boundingRectFor(boundingRect(Qt::DeviceCoordinates)); + if (system == Qt::LogicalCoordinates) + effectRectF = info->painter->worldTransform().inverted().mapRect(effectRectF); } else if (mode == QGraphicsEffect::PadToTransparentBorder) { // adjust by 1.5 to account for cosmetic pens - effectRect = sourceRect.adjusted(-1.5, -1.5, 1.5, 1.5).toAlignedRect(); + effectRectF = sourceRect.adjusted(-1.5, -1.5, 1.5, 1.5); } else { - effectRect = sourceRect.toAlignedRect(); + effectRectF = sourceRect; } + QRect effectRect = effectRectF.toAlignedRect(); + if (offset) *offset = effectRect.topLeft(); diff --git a/src/gui/graphicsview/qgraphicsitem_p.h b/src/gui/graphicsview/qgraphicsitem_p.h index 8f9fe54..2d8de65 100644 --- a/src/gui/graphicsview/qgraphicsitem_p.h +++ b/src/gui/graphicsview/qgraphicsitem_p.h @@ -236,6 +236,8 @@ public: QRectF effectiveBoundingRect() const; QRectF sceneEffectiveBoundingRect() const; + QRectF effectiveBoundingRect(const QRectF &rect) const; + virtual void resolveFont(uint inheritedMask) { for (int i = 0; i < children.size(); ++i) diff --git a/src/gui/graphicsview/qgraphicsview.cpp b/src/gui/graphicsview/qgraphicsview.cpp index 3f9f443..ffe64aa 100644 --- a/src/gui/graphicsview/qgraphicsview.cpp +++ b/src/gui/graphicsview/qgraphicsview.cpp @@ -545,6 +545,32 @@ qint64 QGraphicsViewPrivate::verticalScroll() const /*! \internal + + Maps the given rectangle to the scene using QTransform::mapRect() +*/ +QRectF QGraphicsViewPrivate::mapRectToScene(const QRect &rect) const +{ + if (dirtyScroll) + const_cast(this)->updateScroll(); + QRectF scrolled = QRectF(rect.translated(scrollX, scrollY)); + return identityMatrix ? scrolled : matrix.inverted().mapRect(scrolled); +} + + +/*! + \internal + + Maps the given rectangle from the scene using QTransform::mapRect() +*/ +QRectF QGraphicsViewPrivate::mapRectFromScene(const QRectF &rect) const +{ + if (dirtyScroll) + const_cast(this)->updateScroll(); + return (identityMatrix ? rect : matrix.mapRect(rect)).translated(-scrollX, -scrollY); +} + +/*! + \internal */ void QGraphicsViewPrivate::updateScroll() { diff --git a/src/gui/graphicsview/qgraphicsview.h b/src/gui/graphicsview/qgraphicsview.h index 2aed0e6..90576e5 100644 --- a/src/gui/graphicsview/qgraphicsview.h +++ b/src/gui/graphicsview/qgraphicsview.h @@ -278,6 +278,7 @@ private: friend class QGraphicsSceneWidget; friend class QGraphicsScene; friend class QGraphicsScenePrivate; + friend class QGraphicsItemPrivate; }; Q_DECLARE_OPERATORS_FOR_FLAGS(QGraphicsView::CacheMode) diff --git a/src/gui/graphicsview/qgraphicsview_p.h b/src/gui/graphicsview/qgraphicsview_p.h index cd161ad..d4b718e 100644 --- a/src/gui/graphicsview/qgraphicsview_p.h +++ b/src/gui/graphicsview/qgraphicsview_p.h @@ -86,6 +86,9 @@ public: qint64 horizontalScroll() const; qint64 verticalScroll() const; + QRectF mapRectToScene(const QRect &rect) const; + QRectF mapRectFromScene(const QRectF &rect) const; + QPointF mousePressItemPoint; QPointF mousePressScenePoint; QPoint mousePressViewPoint; diff --git a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp index 6266933..3e7a2eb 100644 --- a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp +++ b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp @@ -233,6 +233,7 @@ public: void paint(QPainter *painter, const QStyleOptionGraphicsItem *, QWidget *) { hints = painter->renderHints(); + painter->setBrush(brush); painter->drawRect(boundingRect()); ++repaints; } @@ -247,6 +248,7 @@ public: QPainter::RenderHints hints; int repaints; QRectF br; + QBrush brush; }; class tst_QGraphicsItem : public QObject @@ -7646,17 +7648,20 @@ void tst_QGraphicsItem::hitTestGraphicsEffectItem() EventTester *item1 = new EventTester; item1->br = itemBoundingRect; item1->setPos(-200, -200); + item1->brush = Qt::red; EventTester *item2 = new EventTester; item2->br = itemBoundingRect; item2->setFlag(QGraphicsItem::ItemIgnoresTransformations); item2->setParentItem(item1); item2->setPos(200, 200); + item2->brush = Qt::green; EventTester *item3 = new EventTester; item3->br = itemBoundingRect; item3->setParentItem(item2); item3->setPos(80, 80); + item3->brush = Qt::blue; scene.addItem(item1); QTest::qWait(100); @@ -7671,8 +7676,8 @@ void tst_QGraphicsItem::hitTestGraphicsEffectItem() item1->setGraphicsEffect(shadow); QTest::qWait(50); - // Make sure all items are repainted. - QCOMPARE(item1->repaints, 1); + // Make sure all visible items are repainted. + QCOMPARE(item1->repaints, 0); QCOMPARE(item2->repaints, 1); QCOMPARE(item3->repaints, 1); -- cgit v0.12 From bf9456c5a2d8dfe9a35a2175186630cb426858ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Trond=20Kjern=C3=A5sen?= Date: Thu, 10 Dec 2009 17:38:30 +0100 Subject: Optimize our GL extension checks to avoid mallocs. We want to avoid any unnecessary mallocs when checking GL/GLX extensions. The GL extension string can be quite long and contain several hundred extensions. The old code forced one malloc for each extension + 1 extra malloc for the extension string itself when it was copied into the QByteArray. Reviewed-by: Kim --- src/opengl/qgl.cpp | 47 ++++++++++++++++++++++++----------------------- src/opengl/qgl_p.h | 43 +++++++++++++++++++++++++++++++++++++++++++ src/opengl/qgl_x11.cpp | 24 ++++++++++++------------ 3 files changed, 79 insertions(+), 35 deletions(-) diff --git a/src/opengl/qgl.cpp b/src/opengl/qgl.cpp index dcf8c00..b6f8919 100644 --- a/src/opengl/qgl.cpp +++ b/src/opengl/qgl.cpp @@ -4822,38 +4822,39 @@ QGLWidget::QGLWidget(QGLContext *context, QWidget *parent, void QGLExtensions::init_extensions() { - QList extensions = QByteArray(reinterpret_cast(glGetString(GL_EXTENSIONS))).split(' '); - if (extensions.contains("GL_ARB_texture_rectangle")) + QGLExtensionMatcher extensions(reinterpret_cast(glGetString(GL_EXTENSIONS))); + + if (extensions.match("GL_ARB_texture_rectangle")) glExtensions |= TextureRectangle; - if (extensions.contains("GL_ARB_multisample")) + if (extensions.match("GL_ARB_multisample")) glExtensions |= SampleBuffers; - if (extensions.contains("GL_SGIS_generate_mipmap")) + if (extensions.match("GL_SGIS_generate_mipmap")) glExtensions |= GenerateMipmap; - if (extensions.contains("GL_ARB_texture_compression")) + if (extensions.match("GL_ARB_texture_compression")) glExtensions |= TextureCompression; - if (extensions.contains("GL_EXT_texture_compression_s3tc")) + if (extensions.match("GL_EXT_texture_compression_s3tc")) glExtensions |= DDSTextureCompression; - if (extensions.contains("GL_OES_compressed_ETC1_RGB8_texture")) + if (extensions.match("GL_OES_compressed_ETC1_RGB8_texture")) glExtensions |= ETC1TextureCompression; - if (extensions.contains("GL_IMG_texture_compression_pvrtc")) + if (extensions.match("GL_IMG_texture_compression_pvrtc")) glExtensions |= PVRTCTextureCompression; - if (extensions.contains("GL_ARB_fragment_program")) + if (extensions.match("GL_ARB_fragment_program")) glExtensions |= FragmentProgram; - if (extensions.contains("GL_ARB_fragment_shader")) + if (extensions.match("GL_ARB_fragment_shader")) glExtensions |= FragmentShader; - if (extensions.contains("GL_ARB_texture_mirrored_repeat")) + if (extensions.match("GL_ARB_texture_mirrored_repeat")) glExtensions |= MirroredRepeat; - if (extensions.contains("GL_EXT_framebuffer_object")) + if (extensions.match("GL_EXT_framebuffer_object")) glExtensions |= FramebufferObject; - if (extensions.contains("GL_EXT_stencil_two_side")) + if (extensions.match("GL_EXT_stencil_two_side")) glExtensions |= StencilTwoSide; - if (extensions.contains("GL_EXT_stencil_wrap")) + if (extensions.match("GL_EXT_stencil_wrap")) glExtensions |= StencilWrap; - if (extensions.contains("GL_EXT_packed_depth_stencil")) + if (extensions.match("GL_EXT_packed_depth_stencil")) glExtensions |= PackedDepthStencil; - if (extensions.contains("GL_NV_float_buffer")) + if (extensions.match("GL_NV_float_buffer")) glExtensions |= NVFloatBuffer; - if (extensions.contains("GL_ARB_pixel_buffer_object")) + if (extensions.match("GL_ARB_pixel_buffer_object")) glExtensions |= PixelBufferObject; #if defined(QT_OPENGL_ES_2) glExtensions |= FramebufferObject; @@ -4861,26 +4862,26 @@ void QGLExtensions::init_extensions() glExtensions |= FragmentShader; #endif #if defined(QT_OPENGL_ES_1) || defined(QT_OPENGL_ES_1_CL) - if (extensions.contains("GL_OES_framebuffer_object")) + if (extensions.match("GL_OES_framebuffer_object")) glExtensions |= FramebufferObject; #endif #if defined(QT_OPENGL_ES) - if (extensions.contains("GL_OES_packed_depth_stencil")) + if (extensions.match("GL_OES_packed_depth_stencil")) glExtensions |= PackedDepthStencil; #endif - if (extensions.contains("GL_ARB_framebuffer_object")) { + if (extensions.match("GL_ARB_framebuffer_object")) { // ARB_framebuffer_object also includes EXT_framebuffer_blit. glExtensions |= FramebufferObject; glExtensions |= FramebufferBlit; } - if (extensions.contains("GL_EXT_framebuffer_blit")) + if (extensions.match("GL_EXT_framebuffer_blit")) glExtensions |= FramebufferBlit; - if (extensions.contains("GL_ARB_texture_non_power_of_two")) + if (extensions.match("GL_ARB_texture_non_power_of_two")) glExtensions |= NPOTTextures; - if (extensions.contains("GL_EXT_bgra")) + if (extensions.match("GL_EXT_bgra")) glExtensions |= BGRATextureFormat; } diff --git a/src/opengl/qgl_p.h b/src/opengl/qgl_p.h index 8a0b31f..11770d3 100644 --- a/src/opengl/qgl_p.h +++ b/src/opengl/qgl_p.h @@ -610,6 +610,49 @@ private: friend class QGLContextGroup; }; + +// This class can be used to match GL extensions with doing any mallocs. The +// class assumes that the GL extension string ends with a space character, +// which it should do on all conformant platforms. Create the object and pass +// in a pointer to the extension string, then call match() on each extension +// that should be matched. The match() function takes the extension name +// *without* the terminating space character as input. + +class QGLExtensionMatcher +{ +public: + QGLExtensionMatcher(const char *str) + : gl_extensions(str), gl_extensions_length(qstrlen(str)) + {} + + bool match(const char *str) { + int str_length = qstrlen(str); + const char *extensions = gl_extensions; + int extensions_length = gl_extensions_length; + + while (1) { + // the total length that needs to be matched is the str_length + + // the space character that terminates the extension name + if (extensions_length < str_length + 1) + return false; + if (qstrncmp(extensions, str, str_length) == 0 && extensions[str_length] == ' ') + return true; + + int split_pos = 0; + while (split_pos < extensions_length && extensions[split_pos] != ' ') + ++split_pos; + ++split_pos; // added for the terminating space character + extensions += split_pos; + extensions_length -= split_pos; + } + return false; + } + +private: + const char *gl_extensions; + int gl_extensions_length; +}; + QT_END_NAMESPACE #endif // QGL_P_H diff --git a/src/opengl/qgl_x11.cpp b/src/opengl/qgl_x11.cpp index a037282..9c3fc79 100644 --- a/src/opengl/qgl_x11.cpp +++ b/src/opengl/qgl_x11.cpp @@ -343,8 +343,8 @@ void* qglx_getProcAddress(const char* procName) static bool triedResolvingGlxGetProcAddress = false; if (!triedResolvingGlxGetProcAddress) { triedResolvingGlxGetProcAddress = true; - QList glxExt = QByteArray(glXGetClientString(QX11Info::display(), GLX_EXTENSIONS)).split(' '); - if (glxExt.contains("GLX_ARB_get_proc_address")) { + QGLExtensionMatcher extensions(glXGetClientString(QX11Info::display(), GLX_EXTENSIONS)); + if (extensions.match("GLX_ARB_get_proc_address")) { #if defined(Q_OS_LINUX) || defined(Q_OS_BSD4) void *handle = dlopen(NULL, RTLD_LAZY); if (handle) { @@ -523,8 +523,8 @@ bool QGLContext::chooseContext(const QGLContext* shareContext) if (!d->gpm) return false; } - QList glxExt = QByteArray(glXQueryExtensionsString(xinfo->display(), xinfo->screen())).split(' '); - if (glxExt.contains("GLX_SGI_video_sync")) { + QGLExtensionMatcher extensions(glXQueryExtensionsString(xinfo->display(), xinfo->screen())); + if (extensions.match("GLX_SGI_video_sync")) { if (d->glFormat.swapInterval() == -1) d->glFormat.setSwapInterval(0); } else { @@ -630,8 +630,8 @@ void *QGLContext::tryVisual(const QGLFormat& f, int bufDepth) static bool useTranspExt = false; static bool useTranspExtChecked = false; if (f.plane() && !useTranspExtChecked && d->paintDevice) { - QByteArray estr(glXQueryExtensionsString(xinfo->display(), xinfo->screen())); - useTranspExt = estr.contains("GLX_EXT_visual_info"); + QGLExtensionMatcher extensions(glXQueryExtensionsString(xinfo->display(), xinfo->screen())); + useTranspExt = extensions.match("GLX_EXT_visual_info"); //# (A bit simplistic; that could theoretically be a substring) if (useTranspExt) { QByteArray cstr(glXGetClientString(xinfo->display(), GLX_VENDOR)); @@ -875,8 +875,8 @@ void QGLContext::swapBuffers() const static bool resolved = false; if (!resolved) { const QX11Info *xinfo = qt_x11Info(d->paintDevice); - QList glxExt = QByteArray(glXQueryExtensionsString(xinfo->display(), xinfo->screen())).split(' '); - if (glxExt.contains("GLX_SGI_video_sync")) { + QGLExtensionMatcher extensions(glXQueryExtensionsString(xinfo->display(), xinfo->screen())); + if (extensions.match("GLX_SGI_video_sync")) { glXGetVideoSyncSGI = (qt_glXGetVideoSyncSGI)qglx_getProcAddress("glXGetVideoSyncSGI"); glXWaitVideoSyncSGI = (qt_glXWaitVideoSyncSGI)qglx_getProcAddress("glXWaitVideoSyncSGI"); } @@ -1107,8 +1107,8 @@ void *QGLContext::getProcAddress(const QString &proc) const if (resolved && !glXGetProcAddressARB) return 0; if (!glXGetProcAddressARB) { - QList glxExt = QByteArray(glXGetClientString(QX11Info::display(), GLX_EXTENSIONS)).split(' '); - if (glxExt.contains("GLX_ARB_get_proc_address")) { + QGLExtensionMatcher extensions(glXGetClientString(QX11Info::display(), GLX_EXTENSIONS)); + if (extensions.match("GLX_ARB_get_proc_address")) { #if defined(Q_OS_LINUX) || defined(Q_OS_BSD4) void *handle = dlopen(NULL, RTLD_LAZY); if (handle) { @@ -1609,8 +1609,8 @@ static bool qt_resolveTextureFromPixmap(QPaintDevice *paintDevice) return false; // Can't use TFP without NPOT } const QX11Info *xinfo = qt_x11Info(paintDevice); - QList glxExt = QByteArray(glXQueryExtensionsString(xinfo->display(), xinfo->screen())).split(' '); - if (glxExt.contains("GLX_EXT_texture_from_pixmap")) { + QGLExtensionMatcher extensions(glXQueryExtensionsString(xinfo->display(), xinfo->screen())); + if (extensions.match("GLX_EXT_texture_from_pixmap")) { glXBindTexImageEXT = (qt_glXBindTexImageEXT) qglx_getProcAddress("glXBindTexImageEXT"); glXReleaseTexImageEXT = (qt_glXReleaseTexImageEXT) qglx_getProcAddress("glXReleaseTexImageEXT"); } -- cgit v0.12 From 3a8a1f83d60ec16e4c61e2b0a327a5af02917a5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20R=C3=B8dal?= Date: Thu, 10 Dec 2009 17:22:53 +0100 Subject: Prevented leak of keys in QPixmapCache. Removing a pixmap from the pixmap cache using the new QPixmapCache::Key API left the keys dangling in the QCache's internal QHash. The problem is that the Key is invalidated as soon as the QPixmapCacheEntry is destroyed, thus removing it from the hash failed. Reordering the destruction of the object and the removal of the key in QHash fixes the problem. Reviewed-by: Alexis Reviewed-by: Thiago --- src/corelib/tools/qcache.h | 2 +- src/gui/image/qpixmapcache.cpp | 5 +++++ tests/auto/qpixmapcache/tst_qpixmapcache.cpp | 19 +++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/corelib/tools/qcache.h b/src/corelib/tools/qcache.h index 46e20b1..ee9523f 100644 --- a/src/corelib/tools/qcache.h +++ b/src/corelib/tools/qcache.h @@ -70,8 +70,8 @@ class QCache if (l == &n) l = n.p; if (f == &n) f = n.n; total -= n.c; - delete n.t; hash.remove(*n.keyPtr); + delete n.t; } inline T *relink(const Key &key) { typename QHash::iterator i = hash.find(key); diff --git a/src/gui/image/qpixmapcache.cpp b/src/gui/image/qpixmapcache.cpp index b0b7d72..b7b29d7 100644 --- a/src/gui/image/qpixmapcache.cpp +++ b/src/gui/image/qpixmapcache.cpp @@ -424,6 +424,11 @@ QPixmapCache::KeyData* QPMCache::getKeyData(QPixmapCache::Key *key) Q_GLOBAL_STATIC(QPMCache, pm_cache) +int Q_AUTOTEST_EXPORT q_QPixmapCache_keyHashSize() +{ + return pm_cache()->size(); +} + QPixmapCacheEntry::~QPixmapCacheEntry() { pm_cache()->releaseKey(key); diff --git a/tests/auto/qpixmapcache/tst_qpixmapcache.cpp b/tests/auto/qpixmapcache/tst_qpixmapcache.cpp index 9775d36..c89a182 100644 --- a/tests/auto/qpixmapcache/tst_qpixmapcache.cpp +++ b/tests/auto/qpixmapcache/tst_qpixmapcache.cpp @@ -70,6 +70,7 @@ private slots: void remove(); void clear(); void pixmapKey(); + void noLeak(); }; static QPixmapCache::KeyData* getPrivate(QPixmapCache::Key &key) @@ -482,5 +483,23 @@ void tst_QPixmapCache::pixmapKey() QVERIFY(!getPrivate(key8)); } +extern int q_QPixmapCache_keyHashSize(); + +void tst_QPixmapCache::noLeak() +{ + QPixmapCache::Key key; + + int oldSize = q_QPixmapCache_keyHashSize(); + for (int i = 0; i < 100; ++i) { + QPixmap pm(128, 128); + pm.fill(Qt::transparent); + key = QPixmapCache::insert(pm); + QPixmapCache::remove(key); + } + int newSize = q_QPixmapCache_keyHashSize(); + + QCOMPARE(oldSize, newSize); +} + QTEST_MAIN(tst_QPixmapCache) #include "tst_qpixmapcache.moc" -- cgit v0.12