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.

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

Performance / responsiveness to inputs #32

Open
cengels opened this issue Jul 5, 2020 · 3 comments
Open

Performance / responsiveness to inputs #32

cengels opened this issue Jul 5, 2020 · 3 comments
Assignees
@cengels
Labels
bug Something isn't working text area The issue concerns the text area
Milestone
Core

Comments

@cengels
Copy link
Owner

cengels commented Jul 5, 2020

This issue is a catch-all issue to diagnose and fix all and any problems related to the performance and/or the general input responsiveness in Skywriter.
@cengels cengels added bug Something isn't working text area The issue concerns the text area labels Jul 5, 2020
@cengels cengels added this to the Core milestone Jul 5, 2020
@cengels cengels self-assigned this Jul 5, 2020
@cengels
Copy link
Owner Author

cengels commented Jul 11, 2020

The search suffers from massive performance problems. In other applications, even in large files (1 MB+), something as small as 1 character search results return instantly. Conclusion: there must be a better way to implement a search that is considerably more performant.

It must be diagnosed whether the problem is the actual search itself or the visual highlighting of the search results.

cengels added a commit that referenced this issue Jul 12, 2020
@cengels
Fix incorrect startSegment/endSegment (#32)
ed2981b 
The startSegment/endSegment was not correctly calculated. Due to
logic errors in the conditionals, startSegment was always 0, which
caused the entire document structure to be refreshed on every deletion,
including simple backspacing.
@cengels
Copy link
Owner Author

cengels commented Jul 12, 2020 •
edited
Loading

The cause of the search performance issues is indeed the QSyntaxHighlighter, or more specifically TextHighlighter::refresh(). Normally the QSyntaxHighlighter automatically calls QSyntaxHighlighter::highlightBlock() whenever a text block changes, but this means that external changes (such as entering a new search term) do not trigger a rehighlight. To remedy this, manually calling rehighlight() (or, in TextHighlighter called refresh()) is necessary.

It doesn't really make sense to me why the rehighlighting takes so long. We're applying quite a lot of formats, yes, but none of the search term highlighting formats modify the text layout in any way. Thus, this should not trigger a relayout, which is the only potential cause I can see for such a performance impact.

An alternative solution might be to move the search term highlighting code to updatePaintNode, but that seems a little hacky and I'd like to avoid that if possible.

Unfortunately, simply making the QSyntaxHighlighter asynchronous is (probably) not an option since it operates directly on the UI thread, where it must stay.

@cengels cengels mentioned this issue Aug 7, 2020
Closed
cengels added a commit that referenced this issue Dec 19, 2020
@cengels
Add new paint node-based search highlighter (#32)
2aec0e9 
The new highlighter no longer inherits from QSyntaxHighlighter and
therefore no longer changes the QTextDocument's internal formats when
search matches are set or reset. Instead it is embedded directly in the
painting process of FormattableTextArea::updatePaintNode(), which means
that corresponding rectangle highlighting nodes are only added if they
are actually on-screen, which results in an enormous performance boost,
especially for search inputs with a lot of matches.
@cengels
Copy link
Owner Author

cengels commented Dec 19, 2020

All performance issues related to the search (see above) are now resolved.

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

@cengels cengels

Labels
bug Something isn't working text area The issue concerns the text area
Projects
None yet
Milestone
Core
Development

No branches or pull requests
1 participant
@cengels