From d73497f136fbfad5d367e5af429adb7d38af6dfe Mon Sep 17 00:00:00 2001 From: Fabian Kosmale Date: Wed, 3 Jul 2019 08:37:16 +0200 Subject: QQmlComponent: Check for existence of engine Avoid a crash when setData or create are called after a user mistakenly used the internal constructor of QQmlComponent which does not take an engine. Fixes: QTBUG-55407 Change-Id: Ia4295d1b044723efce1a95607349561d4f1640da Reviewed-by: Ulf Hermann --- src/qml/qml/qqmlcomponent.cpp | 12 ++++++++++++ tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/qml/qml/qqmlcomponent.cpp b/src/qml/qml/qqmlcomponent.cpp index a23a322df9..7ad9310160 100644 --- a/src/qml/qml/qqmlcomponent.cpp +++ b/src/qml/qml/qqmlcomponent.cpp @@ -571,6 +571,12 @@ void QQmlComponent::setData(const QByteArray &data, const QUrl &url) { Q_D(QQmlComponent); + if (!d->engine) { + // ###Qt6: In Qt 6, it should be impossible for users to create a QQmlComponent without an engine, and we can remove this check + qWarning("QQmlComponent: Must provide an engine before calling setData"); + return; + } + d->clear(); d->url = url; @@ -777,6 +783,12 @@ QObject *QQmlComponent::create(QQmlContext *context) Q_D(QQmlComponent); QML_MEMORY_SCOPE_URL(url()); + if (!d->engine) { + // ###Qt6: In Qt 6, it should be impossible for users to create a QQmlComponent without an engine, and we can remove this check + qWarning("QQmlComponent: Must provide an engine before calling create"); + return nullptr; + } + if (!context) context = d->engine->rootContext(); diff --git a/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp b/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp index 7c7c7d3bd0..a872cece96 100644 --- a/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp +++ b/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp @@ -121,6 +121,7 @@ private slots: void setNonExistentInitialProperty(); void relativeUrl_data(); void relativeUrl(); + void setDataNoEngineNoSegfault(); private: QQmlEngine engine; @@ -654,6 +655,17 @@ void tst_qqmlcomponent::relativeUrl() QVERIFY2(!component.isError(), qPrintable(component.errorString())); } +void tst_qqmlcomponent::setDataNoEngineNoSegfault() +{ + QQmlEngine eng; + QQmlComponent comp; + QTest::ignoreMessage(QtWarningMsg, "QQmlComponent: Must provide an engine before calling setData"); + comp.setData("import QtQuick 1.0; QtObject { }", QUrl("")); + QTest::ignoreMessage(QtWarningMsg, "QQmlComponent: Must provide an engine before calling create"); + auto c = comp.create(); + QVERIFY(!c); +} + QTEST_MAIN(tst_qqmlcomponent) #include "tst_qqmlcomponent.moc" -- cgit v1.2.3 From 75ba1ce9114e320cccfbc0c14dd32675ce2e598e Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Tue, 2 Jul 2019 16:21:42 +0200 Subject: QQmlAdapterModel: Guard items against deletion during notification Change-Id: I177ea278b5039688923fc23425a1377522412d08 Reviewed-by: Simon Hausmann --- src/qml/util/qqmladaptormodel.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/qml/util/qqmladaptormodel.cpp b/src/qml/util/qqmladaptormodel.cpp index a9a38c5381..1ff866eb74 100644 --- a/src/qml/util/qqmladaptormodel.cpp +++ b/src/qml/util/qqmladaptormodel.cpp @@ -162,8 +162,14 @@ public: signalIndexes.append(propertyId + signalOffset); } - for (int i = 0, c = items.count(); i < c; ++i) { - QQmlDelegateModelItem *item = items.at(i); + QVarLengthArray> guardedItems; + for (const auto item : items) + guardedItems.append(item); + + for (const auto &item : qAsConst(guardedItems)) { + if (item.isNull()) + continue; + const int idx = item->modelIndex(); if (idx >= index && idx < index + count) { for (int i = 0; i < signalIndexes.count(); ++i) -- cgit v1.2.3 From b79b9e35334b3bdd41329f2ea74bc82f3f05ae52 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Wed, 3 Jul 2019 09:25:53 +0200 Subject: Create a URL from the import string before adding the qmldir path Otherwise the path might get interpreted as some other part of the URL, for example the host name. Fixes: QTBUG-76441 Change-Id: I3edde96153403962db4576e5af794419c21b49a8 Reviewed-by: Fabian Kosmale Reviewed-by: Simon Hausmann --- src/qml/qml/qqmltypeloader.cpp | 6 +++++- tests/auto/qml/qqmltypeloader/data/qrcRootPath.qml | 4 ++++ tests/auto/qml/qqmltypeloader/tst_qqmltypeloader.cpp | 8 ++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 tests/auto/qml/qqmltypeloader/data/qrcRootPath.qml diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp index df6a8f1500..9e5bc0b021 100644 --- a/src/qml/qml/qqmltypeloader.cpp +++ b/src/qml/qml/qqmltypeloader.cpp @@ -1495,7 +1495,11 @@ bool QQmlTypeLoader::Blob::addImport(const QV4::CompiledData::Import *import, QL bool incomplete = false; - QUrl qmldirUrl = finalUrl().resolved(QUrl(importUri + QLatin1String("/qmldir"))); + QUrl importUrl(importUri); + QString path = importUrl.path(); + path.append(QLatin1String(path.endsWith(QLatin1Char('/')) ? "qmldir" : "/qmldir")); + importUrl.setPath(path); + QUrl qmldirUrl = finalUrl().resolved(importUrl); if (!QQmlImports::isLocal(qmldirUrl)) { // This is a remote file; the import is currently incomplete incomplete = true; diff --git a/tests/auto/qml/qqmltypeloader/data/qrcRootPath.qml b/tests/auto/qml/qqmltypeloader/data/qrcRootPath.qml new file mode 100644 index 0000000000..bdbee4eb59 --- /dev/null +++ b/tests/auto/qml/qqmltypeloader/data/qrcRootPath.qml @@ -0,0 +1,4 @@ +import QtQml 2.12 +import "qrc:/" + +QtObject {} diff --git a/tests/auto/qml/qqmltypeloader/tst_qqmltypeloader.cpp b/tests/auto/qml/qqmltypeloader/tst_qqmltypeloader.cpp index 0c4abf19f4..52c722aac8 100644 --- a/tests/auto/qml/qqmltypeloader/tst_qqmltypeloader.cpp +++ b/tests/auto/qml/qqmltypeloader/tst_qqmltypeloader.cpp @@ -56,6 +56,7 @@ private slots: void qmlSingletonWithinModule(); void multiSingletonModule(); void implicitComponentModule(); + void qrcRootPathUrl(); }; void tst_QQMLTypeLoader::testLoadComplete() @@ -503,6 +504,13 @@ void tst_QQMLTypeLoader::implicitComponentModule() checkCleanCacheLoad(QLatin1String("implicitComponentModule")); } +void tst_QQMLTypeLoader::qrcRootPathUrl() +{ + QQmlEngine engine; + QQmlComponent component(&engine, testFileUrl("qrcRootPath.qml")); + QCOMPARE(component.status(), QQmlComponent::Ready); +} + QTEST_MAIN(tst_QQMLTypeLoader) #include "tst_qqmltypeloader.moc" -- cgit v1.2.3 From c5578b16d6454e708c8ce12661a85d41eeaaa758 Mon Sep 17 00:00:00 2001 From: Joerg Bornemann Date: Mon, 1 Jul 2019 21:18:26 +0200 Subject: Yield error if qtquickcompiler is used in non-QML projects Consider a non-QML project that puts .js files in a Qt resource file. The qtquickcompiler feature will pull those files out of the resource and generate QML-specific code, which will lead to build errors. Yield an error message on qmake-time. Fixes: QTBUG-73669 Change-Id: I6bec22f758d884ce4e1c50fca48f452c5f86ce74 Reviewed-by: Simon Hausmann Reviewed-by: Ulf Hermann --- tools/qmlcachegen/qtquickcompiler.prf | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tools/qmlcachegen/qtquickcompiler.prf b/tools/qmlcachegen/qtquickcompiler.prf index b98d8a0198..967d55a5bd 100644 --- a/tools/qmlcachegen/qtquickcompiler.prf +++ b/tools/qmlcachegen/qtquickcompiler.prf @@ -1,5 +1,15 @@ if(qtc_run|lupdate_run): return() +!contains(QT, qml) { + qt_modules = \ + $$replace(QT, -private$, _private) \ + $$replace(QT_PRIVATE, -private$, _private) + qt_modules = $$resolve_depends(qt_modules, "QT.", ".depends" ".run_depends") + !contains(qt_modules, qml): \ + error("The qtquickcompiler feature cannot be used without the QML module.") + unset(qt_modules) +} + qtPrepareTool(QML_CACHEGEN, qmlcachegen, _FILTER) qtPrepareTool(QMAKE_RCC, rcc, _DEP) -- cgit v1.2.3 From 46d72a117df642135718b38995346267312c4808 Mon Sep 17 00:00:00 2001 From: Eskil Abrahamsen Blomfeldt Date: Wed, 26 Jun 2019 13:20:00 +0200 Subject: Fix possible crash when distance field cache spans multiple textures The allocated area for each texture has to have its origin at (0,0) otherwise the actual required height of the cache is too low. When using the texture upload workaround, this would crash, and when not it would lead to missing glyphs. Note that this is because the texture may contain a bit of empty space at the top, since the area allocator may allocate this area to a glyph which actually belongs to the previous texture (we use a single area allocator for the entire cache and some glyphs may overlap the limit between two textures). [ChangeLog][Text] Fixed missing glyphs and in some cases crashes when large character sets were being used on system with a low maximum texture size. Task-number: QTBUG-76528 Change-Id: I710ebbdd2feba4567505393fb12f89b5dd8501f1 Reviewed-by: Simon Hausmann --- src/quick/scenegraph/qsgdefaultdistancefieldglyphcache_p.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/quick/scenegraph/qsgdefaultdistancefieldglyphcache_p.h b/src/quick/scenegraph/qsgdefaultdistancefieldglyphcache_p.h index c64adddd91..fb07191e59 100644 --- a/src/quick/scenegraph/qsgdefaultdistancefieldglyphcache_p.h +++ b/src/quick/scenegraph/qsgdefaultdistancefieldglyphcache_p.h @@ -97,7 +97,7 @@ private: QDistanceField image; int padding = -1; - TextureInfo(const QRect &preallocRect = QRect()) : texture(0), allocatedArea(preallocRect) { } + TextureInfo(const QRect &preallocRect = QRect(0, 0, 1, 1)) : texture(0), allocatedArea(preallocRect) { } }; void createTexture(TextureInfo * texInfo, int width, int height, const void *pixels); -- cgit v1.2.3 From 9a53834f1e7fce2fc3b1ecc2a311faedbc371d37 Mon Sep 17 00:00:00 2001 From: Eskil Abrahamsen Blomfeldt Date: Wed, 26 Jun 2019 12:36:08 +0200 Subject: Fix distance field cache growing outside limit Due to the way the area allocator is used, where one area spans all the textures, it is possible to have the situation where part of a glyph is allocated to a texture N and the rest to N+1, i.e. the split happens inside the glyph. In a case like this, the cache would grow the texture in question to accommodate the glyph, which would in turn cause the height of the texture to be too large. For most systems, this would not happen, since a single texture would be enough for a font, but when it does the texture creation may fail. The fix is to reduce the height of the area allocated to each texture by one row height, so that if the glyph does happen to hit the border between two and the texture needs to extend, it will still not exceed the actual maximum height. [ChangeLog][Text] Fixed a bug when displaying many characters from the same font on a system with a low maximum texture size. Task-number: QTBUG-76528 Change-Id: I1e559aaf90552afe8531872264186daa5ee61939 Reviewed-by: Simon Hausmann --- .../qsgdefaultdistancefieldglyphcache.cpp | 40 ++++++++++++---------- .../qsgdefaultdistancefieldglyphcache_p.h | 7 ++-- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/src/quick/scenegraph/qsgdefaultdistancefieldglyphcache.cpp b/src/quick/scenegraph/qsgdefaultdistancefieldglyphcache.cpp index 8121b4559e..c51358df7c 100644 --- a/src/quick/scenegraph/qsgdefaultdistancefieldglyphcache.cpp +++ b/src/quick/scenegraph/qsgdefaultdistancefieldglyphcache.cpp @@ -68,7 +68,8 @@ DEFINE_BOOL_CONFIG_OPTION(qsgPreferFullSizeGlyphCacheTextures, QSG_PREFER_FULLSI QSGDefaultDistanceFieldGlyphCache::QSGDefaultDistanceFieldGlyphCache(QOpenGLContext *c, const QRawFont &font) : QSGDistanceFieldGlyphCache(font) - , m_maxTextureSize(0) + , m_maxTextureWidth(0) + , m_maxTextureHeight(0) , m_maxTextureCount(3) , m_areaAllocator(nullptr) , m_blitProgram(nullptr) @@ -112,13 +113,23 @@ void QSGDefaultDistanceFieldGlyphCache::requestGlyphs(const QSet &glyph QList glyphPositions; QVector glyphsToRender; + const int padding = QSG_DEFAULT_DISTANCEFIELD_GLYPH_CACHE_PADDING; + const qreal scaleFactor = qreal(1) / QT_DISTANCEFIELD_SCALE(m_doubleGlyphResolution); + + if (m_maxTextureHeight == 0) { + m_funcs->glGetIntegerv(GL_MAX_TEXTURE_SIZE, &m_maxTextureWidth); + + // We need to add a buffer to avoid glyphs that overlap the border between two + // textures causing the height of the textures to extend beyond the limit. + m_maxTextureHeight = m_maxTextureWidth - (qCeil(m_referenceFont.pixelSize() * scaleFactor) + distanceFieldRadius() * 2 + padding * 2); + } + if (m_areaAllocator == nullptr) - m_areaAllocator = new QSGAreaAllocator(QSize(maxTextureSize(), m_maxTextureCount * maxTextureSize())); + m_areaAllocator = new QSGAreaAllocator(QSize(m_maxTextureWidth, m_maxTextureCount * m_maxTextureHeight)); for (QSet::const_iterator it = glyphs.constBegin(); it != glyphs.constEnd() ; ++it) { glyph_t glyphIndex = *it; - int padding = QSG_DEFAULT_DISTANCEFIELD_GLYPH_CACHE_PADDING; QRectF boundingRect = glyphData(glyphIndex).boundingRect; int glyphWidth = qCeil(boundingRect.width()) + distanceFieldRadius() * 2; int glyphHeight = qCeil(boundingRect.height()) + distanceFieldRadius() * 2; @@ -151,8 +162,8 @@ void QSGDefaultDistanceFieldGlyphCache::requestGlyphs(const QSet &glyph continue; } - TextureInfo *tex = textureInfo(alloc.y() / maxTextureSize()); - alloc = QRect(alloc.x(), alloc.y() % maxTextureSize(), alloc.width(), alloc.height()); + TextureInfo *tex = textureInfo(alloc.y() / m_maxTextureHeight); + alloc = QRect(alloc.x(), alloc.y() % m_maxTextureHeight, alloc.width(), alloc.height()); tex->allocatedArea |= alloc; Q_ASSERT(tex->padding == padding || tex->padding < 0); @@ -539,13 +550,6 @@ bool QSGDefaultDistanceFieldGlyphCache::createFullSizeTextures() const return qsgPreferFullSizeGlyphCacheTextures() && glyphCount() > QT_DISTANCEFIELD_HIGHGLYPHCOUNT(); } -int QSGDefaultDistanceFieldGlyphCache::maxTextureSize() const -{ - if (!m_maxTextureSize) - m_funcs->glGetIntegerv(GL_MAX_TEXTURE_SIZE, &m_maxTextureSize); - return m_maxTextureSize; -} - namespace { struct Qtdf { // We need these structs to be tightly packed, but some compilers we use do not @@ -644,7 +648,7 @@ bool QSGDefaultDistanceFieldGlyphCache::loadPregeneratedCache(const QRawFont &fo } qreal pixelSize = qreal(Qtdf::fetch(qtdfTableStart, Qtdf::pixelSize)); - m_maxTextureSize = Qtdf::fetch(qtdfTableStart, Qtdf::textureSize); + m_maxTextureWidth = m_maxTextureHeight = Qtdf::fetch(qtdfTableStart, Qtdf::textureSize); m_doubleGlyphResolution = Qtdf::fetch(qtdfTableStart, Qtdf::flags) == 1; padding = Qtdf::fetch(qtdfTableStart, Qtdf::headerPadding); @@ -653,7 +657,7 @@ bool QSGDefaultDistanceFieldGlyphCache::loadPregeneratedCache(const QRawFont &fo return false; } - if (m_maxTextureSize <= 0) { + if (m_maxTextureWidth <= 0) { qWarning("Invalid texture size in '%s'", qPrintable(font.familyName())); return false; } @@ -661,11 +665,11 @@ bool QSGDefaultDistanceFieldGlyphCache::loadPregeneratedCache(const QRawFont &fo int systemMaxTextureSize; m_funcs->glGetIntegerv(GL_MAX_TEXTURE_SIZE, &systemMaxTextureSize); - if (m_maxTextureSize > systemMaxTextureSize) { + if (m_maxTextureWidth > systemMaxTextureSize) { qWarning("System maximum texture size is %d. This is lower than the value in '%s', which is %d", systemMaxTextureSize, qPrintable(font.familyName()), - m_maxTextureSize); + m_maxTextureWidth); } if (padding != QSG_DEFAULT_DISTANCEFIELD_GLYPH_CACHE_PADDING) { @@ -688,12 +692,12 @@ bool QSGDefaultDistanceFieldGlyphCache::loadPregeneratedCache(const QRawFont &fo return false; } - if (m_areaAllocator->size().height() % m_maxTextureSize != 0) { + if (m_areaAllocator->size().height() % m_maxTextureHeight != 0) { qWarning("Area allocator size mismatch in '%s'", qPrintable(font.familyName())); return false; } - textureCount = m_areaAllocator->size().height() / m_maxTextureSize; + textureCount = m_areaAllocator->size().height() / m_maxTextureHeight; m_maxTextureCount = qMax(m_maxTextureCount, textureCount); const char *textureRecord = allocatorData; diff --git a/src/quick/scenegraph/qsgdefaultdistancefieldglyphcache_p.h b/src/quick/scenegraph/qsgdefaultdistancefieldglyphcache_p.h index fb07191e59..0420ef251e 100644 --- a/src/quick/scenegraph/qsgdefaultdistancefieldglyphcache_p.h +++ b/src/quick/scenegraph/qsgdefaultdistancefieldglyphcache_p.h @@ -80,7 +80,6 @@ public: bool useTextureResizeWorkaround() const; bool useTextureUploadWorkaround() const; bool createFullSizeTextures() const; - int maxTextureSize() const; void setMaxTextureCount(int max) { m_maxTextureCount = max; } int maxTextureCount() const { return m_maxTextureCount; } @@ -106,9 +105,10 @@ private: TextureInfo *textureInfo(int index) { + Q_ASSERT(m_maxTextureWidth > 0 && m_maxTextureHeight > 0); for (int i = m_textures.count(); i <= index; ++i) { if (createFullSizeTextures()) - m_textures.append(QRect(0, 0, maxTextureSize(), maxTextureSize())); + m_textures.append(QRect(0, 0, m_maxTextureWidth, m_maxTextureWidth)); else m_textures.append(TextureInfo()); } @@ -136,7 +136,8 @@ private: m_blitProgram->link(); } - mutable int m_maxTextureSize; + int m_maxTextureWidth; + int m_maxTextureHeight; int m_maxTextureCount; bool m_coreProfile; -- cgit v1.2.3