Closed dados closed 2 years ago
VS code has a limit of 10000 characters per line.
This is a duplicate of #166.
A fix was implemented that would automatically switch WordWrap on when line length exceeds 10000 (what VS Code does). It was then removed. Could you please try the performance with WordWrap on? Also try with the erWrapwithRightEdge on.
Having WordWrap makes a huge difference when scrolling through that single wrapped line. Editing that line is still very slow but about 3.5 - 4x faster with WordWrap on
Funny having a 5 or 10Mb. single line with WordWrap ON and going to the end of the line (with wordwrap it's the bottom) and adding the second line and start typing. Editing that second line works just fine, but if I push arrow up and go to the first line (long line) and try to edit that line every thing slows down. I would say it takes about 1,8 seconds (without WordWrap it takes 7 seconds) after I type a single letter for it to appear with the 5Mb. line and about 3 seconds with 10mb line. And typing multiple characters at once just multiplies the waiting time by count of chars.
Having erWrapwithRightEdge ON did nothing for the performance except when resizing a form with synedit aligned to client (did not have to change the text display width when resizing)
I think that the work on the latest SynEdit version is wonderful and I appreciate the work....But I have a program that I've been developing and using for about 15 years now and I've compiled it with the new SynEdit version and it makes that program so frustrating to use as my main tool at work, just because of this lag with large lines. Just a 12k line is almost "unusable" to work with (no lag with the old SynEdit version on a 12k line)
I have not looked at the SynEdit code and I do not understand the reason behind the lag. But It would be wonderful if this could be optimized somehow without having to use WordWrap (it still lags with WordWrap) Visual Studio, ConTEXT and Notepad++ are able to display and edit a 12K line without any lag
These tests where done by compiling a release and run without debugger in 64bit. (D11, Win10, Dell Precision i7)
The old SynEdit made the assumption that every unicode codepoint (WIdeChar) corresponds to a single displayed character (grapheme) and has a fixed width. This is why it was fast. If you look at the issues described in #26 you see why this is problematic. The changes were made so that SynEdit handles unicode correctly. The downside is processing speed.
However, SynEdit is primarily used as a code editor. Editing these 5Mb lines, although it may have been possible before, it was not within the design objectives of code editor. The most popular code editor on the planet, VS Code, only displays lines 10K long. The current version can handle 100,000s of lines of standard code very well. Dealing with these super long lines is not my priority, but I welcome anyone that comes up with optimizations that improve the handling of those cases. And of course you can always use older versions of SynEdit or other forks that suit your needs better.
Thanks 👍
I agree that this would be a low priority thing, but theoretically (and with limited knowledge of the challenges involved):
The speed issues all come down to performing actions on a text layout of an entire line when the line is very long. The solution would be to work on a portion or "window" of the current line. I've done the very simple test of using the initial portion of the current line instead of the entire thing. Of course, my simple sample falls apart as soon as we scroll right. In that case the layout "window" would have to shift as well along with knowing the exact pixel offset of the beginning of the window and determining the part of the line for the partial layout. I'm not sure how that would work when combined with highlighting, though I'd think that highlighting could be turned off on such a wide file automatically.
Which, I'm guessing, is a lot of work, though the overall payoff could be impressive. I also think that this discussion shouldn't minimize how well the control works now with all reasonable coding files (which is its purpose after all.) The "wide file" issue is almost exclusively a "log" or "data" file issue, not a coding file issue.
Just wait a bit... A fix is coming. @MShark67 Could you please post here some files with very long lines in which the current version is struggling for testing?
Here are some sample files I use for testing. These are, in no way, reasonable real-world files lol. It's a simple 100 char select statement that highlights nicely, Repeated 20K chars per line, 50K chars per line, and 100K chars per line (the 100K one is good for non-highlight testing.) 10 lines per file.
@dados @MShark67 Have not finished yet but do try the latest version.
I was going to implement Mark's suggestion above, but then I came across this: https://en.delphipraxis.net/topic/6360-string-optimization/.
Now it should be quite fast even with Syntax highlighting.
There is still scope for further optimizations, but the performance is good even with the 100K lines and syntax highlighting. Please test that I have not broken anything.
Wow! Initial tests are impressive. I'll test more tomorrow. Well done!
I can confirm that there is a remarkable improvement in performance from these changes on my ridiculously long and wide file with highlighting and without. Thanks for this.
I think I've found a different issue with further testing. If I go to the area of the file where it's wide and scroll to the right one column at a time using the scrollbar button I get the below-looking editor (the lines are not drawn on the right side of the editor) but if I scroll by pages horizontally (clicking to the left of the right scroll button) this doesn't happen.
@DGH2112 I haven't been able to duplicate your issue here yet. Does turning off theming or highlighting change the behavior?
I've just tried without theming and got the same result. I've tried to find a rationale behind it but cannot. Some not so long lines do it and some extremely long lines do it. I thought it was lines greater than 255 chars but that doesn't seem to support what I'm seeing.
I'm now also seeing issues with cursor position when CTRL+LEFT and CTRL+RIGHT. The cursor should be at the start of words but isn't. Also, the selection is not aligned with the cursor when this happens. Now my data is TAB delimited, could this be causing something to not be calculated correctly?
Okay, I think I've found the fundamental issue. I replaced all the TAB characters with 2 spaces (took a while) and the scrolling/paint issue disappeared. I do not know where in the code this will be by it must have something to do with calculating the width of a TAB character.
Fixed. Please try again.
@MShark67 While looking at this I found an issue with horizontal scrolling and esScrollPastEOL.
It is kind of hard to provide instructions on how to replicate. I could only replicate it with long lines.
Still have not found a way to resolve it.
I'll take a look! I notice that the caret positioning is now amazingly fast for wide lines, however any painting that happens out there is slower. I see the Layout is using lastchar to limit the string, but not firstchar. That may be completely as intended, but everything else is so fast now that it stands out. My timings for my 100K file but increasing the linecount to fill the screen is:
Paint: 22ms LeftChar: 631 <-- Near the left side super fast Paint: 1613ms LeftChar: 99866 <-- Near the end (not in debug mode, no highlighting, no hard tabs.)
I almost hate to bring it up, the improvements are stellar, but I like to be thorough. Fantastic work!
Fixed. Please try again.
@pyscripter Yes this seems to have done the trick, thank you.
@pyscripter I haven't been able to cause it to happen here, but it sounds like it gets into a bit of a feedback loop. My thought is to not call UpdateScrollBars if IsScrolling is true (I assume from your steps above that this happens while IsScrolling is true.)
I notice that the caret positioning is now amazingly fast for wide lines, however any painting that happens out there is slower. I see the Layout is using lastchar to limit the string, but not firstchar. That may be completely as intended,
I indent to implement that, but it will take quite a few tricky changes, so I decided to leave it for another day.
I assume from your steps above that this happens while IsScrolling is true
Yes. SetLeftChar - UpdateScrollBars -WMHSCROLL-SetLeftChar It never receives SB_ENDSCROLL
I first got this with your 20K file, but it happens with shorter lines. Just having one line is enough.
https://docs.microsoft.com/en-us/windows/win32/controls/wm-vscroll states that
If an application scrolls the content of the window, it must also reset the position of the scroll box by using the SetScrollPos function.
We do not seem to be calling SetScrollPos. Instead SetScrollInfo is used.
[update] sorry, more testing needed on my part. I'll report back
I looked at how Vcl implements Scrolling say in TCategoryButtons TCategoryButtons.Resize calls SetScrollInfo and WMHScroll/WMVScroll call ScrollPosChanged which calls SetScrollPos
The problem we have is that horizontal if eoScrollPastEOL is True also changes the scrollbar range!
Agreed! My current fix is to use SIF_POS instead of SIF_ALL if IsScrolling.
Agreed! My current fix is to use SIF_POS instead of SIF_ALL if IsScrolling.
Do you have some code I can try?
Here's my quick fix:
Here's my quick fix:
Still the same infinite loop...
Darn it! I really thought that might be it. I wish I could cause it, maybe it's a different setting I'm using. I'll keep at it.
@MShark67 I have committed a fix, based on your idea. Please have a look and test. Note the calls to RedrawWindow. For some reason I had to invalidate the non-client area for the scrollbars to paint correctly after scrolling. This maybe relevant to the discussion about UpdateWindow (maybe not).
Awesome! I've been going at it from a different perspective. I noticed that our "leftchar" based updating scrollinfo values didn't match our "thumbtrack" based values (so the set value would be range adjusted during the feedback loop.). I'm not sure if that would lead to the infinite loop issue, but it seemed like a problem to me. I managed to get that "fixed", though I don't really know if it was an issue in the first place. I'll take a look at your changes and see if it makes sense to merge my changes into it.
Fix looks good! I think this is the way to go. Interestingly enough, if I comment out the "if isscrolling" check I can recreate the issue that I haven't been able to see so far! That makes me feel better about the fix technique... well done. I'll have some very minor adjustments that fix the range fix issue I mentioned (which is unrelated to the infinite loop issue, but I think good to fix.) Currently we scale all the values based on charwidth, but we could also just go back to doing it "per char" which makes the code a bit cleaner I think. Let me know if you'd like me to do that as well (I've already done it while I was trying to verify other issues.) Also, it looks like the line ends got messed up on the commit.
Just committed an improvement that gets rid of the RedrawWindow stuff (the need to use them bothered me), by calling SetScrollPos.
Why don't you submit one or more PRs. whichever is convenient.
Keeping track of the scrolling in terms of "notional" chars may indeed simplify the code. I am happy to go back. But we should always keep in mind that chars don't have the same width, and many WideChars can be displayed as a single grapheme. But since horizontal scrolling is based on LeftChar then it makes sense to base the scrolling on that as well.
My understanding is you aren't supposed to use setscrollpos anymore. The docs say: Note The SetScrollPos function is provided for backward compatibility. New applications should use the SetScrollInfo function. Do you mind if I change that to use SetScrollInfo with sif_pos instead? It will have the same affect, basically we're just bypassing the LeftChar -> setpos with wrong value issue by setting it directly (along with the redraw flag.)
No I don't mind but where does it say so? https://docs.microsoft.com/en-us/windows/win32/controls/wm-vscroll for example says that SetScrollPos should be called.
I see. Good go ahead. It takes a bit more code, but it is more uptodate.
@dados @MShark67 Completed the optimizations for long lines. Please test again for performance but also for errors I may have inadvertently introduced.
The only case which is not optimized is when you are viewing the far right and the displayed text includes hard tabs. If you want fast editing of say tab delimited files with very long lines, you can still get speed by setting the tabwidth to 1.
Thank you so much 👍 I'll test and post my results
Home/End Caret movement is great, mouse caret placements are great, editing is great also. But there is something wrong with Arrow Left/Right commands on long lines. Also when entering a long line from another using arrow up/down (except when CaretX = 1) same delay Ps. I'm using a 10mb xml line to exaggerate the delay (witch is about 4 seconds)
I'm seeing amazing performance on my test files. I haven't noticed any problems so far. @dados, are you only seeing performance issues or are you seeing behavior or painting issues? Are you trying outside of debugging and with highlighers turned off?
Yes I agree, the performance has improved very much. I'm trying outside debugging without highlighters. Here is my test program SynEditBenchTest.zip Start the program and load data via the button and try to move around with arrow key's and see if you experience the delay as well on the long line
@dados I can reproduce the delay. However this file has 11 million characters in one line.
Yes, like I said I used an extra large line to exaggerate any delay. Everything is extremely fast on the huge line: editing, home/end, cursor replacement with mouse And then there is this huge difference when using the arrow keys
The delay is due to ValidTextPosition, which has not been optimized. I' ll try to improve on this,
ValidTextPos optimized. Please try again. Working with syntax highlighting should also be fast.
That did it, super well done :) 👍
Outperforms the old version by a mile. I got tired waiting on the old version to paint the 10Mb xml line with formatting and terminated it after a few minutes :)
@pyscripter, Amazing work! Bravo!
Tested with single line files of various sizes.
Sizes 4kb, 8kb and 16kb where fine.
Tested also with 512Kb, 1Mb, 5Mb and 10Mb. And from 512Kb the single line file the performance degraded very much.
For example If I loaded the 5Mb single line file and pushed the END button eventually the pointer and horizontal scrollbar updated but the text at the end of the line was not painted (no text was painted when cursor was at the end of the line)