From 4114a0ea758d7f808011f9db0524c541880c16ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20de=20la=20Rocha?= Date: Fri, 25 Mar 2022 02:14:12 -0300 Subject: [PATCH] Windows QPA: Fix slowdown with large table/tree views with accessibility The accessibility code for notifying focus changes related to a table/tree view was iterating over all items to find the focused one, which for a very large number of items could cause a major slowdown and UI freeze. This patch avoids the issue by removing the loop and implementing the focusChild() method in the table/tree accessibility classes. Fixes: QTBUG-100997 Pick-to: 6.2 6.3 5.15 Change-Id: I04c847a5e65223b7a93ab82363feb32e1ebab9f3 Reviewed-by: Volker Hilsheimer --- .../uiautomation/qwindowsuiamainprovider.cpp | 16 +-- src/widgets/accessible/itemviews.cpp | 23 ++++ src/widgets/accessible/itemviews_p.h | 2 + .../qaccessibility/tst_qaccessibility.cpp | 107 ++++++++++++++++++ 4 files changed, 136 insertions(+), 12 deletions(-) diff --git a/src/plugins/platforms/windows/uiautomation/qwindowsuiamainprovider.cpp b/src/plugins/platforms/windows/uiautomation/qwindowsuiamainprovider.cpp index 2913f0ed68..63bf6bc87c 100644 --- a/src/plugins/platforms/windows/uiautomation/qwindowsuiamainprovider.cpp +++ b/src/plugins/platforms/windows/uiautomation/qwindowsuiamainprovider.cpp @@ -108,19 +108,11 @@ void QWindowsUiaMainProvider::notifyFocusChange(QAccessibleEvent *event) { if (QAccessibleInterface *accessible = event->accessibleInterface()) { // If this is a table/tree/list, raise event for the focused cell/item instead. - if (accessible->tableInterface()) { - int count = accessible->childCount(); - for (int i = 0; i < count; ++i) { - QAccessibleInterface *item = accessible->child(i); - if (item && item->isValid() && item->state().focused) { - accessible = item; - break; - } - } - } - if (QWindowsUiaMainProvider *provider = providerForAccessible(accessible)) { + if (accessible->tableInterface()) + if (QAccessibleInterface *child = accessible->focusChild()) + accessible = child; + if (QWindowsUiaMainProvider *provider = providerForAccessible(accessible)) QWindowsUiaWrapper::instance()->raiseAutomationEvent(provider, UIA_AutomationFocusChangedEventId); - } } } diff --git a/src/widgets/accessible/itemviews.cpp b/src/widgets/accessible/itemviews.cpp index a7b536ae54..e485744dc8 100644 --- a/src/widgets/accessible/itemviews.cpp +++ b/src/widgets/accessible/itemviews.cpp @@ -425,6 +425,15 @@ QAccessibleInterface *QAccessibleTable::childAt(int x, int y) const return nullptr; } +QAccessibleInterface *QAccessibleTable::focusChild() const +{ + QModelIndex index = view()->currentIndex(); + if (!index.isValid()) + return nullptr; + + return child(logicalIndex(index)); +} + int QAccessibleTable::childCount() const { if (!view()->model()) @@ -692,6 +701,20 @@ QAccessibleInterface *QAccessibleTree::childAt(int x, int y) const return child(i); } +QAccessibleInterface *QAccessibleTree::focusChild() const +{ + QModelIndex index = view()->currentIndex(); + if (!index.isValid()) + return nullptr; + + const QTreeView *treeView = qobject_cast(view()); + int row = treeView->d_func()->viewIndex(index) + (horizontalHeader() ? 1 : 0); + int column = index.column(); + + int i = row * view()->model()->columnCount() + column; + return child(i); +} + int QAccessibleTree::childCount() const { const QTreeView *treeView = qobject_cast(view()); diff --git a/src/widgets/accessible/itemviews_p.h b/src/widgets/accessible/itemviews_p.h index 683ae9c948..1fc6a3e521 100644 --- a/src/widgets/accessible/itemviews_p.h +++ b/src/widgets/accessible/itemviews_p.h @@ -79,6 +79,7 @@ public: QRect rect() const override; QAccessibleInterface *childAt(int x, int y) const override; + QAccessibleInterface *focusChild() const override; int childCount() const override; int indexOfChild(const QAccessibleInterface *) const override; @@ -154,6 +155,7 @@ public: QAccessibleInterface *childAt(int x, int y) const override; + QAccessibleInterface *focusChild() const override; int childCount() const override; QAccessibleInterface *child(int index) const override; diff --git a/tests/auto/other/qaccessibility/tst_qaccessibility.cpp b/tests/auto/other/qaccessibility/tst_qaccessibility.cpp index 95223057a0..b7b50ccf4f 100644 --- a/tests/auto/other/qaccessibility/tst_qaccessibility.cpp +++ b/tests/auto/other/qaccessibility/tst_qaccessibility.cpp @@ -36,6 +36,7 @@ # include #endif #include +#include #include #include #include @@ -4118,6 +4119,112 @@ void tst_QAccessibility::focusChild() QTestAccessibility::clearEvents(); } + + { + auto tableView = std::make_unique(3, 3); + + QSignalSpy spy(tableView.get(), SIGNAL(currentCellChanged(int,int,int,int))); + + tableView->setColumnCount(3); + QStringList hHeader; + hHeader << "h1" << "h2" << "h3"; + tableView->setHorizontalHeaderLabels(hHeader); + + QStringList vHeader; + vHeader << "v1" << "v2" << "v3"; + tableView->setVerticalHeaderLabels(vHeader); + + for (int i = 0; i < 9; ++i) { + QTableWidgetItem *item = new QTableWidgetItem; + item->setText(QString::number(i/3) + QString(".") + QString::number(i%3)); + tableView->setItem(i/3, i%3, item); + } + + tableView->resize(600, 600); + tableView->show(); + QVERIFY(QTest::qWaitForWindowExposed(tableView.get())); + + tableView->setFocus(); + + QAccessibleInterface *iface = QAccessible::queryAccessibleInterface(tableView.get()); + QVERIFY(iface); + + spy.clear(); + tableView->setCurrentCell(2, 1); + QTRY_COMPARE(spy.count(), 1); + + QAccessibleInterface *child = iface->focusChild(); + QVERIFY(child); + QCOMPARE(child->text(QAccessible::Name), QStringLiteral("2.1")); + + spy.clear(); + tableView->setCurrentCell(1, 2); + QTRY_COMPARE(spy.count(), 1); + + child = iface->focusChild(); + QVERIFY(child); + QCOMPARE(child->text(QAccessible::Name), QStringLiteral("1.2")); + } + + { + auto treeView = std::make_unique(); + + QSignalSpy spy(treeView.get(), SIGNAL(currentItemChanged(QTreeWidgetItem*,QTreeWidgetItem*))); + + treeView->setColumnCount(2); + QTreeWidgetItem *header = new QTreeWidgetItem; + header->setText(0, "Artist"); + header->setText(1, "Work"); + treeView->setHeaderItem(header); + + QTreeWidgetItem *root1 = new QTreeWidgetItem; + root1->setText(0, "Spain"); + treeView->addTopLevelItem(root1); + + QTreeWidgetItem *item1 = new QTreeWidgetItem; + item1->setText(0, "Picasso"); + item1->setText(1, "Guernica"); + root1->addChild(item1); + + QTreeWidgetItem *item2 = new QTreeWidgetItem; + item2->setText(0, "Tapies"); + item2->setText(1, "Ambrosia"); + root1->addChild(item2); + + QTreeWidgetItem *root2 = new QTreeWidgetItem; + root2->setText(0, "Austria"); + treeView->addTopLevelItem(root2); + + QTreeWidgetItem *item3 = new QTreeWidgetItem; + item3->setText(0, "Klimt"); + item3->setText(1, "The Kiss"); + root2->addChild(item3); + + treeView->resize(400, 400); + treeView->show(); + QVERIFY(QTest::qWaitForWindowExposed(treeView.get())); + + treeView->setFocus(); + + QAccessibleInterface *iface = QAccessible::queryAccessibleInterface(treeView.get()); + QVERIFY(iface); + + spy.clear(); + treeView->setCurrentItem(item2); + QTRY_COMPARE(spy.count(), 1); + + QAccessibleInterface *child = iface->focusChild(); + QVERIFY(child); + QCOMPARE(child->text(QAccessible::Name), QStringLiteral("Tapies")); + + spy.clear(); + treeView->setCurrentItem(item3); + QTRY_COMPARE(spy.count(), 1); + + child = iface->focusChild(); + QVERIFY(child); + QCOMPARE(child->text(QAccessible::Name), QStringLiteral("Klimt")); + } }