Recent

Author Topic: 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect  (Read 27490 times)

d7_2_laz

  • Hero Member
  • *****
  • Posts: 506
Unfortunately there's another one that lets a program's code fail.
As i don't have the trunk installed, i plan to test it against an upcoming RC2.
But maybe it's more reasonable to post it here too, for anybody who might be interested in yet to do a quick check against the trunk earlier.

Windows, virtual treeview, with MultiSelect: navigating with the arrow keys, i.e event KeyUp with key VK_DOWN, in combination with  Shift, looses "Selected" to have a correct value (is Nil).

Test project attached for anybody who might be interested in. - I hope it's only an issue on my local systen, where it behaves differently than with 2.0.12
Lazarus 3.2  FPC 3.2.2 Win10 64bit

d7_2_laz

  • Hero Member
  • *****
  • Posts: 506
Thanks to the new Gitlab repo, where i could easily fetch the really last version of customlistview.inc i can say that both issues remaining are indeed fixed here. It appears to be fine again  :)
Lazarus 3.2  FPC 3.2.2 Win10 64bit

d7_2_laz

  • Hero Member
  • *****
  • Posts: 506
I'll enter this note as a kind of early warning that there might be a problem with this implementation.
I encounttered a dramatic breakdown of performance on highlighy populated listview's contents. // listview virtual plus MultiSelect
4755 objects:
   2.0.12:  110 ms
   2.2:     6944 ms
17704 objects:
   2.0.12:  531 ms
   2.2:     28908 ms  nearly 30 seconds!
This breakdown splits down into various places within the program where it is accumulated..
If it should be happen with the RC2, this would be a real showstopper for me to use.

I could locate the problem within customlistview.inc, TCustomListView.GetSelection - and here, it's due to the for-loop
potentiall along all items, if nothing is selected.
Now, a simple "if Selected = nil then exit; " might cost me 8 seconds
Until 2.0.12 all was fine, the listview did behave correct and fast. Why only had it been changed. A real nightmare!
Lazarus 3.2  FPC 3.2.2 Win10 64bit

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4458
  • I like bugs.
I could locate the problem within customlistview.inc, TCustomListView.GetSelection - and here, it's due to the for-loop
potentiall along all items, if nothing is selected.
Now, a simple "if Selected = nil then exit; " might cost me 8 seconds
Until 2.0.12 all was fine, the listview did behave correct and fast. Why only had it been changed. A real nightmare!
No, it was not correct in 2.0.12. There "Selected" returned a wrong item in virtual mode. Now the feature is identical with the "normal" mode, both functionally and speed-wise.
If you find better ways to fix it, please tell us and provide a patch.
[Edit] Actually I have an idea. There is variable FSelectedIdx for the virtual mode. With MultiSelect enabled it picked a wrong line but maybe it can be used to optimize the case when no lines are selected. It is then Nil, although you found another bug about the last line selected.
Please play and test with it.

Quote
Thanks to the new Gitlab repo, where i could easily fetch the really last version of customlistview.inc
There was no need to wait for Gitlab really. Getting the sources from SVN used to be equally easy.
Git has other benefits like easy local branches but they are not needed to test latest changes in trunk.
« Last Edit: July 28, 2021, 04:38:22 pm by JuhaManninen »
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

wp

  • Hero Member
  • *****
  • Posts: 11831
I encounttered a dramatic breakdown of performance on highlighy populated listview's contents.
[...]
Now, a simple "if Selected = nil then exit; " might cost me 8 seconds
Until 2.0.12 all was fine, the listview did behave correct and fast. Why only had it been changed. A real nightmare!
It would be helpful if you'd provide a simple basic demo showing this issue.

d7_2_laz

  • Hero Member
  • *****
  • Posts: 506
Being not a core developer i cannot fix it, not deep enough inside But i can say that I haven't had any problem with the listview upfrom Delphi 7 along Laz. 2.0.10 and then 2.0.12. After that i ran from one side effect and test case into another, i really don't rely it it anymore and with increasing response times from 110 ms to 6944 ms it would be unusable for me.

The issue seems to depend on some specific contexts (regularity unknown), mostly the call (if Selected) is harmless, but maybe there are "bad places" (unlucky if one just hits them) to call it where processing takes foreever. It should be more robust.

If i should guess, i would guess that, when calling from a "bad place", an unintended (too early) initialization of the item objects will be triggered. Maybe it comes somewhere after Items.Count had been set, but OnDate not yet fully accomplished
My öast resort is really to hope that it won't occur with the RC2.
Lazarus 3.2  FPC 3.2.2 Win10 64bit

d7_2_laz

  • Hero Member
  • *****
  • Posts: 506
Helloi wp, thanks! I already did spend many hours to track it down without getting the clue. But just got an idea, i'll verify and come back here with it soon.
Lazarus 3.2  FPC 3.2.2 Win10 64bit

d7_2_laz

  • Hero Member
  • *****
  • Posts: 506
The  functional context is not the same as within the app, but at least it provokes the same behaviour. Not quite sure though if it represents all of the issue

I apologize by the author of ShellViewEx that i take that again as test project, but it's somehow comparable and small enough.

Search a highly populated folder in the treeview (eg. windows\system32 or windows\winsxsI and click on the node; see what happens iin the listview.

The intervening statement is simply  " if SLV.Selected = nil then  xyz := 'dummy'; " 
Lazarus 3.2  FPC 3.2.2 Win10 64bit

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4458
  • I like bugs.
Being not a core developer i cannot fix it, not deep enough inside
You didn't even try. It is Pascal code just like any other Pascal code.
A "core developer" is not some magical super-human who knows everything once he got commit rights.

Quote
My öast resort is really to hope that it won't occur with the RC2.
Sure it will be in RC2 because the fix was merged there.
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

wp

  • Hero Member
  • *****
  • Posts: 11831
it's due to the for-loop
potentiall along all items, if nothing is selected.
Now, a simple "if Selected = nil then exit; " might cost me 8 seconds
I can't imagine that iterating over 17000 items just for checking whether the loop item is selected takes 8 seconds (**)...

Your demo is too complex for your theory because it contains code how to find the image indexes for listitems (which in fact may take a while for a folder with 17000 items (*) ). I removed the imagelists, the code setting the imageindex and forced the treeview and listview not to use the built-in images. Now the listview is lightning fast even when opening and clicking the c:\windows\winsxs folder.

Therefore I'd conclude that the delay that you observed (and I confirm this) is due to the item icons, not the Item.Selected loop.

* [EDIT]
OK, in virtual mode only the image indices of the visible items are determined. I guess I need to study your code in more detail.

** [EDIT]
Hmm... Debugging into TOwnerDataListItems.GetItem I notice that there is a call to ListView_GetItemState in the win32 widgetset - and this really may take some time. And since this happens for every list item due to the Selected loop, I think now that your observation is correct.
« Last Edit: July 28, 2021, 08:40:02 pm by wp »

balazsszekely

  • Guest
I really don't understand what's the problem. Everything works fine for me and it's lightning fast, no exceptions whatsoever(both with 2.0.12 and Trunk).

balazsszekely

  • Guest
OK. Now I understand what you meant:
Code: Pascal  [Select][+][-]
  1.     //...
  2.     // Oh, oh !
  3.     if SLV.Selected = nil then
  4.        xyz := 'dummy';
When you check for SLV.Selected, the ListView will loop through all its items again(internally), making it slow. However there is no reason to check for SLV.Selected on STVChange, since the SLV it was freshly populated, so nothing is selected. A better event for selection check is OnChanging.

PS: Even with the above code it does not crash for me, just becomes slower.
« Last Edit: July 28, 2021, 08:40:43 pm by GetMem »

d7_2_laz

  • Hero Member
  • *****
  • Posts: 506
@GetMem, yes, that's exactly what i meant. And yes, the sample looks crazy. But it shows up a problem.  And in practice the programs are far more complex and there is a dependency of functions .... and not everybody has s navigator builtin for to decide which place is a good one and which one is  bad. Should me or other redesign programs only because the technique of GetSelection had changeed?

@wp, right, without icons it's much faster, but not even close as it had been before;
in my app it would go down from7000 ms onto 680ms. Previously (before the for-loop) it had been 110 ms).
I didn't suspect the runtime of the for-loop itself (it's by far not as long), but i thought, maybe all objects would initialize at once now, conflicting the virtual paradigm.
One might omit the icons (however, won't like to do omit them), but there're also more data to fetch  and prepare within the OnData callback.

Argement pro the theory:
i applied a Counter within "OnData".
 Clicking on node "system32", it says: 4765 calls of SLVOnData.
 Without the line "if SLV.Selected = nil then xyz:='dummy'" it says: 0 (! womehow strange ..) calls of SLVOnData.
 So i believe that the for-loop within the GetSelection might kill the virtual paradigm, when - unluckywise -  the "if Select" is done from a "wrong place".
 
 Just besides (because of course not applicable for cross platform):
 i looked was Delphi 7 had done at "GetSelected". At the end it calls (Windows API) ListView_GetNextItem(
 with the flags  'LVNI_ALL or LVNI_SELECTED', where, maybe of interest;
https://docs.microsoft.com/en-us/windows/win32/api/commctrl/nf-commctrl-listview_getnextitem
LVNI_ALL - Searches for a subsequent item by index, the default value.

@Juha: i know my limits here. I've a lot of respect for the complexity of Lazarus and the expertise and experience of the collegues here, it's unbelievable. With my two or three little apps i'm just at my boundaries.

Lazarus 3.2  FPC 3.2.2 Win10 64bit

wp

  • Hero Member
  • *****
  • Posts: 11831
Being not a core developer i cannot fix it, not deep enough inside
I agree with Juha. Core developers are no super-men. You are debugging deeply into the code, and it is hard to understand why you don't go deeper.

I think your problem is that you refrain from building a Lazarus trunk installation and you need some instructions how to set up one. On Windows it is super easy. I have the impression that you work with git. So, do the following steps:
  • Clone the official Lazarus repository into an empty directory ("trunk" is now called "main"). Assume, it is called "c:\laz-git"
  • In this directory create a file "lazarus.cfg" with the folder name for the Lazarus configuration. This is highly recommended in order to leave your current Laz installation alone. The contents of this file should be something like
Code: [Select]
--primary-config-path=C:\laz-configs\laz-git
  • Now I assume that you already have a recent Laz release on your disk. It must come with fpc3.2.0 or 3.2.2 (maybe 3.0.4 is working, too). Remember the path to fpc.exe, e.g. c:\lazarus\fpc\3.2.2\bin\i386-win32
  • I use a batch file for building Lazarus (make_laz.bat); this way I can easily repeat the build process without thinking. It also has the advantage that you can locally adjust the path to avoid confusion with other FPC installations or, even worse, with Delphi installations. The batch file is simply as follows; store it in the c:\laz-git folder:
Code: [Select]
set path=c:\lazarus\fpc\fpc\3.2.2\bin\i386-win32
make bigide
  • Execute this batch file from c:\laz-git. It will run for a while, but ends with a new lazarus.exe in c:\laz-git.
  • When you start the new Lazarus a window appears that the fpc and the debugger cannot be found: Simply use those of the release version that you took for building. Note that the debugger is in folder mingw.
  • Install your components, setup the user interface as you like, and you are ready to go.
« Last Edit: July 28, 2021, 09:43:39 pm by wp »

balazsszekely

  • Guest
@d7_2_laz
After you set up your trunk installation, as @wp suggested, you can play with the attached patch. No guarantees though.
« Last Edit: July 28, 2021, 09:34:30 pm by GetMem »

 

TinyPortal © 2005-2018