Print Page - [SOLVED] 2.2 win x64: ugly issue with virtual listview "Selected" performance

Programming => LCL => Topic started by: d7_2_laz on November 08, 2021, 01:56:43 pm

Title: [SOLVED] 2.2 win x64: ugly issue with virtual listview "Selected" performance
Post by: d7_2_laz on November 08, 2021, 01:56:43 pm

First of all: after recompilaton of two bigger progs prviously ported from Delphi 7 with 2.2 RC2 Win x64 i encountered nearly no problems. Many compliments to the developers here, that is impressive! And many thanks from my side!

Except those two issues with the virtual listview that, introduced with RC1, - unfortunately - still do not have any status at all.

Those issues do persist:

(1) Virtual listview, MultiSelect: early reference to Selected object may cause performace problems
https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/39324
-> This issue kills the performance of my prog completely, it breaks long existing code without any need. Only solution so far is a workaround.
Remark: within the test project, the guilty statemant might appear to be very obvious. I intended that of course. But, how to determine what is a good place and what a wrong place?
In reality, the reference to "Selected" or similar within some subordinary routines might be somewhere in the code. And so it might be very hard to find out why all slows down.
A simple "if Selected" at the wrong place, and .. boom, the virtual listview behaves just the contrary as "virtual", as it immediately instantiates all objects at once. The culprit may be deep in the code. No fun at all to rewrite parts of the app only due to this shortly arriving issue.

(2) 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
-> This issue results in wrong effects when doing owner drawing. Items may appear to be selected ... although they aren't.
Remark: for a solution, i'd recommend to avoid expensive loops around all items for to do a correction of the property.

I would be very happy to see at least somwthing like a status on the issues. Would this be possible?
For information i attach my workaround (based on RC2) here that lets the program behave as expected again.

The good news is: except those two ugly beasts no problems at all with RC2 so far!

Title: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: d7_2_laz on January 15, 2022, 01:07:15 pm

Best congratulations for the excellent 2.2! (i like to wokr with Lazarus a lot)
Nearly no problems at all ... except those remaining two (win x64):

No change here with 2.2, same as newly introduced with RC1 and RC2:

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

The only thing i'd like to have is to be downward-compatible about basic objects and properties.
So that one can refer to "Selected" (or "SelCount") anywhere within the code.
Without time-consuming loops around all items - and without (and this is even more worse:) all items of the list are unintendedly instantiated at once bulk-wise!

Code: Pascal [Select][+]

  ListView1.Items.Count := 20000;
  // Probably somewhere later, but for demo in its elementary form here:
  if ListView1.Selected = nil then   -> boom! 20000 items will be instantiated at once, destroying virtualism
  // Just saw that it is the same with "SelCount":
  if ListView1.SelCount > 0 then     -> boom! all 20000 items will be instantiated at once

"Ugly", because the instantiating "Selected" (or "SelCount") might be located anywhere.
For instance in some multi-called helper routines. Hard to find out what's going on here and old code breaks! Initially it did drive me crazy
until i found out what caused the drastic performance decrease in my app.
Remark: the windows folder "C:\Windows\WinSxS" does contain nearly 20000 items. that was causing my observation.
Question: how to decide under which circumstances one can refer to
"Selected" or "SelCount" without harm??

The problem did not exist with 2.0.12 and before, but with 2.2 RC1, 2.2 RC2 and 2.2.

Virtual listview, MultiSelect: problem: having 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

After clicking on empty area, it should be possible to rely on the last item correctly being de-selected.
For instance for to achieve correct owner-drawing if one use that for specific paint of selected objects!
Imo, the evaluation should be done best without time-expensive loops along all 50000 items.

I'd highly wish to see, after four months, at least something like a status resp. label on these issues so that there is an indication that they don't will get forgotten.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: ASerge on January 15, 2022, 02:09:10 pm

Quote from: d7_2_laz on January 15, 2022, 01:07:15 pm

Code: Pascal [Select][+][-]
ListView1.Items.Count := 20000;
// Probably somewhere later, but for demo in its elementary form here:
if ListView1.Selected = nil then -> boom! 20000 items will be instantiated at once, destroying virtualism
// Just saw that it is the same with "SelCount":
if ListView1.SelCount > 0 then -> boom! all 20000 items will be instantiated at once

Windows 7 x64, Lazarus 2.2.0
Form with ListView (OwnerData=True, ViewStyle=vsReport, 2 columns), Memo and two buttons:

Code: Pascal [Select][+]

procedure TForm1.Button1Click(Sender: TObject);
begin
  ListView1.Items.Count := 20000;
end;
 
procedure TForm1.Button2Click(Sender: TObject);
begin
  if ListView1.Selected = nil then
    Memo1.Append('nil');
end;
 
procedure TForm1.ListView1Data(Sender: TObject; Item: TListItem);
var
  S: string;
begin
  S := IntToStr(Item.Index);
  Memo1.Append(S); // Logging of requested items
  Item.Caption := S;
  Item.SubItems.Append(S + ' ' + S);
end;

After pressing Button1, only visible elements are requested. After pressing Button2 - not one at all.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: d7_2_laz on January 15, 2022, 02:50:49 pm

Hello ASerge,

many thanks for your reply!
Unfortunately that's not the point.
Could you
- set the ListView to "MultiSelect" (problem only here)
- and move your lines 8 and 9 (if ListView1.Selected = nil etc.) directly after line3? (but this is meant only as a simple demo)
What do you see how many items are initialized at once?

My testcases are placed within the issue reports.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: ASerge on January 15, 2022, 03:22:46 pm

Quote from: d7_2_laz on January 15, 2022, 02:50:49 pm

Could you
- set the ListView to "MultiSelect" (problem only here)

You're right. After that, when press Button 2, all the elements are requested.
But if I change (ListView1.Selected = nil) to (ListView1.SelCount = 0) no items requested. In this case, a call is made via the widget without iterating over the elements.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: d7_2_laz on January 15, 2022, 03:42:58 pm

You are right about the SelCount (but that was only an observation besides).
But if you move the assignment to Items.Count and the "if ListView1.SelCount" etc both from within Button1Click into FormCreate, it will happen too (strange).

Code: Pascal [Select][+]

procedure TForm1.FormCreate(Sender: TObject);
begin
   ListView1.Items.Count := 20000;
   if ListView1.SelCount = 0  then

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: ASerge on January 16, 2022, 11:10:45 am

Quote from: d7_2_laz on January 15, 2022, 03:42:58 pm

You are right about the SelCount (but that was only an observation besides).
But if you move the assignment to Items.Count and the "if ListView1.SelCount" etc both from within Button1Click into FormCreate, it will happen too (strange).

Since the ListView1.Handle has not been created, Lazarus does not call the OS function.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: d7_2_laz on January 16, 2022, 02:45:50 pm

Hm, right, see TCustomListView.GetSelCount: If not HandleAllocated, then it gooes into a loop with .. guess what? Querying if an item is Selected.

So we're back here at the main issue (with "Selected").

Unintentionally it's an illusration why i said this issue is so treacherous: because the all-data-requesting "Selected" might be located anywhere within the code hierarchy. One never can be sure ...

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: d7_2_laz on January 20, 2022, 05:44:58 pm

Sorry to bother again here, but please allow a small supplementary note.
It's only because i had been quite unhappy myself about my own test case. Because it's restricted as much to it's most minimalistic form.
One might think: this won't affect me, never, i won't do such! To ask for "if Selected .." immediately after having set Items.Count Crazy!

So i tried an at least b bit more "realistic" use case (attached):
- press button 1 for to do some initialization; here: set Item.Count
- press button 2 for to see later and separately if a selection had been done in the listview.
When no selection happened, you can see:

Listview Item.Count had been set to 30000
No item selected
That took 547 ms
===>>>> # Calls of OnData had been: 30013

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: balazsszekely on January 20, 2022, 07:09:30 pm

@ d7_2_laz
VST use a dynamic array to keep a list with selected items. ListView on the other hand, loops through the items and checks each item selected state, no wonder is slow. However in case of owner draw data, you are lucky, because TCustomListView internally keeps track the index of the selected item. So when nothing is selected, you can avoid the loop. Try attached patch against trunk and see if it solves the problem.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: d7_2_laz on January 20, 2022, 07:57:20 pm

Hi Getmen,
thank you!
Could apply it directy to my 2.2.- If i understand the patch file format right,
it's simply about to apply this additional if-condition before the for loop.
Works very fine! Btw it's essentially the same (but shorter coded) than my workaround proposed:

Code: Pascal [Select][+]

      if OwnerData And (FSelectedIdx=-1) then begin
         Result := nil;
         FSelected := nil;
         Exit;
      end;
      FSelected := nil;
      for i := 0 to Items.Count - 1 do
     //  etc. .....

The patch would be fine for me :)

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: balazsszekely on January 20, 2022, 08:22:11 pm

@d7_2_laz

Quote

Could apply it directy to my 2.2.- If i understand the patch file format right,
it's simply about to apply this additional if-condition before the for loop.
Works very fine!

You're welcome! In my opinion the chance of regression for this patch is minimal.

Quote

Btw it's essentially the same (but shorter coded) than my workaround proposed

Where do you proposed a workaround? In the bug tracker?

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: d7_2_laz on January 20, 2022, 08:54:43 pm

Yes GetMem, here (lthe link is mentioned at the begin of this post)
https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/39324

It's based (and slightly modified) on a proposal from yourself around July 28, 2021

Since this time i'm trying to see this issue fixed officiially, until now without any success :o
Endless story, see https://forum.lazarus.freepascal.org/index.php/topic,55398.0.html

It killed my app in at least 3 different respects. Whereas i could use the workaround without any problems.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: dsiders on January 20, 2022, 09:11:55 pm

Quote from: d7_2_laz on January 20, 2022, 08:54:43 pm

It killed my app in at least 3 different respects. Whereas i could use the workaround without any problems.

Are you saying it passes the test case Martin mentioned in the post? If so, please add that comment to the bug report,

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: balazsszekely on January 20, 2022, 09:13:53 pm

Quote

It's based (and slightly modified) on a proposal from yourself around July 28, 2021

Apparently the first bad sectors already appeared in my head. :D

My original proposal was wrong. The new patch from my previous post should be fine. Can you please test if it has side effects?
In my opinion should not cause any issues. I will test your project with gtk2.

PS: The patch works fine with gtk2(Linux Mint 20.02 Cinnamon). As a side not the listview is much faster on linux then on windows.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: d7_2_laz on January 20, 2022, 09:58:14 pm

Hi GetMen, i understand you patch file so that. within TCustomListView.GetSelection,
it adds the condiktion:
if (not FOwnerData) or (FOwnerData and (FSelectedIdx > 0)) then
before the line:
for i := 0 to Items.Count - 1 do

So basically it does the same as my version, but more elegant.
I did try your version modifying the 2.2 Win x64 customlistview.inc on my PC accordingly, rebuilt all and retested
- the testcases written for this special issue
- the whole app
and encountered no problems at all so far. The app did behave (regarding this special issue #39324) just as expected and in the same way as before when having my own patch applied.
So for me your proposal is perfect! :)

Unforrtunately i
- cannot speak about other OSses
- cannot say anything about this runtestsgui.lpi testing scenario,as i don't know nothing about it.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: balazsszekely on January 20, 2022, 10:07:30 pm

@d7_2_laz
In you patch you set Result and FSelected nil, then immediately exit the function. After the for cycle there is one more line that is not executed. I will look at the testcase tomorrow.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: d7_2_laz on January 20, 2022, 10:31:01 pm

@GetMem, yes i see. You as well as me set (if the condition is met) both FSelected and Result to nil (that is both necessary), but you additionally pass the inclusion of lffSelectedValid into FFlags.

Although i noticed no side-effects so far. apparently it needs to be decided if this setting of the flag is necessary resp. wanted or not, regarding the special condition.
This had not been in my focus so far. At least i did not exclude it by intention.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: balazsszekely on January 20, 2022, 10:32:04 pm

I ran the test, there is one failure in the popup menu section, not related to our changes.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: d7_2_laz on January 20, 2022, 11:48:11 pm

There are two usage contexts for the flag lffSelectedValid in speech.

-the one is the current TCustomListView.GetSelection itself,
where apparently it's not critical if the same "if" (with the same result "nil") might be entered twice; it's not costly though.

- the other is TCustomListView.CNNotify around line 346
where your version would prohibit (as having lffSelectedValid included in FFlags) and my version would allow to enter this if:

// select
if (((nm^.uOldState and LVIS_SELECTED) <> (nm^.uNewState and LVIS_SELECTED)))
or (not (lffSelectedValid in FFlags) and (nm^.uNewState and LVIS_SELECTED <> 0)) then
begin
// select state changed

Theoretically .... but for the condition that Selected has detected to be nil, the latter "if" will even not be reached though.
At least a breakpoint here has no effect for me, it does not stop here.

So i would guess: it does not matter if -for the case of Selected is nil - the flag is included or not. At öeast i saw no difference in practice (also when playing aroung with keyboard/mouse with/without Shift and or Ctrl, KeyDown/KeyUp), no impact.

Maybe there is some additional specific reaon to - for the case Selection had been found to be nil - include lffSelectedValid in FFlags or not? My vote so far would be: Go for your change ...

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: Martin_fr on January 21, 2022, 03:01:32 pm

Quote from: d7_2_laz on January 20, 2022, 07:57:20 pm

Code: Pascal [Select][+][-]
if OwnerData And (FSelectedIdx=-1) then begin
...
Exit;
end;
...
for i := 0 to Items.Count - 1 do

This still allows for virtual views (aka "OwnerData") to enter the loop in

Code: Pascal [Select][+]

function TCustomListView.GetSelection: TListItem;
...
  if OwnerData and not MultiSelect then
...
  else begin
...
    if not (lffSelectedValid in FFlags) or MultiSelect then begin
      FSelected := nil;
      for i := 0 to Items.Count - 1 do
 

Which IMHO should not happen. (At least not in as many situations, as the suggested patch allows for).

The patch could be part of the solution. But IMHO it can be improved.

The suggested patch "if OwnerData And (FSelectedIdx=-1)" prevents entry of the loop, if it is known that there is no selection at all.
But, if the last 2 items (out of 50.000 items) are selected, the loop is entered. And (almost) all items are iterated.
Btw, if indeed "FSelectedIdx=-1" always and reliable indicates that there is no selection at all, then this could be added for non-virtual views too (so long as no better solution exists for either).

And even, if with the suggestion to only loop internally, this is still a lot of items to loop.
(See https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/39324#note_811612058 => check "lisSelected in FStates", no need to call "OnData")

TCustomListView.CNNotify should whenever possible keep track of the "first selected item.

If, it can not do that (probably cases where the current first-selected, is un-selected via ctrl-click, it could still store that index as "start point" for the search (but that is optional).

function TCustomListView.GetSelection: TListItem;
Should at the very least cache the found index. (CNNotify can unset the cache)

This still leaves cases, where all items need to be searched. But it should reduce the odds as much as possible.

Code: Pascal [Select][+]

procedure TCustomListView.SetSelection(const AValue: TListItem);

has another loop, that for would call "OnData" for all items, despite only needing "lisSelected in FStates".

Btw: for OwnerData "ListView.ItemFocused" is not updated.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: d7_2_laz on January 21, 2022, 04:26:22 pm

Martin_fr, i think you are right. Tried to verify the scenario with my testcase (as of reply #8) as well as with myp and this happens:

With the patch (as from GetMems reply #9) the early population phase of the programs no is fast again.
Before, without the patch, runtime might explode due to some "if Selected = nil" somewhere within routins).
This is now solved by the patch-

But when navigating to the last element in the list and selecting it, then "if Selected = nil" lets the runtime, via calls of OnData, explode again.

In my app the use case is as follows: folder treeview, tabbed listview(s): select "C:\Windows\WinSxS".
Without the patch: listview's population takes forever. With the patch: runs instantly again.

Select item #5 within the list: processing is instantly
Select last item within the list: runtime takes forever again (when "if Selected .." comes into play)

So the patch solves parts of the problem, but obviously is not sufficient for itself.

I retried with versions of the app generated with 2.0.12 (and 2.ß.10 and Delphi 7):
population: instantly
selecting last item: processing is instantly
Good times, sigh ....

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: balazsszekely on January 21, 2022, 04:55:13 pm

@Martin_fr, @d7_2_laz
Another solution is to revert d6c215a4("make ListView.Selected return the first selected item also when OwnerData=True). Everything will be fast again.
I don't think we should care if ListView.Selected points to the first or last selection. If somebody wants to know the selected items when multiselect is true, it has to loop through the items anyway.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: d7_2_laz on January 21, 2022, 05:14:51 pm

That's exactly what i did before (on application level).
So to say, it's a loop just by demand, just when needed. But not affecting several ops continuously.

One needs to keep in mind that this might be a loop around all items though
(as with keystroke Ctrl and Click, you can select interrupted sequences, eg. item #1, item #10, item #99)

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: d7_2_laz on January 22, 2022, 11:38:27 am

A. Current status; just more realistic test case

For to have a more real-life impression showing the impact of the remaining part of the issue that is not yet covered by the patch, i created a test case using a ShellViewDemo i picked up from an older discussion in this forum. (Attached)
My motiviation to post this is to show that it is not a purely theoretical thing.

- Select a larger folder in the treeview, eg. C:\Windows\System32 or (hard-core:) C:\Windows\WinSxS
- In the listview, press <Ctrl><End> for to go to the end of the list
- Click the last row ... and wait ... wait ... until a message box appears that tells which item had been selected
More than 6 seconds for "System32", 30 seconds for "WinSxS" ...

That's exactly the situation i'm having in my own app.
New behaviour since 2.2. RC1.

B. Solution compromise

The following modification of TCustomListView.GetSelection would (equivalently to reverting the RC1 patch which caused the issue) regain the full speed of 2.0.12 with the price, of course, to return again a last element instead of the first element.

Code: Pascal [Select][+]

    if not (lffSelectedValid in FFlags) or MultiSelect then
    begin
      FSelected := nil;
 
      // d7_2_laz: Patch https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/39324
      if (not FOwnerData) or (FOwnerData and (FSelectedIdx > 0)) then begin
         if (MultiSelect And (FOwnerData and (FSelectedIdx > 0))) then
             FSelected := Items[FSelectedIdx]
         else
         for i := 0 to Items.Count - 1 do begin
              if Items[i].Selected then begin
                 FSelected := Items[i];
                 break;
              end;
         end;
      end;
 
      Include(FFlags, lffSelectedValid);
    end;
 

Try the change using the test case ....
Both are attached here.
Plus: full performance is back again ! The overall navigation performance of the app is noticeably better (and better than with 2.0.12)!
Minus: Last instead of first selected element will be returned at multiselect
Personally for me, the Plus would be much much more important than the Minus. First/last might be handled on application level.

Are there any different opinions?

What about - as compromise - to imply a property (maybe defaulted to "return first item", Delphi-like) that allows the developer to choose which behaviour / option he prefers?
Something like FOwnerDataMultiSelectReturnLast := True (defaulted to False) .. FMultiSelectDelphiCompatility := true or such, let it up to the user.
if OwnerDataMultiSelectDelphiCompatility then .. existing code .. else ... reverted code as aboce.
I'd definitely (definitely!) would choose "speed" in very basic, fundamental operations and invest a bit of work on application level for the few cases when "first" is needed instead of "last".

What do you think? I'd highly be interested in!

Could this be a way out to shorten up this endless story, where i'm triggering the same issue again and again, because i'm suffering on it?

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: ASerge on January 22, 2022, 03:03:33 pm

Micro remark: the following two expressions are equal:
if (not FOwnerData) or (FOwnerData and (FSelectedIdx > 0)) then
if (not FOwnerData) or (FSelectedIdx > 0) then

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: d7_2_laz on January 23, 2022, 01:04:53 pm

Yep ASerge, right .. :
Additionally, the "FSelectedIdx > 0" better should read ">= 0", otherwise a first item won't be fetched..

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: balazsszekely on January 23, 2022, 01:34:03 pm

@d7_2_laz
Please add the latest info to the bugtracker, otherwise it will be lost. I see the issue is now assigned, maybe a fix will be available soon in trunk.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: d7_2_laz on January 23, 2022, 03:15:11 pm

GetMem, is there even an interest to have such kind of workaround? No substantially feedback Yes/No in this respect so far.
Everybody would like to have a proper solution, where the problem does not arise in the first place. Me too.
The more because on application level there will be still consecutive struggles with Item loops.
Is such a workaround (which is not a comprehensive solution, as Martin pointed out) in the path of decision?
I don't know. Meanwhile i'm loosing energy to trigger this topic again and again and close to give up.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: balazsszekely on January 23, 2022, 03:34:11 pm

Quote from: d7_2_laz on January 23, 2022, 03:15:11 pm

GetMem, is there even an interest to have such kind of workaround? No substantially feedback Yes/No in this respect so far.
Everybody would like to have a proper solution, where the problem does not arise in the first place. Me too.
The more because on application level there will be still consecutive struggles with Item loops.
Is such a workaround (which is not a comprehensive solution, as Martin pointed out) in the path of decision?
I don't know. Meanwhile i'm loosing energy to trigger this topic again and again and close to give up.

I don't know. Personally I would revert d6c215a4, this would solve the speed issue, then I would try to address the "last instead of first selected element on multiselect" separately. In my opinion the later issue is not critical, it's just a mild inconvenience at best. However fixing it is no walk in the park and most likely will introduce new issues.
Again the bug is not assigned to me, so I don't want to insist on going in one way or the other.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: d7_2_laz on January 23, 2022, 04:01:28 pm

Fully understandable GetMen .... and many thanks for all your replies here as well as in the past, which i highly appreciate!

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: JuhaManninen on January 27, 2022, 07:28:42 pm

Please test with latest trunk (main).
See my note in issue
https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/39324

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: d7_2_laz on January 27, 2022, 10:14:12 pm

Juha, great thanks for this attention!

As i hadn't time today to reestablish a recent trunk test, i tried it - after inspecting the patch and, so, having no doubt that it would work :-) - to test with an accordingly adapted customlistview.inc.
In short i retried, after applying this patch, all my test cases related. Result just as expected: that works well and solves most of the performance problems as discussed here. It helps!

One very little thing needs to be mentioned though (not occuring with a pure 2.2 customlistview.inc). Ownerdata + Multiselect + patch: "Shift" and VK_UP will return nil as result value for "Selected". Test project and workaround proposal (a simple 2-liner) do exist. I hadn't that mentioned here yet for not to flood even more stuff or confusion.

As the patch is highly appreciated from my side, i'd be happy we could keep this little thing completely apart from here (ok?), as the patch does solve the urgest pain.

(yes there might be some performance issues lieft, ie. when looping around all items and asking if Items.Selected, but this is nothing new or directly related to this specific topic) The patch does already help a lot)

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: JuhaManninen on January 28, 2022, 11:48:29 am

Quote from: d7_2_laz on January 27, 2022, 10:14:12 pm

One very little thing needs to be mentioned though (not occuring with a pure 2.2 customlistview.inc). Ownerdata + Multiselect + patch: "Shift" and VK_UP will return nil as result value for "Selected". Test project and workaround proposal (a simple 2-liner) do exist. I hadn't that mentioned here yet for not to flood even more stuff or confusion.

Is the attached patch OK? I could not reproduce the problem on Linux.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: d7_2_laz on January 28, 2022, 12:55:47 pm

Yes! (if i interprete the patch file right): that matches exactly the workaround i would have proposed:
'

Code: Pascal [Select][+]

 
// (TCustomListView.CNNotify, around line 359 in the recent trunk file:
               if (nm^.iItem < 0) or (nm^.iItem = FSelectedIdx) then
                 // Replace this line ...
                 //InvalidateSelected;
                 // ... by these lines:
                   if not MultiSelect then
                      InvalidateSelected;
 

. Obviously, without this, InvalidateSelected would be executed too general here
- Ccouldn't see yet any negative effect when suppressing it for MultiSelect

Unless there are more basic considerations i think as of today it's the best me could do.

Title: Re: 2.2 win x64: two persistent ugly issues with virtual listview "Selected"
Post by: JuhaManninen on January 28, 2022, 04:33:35 pm

Applied. The relevant changes will be merged to 2.2.2.

Quote from: d7_2_laz on January 28, 2022, 12:55:47 pm

Yes! (if i interprete the patch file right): that matches exactly the workaround i would have proposed:

A patch does not need interpreting. It represents the change exactly. The tool chain supports that exact information, meaning patches can be generated and applied easily.

You really must learn to use the development branch "main" and to work with patches.
Everybody involved in the development (as you are now) really has to do it. It is super-easy. Did you even try?
Now I had to dig the suggested changes from long forum threads etc.

Merge requests in GitLab are an alternative to patches. I personally don't like them. Typically a merge request is associated with a bug report, thus splitting the relevant information into 2 places. To test a merge request I must download the changes as a patch and apply it. A patch uploaded into the original bug report would make more sense.
Trivial changes can be merged without testing by clicking a "Merge" button. Then a merge request is useful but those cases are rare.
Thus, a good old patch is still recommended.

I understood the iteration of selected ListView items could be optimized further. If you do it, please open a new report and attach a patch.

Title: [SOLVED] 2.2 win x64: persistent ugly issue with virtual listview "Selected" per
Post by: d7_2_laz on January 28, 2022, 08:45:13 pm

I'd have to accept the critics. It's because i'm less familiar with the repository procedures tools and more comfortable to, when encountering a problem, find out the causing reason, make it reproducable and to search for possible ways out with having as best no side effects. Oftenly enough more overall insights will exist, ie. regarding cross-platform related impacts or conceptual reasons where to patch best.

With the patch the performance is back again, and happy enough about that, i don't really search for further optimizations, at least not now.

As the exchange so far of course was focused on the most important thing, i'll take the other initially mentioned (39325, only one item in the list: ...etc.) out here and adapt the topic's title accordingly.

Great thanks from my side to all who took part!