From 445ef8847979dab72893aab1924d46d0fe1a8a3e Mon Sep 17 00:00:00 2001 From: Gabriel de Dietrich Date: Tue, 2 Nov 2010 17:25:54 +0100 Subject: With some locales, QDoubleValidator would not accept "C" locale valid numbers Locales using dot as thousands delimiter and comma as decimal separator are prone to this error. This is a regression introduced by commit b81b8e43ad57183ed66. Auto-tests included. Reviewed-by: Olivier Task-number: QTBUG_14935 --- src/corelib/tools/qlocale.h | 2 +- src/gui/widgets/qvalidator.cpp | 45 +++++++++++++--------- .../auto/qdoublevalidator/tst_qdoublevalidator.cpp | 4 ++ 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/src/corelib/tools/qlocale.h b/src/corelib/tools/qlocale.h index 8b424bb..1af2cb4 100644 --- a/src/corelib/tools/qlocale.h +++ b/src/corelib/tools/qlocale.h @@ -114,7 +114,7 @@ class Q_CORE_EXPORT QLocale friend class QString; friend class QByteArray; friend class QIntValidator; - friend class QDoubleValidator; + friend class QDoubleValidatorPrivate; friend class QTextStream; friend class QTextStreamPrivate; diff --git a/src/gui/widgets/qvalidator.cpp b/src/gui/widgets/qvalidator.cpp index b75db45..130d091 100644 --- a/src/gui/widgets/qvalidator.cpp +++ b/src/gui/widgets/qvalidator.cpp @@ -499,6 +499,8 @@ public: } QDoubleValidator::Notation notation; + + QValidator::State validateWithLocale(QString & input, QLocalePrivate::NumberMode numMode, const QLocale &locale) const; }; @@ -654,42 +656,49 @@ QValidator::State QDoubleValidator::validate(QString & input, int &) const break; } + State currentLocaleValidation = d->validateWithLocale(input, numMode, locale()); + if (currentLocaleValidation == Acceptable || locale().language() == QLocale::C) + return currentLocaleValidation; + State cLocaleValidation = d->validateWithLocale(input, numMode, QLocale(QLocale::C)); + return qMax(currentLocaleValidation, cLocaleValidation); +} + +QValidator::State QDoubleValidatorPrivate::validateWithLocale(QString &input, QLocalePrivate::NumberMode numMode, const QLocale &locale) const +{ + Q_Q(const QDoubleValidator); QByteArray buff; - if (!locale().d()->validateChars(input, numMode, &buff, dec)) { - QLocale cl(QLocale::C); - if (!cl.d()->validateChars(input, numMode, &buff, dec)) - return Invalid; - } + if (!locale.d()->validateChars(input, numMode, &buff, q->dec)) + return QValidator::Invalid; if (buff.isEmpty()) - return Intermediate; + return QValidator::Intermediate; - if (b >= 0 && buff.startsWith('-')) - return Invalid; + if (q->b >= 0 && buff.startsWith('-')) + return QValidator::Invalid; - if (t < 0 && buff.startsWith('+')) - return Invalid; + if (q->t < 0 && buff.startsWith('+')) + return QValidator::Invalid; bool ok, overflow; double i = QLocalePrivate::bytearrayToDouble(buff.constData(), &ok, &overflow); if (overflow) - return Invalid; + return QValidator::Invalid; if (!ok) - return Intermediate; + return QValidator::Intermediate; - if (i >= b && i <= t) - return Acceptable; + if (i >= q->b && i <= q->t) + return QValidator::Acceptable; - if (d->notation == StandardNotation) { - double max = qMax(qAbs(b), qAbs(t)); + if (notation == QDoubleValidator::StandardNotation) { + double max = qMax(qAbs(q->b), qAbs(q->t)); if (max < LLONG_MAX) { qlonglong n = pow10(numDigits(qlonglong(max))) - 1; if (qAbs(i) > n) - return Invalid; + return QValidator::Invalid; } } - return Intermediate; + return QValidator::Intermediate; } diff --git a/tests/auto/qdoublevalidator/tst_qdoublevalidator.cpp b/tests/auto/qdoublevalidator/tst_qdoublevalidator.cpp index 26890b3..3470456 100644 --- a/tests/auto/qdoublevalidator/tst_qdoublevalidator.cpp +++ b/tests/auto/qdoublevalidator/tst_qdoublevalidator.cpp @@ -214,6 +214,10 @@ void tst_QDoubleValidator::validate_data() arabicNum += QChar(1643); arabicNum += QChar(1636); QTest::newRow("arabic") << "ar" << 0.0 << 20.0 << 2 << arabicNum << ACC << ACC; + + QTest::newRow("data_QTBUG_14935-1") << "de" << 0.0 << 1.0 << 5 << QString("0.31") << ACC << ACC; + QTest::newRow("data_QTBUG_14935-2") << "de" << 0.0 << 1000000.0 << 5 << QString("3.123") << ACC << ACC; + QTest::newRow("data_QTBUG_14935-3") << "de" << 0.0 << 1000000.0 << 5 << QString("123,345.678") << ACC << ACC; } void tst_QDoubleValidator::validate() -- cgit v0.12 From d50fb14395e8ea35d09bdfc8bb420f051b6c140a Mon Sep 17 00:00:00 2001 From: Sami Merila Date: Tue, 9 Nov 2010 12:50:09 +0200 Subject: QS60Style: Color calculation should be optimized Currently, QS60Style calculates some palette colors (tooltip base and button). Since native side does not have a color for these, but a nine-part theme graphic, the style tries to estimate the color of the bitmap by taking a small sample of the QPixmap and calculate the RGB colors of 32*32 pixels. This is rather slow, it takes around 110 msecs for each QApplication, when the application is started. Note that color is cached to member variable of style, but it is very rarely asked again (as the color is polished to all widgets/apps) and the cache is not shared across processes. As a fix, style now calculates the button color (tooltip color is no longer calculated, as no other QStyle does that, and tooltips do not anyway work in the Qt/Symbian) and stores the calculated value to Global QSettings together with active theme ID. Now, when a second Qt application is launched, the stored theme ID value is matched with currently active theme. If it matches, then the stored Button color is used. Otherwise, color is again calculated and stored. If theme is unchanged, the application launch is ~95msecs faster. Task-number: QTBUG-14860 Reviewed-by: Jani Hautakangas --- src/gui/styles/qs60style.cpp | 107 ++++++++++++++++++--------------------- src/gui/styles/qs60style_p.h | 7 ++- src/gui/styles/qs60style_s60.cpp | 75 +++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 59 deletions(-) diff --git a/src/gui/styles/qs60style.cpp b/src/gui/styles/qs60style.cpp index f3fb2f5..5e5adab 100644 --- a/src/gui/styles/qs60style.cpp +++ b/src/gui/styles/qs60style.cpp @@ -405,13 +405,14 @@ void QS60StylePrivate::clearCaches(CacheClearReason reason) QPixmapCache::clear(); break; case CC_ThemeChange: - m_colorCache.clear(); QPixmapCache::clear(); +#ifdef Q_WS_S60 + deleteStoredSettings(); +#endif deleteBackground(); break; case CC_UndefinedChange: default: - m_colorCache.clear(); m_mappedFontsCache.clear(); QPixmapCache::clear(); deleteBackground(); @@ -419,64 +420,53 @@ void QS60StylePrivate::clearCaches(CacheClearReason reason) } } -// Since S60Style has 'button' and 'tooltip' as a graphic, we don't have any native color which to use -// for QPalette::Button and QPalette::ToolTipBase. Therefore S60Style needs to guesstimate -// palette colors by calculating average rgb values for button pixels. -// Returns Qt::black if there is an issue with the graphics (image is NULL, or no bits() found). -QColor QS60StylePrivate::colorFromFrameGraphics(SkinFrameElements frame) const +QColor QS60StylePrivate::calculatedColor(SkinFrameElements frame) const { - const bool cachedColorExists = m_colorCache.contains(frame); - if (!cachedColorExists) { - const int frameCornerWidth = pixelMetric(PM_FrameCornerWidth); - const int frameCornerHeight = pixelMetric(PM_FrameCornerHeight); - Q_ASSERT(2 * frameCornerWidth < 32); - Q_ASSERT(2 * frameCornerHeight < 32); - - const QImage frameImage = QS60StylePrivate::frame(frame, QSize(32, 32)).toImage(); - Q_ASSERT(frameImage.bytesPerLine() > 0); - if (frameImage.isNull()) - return Qt::black; - - const QRgb *pixelRgb = (const QRgb*)frameImage.bits(); - const int pixels = frameImage.byteCount()/sizeof(QRgb); - - int estimatedRed = 0; - int estimatedGreen = 0; - int estimatedBlue = 0; - - int skips = 0; - int estimations = 0; - - const int topBorderLastPixel = frameCornerHeight*frameImage.width() - 1; - const int bottomBorderFirstPixel = frameImage.width() * frameImage.height() - frameCornerHeight*frameImage.width() - 1; - const int rightBorderFirstPixel = frameImage.width() - frameCornerWidth; - const int leftBorderLastPixel = frameCornerWidth; - - while ((skips + estimations) < pixels) { - if ((skips + estimations) > topBorderLastPixel && - (skips + estimations) < bottomBorderFirstPixel) { - for (int rowIndex = 0; rowIndex < frameImage.width(); rowIndex++) { - if (rowIndex > leftBorderLastPixel && - rowIndex < rightBorderFirstPixel) { - estimatedRed += qRed(*pixelRgb); - estimatedGreen += qGreen(*pixelRgb); - estimatedBlue += qBlue(*pixelRgb); - } - pixelRgb++; - estimations++; + const int frameCornerWidth = pixelMetric(PM_FrameCornerWidth); + const int frameCornerHeight = pixelMetric(PM_FrameCornerHeight); + Q_ASSERT(2 * frameCornerWidth < 32); + Q_ASSERT(2 * frameCornerHeight < 32); + + const QImage frameImage = QS60StylePrivate::frame(frame, QSize(32, 32)).toImage(); + Q_ASSERT(frameImage.bytesPerLine() > 0); + if (frameImage.isNull()) + return Qt::black; + + const QRgb *pixelRgb = (const QRgb*)frameImage.constBits(); + const int pixels = frameImage.byteCount() / sizeof(QRgb); + + int estimatedRed = 0; + int estimatedGreen = 0; + int estimatedBlue = 0; + + int skips = 0; + int estimations = 0; + + const int topBorderLastPixel = frameCornerHeight * frameImage.width() - 1; + const int bottomBorderFirstPixel = frameImage.width() * frameImage.height() - topBorderLastPixel; + const int rightBorderFirstPixel = frameImage.width() - frameCornerWidth; + const int leftBorderLastPixel = frameCornerWidth; + + while ((skips + estimations) < pixels) { + if ((skips + estimations) > topBorderLastPixel && + (skips + estimations) < bottomBorderFirstPixel) { + for (int rowIndex = 0; rowIndex < frameImage.width(); rowIndex++) { + if (rowIndex > leftBorderLastPixel && + rowIndex < rightBorderFirstPixel) { + estimatedRed += qRed(*pixelRgb); + estimatedGreen += qGreen(*pixelRgb); + estimatedBlue += qBlue(*pixelRgb); } - } else { pixelRgb++; - skips++; + estimations++; } + } else { + pixelRgb++; + skips++; } - QColor frameColor(estimatedRed/estimations, estimatedGreen/estimations, estimatedBlue/estimations); - m_colorCache.insert(frame, frameColor); - return !estimations ? Qt::black : frameColor; - } else { - return m_colorCache.value(frame); } - + QColor frameColor(estimatedRed/estimations, estimatedGreen/estimations, estimatedBlue/estimations); + return !estimations ? Qt::black : frameColor; } void QS60StylePrivate::setThemePalette(QApplication *app) const @@ -731,11 +721,14 @@ void QS60StylePrivate::setThemePalette(QPalette *palette) const palette->setBrush(QPalette::Window, backgroundTexture()); // set as transparent so that styled full screen theme background is visible palette->setBrush(QPalette::Base, Qt::transparent); - // set button and tooltipbase based on pixel colors + // set button color based on pixel colors +#ifndef Q_WS_S60 + //For emulated style, just calculate the color everytime + const QColor buttonColor = calculatedColor(SF_ButtonNormal); +#else const QColor buttonColor = colorFromFrameGraphics(SF_ButtonNormal); +#endif palette->setColor(QPalette::Button, buttonColor); - const QColor toolTipColor = colorFromFrameGraphics(SF_ToolTip); - palette->setColor(QPalette::ToolTipBase, toolTipColor); palette->setColor(QPalette::Light, palette->color(QPalette::Button).lighter()); palette->setColor(QPalette::Dark, palette->color(QPalette::Button).darker()); palette->setColor(QPalette::Midlight, palette->color(QPalette::Button).lighter(125)); diff --git a/src/gui/styles/qs60style_p.h b/src/gui/styles/qs60style_p.h index b46f75e..7a7991a 100644 --- a/src/gui/styles/qs60style_p.h +++ b/src/gui/styles/qs60style_p.h @@ -523,8 +523,12 @@ public: static bool isSingleClickUi(); static bool isWidgetPressed(const QWidget *widget); - // calculates average color based on button skin graphics (minus borders). +#ifdef Q_WS_S60 + static void deleteStoredSettings(); + // calculates average color based on theme graphics (minus borders). QColor colorFromFrameGraphics(SkinFrameElements frame) const; +#endif + QColor calculatedColor(SkinFrameElements frame) const; //set theme palette for application void setThemePalette(QApplication *application) const; @@ -542,7 +546,6 @@ public: static const int m_numberOfLayouts; mutable QHash, QFont> m_mappedFontsCache; - mutable QHash m_colorCache; // Has one entry per SkinFrameElements static const struct frameElementCenter { diff --git a/src/gui/styles/qs60style_s60.cpp b/src/gui/styles/qs60style_s60.cpp index a1ea308..204699d 100644 --- a/src/gui/styles/qs60style_s60.cpp +++ b/src/gui/styles/qs60style_s60.cpp @@ -48,6 +48,7 @@ #include "private/qpixmap_s60_p.h" #include "private/qcore_symbian_p.h" #include "qapplication.h" +#include "qsettings.h" #include "qpluginloader.h" #include "qlibraryinfo.h" @@ -69,6 +70,8 @@ #include #include +#include + #if !defined(QT_NO_STYLE_S60) || defined(QT_PLUGIN) QT_BEGIN_NAMESPACE @@ -81,6 +84,8 @@ enum TDrawType { ENoDraw }; +const TUid personalisationUID = { 0x101F876F }; + enum TSupportRelease { ES60_None = 0x0000, //indicates that the commonstyle should draw the graphics ES60_3_1 = 0x0001, @@ -689,6 +694,76 @@ bool QS60StylePrivate::isSingleClickUi() return (QSysInfo::s60Version() > QSysInfo::SV_S60_5_0); } +void QS60StylePrivate::deleteStoredSettings() +{ + QSettings settings(QSettings::UserScope, QLatin1String("Trolltech")); + settings.beginGroup(QLatin1String("QS60Style")); + settings.remove(""); + settings.endGroup(); +} + +// Since S60Style has 'button' as a graphic, we don't have any native color which to use +// for QPalette::Button. Therefore S60Style needs to guesstimate palette color by calculating +// average rgb values for button pixels. +// Returns Qt::black if there is an issue with the graphics (image is NULL, or no constBits() found). +QColor QS60StylePrivate::colorFromFrameGraphics(SkinFrameElements frame) const +{ +#ifndef QT_NO_SETTINGS + TInt themeID = 0; + //First we need to fetch active theme ID. We need to store the themeID at the same time + //as color, so that we can later check if the stored color is still from the same theme. + //Native side stores active theme UID/Timestamp into central repository. + int error = 0; + QT_TRAP_THROWING( + CRepository *themeRepository = CRepository::NewLC(personalisationUID); + if (themeRepository) { + static const TInt KThemePkgIDDesSize = 23; //size of the stored theme package ID + TBuf<32> value; //themeID is currently max of 8 + 1 + 8 characters, but lets have some extra space + const TUint32 key = 0x00000002; //active theme key in the repository + error = themeRepository->Get(key, value); + if (error == KErrNone) { + TLex lex(value); + TPtrC numberToken(lex.NextToken()); + if(numberToken.Length()) + error = TLex(numberToken).Val(themeID); + else + error = KErrArgument; + } + } + CleanupStack::PopAndDestroy(themeRepository); + ); + + QSettings settings(QSettings::UserScope, QLatin1String("Trolltech")); + settings.beginGroup(QLatin1String("QS60Style")); + if (themeID != 0) { + QVariant buttonColor = settings.value(QLatin1String("ButtonColor")); + if (!buttonColor.isNull()) { + //there is a stored color value, lets see if the theme ID matches + if (error == KErrNone) { + QVariant themeUID = settings.value(QLatin1String("ThemeUID")); + if (!themeUID.isNull() && themeUID.toInt() == themeID) { + QColor storedColor(buttonColor.value()); + if (storedColor.isValid()) + return storedColor; + } + } + settings.remove(""); //if color was invalid, or theme has been changed, just delete all stored settings + } + } +#endif + + QColor color = calculatedColor(frame); + +#ifndef QT_NO_SETTINGS + settings.setValue(QLatin1String("ThemeUID"), QVariant(themeID)); + if (frame == SF_ButtonNormal) //other colors are not currently calculated from graphics + settings.setValue(QLatin1String("ButtonColor"), QVariant(color)); + settings.endGroup(); +#endif + + return color; +} + QPoint qt_s60_fill_background_offset(const QWidget *targetWidget) { CCoeControl *control = targetWidget->effectiveWinId(); -- cgit v0.12 From 0870d766b9e302081ba31a9c8f6dfa3a1e8c1e52 Mon Sep 17 00:00:00 2001 From: Sami Merila Date: Tue, 9 Nov 2010 12:53:07 +0200 Subject: QS60Style: Color calculation should be optimized Addendum to the previous fix. Remove trailing white space as well. Task-number: QTBUG-14860 Reviewed-by: Jani Hautakangas --- src/gui/styles/qs60style_p.h | 2 +- src/gui/styles/qs60style_s60.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/gui/styles/qs60style_p.h b/src/gui/styles/qs60style_p.h index 7a7991a..db4285d 100644 --- a/src/gui/styles/qs60style_p.h +++ b/src/gui/styles/qs60style_p.h @@ -527,7 +527,7 @@ public: static void deleteStoredSettings(); // calculates average color based on theme graphics (minus borders). QColor colorFromFrameGraphics(SkinFrameElements frame) const; -#endif +#endif QColor calculatedColor(SkinFrameElements frame) const; //set theme palette for application diff --git a/src/gui/styles/qs60style_s60.cpp b/src/gui/styles/qs60style_s60.cpp index 204699d..7b75d40 100644 --- a/src/gui/styles/qs60style_s60.cpp +++ b/src/gui/styles/qs60style_s60.cpp @@ -709,7 +709,7 @@ void QS60StylePrivate::deleteStoredSettings() QColor QS60StylePrivate::colorFromFrameGraphics(SkinFrameElements frame) const { #ifndef QT_NO_SETTINGS - TInt themeID = 0; + TInt themeID = 0; //First we need to fetch active theme ID. We need to store the themeID at the same time //as color, so that we can later check if the stored color is still from the same theme. //Native side stores active theme UID/Timestamp into central repository. @@ -718,13 +718,13 @@ QColor QS60StylePrivate::colorFromFrameGraphics(SkinFrameElements frame) const CRepository *themeRepository = CRepository::NewLC(personalisationUID); if (themeRepository) { static const TInt KThemePkgIDDesSize = 23; //size of the stored theme package ID - TBuf<32> value; //themeID is currently max of 8 + 1 + 8 characters, but lets have some extra space + TBuf<32> value; //themeID is currently max of 8 + 1 + 8 characters, but lets have some extra space const TUint32 key = 0x00000002; //active theme key in the repository error = themeRepository->Get(key, value); if (error == KErrNone) { - TLex lex(value); + TLex lex(value); TPtrC numberToken(lex.NextToken()); - if(numberToken.Length()) + if (numberToken.Length()) error = TLex(numberToken).Val(themeID); else error = KErrArgument; -- cgit v0.12 From b7d91c0d798bc10dd4f8a750723285076fb96935 Mon Sep 17 00:00:00 2001 From: Gabriel de Dietrich Date: Wed, 10 Nov 2010 13:21:02 +0100 Subject: Fix auto-test regression in tst_QDoubleValidator The locale from a previous test was not cleared. Another issue has been detected, though, and reported as QTBUG-15210. Reviewed-by: Thierry --- tests/auto/qdoublevalidator/tst_qdoublevalidator.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/auto/qdoublevalidator/tst_qdoublevalidator.cpp b/tests/auto/qdoublevalidator/tst_qdoublevalidator.cpp index 3470456..98c4740 100644 --- a/tests/auto/qdoublevalidator/tst_qdoublevalidator.cpp +++ b/tests/auto/qdoublevalidator/tst_qdoublevalidator.cpp @@ -218,6 +218,9 @@ void tst_QDoubleValidator::validate_data() QTest::newRow("data_QTBUG_14935-1") << "de" << 0.0 << 1.0 << 5 << QString("0.31") << ACC << ACC; QTest::newRow("data_QTBUG_14935-2") << "de" << 0.0 << 1000000.0 << 5 << QString("3.123") << ACC << ACC; QTest::newRow("data_QTBUG_14935-3") << "de" << 0.0 << 1000000.0 << 5 << QString("123,345.678") << ACC << ACC; + + QTest::newRow("data_de_problem-1") << "de" << 0.0 << 10.0 << 0 << QString("1.0") << ITM << ITM; + QTest::newRow("data_de_problem-2") << "de" << 0.0 << 10.0 << 0 << QString("0.1") << INV << INV; } void tst_QDoubleValidator::validate() @@ -230,6 +233,9 @@ void tst_QDoubleValidator::validate() QFETCH(QValidator::State, scientific_state); QFETCH(QValidator::State, standard_state); + QEXPECT_FAIL("data_de_problem-1", "To be fixed. See QTBUG-15210.", Abort); + QEXPECT_FAIL("data_de_problem-2", "To be fixed. See QTBUG-15210.", Abort); + QLocale::setDefault(QLocale(localeName)); QDoubleValidator dv(minimum, maximum, decimals, 0); @@ -312,6 +318,8 @@ void tst_QDoubleValidator::validateIntEquiv() QFETCH(QString, input); QFETCH(QValidator::State, state); + QLocale::setDefault(QLocale("C")); + QDoubleValidator dv(minimum, maximum, 0, 0); dv.setNotation(QDoubleValidator::StandardNotation); int dummy; -- cgit v0.12 From 285f49d680a824e788eb6cf107f70cd94d358dfe Mon Sep 17 00:00:00 2001 From: Sergio Ahumada Date: Thu, 11 Nov 2010 11:38:15 +0100 Subject: Doc: Fixing typo --- src/gui/styles/qs60style.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/styles/qs60style.cpp b/src/gui/styles/qs60style.cpp index 5e5adab..5fe9050 100644 --- a/src/gui/styles/qs60style.cpp +++ b/src/gui/styles/qs60style.cpp @@ -723,7 +723,7 @@ void QS60StylePrivate::setThemePalette(QPalette *palette) const palette->setBrush(QPalette::Base, Qt::transparent); // set button color based on pixel colors #ifndef Q_WS_S60 - //For emulated style, just calculate the color everytime + //For emulated style, just calculate the color every time const QColor buttonColor = calculatedColor(SF_ButtonNormal); #else const QColor buttonColor = colorFromFrameGraphics(SF_ButtonNormal); -- cgit v0.12