Recent

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

GetMem

  • Hero Member
  • *****
  • Posts: 3492
@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 »

d7_2_laz

  • Full Member
  • ***
  • Posts: 198
Ok, although not being enthusiastic, i'll put it (trunk) onto my agenda and will give it a try.

About the patch (so easy, hu?): yes, i can confirm that it fixes the issue in all it's occurances .. no side effects so far that i could see.
So, the virtual listview / with MultiSelect is back again in it's old lightning speed (not measured, but by subjective comparison  2.0.12 version / new version, on a folder folder having 17000 objects).

Hard struggle, needed days to get an idea what might have gone wrong. Thanks to all of you having had the patience to follow this (again) probably too long story !!

GetMem

  • Hero Member
  • *****
  • Posts: 3492
@d7_2_laz
Quote
About the patch (so easy, hu?): yes, i can confirm that it fixes the issue in all it's occurances .. no side effects so far that i could see.
No, it's not easy. Due to the complexity of LCL, every change most likely will cause some kind of regression. It must be tested thoroughly.

d7_2_laz

  • Full Member
  • ***
  • Posts: 198
That's fully true GetMem, and so i think that, wiithin this thread, i did take part at this beta testing, encountering a couple of issues ....  (3 at least here discovered and solved by your team)

But that's simply due to the fact that i try tu assure in my more or less complex app that none of the base functions gets broken.Otherwise you'd need a lot of test cases around that do cover that all.
Nevertheless, i'm happy that's sorted out before the official release arrives.
The good news is: otherwise none (none!) side effects with the 2.2 i did see so far.

wp

  • Hero Member
  • *****
  • Posts: 8895
Just removed the Windows-specifics from GetMem's sample project of reply #41 and tested it on Linux gtk2/gtk3/qt/qt5, as well as macOS cocoa to investigate if ListView virtual mode is working there too: yes, it is working in all these widgetsets (except for gtk3 which has problems with the ListView anyway).

So, maybe it's time for some adventurous guys to convert the TShellListView to virtual mode, because the current non-virtual-mode ShellListView still has speed issues when reading folders with many items (at least on Windows where the built-in shell icons are extracted).

Mainly Lazarus trunk / fpc 3.2.0 / all 32-bit on Win-10, but many more...

GetMem

  • Hero Member
  • *****
  • Posts: 3492
@wp
Quote
Just removed the Windows-specifics from GetMem's sample project of reply #41 and tested it on Linux gtk2/gtk3/qt/qt5, as well as macOS cocoa to investigate if ListView virtual mode is working there too: yes, it is working in all these widgetsets (except for gtk3 which has problems with the ListView anyway).
Thank you!

Quote
So, maybe it's time for some adventurous guys to convert the TShellListView to virtual mode, because the current non-virtual-mode ShellListView still has speed issues when reading folders with many items (at least on Windows where the built-in shell icons are extracted).
I did take a quick look to ShellCtrls.pas, but is a non-trivial modification. More over is not my itch as @Juha would put it :D, I still prefer VST over anything.  However if nobody offers to do the conversion in a reasonable amount of time, I will do it.

d7_2_laz

  • Full Member
  • ***
  • Posts: 198
A virtual approach of the ShellListView would definitely give a tremendous boost forwards to the ShellControls!

But for today i'd need to throw a bad penny herein again. Hope nobodoy hates me for tipping again at this beast .. everybody surely is already bored. So you might take this just as a short note:

In my successful testings yesterday maybe i had been a bit too euphemistic .. it looked so fine, but i did miss a little thing. Namely the navigation by ArrowUp/ArrowDown keys. That falls back again to say 'Selected is Nil' where in fact items had been selected) by krys.

But how to know, if Selected is said to be nil here, what's the actually selected item without calling the big loop army? For to come to an end, i think i'll cirucmventt with ItemFocused as a helper, that (somehow indirectly) delivers just what is needed:
arrow up  : the item that just changed it's state to selected: the first item in the selected group
arrow down: the item that just changed it's state to selected: the last item in the selected group
Pseudo code: if ListView's Selected is nil, but Listview is focused, then:
if ListView1.ItemFocused.Selected ... this one is probably of interest (just the item that actually became selected)

Test project for those interested attached.


GetMem

  • Hero Member
  • *****
  • Posts: 3492
@d7_2_laz
Quote
In my successful testings yesterday maybe i had been a bit too euphemistic .. it looked so fine, but i did miss a little thing. Namely the navigation by ArrowUp/ArrowDown keys. That falls back again to say 'Selected is Nil' where in fact items had been selected) by krys.
Here we go again. :). I told you that each LCL modification is a double edged sword. Anyway I'm out of the office until Sunday. Please try to fix the issue if possible, if not I will take a look Sunday. Thank you.!

d7_2_laz

  • Full Member
  • ***
  • Posts: 198
Double sided sword, really ... but why does it hurt so hard since RC1 ??  :o
It won't be my baby anyhow, but that one might be promising (test case as well as app)

Code: Pascal  [Select][+][-]
  1. TCustomListView.CNNotify
  2.           // select
  3.           if (((nm^.uOldState and LVIS_SELECTED) <> (nm^.uNewState and LVIS_SELECTED)))
  4.           or (not (lffSelectedValid in FFlags) and (nm^.uNewState and LVIS_SELECTED <> 0)) then
  5.           begin
  6.             // select state changed
  7.             if (nm^.uNewState and LVIS_SELECTED) = 0 then
  8.             begin
  9.               if not OwnerData then
  10.               begin
  11.                 if FSelected = Item then
  12.                   InvalidateSelected
  13.               end else
  14.                 if (nm^.iItem < 0) or (nm^.iItem = FSelectedIdx) then
  15.                   //InvalidateSelected;
  16.                   if not MultiSelect then  // --- addition of this condition appears to help
  17.                      InvalidateSelected;
  18.             end else
  19.  


I'll stop my activities here, already lost so much time suffering on this special RC1 paradigm change ... since two weeks ...

« Last Edit: July 30, 2021, 09:56:27 pm by d7_2_laz »

d7_2_laz

  • Full Member
  • ***
  • Posts: 198
But i noticed another thing that s unexpected:
if there is only one item in the list )think of a folder containing only one file):
if you select this, then click on empty space .....  ->  the item keeps to be selected.

Normally you won't see that at all. Unless when using ownerdraw! being dependent here on referrable matching objects and properties.
So a test project is here attached that does simply some related IFs on mouse click.
Btw, that all didn't happen with the 2.0.12

Tried to fix it too and that does what it is expected to do, but an expert would do it conceptually cleaner correcting the real cause. Couldn't go deeper as i ran out of time now.

On top of Patch GetMem July 28, 2021:
customlistview.inc, around line 350:
Code: Pascal  [Select][+][-]
  1.             // select state changed
  2.             if (nm^.uNewState and LVIS_SELECTED) = 0 then
  3.             begin
  4.               if not OwnerData then
  5.               begin
  6.                 if FSelected = Item then
  7.                   InvalidateSelected
  8.               end else
  9.                if (nm^.iItem < 0) or (nm^.iItem = FSelectedIdx) then
  10.                  // InvalidateSelected;
  11.                // ---- Patch attempts d7_2_laz ----
  12.                begin
  13.                     // Patch #1: problem Selected is nil, although items are selected by ArrowDown/ArrowUp:
  14.                     if not MultiSelect then    // <- addistional condition
  15.                        InvalidateSelected;
  16.  
  17.                     // Patch #2: problem: after click on emptry space single item keeps to stay selected
  18.                     if (MultiSelect And  (nm^.iItem < 0)) then
  19.                         if Assigned(FListItems[0]) then
  20.                            FListItems[0].Selected:=False;
  21.                  end;
  22.               // ---- End patch d7_2_laz ----
  23.  
  24.             end else

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 3972
  • I like bugs.
On top of Patch GetMem July 28, 2021:
customlistview.inc, around line 350:
I ask you politely again to use Lazarus trunk sources and create a patch against it, or create a pull request in GitLab.
Refusing to do so again and again while participating in the development is just ridiculous.
The development process including revision control tools, patches etc. make things easier. That is why they were invented.
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

d7_2_laz

  • Full Member
  • ***
  • Posts: 198
And i already told politely that i don't feel qualified enough and not familiar to jump into that role. If this is ridiculous, and not good enough to participate within th forum, this is no problem. Anyway i'll stay at the 2.0.12 that allow to work with without such listview's  trouble.

wp

  • Hero Member
  • *****
  • Posts: 8895
And i already told politely that i don't feel qualified enough and not familiar to jump into that role.
I absolutely disagree. You already took that role because you publish fixes here in the forum. Adding them to bugtracker/GitLab is just a formal operation to make life easier for the developers. Everybody can write bug reports, you only must register to GitLab. A bugreport only requires you to write a bug description, steps to reproduce and a test project. And if available you should add a patch for the devs to see your solution. You show all these items here in your forum posts.
Mainly Lazarus trunk / fpc 3.2.0 / all 32-bit on Win-10, but many more...

Bart

  • Hero Member
  • *****
  • Posts: 4472
    • Bart en Mariska's Webstek
Reporting issues in the bugtracker (on GitLab now) also makes sure that this issue will not be forgotten.
You will be doing us a favour and we'll be happy to apply a patch (after review).

So: please, pretty please: file a bugreport  O:-)

Bart

d7_2_laz

  • Full Member
  • ***
  • Posts: 198
Found the time now to install the trunk (as described by wp in reply #44 - thanks wp!) and could verify the findings, - So here's the problem note (a second small one will be added later):
Virtual listview, MultiSelect: early reference to Selected object may cause performace problems
https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/39324

 

TinyPortal © 2005-2018