chore(textedit): add fastwrap - #615
Conversation
|
It works, but after when Quit Nicotine+ always crash IndexError exception... I think this happens because I have |
|
I added a quick fix, anyway, I am still reviewing the PR. |
|
No even after applying fix 0726665 the crash happen after exit because of It looks like it could also happen any time |
|
I improved thread safety, still I am reviewing the code because I think I made the fastwrap a bit overcomplicated. |
| # Lower bound before any sample is collected. | ||
| return processedRows + rem | ||
| avgRowsPerLine = processedRows / self._processedLines | ||
| return processedRows + max(rem, int(rem * avgRowsPerLine)) |
There was a problem hiding this comment.
The returned number needs to be exactly accurate, or there needs to be another function that can force a full wrap of the entire document in order to obtain the actual total number of wrapped lines.
This accuracy is required to implement a "follow" feature (like the TTkLogViewer has), where it auto-scrolls whenever a new line is appended. If it is off-by-one or more then the auto-following stops when it shouldn't.
There was a problem hiding this comment.
fastwrap cannot be accurate because it works in chunks and the full size is estimated,
I guess you want to display a growing log dynamically with wrap.
I can add a full wrap api, consider that if the size change the wrap must be recalculated.
What is your usecase?
I think that different wrapping routine can be implemented based on the use cases.
There was a problem hiding this comment.
My use case is a chat interface, so new messages are appended to the bottom and have to be shown immediately when they arrive (i.e. "follow" mode, except when scrolling up to view the old messages in the chat history).
For this purpose I made a compound widget that is basically a TTkTextEdit and a TTkLineEdit together...
https://github.com/slook/nicotine-plus/blob/ttk/pynicotine/ttktui/widgets/chatter.py
git clone https://github.com/slook/nicotine-plus.git -b ttk
cd nicotine-plus
./nicotine --tui
This same Chatter() compound widget class is used for the "Chat Rooms" as well as "Private Chats". It is a read-only use case so I consider changing the implementation to something more like the LogViewer console widget.
There was a problem hiding this comment.
Sorry for the late reply,
Those days I am more lazy than busy.
I realised that I don't have a proper scrolling api.
what if, I allow to mark tags, or define blocks and include some api to
- scroll to line
- scroll to tag
- scroll to paragraph
- scroll to block
Allowing the scroll to align to Top,Center,Bottom
So, each message can be a single block and you can scroll the page to have the required message at the bottom aligned to its size.
But, we need to think about infinite scrolling as well, I think eventually you don't want to load the full chat history in the memory.
There was a problem hiding this comment.
There is no need for any apology on your part, since my feature request is primarily intended to be for your information only from my perspective in a real world use case, and I only offer my findings because I know that rendering performance is something that you are interested in relevant for your project, but not due to any sort of urgent need on my part. I was originally expecting that I was going to need to write my own widget entirely from scratch (but I probably wouldn't have much motivation to do it just for the sake of my own project unless others could benefit from such effort). Thank you for opening this PR.
Marking tags is something that I do want to do (to have username colors per online status and hotspots for each user context menu), yet I did not think that wrapping would become relevant for tagging as well as scrolling. I suppose tackling this at the same time could indeed be helpful way to assist the wanted scrolling position as well, without the need for worrying about the scroll offset having to be completely accurate.
As to the infinite scrolling idea, this is also something I would like to have too. Having previously attempted to do this in Gtk (by jerry-rigging its get_page_increment() method and its "edge-reached" signal), I learned that this is a complicated aspect to deal with.
In this particular case the old message data is not always saved into a file so everything has to remain in a buffer anyway, so that same buffer might also become applicable to the tags. Even when there is a log file, it is not practical to load all of it so there is a need for a maximum limit of lines to store in the memory and the capability to dynamically prune it somehow. At present, attempting to delete/offload lines from the beginning results in some kind of IndexError which is actually what led me to investigate this code path in the first place.
Even without adding custom tags, there is already at least a Start, Insertion and an End which could serve the essential purposes of being able to jump to those points, wherever they might happen to be located at any point in time.
Yes it could be simpler. It looks like you have tried to use estimates to optimize the initial loading and resizing, whereas the main areas of concern were freezing during navigation, editing and appending. A couple of seconds to make the proper calculations at startup isn't really a big deal. |
687703d to
165103c
Compare
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
|
Testing this new FastWrap engine in nicotine-plus/ttk that you have got it working pretty well as it is similar performance to VimWrap. This is what my auto follow function looks like with it now... ... so this is getting simpler to use than VimWrap was, while we don't have a follow mode in the library widget yet. I guess the Note I am not using |
|
I will add scrolling api in the text editor and also a follow mode. |
|
anyway, I was thinking to modify the vimwrap to allows it to snap at the bottom and maybe increase its wrapping size to allows for a smoother scrolling. |
|
Both of the engines work efficiently and perhaps the scrolling isn't expected to be smooth during the first navigation of the document until all the positions are known when all the integer values could be discovered and remembered by the engine forever so that after the second time then the view offset for the scroll bar handle becomes more precise and predictable. Maybe just as an idea it could be handy if there was a way to have a callable method like from the FullWrap engine that can calculate all of its positions on demand (or in a background thread?) for use when switching to one of the lazy engines, in cases where absolute precision is desired at the expense of waiting for a longer time opening the document. Edit: Actually, it seems like it already is working something like this so the current behaviour is satisfactory enough. The main negative breakage is when the scroll bar handle suddenly jumps to another place while it is being dragged, so it would be nice to avoid that if possible but really it isn't too bad, considering that it is not normally needed to navigate within pages of such a large document. In my use case the most important operations are go to the exact start and end locations, and then always follow the end when the scroll is at the bottom. |
|
Yes, the nav bar jumping is something I was thinking about, I noticed that other editors have the same issue (i.e. vscode) because they use some wrapping estimation as well, |
2 participants
No description provided.