From e45e6efa723297e8aaa1132251ef55b65201d174 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Fri, 23 Aug 2019 09:51:39 +0200 Subject: [PATCH 1/7] QTouchDevice: don't play ping-pong with q*PostRoutine() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removing a qPostRoutine is an O(N) operation. It's perfectly ok to keep calling it even if the list has become empty before. Just qAddPostRoutine in the deviceList's ctor and never remove it again. Add a missing qAsConst as a drive-by. Change-Id: I25f824b74012146214568cfccb22c5dba3ca38ef Reviewed-by: Jan Arve Sæther --- src/gui/kernel/qtouchdevice.cpp | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/gui/kernel/qtouchdevice.cpp b/src/gui/kernel/qtouchdevice.cpp index 511e92566e..ea187f54aa 100644 --- a/src/gui/kernel/qtouchdevice.cpp +++ b/src/gui/kernel/qtouchdevice.cpp @@ -44,6 +44,7 @@ #include #include +#include QT_BEGIN_NAMESPACE @@ -201,15 +202,20 @@ void QTouchDevice::setName(const QString &name) d->name = name; } -typedef QList TouchDevices; -Q_GLOBAL_STATIC(TouchDevices, deviceList) static QBasicMutex devicesMutex; -static void cleanupDevicesList() +struct TouchDevices { + TouchDevices(); + QList list; +}; +Q_GLOBAL_STATIC(TouchDevices, deviceList) + +TouchDevices::TouchDevices() { - QMutexLocker lock(&devicesMutex); - qDeleteAll(*deviceList()); - deviceList()->clear(); + qAddPostRoutine([]{ + const auto locker = qt_scoped_lock(devicesMutex); + qDeleteAll(qExchange(deviceList->list, {})); + }); } /*! @@ -223,7 +229,7 @@ static void cleanupDevicesList() QList QTouchDevice::devices() { QMutexLocker lock(&devicesMutex); - return *deviceList(); + return deviceList->list; } /*! @@ -232,13 +238,13 @@ QList QTouchDevice::devices() bool QTouchDevicePrivate::isRegistered(const QTouchDevice *dev) { QMutexLocker locker(&devicesMutex); - return deviceList()->contains(dev); + return deviceList->list.contains(dev); } const QTouchDevice *QTouchDevicePrivate::deviceById(quint8 id) { QMutexLocker locker(&devicesMutex); - for (const QTouchDevice *dev : *deviceList()) + for (const QTouchDevice *dev : qAsConst(deviceList->list)) if (QTouchDevicePrivate::get(const_cast(dev))->id == id) return dev; return nullptr; @@ -250,9 +256,7 @@ const QTouchDevice *QTouchDevicePrivate::deviceById(quint8 id) void QTouchDevicePrivate::registerDevice(const QTouchDevice *dev) { QMutexLocker lock(&devicesMutex); - if (deviceList()->isEmpty()) - qAddPostRoutine(cleanupDevicesList); - deviceList()->append(dev); + deviceList->list.append(dev); } /*! @@ -261,9 +265,7 @@ void QTouchDevicePrivate::registerDevice(const QTouchDevice *dev) void QTouchDevicePrivate::unregisterDevice(const QTouchDevice *dev) { QMutexLocker lock(&devicesMutex); - bool wasRemoved = deviceList()->removeOne(dev); - if (wasRemoved && deviceList()->isEmpty()) - qRemovePostRoutine(cleanupDevicesList); + deviceList->list.removeOne(dev); } #ifndef QT_NO_DEBUG_STREAM From 8ce653e048746d65ae46773f555ee8786b10ff68 Mon Sep 17 00:00:00 2001 From: Kevin Funk Date: Fri, 30 Aug 2019 10:18:22 +0200 Subject: [PATCH 2/7] Make sure QLatin1Literal fwd header is generated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This broke when QLatin1Literal got deprecated in change 45373c19243aea335897ba0f371a1dd53ae8f079 Both hunks in this patch are needed. The hunk in syncqt.pl removes the QT_DEPRECATED_X(...) macro if it appears solely on a line in source code, the second hunk inserts a newline after the QT_DEPRECATED_X(...) macro usage to trigger that code path in the Perl script. Before/after comparison of the headers generated in include/QtCore: ``` % diff ~/old.txt ~/new.txt 105a106 > QLatin1Literal ``` Change-Id: I468dd2dd54bf115521ed82c6182236905556f568 Reviewed-by: Jörg Bornemann --- bin/syncqt.pl | 1 + src/corelib/text/qstring.h | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/bin/syncqt.pl b/bin/syncqt.pl index 43ecff53dd..1258994f93 100755 --- a/bin/syncqt.pl +++ b/bin/syncqt.pl @@ -228,6 +228,7 @@ sub classNames { $line .= ";" if($line =~ m/^Q_[A-Z_0-9]*\(.*\)[\r\n]*$/); #qt macro $line .= ";" if($line =~ m/^QT_(BEGIN|END)_HEADER[\r\n]*$/); #qt macro $line .= ";" if($line =~ m/^QT_(BEGIN|END)_NAMESPACE(_[A-Z]+)*[\r\n]*$/); #qt macro + $line .= ";" if($line =~ m/^QT_DEPRECATED_X\(.*\)[\r\n]*$/); #qt macro $line .= ";" if($line =~ m/^QT_MODULE\(.*\)[\r\n]*$/); # QT_MODULE macro $line .= ";" if($line =~ m/^QT_WARNING_(PUSH|POP|DISABLE_\w+\(.*\))[\r\n]*$/); # qt macros $$requires = $1 if ($line =~ m/^QT_REQUIRE_CONFIG\((.*)\);[\r\n]*$/); diff --git a/src/corelib/text/qstring.h b/src/corelib/text/qstring.h index 7b1351666d..5def2c81a1 100644 --- a/src/corelib/text/qstring.h +++ b/src/corelib/text/qstring.h @@ -220,7 +220,8 @@ Q_DECLARE_TYPEINFO(QLatin1String, Q_MOVABLE_TYPE); // Qt 4.x compatibility #if QT_DEPRECATED_SINCE(5, 14) -QT_DEPRECATED_X("Use QLatin1String") typedef QLatin1String QLatin1Literal; +QT_DEPRECATED_X("Use QLatin1String") +typedef QLatin1String QLatin1Literal; #endif // From 60bea1c85a4efa21e34b374ca54b6a3635680acb Mon Sep 17 00:00:00 2001 From: Friedemann Kleint Date: Mon, 2 Sep 2019 11:36:14 +0200 Subject: [PATCH 3/7] QFileSystemModel: Fix naming of Option enumeration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix an API review finding in 5.14, amending 6b9d319b26da2e4b6f939ee92a176f8604c1c539. Change-Id: I6c67ebde91021b87a43a86ff831b724f098019aa Reviewed-by: Tor Arne Vestbø --- examples/widgets/itemviews/dirview/main.cpp | 2 +- src/widgets/dialogs/qfilesystemmodel.cpp | 10 +++++----- src/widgets/dialogs/qfilesystemmodel.h | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/examples/widgets/itemviews/dirview/main.cpp b/examples/widgets/itemviews/dirview/main.cpp index bf485e6c3c..9fecffda40 100644 --- a/examples/widgets/itemviews/dirview/main.cpp +++ b/examples/widgets/itemviews/dirview/main.cpp @@ -79,7 +79,7 @@ int main(int argc, char *argv[]) if (parser.isSet(dontUseCustomDirectoryIconsOption)) model.setOption(QFileSystemModel::DontUseCustomDirectoryIcons); if (parser.isSet(dontWatchOption)) - model.setOption(QFileSystemModel::DontWatch); + model.setOption(QFileSystemModel::DontWatchForChanges); QTreeView tree; tree.setModel(&model); if (!rootPath.isEmpty()) { diff --git a/src/widgets/dialogs/qfilesystemmodel.cpp b/src/widgets/dialogs/qfilesystemmodel.cpp index 212223359a..e9e49e0c33 100644 --- a/src/widgets/dialogs/qfilesystemmodel.cpp +++ b/src/widgets/dialogs/qfilesystemmodel.cpp @@ -1267,7 +1267,7 @@ Qt::DropActions QFileSystemModel::supportedDropActions() const \enum QFileSystemModel::Option \since 5.14 - \value DontWatch Do not add file watchers to the paths. + \value DontWatchForChanges Do not add file watchers to the paths. This reduces overhead when using the model for simple tasks like line edit completion. @@ -1331,8 +1331,8 @@ void QFileSystemModel::setOptions(Options options) #if QT_CONFIG(filesystemwatcher) Q_D(QFileSystemModel); - if (changed.testFlag(DontWatch)) - d->fileInfoGatherer.setWatching(!options.testFlag(DontWatch)); + if (changed.testFlag(DontWatchForChanges)) + d->fileInfoGatherer.setWatching(!options.testFlag(DontWatchForChanges)); #endif if (changed.testFlag(DontUseCustomDirectoryIcons)) { @@ -1353,9 +1353,9 @@ QFileSystemModel::Options QFileSystemModel::options() const result.setFlag(DontResolveSymlinks, !resolveSymlinks()); #if QT_CONFIG(filesystemwatcher) Q_D(const QFileSystemModel); - result.setFlag(DontWatch, !d->fileInfoGatherer.isWatching()); + result.setFlag(DontWatchForChanges, !d->fileInfoGatherer.isWatching()); #else - result.setFlag(DontWatch); + result.setFlag(DontWatchForChanges); #endif if (auto provider = iconProvider()) { result.setFlag(DontUseCustomDirectoryIcons, diff --git a/src/widgets/dialogs/qfilesystemmodel.h b/src/widgets/dialogs/qfilesystemmodel.h index 877b7891d4..b0f289dfcd 100644 --- a/src/widgets/dialogs/qfilesystemmodel.h +++ b/src/widgets/dialogs/qfilesystemmodel.h @@ -78,7 +78,7 @@ public: enum Option { - DontWatch = 0x00000001, + DontWatchForChanges = 0x00000001, DontResolveSymlinks = 0x00000002, DontUseCustomDirectoryIcons = 0x00000004 }; From c9f8893000249bd5701674c53d18a823b4a1c629 Mon Sep 17 00:00:00 2001 From: BogDan Vatra Date: Mon, 26 Aug 2019 16:58:47 +0300 Subject: [PATCH 4/7] Add support for aab Change-Id: I4814a51b25d5e3953e5e1c71d9a0e1bf3fed7385 Reviewed-by: Eskil Abrahamsen Blomfeldt --- src/tools/androiddeployqt/main.cpp | 115 ++++++++++++++++++----------- 1 file changed, 72 insertions(+), 43 deletions(-) diff --git a/src/tools/androiddeployqt/main.cpp b/src/tools/androiddeployqt/main.cpp index 4cba67051b..206c0ff374 100644 --- a/src/tools/androiddeployqt/main.cpp +++ b/src/tools/androiddeployqt/main.cpp @@ -121,8 +121,8 @@ struct Options , auxMode(false) , deploymentMechanism(Bundled) , releasePackage(false) - , digestAlg(QLatin1String("SHA1")) - , sigAlg(QLatin1String("SHA1withRSA")) + , digestAlg(QLatin1String("SHA-256")) + , sigAlg(QLatin1String("SHA256withRSA")) , internalSf(false) , sectionsOnly(false) , protectedAuthenticationPath(false) @@ -181,6 +181,8 @@ struct Options QString currentArchitecture; QString toolchainPrefix; QString ndkHost; + bool buildAAB = false; + // Package information DeploymentMechanism deploymentMechanism; @@ -412,7 +414,10 @@ Options parseOptions() options.helpRequested = true; else options.inputFileName = arguments.at(++i); - } else if (argument.compare(QLatin1String("--no-build"), Qt::CaseInsensitive) == 0) { + } else if (argument.compare(QLatin1String("--aab"), Qt::CaseInsensitive) == 0) { + options.buildAAB = true; + options.build = true; + } else if (options.buildAAB && argument.compare(QLatin1String("--no-build"), Qt::CaseInsensitive) == 0) { options.build = false; } else if (argument.compare(QLatin1String("--install"), Qt::CaseInsensitive) == 0) { options.installApk = true; @@ -553,6 +558,7 @@ void printHelp() " --deployment : Supported deployment mechanisms:\n" " bundled (default): Include Qt files in stand-alone package.\n" " ministro: Use the Ministro service to manage Qt files.\n" + " --aab: Build an Android App Bundle.\n" " --no-build: Do not build the package, it is useful to just install\n" " a package previously built.\n" " --install: Installs apk to device/emulator. By default this step is\n" @@ -2272,6 +2278,9 @@ bool buildAndroidProject(const Options &options) } QString commandLine = QLatin1String("%1 --no-daemon %2").arg(shellQuote(gradlePath), options.releasePackage ? QLatin1String(" assembleRelease") : QLatin1String(" assembleDebug")); + if (options.buildAAB) + commandLine += QLatin1String(" bundle"); + if (options.verbose) commandLine += QLatin1String(" --info"); @@ -2332,28 +2341,38 @@ bool uninstallApk(const Options &options) } enum PackageType { + AAB, UnsignedAPK, SignedAPK }; -QString apkPath(const Options &options, PackageType pt) +QString packagePath(const Options &options, PackageType pt) { QString path(options.outputDirectory); - path += QLatin1String("/build/outputs/apk/"); + path += QLatin1String("/build/outputs/%1/").arg(pt >= UnsignedAPK ? QStringLiteral("apk") : QStringLiteral("bundle")); QString buildType(options.releasePackage ? QLatin1String("release/") : QLatin1String("debug/")); if (QDir(path + buildType).exists()) path += buildType; path += QDir(options.outputDirectory).dirName() + QLatin1Char('-'); if (options.releasePackage) { path += QLatin1String("release-"); - if (pt == UnsignedAPK) - path += QLatin1String("un"); - path += QLatin1String("signed.apk"); + if (pt >= UnsignedAPK) { + if (pt == UnsignedAPK) + path += QLatin1String("un"); + path += QLatin1String("signed.apk"); + } else { + path.chop(1); + path += QLatin1String(".aab"); + } } else { path += QLatin1String("debug"); - if (pt == SignedAPK) - path += QLatin1String("-signed"); - path += QLatin1String(".apk"); + if (pt >= UnsignedAPK) { + if (pt == SignedAPK) + path += QLatin1String("-signed"); + path += QLatin1String(".apk"); + } else { + path += QLatin1String(".aab"); + } } return shellQuote(path); } @@ -2370,7 +2389,7 @@ bool installApk(const Options &options) FILE *adbCommand = runAdb(options, QLatin1String(" install -r ") - + apkPath(options, options.keyStore.isEmpty() ? UnsignedAPK + + packagePath(options, options.keyStore.isEmpty() ? UnsignedAPK : SignedAPK)); if (adbCommand == 0) return false; @@ -2396,7 +2415,7 @@ bool installApk(const Options &options) bool copyPackage(const Options &options) { fflush(stdout); - auto from = apkPath(options, options.keyStore.isEmpty() ? UnsignedAPK : SignedAPK); + auto from = packagePath(options, options.keyStore.isEmpty() ? UnsignedAPK : SignedAPK); QFile::remove(options.apkPath); return QFile::copy(from, options.apkPath); } @@ -2479,29 +2498,39 @@ bool jarSignerSignPackage(const Options &options) if (options.protectedAuthenticationPath) jarSignerTool += QLatin1String(" -protected"); - jarSignerTool += QLatin1String(" %1 %2") - .arg(apkPath(options, UnsignedAPK)) - .arg(shellQuote(options.keyStoreAlias)); + auto signPackage = [&](const QString &file) { + fprintf(stdout, "Signing file %s\n", qPrintable(file)); + fflush(stdout); + auto command = jarSignerTool + QLatin1String(" %1 %2") + .arg(file) + .arg(shellQuote(options.keyStoreAlias)); - FILE *jarSignerCommand = openProcess(jarSignerTool); - if (jarSignerCommand == 0) { - fprintf(stderr, "Couldn't run jarsigner.\n"); + FILE *jarSignerCommand = openProcess(command); + if (jarSignerCommand == 0) { + fprintf(stderr, "Couldn't run jarsigner.\n"); + return false; + } + + if (options.verbose) { + char buffer[512]; + while (fgets(buffer, sizeof(buffer), jarSignerCommand) != 0) + fprintf(stdout, "%s", buffer); + } + + int errorCode = pclose(jarSignerCommand); + if (errorCode != 0) { + fprintf(stderr, "jarsigner command failed.\n"); + if (!options.verbose) + fprintf(stderr, " -- Run with --verbose for more information.\n"); + return false; + } + return true; + }; + + if (!signPackage(packagePath(options, UnsignedAPK))) return false; - } - - if (options.verbose) { - char buffer[512]; - while (fgets(buffer, sizeof(buffer), jarSignerCommand) != 0) - fprintf(stdout, "%s", buffer); - } - - int errorCode = pclose(jarSignerCommand); - if (errorCode != 0) { - fprintf(stderr, "jarsigner command failed.\n"); - if (!options.verbose) - fprintf(stderr, " -- Run with --verbose for more information.\n"); + if (options.buildAAB && !signPackage(packagePath(options, AAB))) return false; - } QString zipAlignTool = options.sdkPath + QLatin1String("/tools/zipalign"); #if defined(Q_OS_WIN32) @@ -2522,8 +2551,8 @@ bool jarSignerSignPackage(const Options &options) zipAlignTool = QLatin1String("%1%2 -f 4 %3 %4") .arg(shellQuote(zipAlignTool), options.verbose ? QLatin1String(" -v") : QLatin1String(), - apkPath(options, UnsignedAPK), - apkPath(options, SignedAPK)); + packagePath(options, UnsignedAPK), + packagePath(options, SignedAPK)); FILE *zipAlignCommand = openProcess(zipAlignTool); if (zipAlignCommand == 0) { @@ -2535,7 +2564,7 @@ bool jarSignerSignPackage(const Options &options) while (fgets(buffer, sizeof(buffer), zipAlignCommand) != 0) fprintf(stdout, "%s", buffer); - errorCode = pclose(zipAlignCommand); + int errorCode = pclose(zipAlignCommand); if (errorCode != 0) { fprintf(stderr, "zipalign command failed.\n"); if (!options.verbose) @@ -2543,7 +2572,7 @@ bool jarSignerSignPackage(const Options &options) return false; } - return QFile::remove(apkPath(options, UnsignedAPK)); + return QFile::remove(packagePath(options, UnsignedAPK)); } bool signPackage(const Options &options) @@ -2577,8 +2606,8 @@ bool signPackage(const Options &options) zipAlignTool = QLatin1String("%1%2 -f 4 %3 %4") .arg(shellQuote(zipAlignTool), options.verbose ? QLatin1String(" -v") : QLatin1String(), - apkPath(options, UnsignedAPK), - apkPath(options, SignedAPK)); + packagePath(options, UnsignedAPK), + packagePath(options, SignedAPK)); FILE *zipAlignCommand = openProcess(zipAlignTool); if (zipAlignCommand == 0) { @@ -2614,7 +2643,7 @@ bool signPackage(const Options &options) apkSignerCommandLine += QLatin1String(" --verbose"); apkSignerCommandLine += QLatin1String(" %1") - .arg(apkPath(options, SignedAPK)); + .arg(packagePath(options, SignedAPK)); auto apkSignerRunner = [&] { FILE *apkSignerCommand = openProcess(apkSignerCommandLine); @@ -2642,10 +2671,10 @@ bool signPackage(const Options &options) return false; apkSignerCommandLine = QLatin1String("%1 verify --verbose %2") - .arg(shellQuote(apksignerTool), apkPath(options, SignedAPK)); + .arg(shellQuote(apksignerTool), packagePath(options, SignedAPK)); // Verify the package and remove the unsigned apk - return apkSignerRunner() && QFile::remove(apkPath(options, UnsignedAPK)); + return apkSignerRunner() && QFile::remove(packagePath(options, UnsignedAPK)); } bool generateAssetsFileList(const Options &options) @@ -2869,7 +2898,7 @@ int main(int argc, char *argv[]) if (options.installApk) fprintf(stdout, " -- It can now be run from the selected device/emulator.\n"); - fprintf(stdout, " -- File: %s\n", qPrintable(apkPath(options, options.keyStore.isEmpty() ? UnsignedAPK + fprintf(stdout, " -- File: %s\n", qPrintable(packagePath(options, options.keyStore.isEmpty() ? UnsignedAPK : SignedAPK))); fflush(stdout); return 0; From 162e23d838101ddcd8e6416dd346465ac20bd05d Mon Sep 17 00:00:00 2001 From: BogDan Vatra Date: Tue, 27 Aug 2019 12:42:29 +0300 Subject: [PATCH 5/7] Android: Add aab target Move aab, apk, apk_install_target to !build_pass, otherwise these targets will be executed for each android abi. Change-Id: I18f6c8946f503f2c08338f24758bf9059987fe0f Reviewed-by: Eskil Abrahamsen Blomfeldt --- mkspecs/features/android/android.prf | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/mkspecs/features/android/android.prf b/mkspecs/features/android/android.prf index a12c17c4ed..fc0ff553d0 100644 --- a/mkspecs/features/android/android.prf +++ b/mkspecs/features/android/android.prf @@ -4,17 +4,22 @@ APK_PATH = $$shell_path($$OUT_PWD/android-build/$${TARGET}.apk) apk_install_target.depends = first apk_install_target.commands = $(MAKE) -f $(MAKEFILE) INSTALL_ROOT=$$OUT_PWD/android-build install - apk.target = apk - apk.depends = apk_install_target qtPrepareTool(ANDROIDDEPLOYQT, androiddeployqt) isEmpty(ANDROID_DEPLOYMENT_SETTINGS_FILE): ANDROID_DEPLOYMENT_SETTINGS_FILE = $$OUT_PWD/android-$$TARGET-deployment-settings.json contains(QMAKE_HOST.os, Windows): extension = .exe + + apk.target = apk + apk.depends = apk_install_target apk.commands = $$ANDROIDDEPLOYQT --input $$ANDROID_DEPLOYMENT_SETTINGS_FILE --output $$OUT_PWD/android-build --apk $$APK_PATH + + aab.target = aab + aab.depends = apk_install_target + aab.commands = $$ANDROIDDEPLOYQT --input $$ANDROID_DEPLOYMENT_SETTINGS_FILE --output $$OUT_PWD/android-build --aab --apk $$APK_PATH } else { + prepareRecursiveTarget(aab) prepareRecursiveTarget(apk) prepareRecursiveTarget(apk_install_target) } -QMAKE_EXTRA_TARGETS *= apk apk_install_target build_pass { contains(TEMPLATE, ".*app") { @@ -34,4 +39,11 @@ build_pass { target.path = /libs/$$ANDROID_TARGET_ARCH/ INSTALLS *= target } +} else { + QMAKE_EXTRA_TARGETS *= aab apk apk_install_target + + android-build-distclean.commands = \ + $$QMAKE_DEL_TREE $$shell_quote($$shell_path($$OUT_PWD/android-build)) + QMAKE_EXTRA_TARGETS *= android-build-distclean + CLEAN_DEPS += android-build-distclean } From c8d3eb35e73e650e73fcc47c78af6d86ff58bfd9 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Fri, 12 Jul 2019 15:02:09 +0200 Subject: [PATCH 6/7] Fixup move semantics of QColorSpace Stop using QExplicitlySharedDataPointer, makes it possible to inline the move constructor and assign operator. Also protect other methods from nullptr d_ptr, and change the default constructed value to also have a null d_ptr, to match the result after a move. Change-Id: I40928feef90cc956ef84d0516a77b0ee0f8986c7 Reviewed-by: Allan Sandfeld Jensen --- src/gui/painting/qcolorspace.cpp | 72 +++++++++++-------- src/gui/painting/qcolorspace.h | 14 +++- src/gui/painting/qcolorspace_p.h | 15 +++- .../painting/qcolorspace/tst_qcolorspace.cpp | 23 ++++++ 4 files changed, 88 insertions(+), 36 deletions(-) diff --git a/src/gui/painting/qcolorspace.cpp b/src/gui/painting/qcolorspace.cpp index 043a951521..86d0c57cfe 100644 --- a/src/gui/painting/qcolorspace.cpp +++ b/src/gui/painting/qcolorspace.cpp @@ -461,19 +461,19 @@ QColorTransform QColorSpacePrivate::transformationToColorSpace(const QColorSpace Creates a new colorspace object that represents \a colorSpaceId. */ QColorSpace::QColorSpace(QColorSpace::ColorSpaceId colorSpaceId) + : d_ptr(nullptr) { - static QExplicitlySharedDataPointer predefinedColorspacePrivates[QColorSpace::Bt2020]; - if (colorSpaceId <= QColorSpace::Unknown) { - if (!predefinedColorspacePrivates[0]) - predefinedColorspacePrivates[0] = new QColorSpacePrivate(QColorSpace::Undefined); - d_ptr = predefinedColorspacePrivates[0]; // unknown and undefined both returns the static undefined colorspace. - } else { - if (!predefinedColorspacePrivates[colorSpaceId - 1]) - predefinedColorspacePrivates[colorSpaceId - 1] = new QColorSpacePrivate(colorSpaceId); - d_ptr = predefinedColorspacePrivates[colorSpaceId - 1]; + static QColorSpacePrivate *predefinedColorspacePrivates[QColorSpace::Bt2020]; + // Unknown and undefined both returns the static undefined colorspace + if (colorSpaceId > QColorSpace::Unknown) { + if (!predefinedColorspacePrivates[colorSpaceId - 2]) { + predefinedColorspacePrivates[colorSpaceId - 2] = new QColorSpacePrivate(colorSpaceId); + predefinedColorspacePrivates[colorSpaceId - 2]->ref.ref(); + } + d_ptr = predefinedColorspacePrivates[colorSpaceId - 2]; + d_ptr->ref.ref(); + Q_ASSERT(isValid()); } - - Q_ASSERT(colorSpaceId == QColorSpace::Undefined || isValid()); } /*! @@ -483,6 +483,7 @@ QColorSpace::QColorSpace(QColorSpace::ColorSpaceId colorSpaceId) QColorSpace::QColorSpace(QColorSpace::Primaries primaries, QColorSpace::TransferFunction fun, float gamma) : d_ptr(new QColorSpacePrivate(primaries, fun, gamma)) { + d_ptr->ref.ref(); } /*! @@ -492,6 +493,7 @@ QColorSpace::QColorSpace(QColorSpace::Primaries primaries, QColorSpace::Transfer QColorSpace::QColorSpace(QColorSpace::Primaries primaries, float gamma) : d_ptr(new QColorSpacePrivate(primaries, TransferFunction::Gamma, gamma)) { + d_ptr->ref.ref(); } /*! @@ -505,35 +507,34 @@ QColorSpace::QColorSpace(const QPointF &whitePoint, const QPointF &redPoint, QColorSpacePrimaries primaries(whitePoint, redPoint, greenPoint, bluePoint); if (!primaries.areValid()) { qWarning() << "QColorSpace attempted constructed from invalid primaries:" << whitePoint << redPoint << greenPoint << bluePoint; - d_ptr = QColorSpace(QColorSpace::Undefined).d_ptr; + d_ptr = nullptr; return; } d_ptr = new QColorSpacePrivate(primaries, fun, gamma); + d_ptr->ref.ref(); } QColorSpace::~QColorSpace() { -} - -QColorSpace::QColorSpace(QColorSpace &&colorSpace) noexcept - : d_ptr(std::move(colorSpace.d_ptr)) -{ + if (d_ptr && !d_ptr->ref.deref()) + delete d_ptr; } QColorSpace::QColorSpace(const QColorSpace &colorSpace) : d_ptr(colorSpace.d_ptr) { -} - -QColorSpace &QColorSpace::operator=(QColorSpace &&colorSpace) noexcept -{ - d_ptr = std::move(colorSpace.d_ptr); - return *this; + if (d_ptr) + d_ptr->ref.ref(); } QColorSpace &QColorSpace::operator=(const QColorSpace &colorSpace) { + QColorSpacePrivate *oldD = d_ptr; d_ptr = colorSpace.d_ptr; + if (d_ptr) + d_ptr->ref.ref(); + if (oldD && !oldD->ref.deref()) + delete oldD; return *this; } @@ -549,6 +550,8 @@ QColorSpace &QColorSpace::operator=(const QColorSpace &colorSpace) */ QColorSpace::ColorSpaceId QColorSpace::colorSpaceId() const noexcept { + if (Q_UNLIKELY(!d_ptr)) + return QColorSpace::Undefined; return d_ptr->id; } @@ -571,6 +574,8 @@ QColorSpace::Primaries QColorSpace::primaries() const noexcept */ QColorSpace::TransferFunction QColorSpace::transferFunction() const noexcept { + if (Q_UNLIKELY(!d_ptr)) + return QColorSpace::TransferFunction::Custom; return d_ptr->transferFunction; } @@ -583,6 +588,8 @@ QColorSpace::TransferFunction QColorSpace::transferFunction() const noexcept */ float QColorSpace::gamma() const noexcept { + if (Q_UNLIKELY(!d_ptr)) + return 0.0f; return d_ptr->gamma; } @@ -599,7 +606,7 @@ void QColorSpace::setTransferFunction(QColorSpace::TransferFunction transferFunc return; if (d_ptr->transferFunction == transferFunction && d_ptr->gamma == gamma) return; - d_ptr.detach(); + QColorSpacePrivate::getWritable(*this); // detach d_ptr->description.clear(); d_ptr->transferFunction = transferFunction; d_ptr->gamma = gamma; @@ -637,7 +644,7 @@ void QColorSpace::setPrimaries(QColorSpace::Primaries primariesId) return; if (d_ptr->primaries == primariesId) return; - d_ptr.detach(); + QColorSpacePrivate::getWritable(*this); // detach d_ptr->description.clear(); d_ptr->primaries = primariesId; d_ptr->identifyColorSpace(); @@ -663,7 +670,7 @@ void QColorSpace::setPrimaries(const QPointF &whitePoint, const QPointF &redPoin QColorMatrix toXyz = primaries.toXyzMatrix(); if (QColorVector(primaries.whitePoint) == d_ptr->whitePoint && toXyz == d_ptr->toXyz) return; - d_ptr.detach(); + QColorSpacePrivate::getWritable(*this); // detach d_ptr->description.clear(); d_ptr->primaries = QColorSpace::Primaries::Custom; d_ptr->toXyz = toXyz; @@ -685,6 +692,8 @@ void QColorSpace::setPrimaries(const QPointF &whitePoint, const QPointF &redPoin */ QByteArray QColorSpace::iccProfile() const { + if (Q_UNLIKELY(!d_ptr)) + return QByteArray(); if (!d_ptr->iccProfile.isEmpty()) return d_ptr->iccProfile; if (!isValid()) @@ -708,8 +717,9 @@ QColorSpace QColorSpace::fromIccProfile(const QByteArray &iccProfile) QColorSpace colorSpace; if (QIcc::fromIccProfile(iccProfile, &colorSpace)) return colorSpace; - colorSpace.d_ptr->id = QColorSpace::Undefined; - colorSpace.d_ptr->iccProfile = iccProfile; + QColorSpacePrivate *d = QColorSpacePrivate::getWritable(colorSpace); + d->id = QColorSpace::Undefined; + d->iccProfile = iccProfile; return colorSpace; } @@ -718,7 +728,7 @@ QColorSpace QColorSpace::fromIccProfile(const QByteArray &iccProfile) */ bool QColorSpace::isValid() const noexcept { - return d_ptr->id != QColorSpace::Undefined && d_ptr->toXyz.isValid() + return d_ptr && d_ptr->id != QColorSpace::Undefined && d_ptr->toXyz.isValid() && d_ptr->trc[0].isValid() && d_ptr->trc[1].isValid() && d_ptr->trc[2].isValid(); } @@ -731,6 +741,8 @@ bool operator==(const QColorSpace &colorSpace1, const QColorSpace &colorSpace2) { if (colorSpace1.d_ptr == colorSpace2.d_ptr) return true; + if (!colorSpace1.d_ptr || !colorSpace2.d_ptr) + return false; if (colorSpace1.colorSpaceId() == QColorSpace::Undefined && colorSpace2.colorSpaceId() == QColorSpace::Undefined) return colorSpace1.d_ptr->iccProfile == colorSpace2.d_ptr->iccProfile; @@ -780,7 +792,7 @@ QColorTransform QColorSpace::transformationToColorSpace(const QColorSpace &color if (!isValid() || !colorspace.isValid()) return QColorTransform(); - return d_ptr->transformationToColorSpace(colorspace.d_ptr.constData()); + return d_ptr->transformationToColorSpace(colorspace.d_ptr); } /***************************************************************************** diff --git a/src/gui/painting/qcolorspace.h b/src/gui/painting/qcolorspace.h index a7c1091911..5941682bbb 100644 --- a/src/gui/painting/qcolorspace.h +++ b/src/gui/painting/qcolorspace.h @@ -90,11 +90,19 @@ public: TransferFunction fun, float gamma = 0.0f); ~QColorSpace(); - QColorSpace(QColorSpace &&colorSpace) noexcept; QColorSpace(const QColorSpace &colorSpace); - QColorSpace &operator=(QColorSpace &&colorSpace) noexcept; QColorSpace &operator=(const QColorSpace &colorSpace); + QColorSpace(QColorSpace &&colorSpace) noexcept + : d_ptr(qExchange(colorSpace.d_ptr, nullptr)) + { } + QColorSpace &operator=(QColorSpace &&colorSpace) noexcept + { + // Make the deallocation of this->d_ptr happen in ~QColorSpace() + QColorSpace(std::move(colorSpace)).swap(*this); + return *this; + } + void swap(QColorSpace &colorSpace) noexcept { qSwap(d_ptr, colorSpace.d_ptr); } @@ -123,7 +131,7 @@ public: private: Q_DECLARE_PRIVATE(QColorSpace) - QExplicitlySharedDataPointer d_ptr; + QColorSpacePrivate *d_ptr; }; bool Q_GUI_EXPORT operator==(const QColorSpace &colorSpace1, const QColorSpace &colorSpace2); diff --git a/src/gui/painting/qcolorspace_p.h b/src/gui/painting/qcolorspace_p.h index 2a40a0cfd8..037111a0ae 100644 --- a/src/gui/painting/qcolorspace_p.h +++ b/src/gui/painting/qcolorspace_p.h @@ -95,15 +95,24 @@ public: QColorSpacePrivate(const QColorSpacePrimaries &primaries, QColorSpace::TransferFunction fun, float gamma); QColorSpacePrivate(const QColorSpacePrivate &other) = default; + // named different from get to avoid accidental detachs static QColorSpacePrivate *getWritable(QColorSpace &colorSpace) { - colorSpace.d_ptr.detach(); - return colorSpace.d_ptr.data(); + if (!colorSpace.d_ptr) { + colorSpace.d_ptr = new QColorSpacePrivate; + colorSpace.d_ptr->ref.ref(); + } else if (colorSpace.d_ptr->ref.loadRelaxed() != 1) { + colorSpace.d_ptr->ref.deref(); + colorSpace.d_ptr = new QColorSpacePrivate(*colorSpace.d_ptr); + colorSpace.d_ptr->ref.ref(); + } + Q_ASSERT(colorSpace.d_ptr->ref.loadRelaxed() == 1); + return colorSpace.d_ptr; } static const QColorSpacePrivate *get(const QColorSpace &colorSpace) { - return colorSpace.d_ptr.data(); + return colorSpace.d_ptr; } void initialize(); diff --git a/tests/auto/gui/painting/qcolorspace/tst_qcolorspace.cpp b/tests/auto/gui/painting/qcolorspace/tst_qcolorspace.cpp index bc1a45013c..9e13dc80b4 100644 --- a/tests/auto/gui/painting/qcolorspace/tst_qcolorspace.cpp +++ b/tests/auto/gui/painting/qcolorspace/tst_qcolorspace.cpp @@ -47,6 +47,7 @@ public: tst_QColorSpace(); private slots: + void movable(); void namedColorSpaces_data(); void namedColorSpaces(); @@ -75,6 +76,28 @@ tst_QColorSpace::tst_QColorSpace() { } +void tst_QColorSpace::movable() +{ + QColorSpace cs1 = QColorSpace::SRgb; + QColorSpace cs2 = QColorSpace::SRgbLinear; + QVERIFY(cs1.isValid()); + QVERIFY(cs2.isValid()); + QCOMPARE(cs1.colorSpaceId(), QColorSpace::SRgb); + + cs2 = std::move(cs1); + QVERIFY(!cs1.isValid()); + QVERIFY(cs2.isValid()); + QCOMPARE(cs2.colorSpaceId(), QColorSpace::SRgb); + QCOMPARE(cs1.colorSpaceId(), QColorSpace::Undefined); + QCOMPARE(cs1, QColorSpace()); + + QColorSpace cs3(std::move(cs2)); + QVERIFY(!cs2.isValid()); + QVERIFY(cs3.isValid()); + QCOMPARE(cs3.colorSpaceId(), QColorSpace::SRgb); + QCOMPARE(cs2.colorSpaceId(), QColorSpace::Undefined); +} + void tst_QColorSpace::namedColorSpaces_data() { QTest::addColumn("colorSpaceId"); From bc1f7c7cce30dc158b4a89f65acf7288f10447dd Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Fri, 30 Aug 2019 11:48:50 +0200 Subject: [PATCH 7/7] Fixup includes One include too many and one too little. Change-Id: I9963adb02523305d753135c0f5a6baefb83a06f1 Reviewed-by: Giuseppe D'Angelo --- src/gui/painting/qcolorspace.h | 2 ++ src/gui/painting/qcolortransform.h | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/gui/painting/qcolorspace.h b/src/gui/painting/qcolorspace.h index 5941682bbb..880f0ad4cf 100644 --- a/src/gui/painting/qcolorspace.h +++ b/src/gui/painting/qcolorspace.h @@ -42,11 +42,13 @@ #include #include +#include #include QT_BEGIN_NAMESPACE class QColorSpacePrivate; +class QPointF; class Q_GUI_EXPORT QColorSpace { diff --git a/src/gui/painting/qcolortransform.h b/src/gui/painting/qcolortransform.h index 5fb51739a7..94b6b3a385 100644 --- a/src/gui/painting/qcolortransform.h +++ b/src/gui/painting/qcolortransform.h @@ -41,7 +41,6 @@ #define QCOLORTRANSFORM_H #include -#include #include QT_BEGIN_NAMESPACE