Alexey-T / CudaText

Cross-platform text editor, written in Free Pascal
Mozilla Public License 2.0
2.55k stars 174 forks source link

"Highlight all" when max_lines limit is over - changed #5424

Closed Alexey-T closed 8 months ago

Alexey-T commented 8 months ago

@pintassilgo let's test this change (idea from you).

the 'lines gap' is 5 lines - count of lines over the visible area, to also highlight

beta: http://uvviewsoft.com/c/

pintassilgo commented 8 months ago

Thanks. It works for a while, but after scrolling a bit the lines at the bottom don't have highlight.

Example: I started find in line 80, with the visible area being up to line 99. For 3 scroll down movements, the matches that arise are highlighted, but starting from the 4th scroll movement there are matches without highlight.

Start: image

After two scrolls, most of the find highlight are gone (border highlight), I can see only the highlight made by Highlight_Occurrences (filled background highlight): image

Then, after the 4th scroll down, there are matches without highlight at the bottom: image

CudaText-addons commented 8 months ago

now pascal code uses const=5 lines as 'count of lines to highlite after screen bottom'. HiOccur plugin uses different value. maybe bigger count. so it is expected? both Cud code and HiOccur plugin don't update marks on scrolling.

pintassilgo commented 8 months ago

both Cud code and HiOccur plugin don't update marks on scrolling.

Well, I guess they should update, because it's supposed to highlight matches in the visible area, and the visible area changes when user scrolls.

CudaText-addons commented 8 months ago

yes, but handler of scrolling will take CPU and scrolling will be slower. I did not wait it...

pintassilgo commented 8 months ago

I guess they do update on scrolling, at least Highlight_Occurrences, because I can scroll very much and visible matches are always highlighted, except at the very top or the very bottom because as I said it's needed to fix the accuracy of what is the visible range (the fix is probably just expanding the range a bit).

Alexey-T commented 8 months ago

HiOccur plugin - will discuss it in separate issue.

I changed logic of HiAll in too big document (count of lines > MaxLines). now it makes very big 'gap' to the up and to the down. 'gap' is about MaxLines/2. this is visible in the code:

NLinesGap:= Max(0, (UiOps.FindHiAll_MaxLines-Ed.GetVisibleLines) div 2);
NLineTop:= Max(0, Ed.LineTop-NLinesGap);
NLineBottom:= Min(NLineCount-1, Ed.LineBottom+NLinesGap);
//then we find marks from NLineTop to NLineBottom

do you get the idea? idea is to AVOID using onScroll event at all. it is very bad to use onScroll, makes work slower. Cud is not fast in scrolling speed. slower - very bad.

beta http://uvviewsoft.com/c/

pintassilgo commented 8 months ago

Thanks.

Missing highlights at the edges, on resizing and on small scrolls: fixed.

But as you know, the new logic misses all the matches when you scroll big distance, even by simply clicking over minimap/scrollbar. I think this is bad and important. User expects to have highlights when he moves across the text.

Do you have any way to measure the impact of onScroll? Btw, is there an easy way to temporarily disable a plugin? For instance, I could want to disable HiOccur to see if I notice any improvement in performance while scrolling, supposing this plugin is the only one using onScroll.

Maybe there's no need to process the matches at every onScroll event. NLinesGap could be big enough so Cuda will only process new matches when current line is close or exceeds gap limit. Doing simple checks of current visible line versus current highlighted lines, without processing highlights on most onScroll events, seems cheap. I guess it's safe to say that because Cuda interface already do work onscroll, for instance to update the line numbers at the left.

Alexey-T commented 8 months ago

Btw, is there an easy way to temporarily disable a plugin?

this one: https://wiki.freepascal.org/CudaText#How_to_disable_plugins

pintassilgo commented 8 months ago

Thanks. I tested and couldn't notice relevant performance impact comparing Cuda with and without HiOccur, in both cases testing a big file off the limits with many highlights (when HiOccur was enabled) and doing very fast and sequential scrollings. It's the same file from the images in my first comment here, with 12k lines and 22MB.

So I couldn't see the big problem of onScroll. And HiAll don't need to process highlights often, as I suggested it currently highlights a big gap, so it only needs to process when visible portion is near the edges of the gap. It could also respect a timer to give a breather in case of user abuse, like by quickly dragging scrollbar thumb up and down to navigate across the entire document.

Alexey-T commented 8 months ago

implemented the new behaviour. I will post beta here soon.

Alexey-T commented 8 months ago

new beta: http://uvviewsoft.com/c/

1.211.1 (2024/03)

pintassilgo commented 8 months ago

Thanks! It's working good.