In AutoScale the "margin" should be DPI scaled

[pyscripter]

procedure TBaseVirtualTree.AutoScale();

// If toAutoChangeScale is set, this method ensures that the default node height is set correctly.

var
  lTextHeight: TDimension;
begin
  if HandleAllocated and (toAutoChangeScale in TreeOptions.AutoOptions) then
  begin
    Canvas.Font.Assign(Self.Font);
    lTextHeight := Canvas.TextHeight('Tg') + 2;

Shouldn't the last statement be:

    lTextHeight := Canvas.TextHeight('Tg') + ScaledPixels(2);

My laptop has a scaling factor of 300% and that scaling may make a difference in the visual density of rows.

Looking at TControl.Change scale I saw that the FCurrentPPI is updated after the font is changed. So the following would work also when ParentFont is False, since CurrentPPI gets the PPI from the Window handle.

    lTextHeight := Canvas.TextHeight('Tg') + MulDiv(2, CurrentPPI, 96);

[pyscripter]

One more thing regarding AutoScale.

In your recent commit the following was added:

    if (lTextHeight > Self.DefaultNodeHeight) then begin
      ScaleNodeHeights(lTextHeight, DefaultNodeHeight);
      Self.DefaultNodeHeight := lTextHeight;
      TVTHeaderCracker(FHeader).ChangeScale(lTextHeight, DefaultNodeHeight);
    end;// if

It is not a good idea to call TVTHeaderCracker(FHeader).ChangeScale here. This does not just scale the height but also scales the font size and more.

TVTHeader.AutoScale() will be called anyway when the header font changes. So there is no need to separately call it from the VT AutoScale. Instead remove the conditional compilation from:

procedure TVTHeader.FontChanged(Sender : TObject);
begin
  inherited;
  {$IF CompilerVersion < 31}
  AutoScale();
  {$IFEND}
end;

and

procedure TVTHeader.StyleChanged();
begin
  {$IF CompilerVersion < 31}
  AutoScale(False); //Elements may have changed in size
  {$IFEND}
end;

Otherwise the header AutoScale would never be called.

Related to this:

procedure TVTHeader.ChangeScale(M, D : TDimension);
var
  I : Integer;
begin
  //This method is only executed if toAutoChangeScale is set
  FMinHeight := MulDiv(FMinHeight, M, D);
  FMaxHeight := MulDiv(FMaxHeight, M, D);
  Self.Height := MulDiv(FHeight, M, D);
  if not ParentFont then
    Font.Height := MulDiv(Font.Height, M, D);
  //Scale the columns widths too
  for I := 0 to FColumns.Count - 1 do
    TVirtualTreeColumnCracker(Self.FColumns[I]).ChangeScale(M, D);
end;

Since the change of font will trigger AutoScale which uses the column spacing, the font scaling should happen after the columns are scaled:

procedure TVTHeader.ChangeScale(M, D : TDimension);
var
  I : Integer;
begin
  //This method is only executed if toAutoChangeScale is set
  FMinHeight := MulDiv(FMinHeight, M, D);
  FMaxHeight := MulDiv(FMaxHeight, M, D);
  Self.Height := MulDiv(FHeight, M, D);
  //Scale the columns widths too
  for I := 0 to FColumns.Count - 1 do
    TVirtualTreeColumnCracker(Self.FColumns[I]).ChangeScale(M, D);
  if not ParentFont then
    Font.Height := MulDiv(Font.Height, M, D);
end;

Could this be the reason you disabled header AutoScale?

I have tested the above changes and I do not see any negative effects.

[joachimmarder]

Looking at TControl.Change scale I saw that the FCurrentPPI is updated after the font is changed. So the following would work also when ParentFont is False, since CurrentPPI gets the PPI from the Window handle.

But Shouldn't we then change ChangeScale() accordingly? And use the property instead of the member?

[pyscripter]

ScalePixels will not work when ParentFont is False.

procedure TControl.ChangeScale(M, D: Integer; isDpiChange: Boolean);
var
  X, Y, W, H: Integer;
  Flags: TScalingFlags;
begin
 ...
 if M <> D then
  begin
    ... 
    if not ParentFont and (sfFont in Flags) then 
      Font.Height := MulDiv(Font.Height, M, D);
    if [sfLeft, sfWidth] * Flags <> [] then
      FOriginalParentSize.X := MulDiv(FOriginalParentSize.X, M, D);
    if [sfTop, sfHeight] * Flags <> [] then
      FOriginalParentSize.Y := MulDiv(FOriginalParentSize.Y, M, D);
  end;
  FScalingFlags := [];
  if isDpiChange then
    FCurrentPPI := M;
end;

When you call inherited ChangeScale, eventually the above method will be called. When ParentFont is False the font is scaled, and the change of the font will trigger AutoScale. But FCurrentPPI, will still be the old FPCurrentPPI. So ScaledPixels will not scale correctly, This is why I suggested that we use:

lTextHeight := Canvas.TextHeight('Tg') + MulDiv(2, CurrentPPI, 96);

instead.

[joachimmarder]

Sorry, maybe I missed something, this is ScaledPixels():

function TBaseVirtualTree.ScaledPixels(pPixels: TDimension): TDimension;
begin
  Result := MulDiv(pPixels, {$if CompilerVersion > 31}Self.FCurrentPPI{$else}Screen.PixelsPerInch{$ifend}, 96);
end;

Called with pPixels=2 this makes:

  Result := MulDiv(2, FCurrentPPI, 96);

Which is exactly your code, with the difference that the member and not the property is used. Did I get this correct?

If yes, wouldn't it make sense to apply this change to ScaledPixels()directly?

[pyscripter]

Which is exactly your code, with the difference that the member and not the property is used

    property CurrentPPI: Integer read GetCurrentPPI;


function TControl.GetCurrentPPI: Integer;
var
  LForm: TCustomForm;
  Handle: HWnd;
begin
  Handle := GetAllocatedWindowHandle;
  if not (csDesigning in ComponentState) and (Handle <> 0) and
    (TOSVersion.Major >= 10) and (TOSVersion.Build >= 14393) then
  begin
    LForm := GetParentForm(Self);
    if (LForm = nil) or (LForm.Scaled) then
      Result := GetDPIforWindow(Handle)
    else
      Result := FCurrentPPI;
  end
  else
    Result := FCurrentPPI;
end;

In almost all cases ParentForm will not be nil and will Scaled. The result will be GetDPIforWindow(Handle) which does not use FCurrentPPI. CurrentPPI, will return the correct PPI, even before FCurrentPPI is changed.

[joachimmarder]

So is the core of your suggestion to use the property CurrentPPI instead of the member FCurrentPPI?

[pyscripter]

So is the core of your suggestion to use the property CurrentPPI instead of the member FCurrentPPI?

Indeed, in this special case.

[joachimmarder]

Why do you think the other many call to ScaledPixels() will work correctly if ParentFont is False?

[pyscripter]

Why do you think the other many call to ScaledPixels() will work correctly if ParentFont is False?

Yes they will, if they are used outside ChangeScale or inside ChangeScale after the inherited call. In this special case, AutoScale will be called from inside the inherited call, before FCurrentPPI is set.

I am pretty sure the rest of its uses are fine.

Incidentally, this is why CreateSysImageList currently works (see #1204), despite being called before the inherited ChangeScale. Because it uses CurrentPPI instead of FCurrentPPI.