Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Method TBaseVirtualTree.GetOffsets unnecessarily applies FImagesMargin in most cases #1191

Closed
hermannoffen opened this issue May 10, 2023 · 1 comment · Fixed by #1192
Closed
Labels
Milestone

Comments

@hermannoffen
Copy link
Contributor

Problem

Method TBaseVirtualTree.GetOffsets (introduced in #369) incorrectly applies FImagesMargin for TVTElement.ofsCheckBox when no check image is needed. Thus, its out-param pOffsets then contains values two pixels too high for TVTElement.ofsCheckBox and all following elements.

Observations

This can, for example, cause a wrong result of TBaseVirtualTree.GetMaxColumnWidth, making autosized columns wider than necessary.
It might also be causing #811, I guess.

Reproduction

It's nicely reproducible by extending Minimal demo project:

  1. Get write access to private field FImagesMargin using the with Self do class helper hack:
type
  TVirtualTreeHelper = class helper for TBaseVirtualTree
    procedure SetImagesMargin(const AValue: Integer);
  end;

procedure TVirtualTreeHelper.SetImagesMargin(const AValue: Integer);
begin
  with Self do
    FImagesMargin := AValue;
end;
  1. Add a TSpinEdit control to TMainForm.dfm and set its Value to 2 as it is the hard coded value of FImagesMargin:
  object SpinEdit1: TSpinEdit
    Value = 2
  end
  1. Implement the spin edit's OnChangeEvent to apply its value to FImagesMargin:
procedure TMainForm.SpinEdit1Change(Sender: TObject);
begin
  VST.SetImagesMargin(SpinEdit1.Value);
  VST.Invalidate;
end;
  1. Compile and start to initially see the unchanged behavior:
    image
  2. Use the spin edit to change FImagesMargin and unexpectibly see the nodes' labels move to the right when increasing it or to the left when decreasing it:
    Animation
  3. As there are no images visible at all, FImagesMargin shouldn't change the display at all.
hermannoffen added a commit to hermannoffen/Virtual-TreeView that referenced this issue May 10, 2023
- updated method `TBaseVirtualTree.GetOffsets`:
  - added new local variable `lNodeIdent` to calculate `MainColumn`'s indentation
  - initialized `pOffsets` elements `ofsToggleButton` and `ofsCheckBox` neutrally with `ofsMargin`
  - for `MainColumn`:
    - increase `pOffsets` for `ofsToggleButton` by full value of `lNodeIndent` first
    - then decrease it, as before, to center the toggle button's position
    - increase `ofsCheckBox` by full value of `lNodeIdent`
  - eventually apply `fImagesMargin` to ` `ofsCheckBox` only if conditions for check image display are met
  - note that `fImagesMargin` is applied for `ofsStateImage` and `ofsImage` within the respective calls of `GetImageSize` as needed
This was referenced May 10, 2023
@joachimmarder joachimmarder added this to the V8.0 milestone May 10, 2023
@joachimmarder
Copy link
Contributor

Thank you for sending the pull request. I didn't repro the issue myself, but your changes look well-thought-out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants