From 7ccaca8dcaab5868987b78b6b2436872ac7f1129 Mon Sep 17 00:00:00 2001 From: Michael Weghorn Date: Fri, 29 Jul 2022 16:07:04 +0200 Subject: tdf#150064 a11y: Swap old/new list before merging information Swap the old and new list before merging information from the old list to the new one, not afterwards. This also moves this into the block guarded by the SolarMutexGuard, which seems to be held primarily due to changes to `maVisibleChildren` going on, which may already be happening in `ChildrenManagerImpl::MergeAccessibilityInformation` since the elements in the vector can be reordered since commit 8f9fd6806ccfbf381a383efe5d143ead86ee49de Date: Wed Jun 29 19:47:20 2022 +0200 tdf#137544 reduce cost of ChildrenManagerImpl::Update This change is primarily in preparation of adapting what list(s) get sorted/reordered in `ChildrenManagerImpl::MergeAccessibilityInformation` in follow-up commit Ie449f76f1b98ffe8e85ca28e938b11d726086721, "tdf#150064 Keep a11y child order intact". Change-Id: I88bd27b6cbca1e7a03702fd7e75f4094bdb5f977 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/137621 Tested-by: Jenkins Reviewed-by: Noel Grandin Reviewed-by: Michael Weghorn --- svx/source/accessibility/ChildrenManagerImpl.cxx | 23 ++++++++++++----------- svx/source/accessibility/ChildrenManagerImpl.hxx | 6 +++--- 2 files changed, 15 insertions(+), 14 deletions(-) (limited to 'svx') diff --git a/svx/source/accessibility/ChildrenManagerImpl.cxx b/svx/source/accessibility/ChildrenManagerImpl.cxx index f6637af3fa18..5ed1bbd4a68e 100644 --- a/svx/source/accessibility/ChildrenManagerImpl.cxx +++ b/svx/source/accessibility/ChildrenManagerImpl.cxx @@ -196,19 +196,20 @@ void ChildrenManagerImpl::Update (bool bCreateNewObjectsOnDemand) ChildDescriptorListType aChildList; CreateListOfVisibleShapes (aChildList); - // 2. Merge the information that is already known about the visible - // shapes from the current list into the new list. - MergeAccessibilityInformation (aChildList); - - // 3. Replace the current list of visible shapes with the new one. Do + // 2. Replace the current list of visible shapes with the new one. Do // the same with the visible area. { SolarMutexGuard g; - adjustIndexInParentOfShapes(aChildList); // Use swap to copy the contents of the new list in constant time. maVisibleChildren.swap (aChildList); + // 3. Merge the information that is already known about the visible + // shapes from the previous list into the new list. + MergeAccessibilityInformation (aChildList); + + adjustIndexInParentOfShapes(maVisibleChildren); + // aChildList now contains all the old children, while maVisibleChildren // contains all the current children @@ -371,15 +372,15 @@ void ChildrenManagerImpl::RemoveNonVisibleChildren ( } void ChildrenManagerImpl::MergeAccessibilityInformation ( - ChildDescriptorListType& raNewChildList) + ChildDescriptorListType& raOldChildList) { // sort the lists by mxShape, and then walk them in parallel, which avoids an O(n^2) loop std::sort(maVisibleChildren.begin(), maVisibleChildren.end(), ChildDescriptorLess()); - std::sort(raNewChildList.begin(), raNewChildList.end(), ChildDescriptorLess()); - ChildDescriptorListType::const_iterator aOldChildDescriptor = maVisibleChildren.begin(); - ChildDescriptorListType::const_iterator aEndVisibleChildren = maVisibleChildren.end(); + std::sort(raOldChildList.begin(), raOldChildList.end(), ChildDescriptorLess()); + ChildDescriptorListType::const_iterator aOldChildDescriptor = raOldChildList.begin(); + ChildDescriptorListType::const_iterator aEndVisibleChildren = raOldChildList.end(); - for (auto& rChild : raNewChildList) + for (auto& rChild : maVisibleChildren) { while (aOldChildDescriptor != aEndVisibleChildren && ChildDescriptorLess()(*aOldChildDescriptor, rChild)) aOldChildDescriptor++; diff --git a/svx/source/accessibility/ChildrenManagerImpl.hxx b/svx/source/accessibility/ChildrenManagerImpl.hxx index 36617291cb62..f2762bfd83f2 100644 --- a/svx/source/accessibility/ChildrenManagerImpl.hxx +++ b/svx/source/accessibility/ChildrenManagerImpl.hxx @@ -353,10 +353,10 @@ private: ChildDescriptorListType& raOldChildList); /** Merge the information that is already known about the visible shapes - from the current list into the new list. + from the old list into the current list. @param raChildList - Information is merged from the current list of visible children - to this list. + Information is merged to the current list of visible children + from this list. The old list can get reordered. */ void MergeAccessibilityInformation (ChildDescriptorListType& raChildList); -- cgit