summaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorEskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@nokia.com>2010-10-28 07:27:24 (GMT)
committerEskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@nokia.com>2010-10-29 10:07:24 (GMT)
commitc76f9f15d1c604cdf3d68d2147a4a06d227871fa (patch)
treeb92a53c12a38e7f8282ce92d47ea80c1510dd6b9 /src
parentcb2a108d79b09ec6ea91106e7f33826dceac4a42 (diff)
downloadQt-c76f9f15d1c604cdf3d68d2147a4a06d227871fa.zip
Qt-c76f9f15d1c604cdf3d68d2147a4a06d227871fa.tar.gz
Qt-c76f9f15d1c604cdf3d68d2147a4a06d227871fa.tar.bz2
Fix possible crash in QStaticText and QDeclarativeTextLayout
The QStaticTextItem held an uncounted reference to QFontEngine. The pointer would dangle in some cases where there was no font object referencing the engine and the cache was cleaned out (e.g. when a new application font is added.) Properly count the reference, and also add reference counting to userData to make it harder to shoot yourself in the foot, since the QStaticTextItem class is now being used in different places, Task-number: QTBUG-14446 Reviewed-by: Martin Jones
Diffstat (limited to 'src')
-rw-r--r--src/declarative/graphicsitems/qdeclarativetextlayout.cpp4
-rw-r--r--src/gui/painting/qpaintengine_raster.cpp2
-rw-r--r--src/gui/painting/qpainter.cpp4
-rw-r--r--src/gui/text/qstatictext.cpp22
-rw-r--r--src/gui/text/qstatictext_p.h54
-rw-r--r--src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp24
-rw-r--r--src/opengl/qpaintengine_opengl.cpp8
-rw-r--r--src/openvg/qpaintengine_vg.cpp2
8 files changed, 88 insertions, 32 deletions
diff --git a/src/declarative/graphicsitems/qdeclarativetextlayout.cpp b/src/declarative/graphicsitems/qdeclarativetextlayout.cpp
index 89a2158..635ceab 100644
--- a/src/declarative/graphicsitems/qdeclarativetextlayout.cpp
+++ b/src/declarative/graphicsitems/qdeclarativetextlayout.cpp
@@ -94,7 +94,7 @@ class DrawTextItemRecorder: public QPaintEngine
if (!m_inertText->items.isEmpty()) {
QStaticTextItem &last = m_inertText->items[m_inertText->items.count() - 1];
- if (last.fontEngine == ti.fontEngine && last.font == ti.font() &&
+ if (last.fontEngine() == ti.fontEngine && last.font == ti.font() &&
(!m_dirtyPen || last.color == state->pen().color())) {
needFreshCurrentItem = false;
@@ -107,7 +107,7 @@ class DrawTextItemRecorder: public QPaintEngine
if (needFreshCurrentItem) {
QStaticTextItem currentItem;
- currentItem.fontEngine = ti.fontEngine;
+ currentItem.setFontEngine(ti.fontEngine);
currentItem.font = ti.font();
currentItem.charOffset = charOffset;
currentItem.numChars = ti.num_chars;
diff --git a/src/gui/painting/qpaintengine_raster.cpp b/src/gui/painting/qpaintengine_raster.cpp
index 8f9d8ba..d4b044b 100644
--- a/src/gui/painting/qpaintengine_raster.cpp
+++ b/src/gui/painting/qpaintengine_raster.cpp
@@ -3300,7 +3300,7 @@ void QRasterPaintEngine::drawStaticTextItem(QStaticTextItem *textItem)
ensureState();
drawCachedGlyphs(textItem->numGlyphs, textItem->glyphs, textItem->glyphPositions,
- textItem->fontEngine);
+ textItem->fontEngine());
}
/*!
diff --git a/src/gui/painting/qpainter.cpp b/src/gui/painting/qpainter.cpp
index 7fed7e4..ab9707d 100644
--- a/src/gui/painting/qpainter.cpp
+++ b/src/gui/painting/qpainter.cpp
@@ -5746,7 +5746,7 @@ void QPainterPrivate::drawGlyphs(const quint32 *glyphArray, const QPointF *posit
QStaticTextItem staticTextItem;
staticTextItem.color = state->pen.color();
staticTextItem.font = state->font;
- staticTextItem.fontEngine = fontEngine;
+ staticTextItem.setFontEngine(fontEngine);
staticTextItem.numGlyphs = glyphCount;
staticTextItem.glyphs = reinterpret_cast<glyph_t *>(const_cast<glyph_t *>(glyphArray));
staticTextItem.glyphPositions = positions.data();
@@ -5938,7 +5938,7 @@ void QPainter::drawStaticText(const QPointF &topLeftPosition, const QStaticText
d->extended->drawStaticTextItem(item);
drawDecorationForGlyphs(this, item->glyphs, item->glyphPositions,
- item->numGlyphs, item->fontEngine, staticText_d->font,
+ item->numGlyphs, item->fontEngine(), staticText_d->font,
QTextCharFormat());
}
if (currentColor != oldPen.color())
diff --git a/src/gui/text/qstatictext.cpp b/src/gui/text/qstatictext.cpp
index 9506006..edf248a 100644
--- a/src/gui/text/qstatictext.cpp
+++ b/src/gui/text/qstatictext.cpp
@@ -445,7 +445,7 @@ namespace {
const QTextItemInt &ti = static_cast<const QTextItemInt &>(textItem);
QStaticTextItem currentItem;
- currentItem.fontEngine = ti.fontEngine;
+ currentItem.setFontEngine(ti.fontEngine);
currentItem.font = ti.font();
currentItem.charOffset = m_chars.size();
currentItem.numChars = ti.num_chars;
@@ -713,4 +713,24 @@ void QStaticTextPrivate::init()
needsRelayout = false;
}
+QStaticTextItem::~QStaticTextItem()
+{
+ if (m_userData != 0 && !m_userData->ref.deref())
+ delete m_userData;
+ if (!m_fontEngine->ref.deref())
+ delete m_fontEngine;
+}
+
+void QStaticTextItem::setFontEngine(QFontEngine *fe)
+{
+ if (m_fontEngine != 0) {
+ if (!m_fontEngine->ref.deref())
+ delete m_fontEngine;
+ }
+
+ m_fontEngine = fe;
+ if (m_fontEngine != 0)
+ m_fontEngine->ref.ref();
+}
+
QT_END_NAMESPACE
diff --git a/src/gui/text/qstatictext_p.h b/src/gui/text/qstatictext_p.h
index cb60626..87ef0d5 100644
--- a/src/gui/text/qstatictext_p.h
+++ b/src/gui/text/qstatictext_p.h
@@ -68,27 +68,60 @@ public:
OpenGLUserData
};
- QStaticTextUserData(Type t) : type(t) {}
+ QStaticTextUserData(Type t) : type(t) { ref = 0; }
virtual ~QStaticTextUserData() {}
+ QAtomicInt ref;
Type type;
};
class Q_GUI_EXPORT QStaticTextItem
{
public:
- QStaticTextItem() : chars(0), numChars(0), fontEngine(0), userData(0),
- useBackendOptimizations(false), userDataNeedsUpdate(0) {}
- ~QStaticTextItem() { delete userData; }
+ QStaticTextItem() : chars(0), numChars(0), useBackendOptimizations(false),
+ userDataNeedsUpdate(0), m_fontEngine(0), m_userData(0) {}
+
+ QStaticTextItem(const QStaticTextItem &other)
+ {
+ operator=(other);
+ }
+
+ void operator=(const QStaticTextItem &other)
+ {
+ glyphPositions = other.glyphPositions;
+ glyphs = other.glyphs;
+ chars = other.chars;
+ numGlyphs = other.numGlyphs;
+ numChars = other.numChars;
+ font = other.font;
+ color = other.color;
+ useBackendOptimizations = other.useBackendOptimizations;
+ userDataNeedsUpdate = other.userDataNeedsUpdate;
+
+ m_fontEngine = 0;
+ m_userData = 0;
+ setUserData(other.userData());
+ setFontEngine(other.fontEngine());
+ }
+
+ ~QStaticTextItem();
void setUserData(QStaticTextUserData *newUserData)
{
- if (userData == newUserData)
+ if (m_userData == newUserData)
return;
- delete userData;
- userData = newUserData;
+ if (m_userData != 0 && !m_userData->ref.deref())
+ delete m_userData;
+
+ m_userData = newUserData;
+ if (m_userData != 0)
+ m_userData->ref.ref();
}
+ QStaticTextUserData *userData() const { return m_userData; }
+
+ void setFontEngine(QFontEngine *fe);
+ QFontEngine *fontEngine() const { return m_fontEngine; }
union {
QFixedPoint *glyphPositions; // 8 bytes per glyph
@@ -108,14 +141,17 @@ public:
// 12 bytes for pointers
int numGlyphs; // 4 bytes per item
int numChars; // 4 bytes per item
- QFontEngine *fontEngine; // 4 bytes per item
QFont font; // 8 bytes per item
QColor color; // 10 bytes per item
- QStaticTextUserData *userData; // 8 bytes per item
char useBackendOptimizations : 1; // 1 byte per item
char userDataNeedsUpdate : 1; //
// ================
// 51 bytes per item
+
+private: // Needs special handling in setters, so private to avoid abuse
+ QFontEngine *m_fontEngine; // 4 bytes per item
+ QStaticTextUserData *m_userData; // 8 bytes per item
+
};
class QStaticText;
diff --git a/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp b/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp
index 84c7fed..1dcb773 100644
--- a/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp
+++ b/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp
@@ -1350,8 +1350,8 @@ void QGL2PaintEngineEx::drawStaticTextItem(QStaticTextItem *textItem)
ensureActive();
- QFontEngineGlyphCache::Type glyphType = textItem->fontEngine->glyphFormat >= 0
- ? QFontEngineGlyphCache::Type(textItem->fontEngine->glyphFormat)
+ QFontEngineGlyphCache::Type glyphType = textItem->fontEngine()->glyphFormat >= 0
+ ? QFontEngineGlyphCache::Type(textItem->fontEngine()->glyphFormat)
: d->glyphCacheType;
if (glyphType == QFontEngineGlyphCache::Raster_RGBMask) {
if (d->device->alphaRequested() || state()->matrix.type() > QTransform::TxTranslate
@@ -1430,7 +1430,7 @@ void QGL2PaintEngineEx::drawTextItem(const QPointF &p, const QTextItem &textItem
{
QStaticTextItem staticTextItem;
staticTextItem.chars = const_cast<QChar *>(ti.chars);
- staticTextItem.fontEngine = ti.fontEngine;
+ staticTextItem.setFontEngine(ti.fontEngine);
staticTextItem.glyphs = glyphs.data();
staticTextItem.numChars = ti.num_chars;
staticTextItem.numGlyphs = glyphs.size();
@@ -1475,18 +1475,18 @@ void QGL2PaintEngineExPrivate::drawCachedGlyphs(QFontEngineGlyphCache::Type glyp
QOpenGL2PaintEngineState *s = q->state();
QGLTextureGlyphCache *cache =
- (QGLTextureGlyphCache *) staticTextItem->fontEngine->glyphCache(ctx, glyphType, QTransform());
+ (QGLTextureGlyphCache *) staticTextItem->fontEngine()->glyphCache(ctx, glyphType, QTransform());
if (!cache || cache->cacheType() != glyphType) {
cache = new QGLTextureGlyphCache(ctx, glyphType, QTransform());
- staticTextItem->fontEngine->setGlyphCache(ctx, cache);
+ staticTextItem->fontEngine()->setGlyphCache(ctx, cache);
}
bool recreateVertexArrays = false;
if (staticTextItem->userDataNeedsUpdate)
recreateVertexArrays = true;
- else if (staticTextItem->userData == 0)
+ else if (staticTextItem->userData() == 0)
recreateVertexArrays = true;
- else if (staticTextItem->userData->type != QStaticTextUserData::OpenGLUserData)
+ else if (staticTextItem->userData()->type != QStaticTextUserData::OpenGLUserData)
recreateVertexArrays = true;
// We only need to update the cache with new glyphs if we are actually going to recreate the vertex arrays.
@@ -1494,8 +1494,8 @@ void QGL2PaintEngineExPrivate::drawCachedGlyphs(QFontEngineGlyphCache::Type glyp
// cache so this text is performed before we test if the cache size has changed.
if (recreateVertexArrays) {
cache->setPaintEnginePrivate(this);
- cache->populate(staticTextItem->fontEngine, staticTextItem->numGlyphs, staticTextItem->glyphs,
- staticTextItem->glyphPositions);
+ cache->populate(staticTextItem->fontEngine(), staticTextItem->numGlyphs,
+ staticTextItem->glyphs, staticTextItem->glyphPositions);
}
if (cache->width() == 0 || cache->height() == 0)
@@ -1515,14 +1515,14 @@ void QGL2PaintEngineExPrivate::drawCachedGlyphs(QFontEngineGlyphCache::Type glyp
if (staticTextItem->useBackendOptimizations) {
QOpenGLStaticTextUserData *userData = 0;
- if (staticTextItem->userData == 0
- || staticTextItem->userData->type != QStaticTextUserData::OpenGLUserData) {
+ if (staticTextItem->userData() == 0
+ || staticTextItem->userData()->type != QStaticTextUserData::OpenGLUserData) {
userData = new QOpenGLStaticTextUserData();
staticTextItem->setUserData(userData);
} else {
- userData = static_cast<QOpenGLStaticTextUserData*>(staticTextItem->userData);
+ userData = static_cast<QOpenGLStaticTextUserData*>(staticTextItem->userData());
}
// Use cache if backend optimizations is turned on
diff --git a/src/opengl/qpaintengine_opengl.cpp b/src/opengl/qpaintengine_opengl.cpp
index 74b6b9a..a04d930 100644
--- a/src/opengl/qpaintengine_opengl.cpp
+++ b/src/opengl/qpaintengine_opengl.cpp
@@ -4918,7 +4918,7 @@ void QOpenGLPaintEngine::drawStaticTextItem(QStaticTextItem *textItem)
d->flushDrawQueue();
// make sure the glyphs we want to draw are in the cache
- qt_glyph_cache()->cacheGlyphs(d->device->context(), textItem->fontEngine, textItem->glyphs,
+ qt_glyph_cache()->cacheGlyphs(d->device->context(), textItem->fontEngine(), textItem->glyphs,
textItem->numGlyphs);
d->setGradientOps(Qt::SolidPattern, QRectF()); // turns off gradient ops
@@ -4941,13 +4941,13 @@ void QOpenGLPaintEngine::drawStaticTextItem(QStaticTextItem *textItem)
glEnableClientState(GL_VERTEX_ARRAY);
glEnableClientState(GL_TEXTURE_COORD_ARRAY);
- bool antialias = !(textItem->fontEngine->fontDef.styleStrategy & QFont::NoAntialias)
+ bool antialias = !(textItem->fontEngine()->fontDef.styleStrategy & QFont::NoAntialias)
&& (d->matrix.type() > QTransform::TxTranslate);
glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, antialias ? GL_LINEAR : GL_NEAREST);
glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, antialias ? GL_LINEAR : GL_NEAREST);
for (int i=0; i< textItem->numGlyphs; ++i) {
- QGLGlyphCoord *g = qt_glyph_cache()->lookup(textItem->fontEngine, textItem->glyphs[i]);
+ QGLGlyphCoord *g = qt_glyph_cache()->lookup(textItem->fontEngine(), textItem->glyphs[i]);
// we don't cache glyphs with no width/height
if (!g)
@@ -5003,7 +5003,7 @@ void QOpenGLPaintEngine::drawTextItem(const QPointF &p, const QTextItem &textIte
{
QStaticTextItem staticTextItem;
staticTextItem.chars = const_cast<QChar *>(ti.chars);
- staticTextItem.fontEngine = ti.fontEngine;
+ staticTextItem.setFontEngine(ti.fontEngine);
staticTextItem.glyphs = glyphs.data();
staticTextItem.numChars = ti.num_chars;
staticTextItem.numGlyphs = glyphs.size();
diff --git a/src/openvg/qpaintengine_vg.cpp b/src/openvg/qpaintengine_vg.cpp
index ce9d11a..f8788b0 100644
--- a/src/openvg/qpaintengine_vg.cpp
+++ b/src/openvg/qpaintengine_vg.cpp
@@ -3407,7 +3407,7 @@ void QVGPaintEngine::drawTextItem(const QPointF &p, const QTextItem &textItem)
void QVGPaintEngine::drawStaticTextItem(QStaticTextItem *textItem)
{
- drawCachedGlyphs(textItem->numGlyphs, textItem->glyphs, textItem->font, textItem->fontEngine,
+ drawCachedGlyphs(textItem->numGlyphs, textItem->glyphs, textItem->font, textItem->fontEngine(),
QPointF(0, 0), textItem->glyphPositions);
}