OnMeasureItem called before data is assigned to node

[nminkov]

tree.AddChild(NULL, userData) will trigger OnMeasureItem() without first "attaching" the data to the node, which is a big issue for VirtualTrees with variable node height. That is, TVirtualNode passed to OnMeasureItem() will not contain the added data while called in AddChild()

Node height is determined by the data contained in the node. Its a "virtual" tree, and its crucial to its concept. This was working properly in the original VirtualTree.

This is caused because of a change in TBaseVirtualTree.SetChildCount.
The original code has even a nice comment:

// The actual node height will later be computed once it is clear
// whether this node has a variable node height or not.
Inc(NewHeight, Child.NodeHeight);

was replaced by

if (toVariableNodeHeight in FOptions.FMiscOptions) then
    GetNodeHeight(Child);
Inc(NewHeight, Child.TotalHeight);

[joachimmarder]

tree.AddChild(NULL, userData) will trigger OnMeasureItem() without first "attaching" the data to the node,

Sorry, I don't understand. GetNodeHeight()first calls OnInitNode and then OnMeasureItem:

      if not (vsInitialized in Node.States) then
        InitNode(Node);

      // Ensure the node's height is determined.
      MeasureItemHeight(Self.Canvas, Node);

The original code has even a nice comment:

The comment might have been nice, but that approach caused issue #447 and so needed to be changed.

[nminkov]

Sorry, I don't understand. GetNodeHeight()first calls OnInitNode and then OnMeasureItem:

Yes, but OnInitNode does not assign the "UserData" to the node.

AddChild() calls SetChildCount() to actually add the node, and SetChildCount() calls OnMeasureItem()
AddChild() then calls SetNodeData(Result, UserData); to assign the UserData to the node, but is done after the inital OnMeasureItem(). For variable height nodes, its the actual UserData determines the height.

Our current workaround is to remove vsHeightMeasured from node state after AddChild() to force it to measure again.

[dtamade]

insertNode try.

[joachimmarder]

InsertNode() and AddChild() contain very similar code, especially after the node has been added to the structure. In fact AddChild() offers a sub-set of the features of InsertNode(). We should unify both methods and finally map AddChild() to InsertNode() which does not suffer from this issue.