Skip to content
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.

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

Redundant braille.BrailleHandler.update executions #16456

Closed
burmancomp opened this issue Apr 27, 2024 · 5 comments · Fixed by #16463 or #16550
Closed

Redundant braille.BrailleHandler.update executions #16456

burmancomp opened this issue Apr 27, 2024 · 5 comments · Fixed by #16463 or #16550
Labels
component/braille p4 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority performance triaged Has been triaged, issue is waiting for implementation.

Comments

@burmancomp
Copy link
Contributor

About

There are following code lines in braille.BrailleHandler._handlePendingUpdate:

if scrollTo is not None: self.scrollToCursorOrSelection(scrollTo) if self.buffer is self.mainBuffer: self.update()

scrollToCursorOrSelection should execute scrollTo when region is type of TextInfoRegion. scrollTo in turn executes updateDisplay which executes braille.BrailleHandler.update.

Because region is type of TextInfoRegion and thus buffer should be mainBuffer, braille.BrailleHandler.update is executed twice.

There is similar case in braille.BrailleHandler._doNewObject function:

self.scrollToCursorOrSelection(region) if self.buffer is self.mainBuffer: self.update()

What is the harm? At least log file, when using debug level, contain redundant ""Braille window dots" lines.

Steps to reproduce:

Log level to debug and investigation of log when there is braille display in use (I have not looked at log when braille display is no display).

Actual behavior:

redundant log lines

Expected behavior:

no redundant log lines

NVDA logs, crash dumps and other attachments:

System configuration

NVDA installed/portable/running from source:

running from source

NVDA version:

current main branch code

Windows version:

windows 10 22h2

Name and version of other software in use when reproducing the issue:

Other information about your system:

Other questions

Does the issue still occur after restarting your computer?

yes

Have you tried any other versions of NVDA? If so, please report their behaviors.

If NVDA add-ons are disabled, is your problem still occurring?

Does the issue still occur after you run the COM Registration Fixing Tool in NVDA's tools menu?

@burmancomp
Copy link
Contributor Author

Fixing this might help to some extend as to issue #16356.

@seanbudd seanbudd added component/braille performance p4 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority triaged Has been triaged, issue is waiting for implementation. labels Apr 29, 2024
seanbudd pushed a commit that referenced this issue May 7, 2024
fixes #16456

Summary of the issue:
In braille.BrailleHandler._handlePendingUpdate and braille.BrailleHandler._doNewObject functions scrollTocursororSelection function is called. scrollToCursorOrSelection calls scrollTo which in turns calls braille.BrailleHandler.update. Both _handlePendingUpdate and _doNewObject call braille.BrailleHandler.update after calling scrollToCursorOrSelection.

Description of user facing changes
There should be less "Braille window dots" log lines when log level is debug. Maybe some minor effect on performance.

Description of development approach
Minor modifications of _handlePendingUpdate and _doNewObject functions.
@nvaccessAuto nvaccessAuto added this to the 2024.3 milestone May 7, 2024
@seanbudd seanbudd reopened this May 10, 2024
@seanbudd
Copy link
Member

seanbudd commented May 10, 2024

Re-opening as #16463 and #16501 are reverted

@burmancomp
Copy link
Contributor Author

I somewhat feel that removing updateDisplay execution from scrollTo could resolve this type of double updates.

May I try pr with this approach?

@seanbudd
Copy link
Member

I'd encourage getting some try build testing before marking it as ready for review, but please do attempt improvements if they are clear improvements

@burmancomp
Copy link
Contributor Author

Maybe less log lines when debugging is major advantage of this.

I am sorry about testing. In this case I did not unfortunately take into account at least braille messages maybe because I have them disabled so I did not think them (maybe not at all) but it is not acceptable reason).

seanbudd pushed a commit that referenced this issue May 10, 2024
Reverts #16463 and #16501

Issues fixed
Issues reopened
Reopens #16456

Reason for revert
complexity

Can this PR be reimplemented? If so, what is required for the next attempt
at least execution of scrollToCursorOrSelection in _doNewObject
@seanbudd seanbudd removed this from the 2024.3 milestone May 10, 2024
XLTechie pushed a commit to XLTechie/xlnvda that referenced this issue May 10, 2024
…6463)

fixes nvaccess#16456

Summary of the issue:
In braille.BrailleHandler._handlePendingUpdate and braille.BrailleHandler._doNewObject functions scrollTocursororSelection function is called. scrollToCursorOrSelection calls scrollTo which in turns calls braille.BrailleHandler.update. Both _handlePendingUpdate and _doNewObject call braille.BrailleHandler.update after calling scrollToCursorOrSelection.

Description of user facing changes
There should be less "Braille window dots" log lines when log level is debug. Maybe some minor effect on performance.

Description of development approach
Minor modifications of _handlePendingUpdate and _doNewObject functions.
XLTechie pushed a commit to XLTechie/xlnvda that referenced this issue May 10, 2024
Reverts nvaccess#16463 and nvaccess#16501

Issues fixed
Issues reopened
Reopens nvaccess#16456

Reason for revert
complexity

Can this PR be reimplemented? If so, what is required for the next attempt
at least execution of scrollToCursorOrSelection in _doNewObject
seanbudd pushed a commit that referenced this issue May 21, 2024
closes #16456

Summary of the issue:
There are often two BrailleHandler.update executions which produce duplicate log entries when log level is debug. As an example, when navigating with up/down arrows in notepad debug level log shows this.

There should not be good reason for two executions.

Description of user facing changes
Less duplicate "Braille window dots" log lines on debug level. This should facilitate debugging a little.

Description of development approach
removed updateDisplay call from BrailleBuffer.scrollTo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/braille p4 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority performance triaged Has been triaged, issue is waiting for implementation.
Projects
None yet
3 participants