From f40583bb6400d4ed67f83bc6e22e88049a314e06 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Wed, 30 Aug 2017 15:58:42 +0200 Subject: Fix accessibility-related performance regressions Before 089dd16f, we had a QQuickControlPrivate::accessibleAttached member that indicated whether accessibility was active. Since 4be38ab in qtdeclarative, it was possible to access QQuickAccessibleAttached:: attachedProperties() directly, but we ended up accidentally accessing it even when accessibility was not active, which added noticeable overhead. This improves those qmlbench test cases that call accessibility-aware setters such as QQuickAbstractButton::setChecked() a lot. For example, the test cases for CheckBox and RadioButton create large amounts of controls with a "checked: index % 2" binding. - CheckBox: 84 => 93 frames - RadioButton: 98 => 113 frames QAccessible::setActive(true) only notifies the observers, but does not really make accessibility active in the sense that the consequent calls to QAccessible::isActive() still return false. tst_accessible had to be changed to use QPlatformAccessible::setActive() instead. Change-Id: I8fd0fe2dddfd5633ce22a080bcda459f2d6e443e Reviewed-by: Liang Qi --- src/quicktemplates2/qquickcontrol.cpp | 17 +++++++++++++---- src/quicktemplates2/qquickcontrol_p_p.h | 3 +++ src/quicktemplates2/qquicklabel.cpp | 2 +- src/quicktemplates2/qquicktextarea.cpp | 4 ++-- src/quicktemplates2/qquicktextfield.cpp | 6 +++--- tests/auto/accessibility/tst_accessibility.cpp | 16 +++++++++++++++- 6 files changed, 37 insertions(+), 11 deletions(-) diff --git a/src/quicktemplates2/qquickcontrol.cpp b/src/quicktemplates2/qquickcontrol.cpp index 4b85e444..646ba7ea 100644 --- a/src/quicktemplates2/qquickcontrol.cpp +++ b/src/quicktemplates2/qquickcontrol.cpp @@ -315,6 +315,13 @@ QAccessible::Role QQuickControlPrivate::accessibleRole() const Q_Q(const QQuickControl); return q->accessibleRole(); } + +QQuickAccessibleAttached *QQuickControlPrivate::accessibleAttached(const QObject *object) +{ + if (!QAccessible::isActive()) + return nullptr; + return QQuickAccessibleAttached::attachedProperties(object); +} #endif /*! @@ -1613,7 +1620,7 @@ void QQuickControl::accessibilityActiveChanged(bool active) QString QQuickControl::accessibleName() const { #if QT_CONFIG(accessibility) - if (QQuickAccessibleAttached *accessibleAttached = QQuickAccessibleAttached::attachedProperties(this)) + if (QQuickAccessibleAttached *accessibleAttached = QQuickControlPrivate::accessibleAttached(this)) return accessibleAttached->name(); #endif return QString(); @@ -1622,7 +1629,7 @@ QString QQuickControl::accessibleName() const void QQuickControl::setAccessibleName(const QString &name) { #if QT_CONFIG(accessibility) - if (QQuickAccessibleAttached *accessibleAttached = QQuickAccessibleAttached::attachedProperties(this)) + if (QQuickAccessibleAttached *accessibleAttached = QQuickControlPrivate::accessibleAttached(this)) accessibleAttached->setName(name); #else Q_UNUSED(name) @@ -1632,7 +1639,8 @@ void QQuickControl::setAccessibleName(const QString &name) QVariant QQuickControl::accessibleProperty(const char *propertyName) { #if QT_CONFIG(accessibility) - return QQuickAccessibleAttached::property(this, propertyName); + if (QAccessible::isActive()) + return QQuickAccessibleAttached::property(this, propertyName); #endif Q_UNUSED(propertyName) return QVariant(); @@ -1641,7 +1649,8 @@ QVariant QQuickControl::accessibleProperty(const char *propertyName) bool QQuickControl::setAccessibleProperty(const char *propertyName, const QVariant &value) { #if QT_CONFIG(accessibility) - return QQuickAccessibleAttached::setProperty(this, propertyName, value); + if (QAccessible::isActive()) + return QQuickAccessibleAttached::setProperty(this, propertyName, value); #endif Q_UNUSED(propertyName) Q_UNUSED(value) diff --git a/src/quicktemplates2/qquickcontrol_p_p.h b/src/quicktemplates2/qquickcontrol_p_p.h index e990e44f..e7e9d46f 100644 --- a/src/quicktemplates2/qquickcontrol_p_p.h +++ b/src/quicktemplates2/qquickcontrol_p_p.h @@ -60,6 +60,8 @@ QT_BEGIN_NAMESPACE +class QQuickAccessibleAttached; + class Q_QUICKTEMPLATES2_PRIVATE_EXPORT QQuickControlPrivate : public QQuickItemPrivate #if QT_CONFIG(accessibility) , public QAccessible::ActivationObserver @@ -100,6 +102,7 @@ public: #if QT_CONFIG(accessibility) void accessibilityActiveChanged(bool active) override; QAccessible::Role accessibleRole() const override; + static QQuickAccessibleAttached *accessibleAttached(const QObject *object); #endif virtual void resolveFont(); diff --git a/src/quicktemplates2/qquicklabel.cpp b/src/quicktemplates2/qquicklabel.cpp index 8d964745..115a64c6 100644 --- a/src/quicktemplates2/qquicklabel.cpp +++ b/src/quicktemplates2/qquicklabel.cpp @@ -176,7 +176,7 @@ void QQuickLabelPrivate::textChanged(const QString &text) { #if QT_CONFIG(accessibility) Q_Q(QQuickLabel); - if (QQuickAccessibleAttached *accessibleAttached = QQuickAccessibleAttached::attachedProperties(q)) + if (QQuickAccessibleAttached *accessibleAttached = QQuickControlPrivate::accessibleAttached(q)) accessibleAttached->setName(text); #else Q_UNUSED(text) diff --git a/src/quicktemplates2/qquicktextarea.cpp b/src/quicktemplates2/qquicktextarea.cpp index eec25848..5f6037db 100644 --- a/src/quicktemplates2/qquicktextarea.cpp +++ b/src/quicktemplates2/qquicktextarea.cpp @@ -409,7 +409,7 @@ void QQuickTextAreaPrivate::readOnlyChanged(bool isReadOnly) { Q_UNUSED(isReadOnly); #if QT_CONFIG(accessibility) - if (QQuickAccessibleAttached *accessibleAttached = QQuickAccessibleAttached::attachedProperties(q_func())) + if (QQuickAccessibleAttached *accessibleAttached = QQuickControlPrivate::accessibleAttached(q_func())) accessibleAttached->set_readOnly(isReadOnly); #endif #if QT_CONFIG(cursor) @@ -528,7 +528,7 @@ void QQuickTextArea::setPlaceholderText(const QString &text) d->placeholder = text; #if QT_CONFIG(accessibility) - if (QQuickAccessibleAttached *accessibleAttached = QQuickAccessibleAttached::attachedProperties(this)) + if (QQuickAccessibleAttached *accessibleAttached = QQuickControlPrivate::accessibleAttached(this)) accessibleAttached->setDescription(text); #endif emit placeholderTextChanged(); diff --git a/src/quicktemplates2/qquicktextfield.cpp b/src/quicktemplates2/qquicktextfield.cpp index 1c350165..9e3eea50 100644 --- a/src/quicktemplates2/qquicktextfield.cpp +++ b/src/quicktemplates2/qquicktextfield.cpp @@ -272,7 +272,7 @@ void QQuickTextFieldPrivate::readOnlyChanged(bool isReadOnly) { Q_UNUSED(isReadOnly); #if QT_CONFIG(accessibility) - if (QQuickAccessibleAttached *accessibleAttached = QQuickAccessibleAttached::attachedProperties(q_func())) + if (QQuickAccessibleAttached *accessibleAttached = QQuickControlPrivate::accessibleAttached(q_func())) accessibleAttached->set_readOnly(isReadOnly); #endif #if QT_CONFIG(cursor) @@ -283,7 +283,7 @@ void QQuickTextFieldPrivate::readOnlyChanged(bool isReadOnly) void QQuickTextFieldPrivate::echoModeChanged(QQuickTextField::EchoMode echoMode) { #if QT_CONFIG(accessibility) - if (QQuickAccessibleAttached *accessibleAttached = QQuickAccessibleAttached::attachedProperties(q_func())) + if (QQuickAccessibleAttached *accessibleAttached = QQuickControlPrivate::accessibleAttached(q_func())) accessibleAttached->set_passwordEdit((echoMode == QQuickTextField::Password || echoMode == QQuickTextField::PasswordEchoOnEdit) ? true : false); #else Q_UNUSED(echoMode) @@ -397,7 +397,7 @@ void QQuickTextField::setPlaceholderText(const QString &text) d->placeholder = text; #if QT_CONFIG(accessibility) - if (QQuickAccessibleAttached *accessibleAttached = QQuickAccessibleAttached::attachedProperties(this)) + if (QQuickAccessibleAttached *accessibleAttached = QQuickControlPrivate::accessibleAttached(this)) accessibleAttached->setDescription(text); #endif emit placeholderTextChanged(); diff --git a/tests/auto/accessibility/tst_accessibility.cpp b/tests/auto/accessibility/tst_accessibility.cpp index 614566e6..7e491b54 100644 --- a/tests/auto/accessibility/tst_accessibility.cpp +++ b/tests/auto/accessibility/tst_accessibility.cpp @@ -43,6 +43,9 @@ #include "../shared/util.h" #if QT_CONFIG(accessibility) +#include +#include +#include #include #endif @@ -113,6 +116,14 @@ void tst_accessibility::a11y_data() QTest::newRow("WeekNumberColumn") << "weeknumbercolumn" << 0x0 << "WeekNumberColumn"; //QAccessible::NoRole } +#if QT_CONFIG(accessibility) +static QPlatformAccessibility *platformAccessibility() +{ + QPlatformIntegration *pfIntegration = QGuiApplicationPrivate::platformIntegration(); + return pfIntegration ? pfIntegration->accessibility() : nullptr; +} +#endif + void tst_accessibility::a11y() { QFETCH(QString, name); @@ -150,7 +161,10 @@ void tst_accessibility::a11y() QVERIFY(acc); } else { QVERIFY(!acc); - QAccessible::setActive(true); + QPlatformAccessibility *accessibility = platformAccessibility(); + if (!accessibility) + QSKIP("No QPlatformAccessibility available."); + accessibility->setActive(true); acc = QQuickAccessibleAttached::attachedProperties(item); } } -- cgit v1.2.3