Lazarus

Programming => LCL => Topic started by: d7_2_laz on July 16, 2021, 01:07:44 pm

Title: 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on July 16, 2021, 01:07:44 pm
I thought i was ready ...migrating to 2.2 / Windows ran very smooth so far, but ...
i saw that my app's custom listview (when procssing WM_CONTEXTMENU) pushed the wrong context menu when having more than one item selected.
Then i tried to look at the MouseDown event and it's the same:

Selecting two items in the listview and check:
Delphi 7 / Lazarus 2.0.12      SelCount is 2; "Seleted" is NOT nil
2.2 RC1:                              SelCount is 2; "Seleted" is NIL

That makes me very nervous, as i rely on the property at various places.

Saw by diff that within CommCtrls.pp regarding that area there had been at least some changes
TListItem class
2.0.12:      property Selected: Boolean index Ord(lisSelected) read GetState write SetState;
2.2 RC1     property Selected: Boolean index lisSelected read GetState write SetState;
Similar with Focused etc.

What's the purpose of this change, and how to get back the old behaviour? Referring to SelCount would not be a desired workaround.

Imo, if i select two items, then "Selected" should not be nil (but point to the first item selected, as it did before)

I retried with some Shwllview app around looking at OnContextPopup: the same.
Title: Re: 2.2 RC1: Custom listview "Selected" not backward compatible at MultiSelect?
Post by: Martin_fr on July 16, 2021, 06:41:13 pm
I don't think that change is related....

LCL: In TListItem, use TListItemState type directly.
SVN 64948@trunk

It is only the declaration, no actual code (other than no longer typecasting the ord int value to the state).

I don't know why the selected was changed. (or if it was intend)
Title: Re: 2.2 RC1: Custom listview "Selected" not backward compatible at MultiSelect?
Post by: Martin_fr on July 16, 2021, 06:43:28 pm
If the release wiki page does not mention it, then please report as a bug
Title: Re: 2.2 RC1: Custom listview "Selected" not backward compatible at MultiSelect?
Post by: JuhaManninen on July 16, 2021, 08:24:25 pm
@d7_2_laz, if you can bisect the guilty revision it would help.
As Martin mentioned the change you found is not a likely candidate.
Title: Re: 2.2 RC1: Custom listview "Selected" not backward compatible at MultiSelect?
Post by: d7_2_laz on July 16, 2021, 09:24:49 pm
I did not see much more than this cast too, but i don't think it's the guiltiy. It was simply an indicator, oha, something had changed in this context, and i believe there is a good reason for .... which one i don't know.

About the changed behaviour itself, personally i would vote: it's a bug, because 1) by contents: when selecting multiple objects, then it's not obvious why the Selected object should be nil;.  2) due to impact on the programmers, many statements or if-branches will fail now, as i already did encounter.

Referring of the documentation, i couldn't find any indication in the Release Notes.

Simply as a kind of additional info (pls. don't misunderstand) i was interested to see, what the newer Delphi docs are saying:
http://docwiki.embarcadero.com/Libraries/Sydney/en/Vcl.ComCtrls.TCustomListView.Selected
"Read Selected to access the properties of the first selected item in the list. If SelCount is 0 ...., Selected is nil"
That's the same as the previous behaviour of the VCL too.

I'd like to wait until one of guys from the VCL might be seeing it and maybe tell an opinion about.
Title: Re: 2.2 RC1: Custom listview "Selected" not backward compatible at MultiSelect?
Post by: Martin_fr on July 16, 2021, 09:42:21 pm
Well, I don't do much on the VCL.

But its certainly worth a bug report. That will also increase the chances that the right person will notice it.

Or try also the mail list. Not everyone is on the forum.
Title: Re: 2.2 RC1: Custom listview "Selected" not backward compatible at MultiSelect?
Post by: JuhaManninen on July 16, 2021, 10:30:02 pm
I'd like to wait until one of guys from the VCL might be seeing it and maybe tell an opinion about.
"Guys from the VCL" hang around Delphi forums. This is FPC/Lazarus forum.
Please bisect the guilty revision.
 https://wiki.lazarus.freepascal.org/How_do_I_create_a_bug_report#Regression_caused_by_a_certain_revision
This is clearly your "itch to scratch" so to speak.
Title: Re: 2.2 RC1: Custom listview "Selected" not backward compatible at MultiSelect?
Post by: d7_2_laz on July 17, 2021, 09:17:18 pm
I won't add any bug reports if i am not sure. My topic's title did end with a question mark.

In fact it turns out to be more complicated than i assumed at the beginning.
** I tried to build an easy testcase from the scratch (simple listview / custom listview / Shell listview as picked up from within the IDE), but that did NOT show up the issue.
** My own app (not using the shell controls) as well as a shell control demo from within this forum (i tried it for cross check), are showing up the issue.

So there is some layer between, and i still could not identify which specific change destroys the correct value of "Selected" when using both apps with RC1. - The impact is: one won't be able to call the right context menu.

Instead of more many words, for everyone who is interested in and wants to see a test case)
Pickup the zip file from Reply #32 within:
https://forum.lazarus.freepascal.org/index.php/topic,31625.msg397071.html#msg397071
and do:
-  Set ShellListView ("SLV") to multiselect true, and add a event for OnContextPopup.
-  Within this event, place a simple check like:
        if SLV.Selected = nil then
             ShowMessage(' Not Ok!!!  Selected is NIL, SelCount is:  '  + inttostr(SLV.SelCount))
        else  ShowMessage('OK, Selected is not  NIL;  SelCount is:   '  + inttostr(SLV.SelCount));;
-  Compile and test (Windows) with 2.012. And then compile and test with 2.2 RC1
-  For testing, select at least two items in the Listview; then do right mouse click
  ---> See what the message box is telling you
___OR SIMPLY___  pickup the zip file herein attached, which already contains those steps described.
(Remark: with new ShellCtrls.pas, sthe event "STVGetImageIndex" is no longer needed)
Title: Re: 2.2 RC1: Custom listview "Selected" not backward compatible at MultiSelect?
Post by: wp on July 17, 2021, 09:44:24 pm
I can confirm the described behaviour with your test application on Laz 2.2.RC1 and on Laz trunk. However, I see that you are using the virtual listview mode here. When I use an umodified TShellListView which is in normal data mode I cannot reproduce the error (see attached project). So, are you sure that the observed behaviour is not due to virtual mode?
Title: Re: 2.2 RC1: Custom listview "Selected" not backward compatible at MultiSelect?
Post by: d7_2_laz on July 17, 2021, 09:56:21 pm
Hi wp.
YEP!
As mentioned, had no prob too with an easy custom listviwe (using items.Add), but my app is using "of course" the virtual mode as well. Note:  encountered no issues here at all upto RC1.
That had been one of the candidates i thought of when saying "additonal layers" that possibly might have interfered.

Do you think this could be the culprit?
Title: Re: 2.2 RC1: Custom listview "Selected" not backward compatible at MultiSelect?
Post by: wp on July 17, 2021, 10:33:07 pm
Yes it is. By means of bisecting the svn revisions and inspecting the svn commit notes I could relatively easily find r60703 as the cause of the issue: up to r60702 the behaviour was correct, starting with r60703 the error appeared.

The commit is dated March 16 2019 and posted by Juha; the commit notes are: "LCL: Improved TListView selection handling. Issue #34877, patch from Yuriy Sydorov."

Please post a bug report and assign it to Juha. To simplify his work maybe you should try to simplify the demo: a multi-select ListView with owner data should show the issue as well.
Title: Re: 2.2 RC1: Custom listview "Selected" not backward compatible at MultiSelect?
Post by: Martin_fr on July 17, 2021, 11:47:04 pm
Small observation, if I use ctrl click to select the 2nd item, then it works ok. (at least Lazarus trunk)
Title: Re: 2.2 RC1: Custom listview "Selected" not backward compatible at MultiSelect?
Post by: d7_2_laz on July 18, 2021, 10:40:01 am
Thank you all for your attention!
wp:   i'd appreciare to create a simplified test project especially around the listview in virtual mode.
It indeed does showup the issue easily -> is attached here.

I could create a user in mantis and enter (and assign) it there, but i don't intend to install additional development subsystems (ie. trunk, version control systems) on my host), at least not these days.
If that would be ok, i'd go ahead. Maybe i elaborate the teset case a bit so that WndProc -> WM_CONTEXTMENU could be checked also. Would the latter be needed?

Martin_fr and Jamie: Oh and oops. Really! Can be seen in the test case too.
- Pressing Shift and doing the selection now, the Selected object goes lost..
- Pressing Ctrl and doing the selection now, the Selected object is preserved.
Might there, for to preserve Selection, somewhere a combination of SelCount and TShiftState is inspected,
where siimply ssCtrl in included but ssShift not?
Title: Re: 2.2 RC1: Custom listview "Selected" not backward compatible at MultiSelect?
Post by: wp on July 18, 2021, 11:44:45 am
I wonder if this demo is indicative of a bug in TCustomListView. You inherit a new class from TCustomListView in which you override, among others, the MouseDown and MouseUp procedures. But you do not call inherited. So, all that is implemented by TCustomListView and its ancestors is not executed.

Please modify the demo such that it does not use a subclassed TCustomListView
Title: Re: 2.2 RC1: Custom listview "Selected" not backward compatible at MultiSelect?
Post by: d7_2_laz on July 18, 2021, 11:57:57 am
A overriden TCustomListView is my special target, so it's an essential part of the test case which i won't miss.
With or without "inherited" didn't make a difference, i previously had checked that.

Here a slighty improved version (with inherited and a Check on WM_CONTEXTMENU):
Title: Re: 2.2 RC1: Custom listview "Selected" not backward compatible at MultiSelect?
Post by: d7_2_laz on July 18, 2021, 12:25:57 pm
Ok - additinally a regular listview added to the test project here
Title: Re: 2.2 RC1: Custom listview "Selected" not backward compatible at MultiSelect?
Post by: wp on July 18, 2021, 12:49:09 pm
Did it myself in the meantime. Reopened the old bug report: https://bugs.freepascal.org/view.php?id=34877

I could create a user in mantis and enter (and assign) it there, but i don't intend to install additional development subsystems (ie. trunk, version control systems) on my host), at least not these days.
In order to report a bug you do not have to install anything.
Title: Re: 2.2 RC1: Custom listview "Selected" not backward compatible at MultiSelect?
Post by: d7_2_laz on July 18, 2021, 03:12:27 pm
*Thousand* thanks again wp!
About the bug tickets, i'll remember [but would never do it without a review cycle in the forum, just as now).
Title: Re: [Waitfix] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: wp on July 18, 2021, 10:56:32 pm
There is a patch in https://bugs.freepascal.org/view.php?id=34877#c131930; in my tests it solves the issue. Can you confirm?
Title: Re: [Waitfix] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: JuhaManninen on July 18, 2021, 11:38:50 pm
I fixed also the other bug I found. Now "Selected" returns the first selected item also in virtual mode. Please test.
Title: Re: [Waitfix] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on July 19, 2021, 12:28:33 am
Saw it ... congratulations and big compliments!   :)
Confirmed, works fine with the testcase as well ss within the prog!
Wouldn't have expected that the solution would arrive so fast - wow! :)
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on July 19, 2021, 02:18:41 pm
Unfortunately i encountered a remaining little flaw in the solution.

Scenario - i added some small IFs in wp's use case -:
- click on an item in the middle of the list, eg. "capt_5"
- click on the empty space in the listview. Here all would be ok so far.
- click on the LAST item in the list, here: "capt_9"
- click on the empty space in the listview. The item identified by capt_9  keeps to have Item.Selected is true

The last item in the list is not set to Item.Selected - false when Selected became nil.
The effect is, that if you rely on Item.Selected is correct whereas it isn't, then some subsequent program steps will fail.
Title: Re: SOLVED? 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on July 19, 2021, 02:48:09 pm
Thought about to make a note in the bug tracker's item, but as the system is just being in progress to be migrated, i leave it to you if it should be corrected.
The solution itself is a good step forwards.
The little flaw oberrvation could be workarounded in a program via:
on Click event:

Code: Pascal  [Select][+][-]
  1.   if Selected = nil then
  2.      Items[Items.Count-1].Selected := False;
Title: Re: SOLVED? 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: JuhaManninen on July 19, 2021, 03:47:34 pm
Thought about to make a note in the bug tracker's item, but as the system is just being in progress to be migrated, i leave it to you if it should be corrected.
I reopened issue:
 https://bugs.freepascal.org/view.php?id=34877
Please copy your findings into a note there.
You found a workaround. Maybe you could fix it properly as well and provide a patch.
Title: Re: SOLVED? 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on July 19, 2021, 04:24:10 pm
No idea how to make TCustomListView.CNNotify fully functional but i added added a note in the bug tracker item.
Title: Re: SOLVED? 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: JuhaManninen on July 19, 2021, 05:36:01 pm
No idea how to make TCustomListView.CNNotify fully functional
LCL code can be debugged, too.
Title: Re: SOLVED? 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: dsiders on July 19, 2021, 08:16:21 pm
No idea how to make TCustomListView.CNNotify fully functional but i added added a note in the bug tracker item.

I think the widgetset implementations of GetItemAt are wrong (in TWin32WSCustomListView and TWinCEWSCustomListView).

Code: Pascal  [Select][+][-]
  1. class function TWin32WSCustomListView.GetItemAt(const ALV: TCustomListView; x,  y: Integer): Integer;
  2. var  HitInfo: LV_HITTESTINFO;
  3. begin
  4.   Result := -1;
  5.   if not WSCheckHandleAllocated(ALV, 'GetItemAt') then Exit;
  6.  
  7.   HitInfo.pt.x:=x;
  8.   HitInfo.pt.y:=y;
  9.   ListView_HitTest(alv.Handle,HitInfo);
  10.   if HitInfo.flags <> LVHT_NOWHERE then Result:=HitInfo.iItem;
  11. end;
  12.  


should probably be:

Code: Pascal  [Select][+][-]
  1. if (HitInfo.flags and LVHT_NOWHERE) <> 0 then Result:=HitInfo.iItem;

I tried it on my Trunk install. OnClick reports selected as being Nil and the selection is removed when I click on a non-item area. It doesn't trigger OnSelectItem or OnChange though. It does fire OnChange and  OnSelectItem.
Title: Re: SOLVED? 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on July 20, 2021, 09:46:41 am
Compiling for Windows x64?  I didn't notice any difference after applying the change, do a "compile new" and using the testcase (listview_virtualMode_multiselect_wp_mod.zip) just as described (click on the last listitem, click on nowhere, and the messagebox still appears telling that for the last item Selected is still true).
Title: Re: SOLVED? 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: dsiders on July 20, 2021, 04:32:19 pm
Compiling for Windows x64?  I didn't notice any difference after applying the change, do a "compile new" and using the testcase (listview_virtualMode_multiselect_wp_mod.zip) just as described (click on the last listitem, click on nowhere, and the messagebox still appears telling that for the last item Selected is still true).

Then we are seeing different results. Try running Tools > Configure Build > Clean All  + Build to force LCL to be rebuilt.
Title: Re: SOLVED? 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on July 20, 2021, 05:10:10 pm
Had this already donw for to avoid old code could have been loaded, but it diid not change.
The best is i'll await a retest with RC2 for to avoid confustion.
Should be no problem, a temporary workaround is available-
Title: Re: SOLVED? 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on July 26, 2021, 10:34:58 am
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
Title: Re: SOLVED? 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on July 26, 2021, 03:26:46 pm
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  :)
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on July 28, 2021, 02:27:11 pm
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!
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: JuhaManninen on July 28, 2021, 04:12:18 pm
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.
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: wp on July 28, 2021, 04:55:10 pm
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.
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on July 28, 2021, 05:34:50 pm
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.
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on July 28, 2021, 06:01:03 pm
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.
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on July 28, 2021, 06:30:00 pm
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'; " 
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: JuhaManninen on July 28, 2021, 06:43:50 pm
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.
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: wp on July 28, 2021, 07:14:59 pm
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.
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: balazsszekely on July 28, 2021, 07:39:49 pm
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).
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: balazsszekely on July 28, 2021, 08:39:07 pm
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.
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on July 28, 2021, 08:45:57 pm
@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.

Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: wp on July 28, 2021, 09:14:05 pm
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:
Code: [Select]
--primary-config-path=C:\laz-configs\laz-git
Code: [Select]
set path=c:\lazarus\fpc\fpc\3.2.2\bin\i386-win32
make bigide
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: balazsszekely on July 28, 2021, 09:32:54 pm
@d7_2_laz
After you set up your trunk installation, as @wp suggested, you can play with the attached patch. No guarantees though.
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on July 28, 2021, 10:59:03 pm
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 !!
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: balazsszekely on July 29, 2021, 06:57:28 am
@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.
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on July 29, 2021, 10:10:56 am
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.
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: wp on July 29, 2021, 08:10:45 pm
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).

Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: balazsszekely on July 30, 2021, 06:36:23 am
@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.
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on July 30, 2021, 02:03:11 pm
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.

Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: balazsszekely on July 30, 2021, 02:59:06 pm
@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.!
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on July 30, 2021, 05:19:42 pm
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 ...

Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on August 01, 2021, 10:13:44 am
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
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: JuhaManninen on August 01, 2021, 11:04:04 am
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.
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on August 01, 2021, 11:42:50 am
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.
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: wp on August 01, 2021, 11:59:40 am
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.
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: Bart on August 01, 2021, 12:06:37 pm
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
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on August 11, 2021, 12:43:46 pm
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
Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on August 11, 2021, 01:02:56 pm
Here is the second one:
Virtual listview, MultiSelect: problem: only one item in the list: after click on empty space this single item keeps to stay selected
https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/39325

Title: Re: [SOLVED] 2.2 RC1: Virtual listview "Selected" incompatible at MultiSelect
Post by: d7_2_laz on October 06, 2021, 12:28:28 pm
As i have no idea what will happen with these two remaining items (no status):

Virtual listview, MultiSelect: early reference to Selected object may cause performace problems
https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/39324

Virtual listview, MultiSelect: problem: only one item in the list: after click on empty space this single item keeps to stay selected
https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/39325

i'll remove teh "Resolved" flag here.
I'll await the 2.2 and open a new item then if needed, for to have a clear and focused reset.
TinyPortal © 2005-2018