Columns EndUpdate and duplicate position

[Dzyszla]

If I have 3 columns, last invisible, but set for two latest same position during columns are under updating, freezing on EndUpdate:

	with VirtualStringTree1.Header.Columns do
	begin
		BeginUpdate;
		Items[0].Position := 0;
		Items[1].Position := 1;
		Items[2].Options := Items[2].Options - [coVisible];
		Items[2].Position := 1;
		EndUpdate;
	end;

bug is in the VirtualTrees.Header file in TotalWidth method, especially on:

    if not (coVisible in Items[LastColumn].Options) then
      LastColumn := GetPreviousVisibleColumn(LastColumn);

But have same position, so still return same column.

Checked on the last version of VST.
Sample project attached.
Projects.zip

[joachimmarder]

Please respect our guidelines on the project homepage for submitting bugs. Please include your version of Virtual TreeView and Delphi, and attach a sample compiling project as ZIP to your report, or instructions how to modify one of the included sample projects to reproduce the problem. If only small changes are required, a description is sufficient how a demo projects needs to be changed in order to replicate the bug. If you already have a solution, please supply a patch file.

[Dzyszla]

Please respect our guidelines on the project homepage for submitting bugs. Please include your version of Virtual TreeView and Delphi, and attach a sample compiling project as ZIP to your report, or instructions how to modify one of the included sample projects to reproduce the problem. If only small changes are required, a description is sufficient how a demo projects needs to be changed in order to replicate the bug. If you already have a solution, please supply a patch file.

Done (issue has been edited). I think this is very simple case. Only VST with 3 columns and my code - that's all.

[joachimmarder]

@Dzyszla : Please check if the recent commit fixes the problem for you.

[Dzyszla]

Problem was fixed, but... Now no change column positions :( Try change (manually, run-time) column c2 to position 0 (first) and press button - no error, but nothing change (c2 will back to second place).

[joachimmarder]

You are right. Could you give it another try?

By the way, it will always work if you don't use BeginUpdate/EndUpdate on the columns.

[Dzyszla]

In first repeat-until loop iteration for positions [0, 1, 1] second column got position 0. Then in second iteration sets to [-1 (without sign), 0, 1] so... Access Violation :)

I think it works this way: write all position into array, sort it by values (positions) and rewrite from index to position.

Maybe something like this:

procedure TVirtualTreeColumns.FixPositions;
// Fixes column positions after loading from DFM or Bidi mode change.
var
  lPositions: System.Generics.Collections.TList<TVirtualTreeColumn>;
  I : Integer;
begin
  lPositions := TList<TVirtualTreeColumn>.Create;
  try
	lPositions.Capacity := Count;
	  for i := 0 to Count - 1 do
		  lPositions.Add(Items[i]);
	lPositions.Sort(
		System.Generics.Defaults.TComparer<TVirtualTreeColumn>.Construct(
			function(const aLeft, aRight: TVirtualTreeColumn): Integer
			begin
				Result := aLeft.Position - aRight.Position;
				if Result = 0 then
					Result := aLeft.Index - aRight.Index; // for not randomize order with same position
			end));
	for i := 0 to Count - 1 do
	begin
		lPositions[i].FPosition := i;
		FPositionToIndex[i] := lPositions[i].Index; // (?)
	end;
  finally
	  lPositions.Free;
  end;

  FNeedPositionsFix := False;
  UpdatePositions(True);
end;

---

I have one more ask - it's possible, when official fix will be available, to apply it to the 7.5.x sable version as fix the bug? Or 8.0 will be soon?

[joachimmarder]

Or 8.0 will be soon?

Yes. Actually, I wanted to release V8 this week. This is the last remaining issue for V8.0.
See: https://github.com/JAM-Software/Virtual-TreeView/milestone/15

[joachimmarder]

You should use BeginUpdate() / EndUpdate() only in case you are setting the entire columns with consistent data. Otherwise just omit it. I will check back to this issue in V8.1.

[Dzyszla]

Everywhere using this two commands are used for block unnecessary operations during changing several items of list, so this will works in all case ;) I keep waiting for fix.

[tecmatia-dp]

I was expieriencing a lot of problems with version 8.0 in my project, with a tree with 45 columns that the user can move, hide, save multiple views. In some cases, resulting in exceptions, columns that are not displayed, different widths in header and content, etc.
I found that problem is repeated Position, including design-time.
I tried to fix positions in my own code, outside the library, but no success.
Finding this open issue, I have seen the way, and fixed method TVirtualTreeColumns.FixPositions. Based on @Dzyszla proposal, this is my final code, that is working fine at runtime:

procedure TVirtualTreeColumns.FixPositions;
// Fixes column positions after loading from DFM or Bidi mode change.
var
  LColumnsByPos: TList<TVirtualTreeColumn>;
  I: Integer;
begin
  LColumnsByPos := TList<TVirtualTreeColumn>.Create;
  try
    LColumnsByPos.Capacity := Self.Count;
    for I := 0 to Self.Count-1 do
      LColumnsByPos.Add(Items[I]);

    LColumnsByPos.Sort(
      TComparer<TVirtualTreeColumn>.Construct(
        function(const A, B: TVirtualTreeColumn): Integer
        begin
          Result := CompareValue(A.Position, B.Position);
          if Result = 0 then
            Result := CompareValue(A.Index, B.Index);
        end)
    );

    for I := 0 to LColumnsByPos.Count-1 do
    begin
      LColumnsByPos[I].FPosition := I;
      Self.FPositionToIndex[I] := LColumnsByPos[I].Index;
    end;

  finally
	  LColumnsByPos.Free;
  end;

  FNeedPositionsFix := False;
  UpdatePositions(True);
end;

It's the same idea, but using CompareValue in Sort (easy to read), and fixing the assignment to the array FPositionToIndex.
Thank you @Dzyszla

[Dzyszla]

And thank's You too, @tecmatia-dp
I see, that line with question mark in my code is a little bit different. Maybe that's why my code wasn't working properly always.
Now waiting for official release with fix :)

[tecmatia-dp]

Oops, @Dzyszla, checking that line again, I think the correct one is yours, mine always assign Position=I to Index=I...
Correct is FPositionToIndex[I] := LColumnsByPos[I].Index
But, why wasn't it working properly always for you?
Now, I think it is working for me, apparently.

Edited my previous code, to fix that line, and change var names to be more easy to read and understand the algorithm (yours).

[joachimmarder]

Thanks for the code, which I just published as part of official release V8.0.1

[Dzyszla]

@tecmatia-dp

But, why wasn't it working properly always for you? Now, I think it is working for me, apparently.

I don't know :( Had some problem with correct changing position... I don't remember now, what exactly case. I try again now after update to 8.0.1, but hope now is all fine :)