funap / npp-explorer-plugin

Explorer plugin for Notepad++ x64/x86
GNU General Public License v2.0
84 stars 10 forks source link

Idea about when building subtree #54

Open klaus101 opened 1 year ago

klaus101 commented 1 year ago

Is your feature request related to a problem? Please describe. This is my 3rd and actually last thought i'd like to share. What is it about? A bug or issue? Surely not. A feature request? Rather not. More an idea to check if it is feasable:

When building a tree structure with probably deep nesting, trying to create the desired target node, it happens that nodes are created. selected and expanded, the needed child nodes are created. selected and expanded and so on. This population process is literally visible in explorer plugin forks.

Describe the solution you'd like From other program contexts / languages i know that it is a well established method to suspend node building painting until the target node had been created. The keywords (pascal:) are BeginUpdate/EndUpdate, and behind this is SendMessage(treeviewhandle ,WM_SETREDRAW, false/true, 0)

Explorer tools written in C++ do use such method also. Rude logic something like:

But wouldn't it be worthy to try it out? (At the end not more than two SendMessages at the right place).

Additional context OS is Windows 10 x64, NPP is 8.5 x64 the portable version The funap explorer plugin is 1.8.2.18, the x64 version.

funap commented 1 year ago

hi @klaus101 Thank you very much for the useful idea.

As you mentioned, in general, stopping the redraw when sequentially adding items to the TreeView will improve performance.

However, the current design of this plug-in is a sub-threaded process that repeats 1-3.

  1. enumerate files and folders
  2. retrieving system icons
  3. adding items to the TreeView

From our investigation, it appears that the extraction of system icons is taking a long time. It is taking 5ms to 10ms to retrieve one icon.

If we stop drawing with the current design, the UI seems to freeze all the time when there are many files.

I thought that optimizing the icon extraction, and separating the TreeView operation from the sub-thread and stopping redrawing would improve performance.

I will try to improve it.

klaus101 commented 1 year ago

@funap, I appreciate your attention a lot! It's no high prio thing of course, more to share an idea; if that doesn't fit within the specific thread approach - ok, might happen, fair. I try to explain a bit why i had focused that. I made an explorer tool 12 years ago, Delphi 32-bit, purely hobby; it did grow over the time and is now ported to Lazarus 64-bit. Also i made an explorer plugin 32-bit/Delphi by interest (i won't spend more time in it, the recent version are miles better). From this context i know about the tremendous effect of the method described, and use it in each treeview population, like many others do. The target node is instantly appearing when starting the app, or when switching tabs.

5-10 ms per icon retrieval is really very hard. Icon retrieval (via SHGetFileInfoW) had not been a real bottleneck for me yet (although 32-bit/Delphi was a bit faster, as it made icons accessible via an index to the system imagelist objects, whereas 64-bit/Lazaraus exposed them via a handle -> icons needed to be extracted/copied and stored). Recently i evaluated another NPP explorer plugin fork (from oviradoi; github), which is very very good as well, and did the same proposal there. He also decided to improve icon retrieval (latest release). If you'd like to take a look, it's issue nbr. 38 there. I also posted a video from my old 32-bit plugin there..

In short: i'm a wondering a bit why the method described shouldn't help in CPP too. But if so, ok, don't worry. Maybe a typical case of: simply keep something in mind, and maybe in another context an idea comes up. Ah, i forgot: CPP: the explorer++, app ( with source): might be interesting, how they do that (build subtree, icon retrieval, and where exactly wm_setredraw is applied).

What i really really like in your plugin (and missed in other forks i saw): the auto-synch option "Auto-navigate to file". By that we can, like as in some file explorers, automatically switch to the belonging folder when switching a tab. Superb!!! Pretty cool!

Thanks for having had the chance to exchange a bit about this matter! And have a nice day!

PS: without further investigation, if I remember right, I hadn’t applied the start/stop redraw globally around the tree population (sorry!), but within the OnExpand callback for each needed sub-node to expand, before/after building the belonging subitems here. Maybe this would work against a freeze impression.

klaus101 commented 1 year ago

Just for interest i looked into the Explorer++ source - https://github.com/derceg/explorerplusplus Just the same - the SendMessages (false, true) are (in ShellTreeView.cpp) part of the function: HRESULT ShellTreeView::ExpandDirectory(HTREEITEM hParent) called by OnItemExpanding

klaus101 commented 1 year ago

Hm, i'm a bit sceptical now if the principle is applicable here. I try to explain as best as i can.

Let's say we have a treepath to build for "D:\T1\T2\T3". I do (and thought i could assume) that there is a outer controlling loop that processes the subfolders step by step, sequentially. Like: Create "D" and populate it. If treenode T1 can be found now, the node is expanded (during the expand: stop/start drawing) and populated ... finished for now. If node T2 can be found now, the node is expanded (during the expand: stop/start drawing) and populated ... finished. And so on. Sequentially; each subnode created is a closed action that is afterwards followed by the next.

When doing a really short look into the sources, i have no clear picture yet how the process the treepath is controlled. But i see, within ExplorerDialog::run_dlgProc there is a case TVN_ITEMEXPANDING where an event EID_EXPAND_ITEM is set. This event is processed within ExplorerDialog::NotifyEvent beyond “case EID_EXPAND_ITEM”. Here: `` UpdateChildren(pathName.c_str(), _hItemExpand); TreeView_Expand(_hTreeCtrl, _hItemExpand, TVE_EXPAND);

If i look into UpdateChildren, that does all itself recursively, possibly for to create all sub-elements needed at once.

Bur that would mean: i we inject a stop/start redraw here within, they would be stacked. And at the end, when the last child (T3) had been built, then it goes back with a series of start/start/start redraw. In a few words: if the building of the tree is not done sequentially, but recursively, the redraws will come into effect when the whole job had been done. That would mean: an overall freeze until the last node had been built. Of course that had not been the intention, definitively not.

So i think, all depends if we could inject the stop/start redraw in a sequential way, one after another, for each subnode to create, or if it is done in a recursive block at once. So unfortunately it might happen it's not applicable here. without other code changes - nobody does want those. Maybe you may want to check by which method the treepath is built at the end. If not possible here, ok, might happen.