New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Navigating by line in Microsoft Edge is now up to 3x faster in the Windows 10 Creaters Update #6994

Merged

michaelDCurran merged 10 commits into master from edgeSpeedup

Apr 12, 2017

+5,706 −289

Member

michaelDCurran commented Mar 21, 2017

Speed improvements gained by making use of UI Automation cache requests when using UIA NVDAObjects to generate controlFields, by making use of the new IUIAutomationTextRange3 caching APIs, and by leveraging the fact that Edge now better conforms to a standard UI Automation text implementation thus the inefficient Edge-specific content fetching algorithm is no longer needed.

michaelDCurran added 3 commits

March 21, 2017 13:32


          Build a caching framework into the UIA NVDAObject class so as to allo…

fa76884

…w for instantiating a UIA NVDAObject with a UIAElement containing prefetched UI Automation property values via a UI Automation CacheRequest, available for the remainder of the core cycle the NVDAObject was instantiated in. This is useful when instantiating an NVDAObject purely to generate a control field, where the caller knows what properties are needed.

All code within UIA NVDAObject and its subclasses should use the new _getUIACacheablePropertyValue method to fetch UI Automation properties as this will check the cache first.

Make use of the new Win 10 RS2 bulk-fetch UIA APIs available on IUIAutomationTextRange3 such as getChildrenBuildCache, getEnclosingElementBuildCache and getAttributeValues. When not available, compatibility code falls back to the older APIs, providing the same cached results, but over multiple calls.

 UIATextInfo._getTextWithFieldsForUIARange and related content fetching code has been updated to use cacheRequests where possible. E.g. getChildrenBuildCache, getEnclosingElementBuildCache etc.


          Edge: split out pre gap removal code into its own TextInfo class and …

…use it in Windows builds less 15048. Windows builds above this now can use the generic UI Automation textWithFields code. Along with the added caching, these changes provide up to a 3x speed-up in reading lines in Microsoft Edge.


          Manually include precompiled UI Automation com interfaces.

f66ce5d

michaelDCurran requested a review from jcsteh

March 21, 2017 03:42

jcsteh approved these changes

View reviewed changes

source/NVDAObjects/UIA/__init__.py Outdated

               		@type alwaysWalkAncestors: bool
               		@param recurseChildren: If true, this function will be recursively called for each child of the given text range, clipped to the bounds of this text range. Formatted text between the children will also be yielded. If false, only formatted text will be yielded.
               		@type recurseChildren: bool
-              		@param _rootElementRange: Optimization argument: the actual text range for the root element, as it is usually already known when making recursive calls.
-              		@type rootElementRange: L{UIAHandler.IUIAutomationTextRange}
+              		@param _rootElementClipped: Indicates of textRange represents all of the given rootElement, or is clipped at the start or end.

Contributor

jcsteh Mar 21, 2017

"Indicates of"? Do you mean "Indicates if"?

source/NVDAObjects/UIA/__init__.py Outdated

@@ @@ -557,6 +638,46 @@ def updateSelection(self): @@
               class UIA(Window):
+              	def _get__coreCycleUIAPropertyCacheElementCache(self):
+              		"""A dictionary per core cycle that is ready to map UIA property IDs to UIAElements with that property already cached."""

Contributor

jcsteh Mar 21, 2017

Can you add a brief explanation of why there might be multiple cache elements; e.g. the initial cache for ControlFields and the cache used to fetch states efficiently?

source/NVDAObjects/UIA/__init__.py Outdated

+              	def _getUIACacheablePropertyValue(self,ID,ignoreDefault=False,onlyCached=False):
+              		"""
+              		Fetches the value for a UI Automation property from an element cache available in this core cycle. If not cached and L{onlyCache} is False then a new value will be fetched.

Contributor

jcsteh Mar 21, 2017

typo: onlyCache should be onlyCached.

source/NVDAObjects/UIA/__init__.py Outdated

+              		Fetches the value for a UI Automation property from an element cache available in this core cycle. If not cached and L{onlyCache} is False then a new value will be fetched.
+              		"""
+              		elementCache=self._coreCycleUIAPropertyCacheElementCache
+              		# If we have a UIAElement who's own cache contains the property, fetch the value from there

Contributor

jcsteh Mar 21, 2017

who's -> whose

source/NVDAObjects/UIA/__init__.py Outdated

+              		else:
+              			raise ValueError("UIA property value not cached")
+              		# cache and return the value
+              		#valueCache[key]=value

Contributor

jcsteh Mar 21, 2017

Looks like you had a value cache at some point. This commented line needs to be removed and the comment above it adjusted.

source/NVDAObjects/UIA/__init__.py Outdated

+              		acceleratorKey = self._getUIACacheablePropertyValue(UIAHandler.UIA_AcceleratorKeyPropertyId)
+              		# Same case as access key.
+              		if acceleratorKey:
+              			shortcuts.append(acceleratorKey)
               			pass

Contributor

jcsteh Mar 21, 2017

Not necessary any more.

source/NVDAObjects/UIA/wordDocument.py Outdated

@@ @@ -66,7 +69,7 @@ def _get_isCollapsed(self): @@
               		# Therefore class a range as collapsed if it has no text
               		return not bool(self.text)
-              	def getTextWithFields(self,formatConfig=None):
+              	def old_getTextWithFields(self,formatConfig=None):

Contributor

jcsteh Mar 21, 2017

Is this no longer needed? If not, it should be removed.

source/_UIAHandler.py Outdated

		@@ -18,6 +18,7 @@

		from comtypes.gen.UIAutomationClient import *

Contributor

jcsteh Mar 21, 2017

Extraneous blank line.

source/UIAUtils.py

@@ @@ -119,3 +120,71 @@ def iterUIARangeByUnit(rangeObj,unit): @@
               	if tempRange.CompareEndpoints(UIAHandler.TextPatternRangeEndpoint_End,rangeObj,UIAHandler.TextPatternRangeEndpoint_End)<0:
               		tempRange.MoveEndpointByRange(UIAHandler.TextPatternRangeEndpoint_End,rangeObj,UIAHandler.TextPatternRangeEndpoint_End)
               		yield tempRange.clone()
+              def getEnclosingElementWithCacheFromUIATextRange(textRange,cacheRequest):

Contributor

jcsteh Mar 21, 2017

I think a docstring would be good here. Only has to point out that it uses getEnclosingElementBuildCache or falls back to getting the element and then building the cache if getEnclosingElementBuildCache is not supported.

source/UIAUtils.py

+              			e=e.buildUpdatedCache(self._cacheRequest)
+              		return e
+              def getChildrenWithCacheFromUIATextRange(textRange,cacheRequest):

Contributor

jcsteh Mar 21, 2017

Docstring as above.

michaelDCurran added 2 commits

March 22, 2017 06:56


          UIA caching: make sure to cache UIA_IsEnabledPropertyId for control f…

f55b431

…ields.


          Address review actions.

bb400d0

michaelDCurran added a commit that referenced this pull request


          Incubates #6994

3c28c06

nvaccessAuto added the incubating label

nvaccessAuto assigned michaelDCurran

Collaborator

josephsl commented Mar 22, 2017

Hi,

Possibly related to this PR: when attempting to retrieve obj.location in places such as Skype Preview's Download file button (when someone sends a file) and reported by folks using Audio Themes 3D add-on, obj.location traceback says:

Traceback (most recent call last):
File "", line 1, in
File "baseObject.pyc", line 34, in get
File "baseObject.pyc", line 110, in getPropertyViaCache
File "NVDAObjects\UIA_init.pyc", line 1147, in _get_location
AttributeError: 'tuple' object has no attribute 'left'

Thanks.

michaelDCurran added a commit that referenced this pull request


          Incubates #6994

e4317aa

Member Author

michaelDCurran commented Mar 22, 2017 via email

Thanks for reporting. Fixed now in 7c61bef. This was also breaking the Focus highlight add-on. Mick

On 23/03/2017 5:10 AM, Joseph Lee wrote: Hi, Possibly related to this PR: when attempting to retrieve obj.location in places such as Skype Preview's Download file button (when someone sends a file) and reported by folks using Audio Themes 3D add-on, obj.location traceback says: Traceback (most recent call last): File "", line 1, in File "baseObject.pyc", line 34, in *get* File "baseObject.pyc", line 110, in /getPropertyViaCache File "NVDAObjects\UIA_/init//.pyc", line 1147, in _get_location AttributeError: 'tuple' object has no attribute 'left' Thanks. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#6994 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ANf5nXcMRYXPLR4G9a1IvzOivOSAYmqbks5roXIIgaJpZM4MjS-S>.

-- -- Michael Curran Executive Director, NV Access Limited PH: +61 7 3149 3306 www: http://www.nvaccess.org/ Twitter: @nvaccess Facebook: http://www.facebook.com/NVAccess


          UIA: fetching boundingRectangle via getCurrentPropertyValueEx rather …

46cd6e6

…than currentBoundingRectangle returns a tuple of left,top,width,height, rather than a specific rectangle object with members of left,top,right and bottom.

michaelDCurran force-pushed the edgeSpeedup branch from 7c61bef to 46cd6e6 Compare

March 22, 2017 22:09

michaelDCurran added a commit that referenced this pull request


          Incubates #6994

8cd1222

Member Author

michaelDCurran commented Mar 22, 2017 via email

Actually, its commit 46cd6e6. There was a typo in the last commit message.

On 23/03/2017 5:10 AM, Joseph Lee wrote: Hi, Possibly related to this PR: when attempting to retrieve obj.location in places such as Skype Preview's Download file button (when someone sends a file) and reported by folks using Audio Themes 3D add-on, obj.location traceback says: Traceback (most recent call last): File "", line 1, in File "baseObject.pyc", line 34, in *get* File "baseObject.pyc", line 110, in /getPropertyViaCache File "NVDAObjects\UIA_/init//.pyc", line 1147, in _get_location AttributeError: 'tuple' object has no attribute 'left' Thanks. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#6994 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ANf5nXcMRYXPLR4G9a1IvzOivOSAYmqbks5roXIIgaJpZM4MjS-S>.

-- -- Michael Curran Executive Director, NV Access Limited PH: +61 7 3149 3306 www: http://www.nvaccess.org/ Twitter: @nvaccess Facebook: http://www.facebook.com/NVAccess

Contributor

jcsteh commented Mar 23, 2017

As reported in this nvda-devel thread, this breaks if UIA is disabled. You can easily test this by setting config.conf["UIA"]["enabled"] = False, saving config and restarting.

Brian1Gaff commented Mar 23, 2017 via email

It certainly does, like on xp for example grin. Brian bglists@blueyonder.co.uk Sent via blueyonder. Please address personal email to:- briang1@blueyonder.co.uk, putting 'Brian Gaff' in the display name field. ----- Original Message ----- From: "James Teh" <notifications@github.com> To: "nvaccess/nvda" <nvda@noreply.github.com> Cc: "Subscribed" <subscribed@noreply.github.com> Sent: Thursday, March 23, 2017 11:13 AM Subject: Re: [nvaccess/nvda] Navigating by line in Microsoft Edge is now up to 3x faster in the Windows 10 Creaters Update (#6994)

As reported in [this nvda-devel thread](http://nabble.nvda-project.org/Last-two-snaps-will-not-install-on-windows-XP-tp41750.html), this breaks if UIA is disabled. You can easily test this by setting `config.conf["UIA"]["enabled"] = False`, saving config and restarting. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: #6994 (comment)

michaelDCurran added 2 commits

March 24, 2017 09:13


          UIA NVDAObject: location property must return int coordinates, not fl…

996ddb5

…oat.


          UIA NVDAObject: don't use any UIA constants at class level unless UIA…

41c1473

… support is enabled.

michaelDCurran requested a review from jcsteh

March 23, 2017 23:39

michaelDCurran mentioned this pull request

performance issues in edge #6973

Closed

michaelDCurran added a commit that referenced this pull request


          Incubates #6994

38d8ee7

jcsteh mentioned this pull request

Nvda does not read the suggested options in the start menu in windows 7 #7013

Closed


          UI Automation client library cannot support properties added in a fut…

a30c217

…ure version of Windows, either in cache requests, or provide defaults for them. Therefore: catch COMError when trhing to add properties to a controlFieldCacheRequest (E.g. sizeInSet on Windows 7), and also reintroduce try blocks around some of the property fetches in UIA NVDAObject's states property, much resembling what it used to be, though still using _getUIACacheablePropertyValue.

Fixes #7013

Member Author

michaelDCurran commented Mar 27, 2017

@jcsteh: another review please. Fixes #7013

jcsteh reviewed

View reviewed changes

Contributor

jcsteh left a comment

I follow the motivation behind a30c217, but I'm a bit confused about some of the changes. Why remove onlyCached from all of these property queries? Doesn't that mean we will try to make queries for them all? I guess those queries will fail at the client library before they even get executed? If so, is there any case where onlyCached is useful any more? If not, perhaps it should be removed to avoid confusion.

jcsteh approved these changes

View reviewed changes

Contributor

jcsteh left a comment

As discussed via voice, approving with the removal of the onlyCached argument to _getUIACacheablePropertyValue.


          UIA NVDAObject's _getUIACacheablePropertyValue: remove the 'onlyCache…

0357e92

…d' parameter as it is really not needed.

michaelDCurran added a commit that referenced this pull request


          Incubates #6994

b9c7808

jcsteh mentioned this pull request

'Can not edit cells in excel. #7015

Closed

michaelDCurran merged commit c1799ee into master

nvaccessAuto removed the incubating label

nvaccessAuto added this to the 2017.2 milestone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment