SilverpointDev / sptbxlib

SpTBXLib is an expansion package for TB2K Delphi components that adds multiple features including support for styles and custom skins.
https://www.silverpointdevelopment.com
Other
73 stars 33 forks source link

Issues with ImageName #90

Closed pyscripter closed 3 years ago

pyscripter commented 3 years ago

Firstly, many thanks for adding the ImageName property!

However there are a few issues:

A) SetImageName and SetImageIndex do not account for "inherited" ImageLists. They need to use something like TTBItemViewer.GetImageList

procedure TTBCustomItem.SetImageName(const Value: TImageName);
begin
  if Value <> FImageName then begin
    FImageName := Value;
    if Assigned(Images) and Images.IsImageNameAvailable then
      SetImageIndex(Images.GetIndexByName(Value));  // Update ImageIndex and invalidate
  end;
end;

procedure TTBCustomItem.SetImageIndex(Value: TImageIndex);
var
  HadNoImage: Boolean;
begin
  if FImageIndex <> Value then begin
    {$IF CompilerVersion >= 34} // Robert: for Delphi Sydney and up
    if Assigned(Images) and Images.IsImageNameAvailable then
      FImageName := Images.GetNameByIndex(Value);
    {$IFEND}
    HadNoImage := FImageIndex = -1;
    FImageIndex := Value;
    Change(HadNoImage xor (Value = -1));
  end;
end;

b) Changing of ImageList is implemented in Vcl by calling ImageList.CheckIndexAndName. e.g.

procedure TCategoryButtons.CheckImageIndexes;
var
  I, J: Integer;
begin
  if (FImages <> nil) and FImages.IsImageNameAvailable then
    for I := 0 to FButtonCategories.Count - 1 do
      for J := 0 to FButtonCategories[I].Items.Count - 1 do
        FButtonCategories[I].Items[J].CheckImageIndexAndName;
end;

procedure TCategoryButtons.ImageListChange(Sender: TObject);
begin
  CheckImageIndexes;
  UpdateAllButtons;
end;

procedure TButtonItem.CheckImageIndexAndName;
var
  LImages: TCustomImageList;
begin
  if CategoryButtons <> nil then
    LImages := CategoryButtons.Images
  else
    LImages := nil;
  if (LImages <> nil) and LImages.IsImageNameAvailable then
    LImages.CheckIndexAndName(FImageIndex, FImageName);
end;
procedure TCustomImageList.CheckIndexAndName(var AIndex: TImageIndex; var AName: TImageName);
var
  LName: String;
begin
  if not IsImageNameAvailable then
    Exit;

  if AName <> '' then
  begin
    if (AIndex >= 0) and (AIndex < Count) then
    begin
      LName := GetNameByIndex(AIndex);
      if not SameText(LName, AName) then
        AIndex := GetIndexByName(AName);
    end
    else
      AIndex := GetIndexByName(AName);
  end
  else
  if (AIndex >= 0) and (AIndex < Count) then
    AName := GetNameByIndex(AIndex);
end;

Something similar is needed in TB2K and SpTBXLib

SilverpointDev commented 3 years ago

Fixed. I think I tested it in all possible scenarios, can you give it a try?

Got bit by 2 Delphi bugs:

1) Deleting an image from VirtualImageList doesn't trigger the change link. Seems to be a Delphi bug, a workaround would be to call VirtualImageList.Change after deleting.

2) ImageName is not reset when the linked VirtualImageList is changed and ImageIndex is not valid anymore. For example, place the following controls on the Form: A) TVirtualImageList with 3 images, name them: png0, png1, png2 B) TSpeedButton, link the Images property and set ImageIndex=2 and ImageName=png2 Deleting png2 from VirtualImageList will not change the ImageIndex, as pointed in 1) Calling VirtualImageList.Change after deleting will change ImageIndex to -1, but ImageName will remain png2, it should reset to empty string. The problem seems to be in TCustomImageList.CheckIndexAndName, when AIndex is returned as -1 AName should be and empty string.

pyscripter commented 3 years ago

Thanks! Will check shortly.

On your two points. I think these are not bugs but they are by design.

Think for example of the following scenario in relation to your example under "bug 2":

After step 2, you decide to change png2. You delete it from the VitualImageList and you add a new one with the same name. As it stands the link between the new image and the speedbutton will be re-established without taking any action. The fact that the ImageIndex was incorrect does not matter. ImageName takes precedence over ImageIndex.

The whole point with ImageNames is that the ImageIndex should not matter. You can rearrange images in the ImageList without breaking the links.

Please note also that VirtualImageList has a property "PreserveItems". This is important and I always set it to true. This does something similar for the link between the ImageCollection and the VirtualImageList. Say you delete an image from the Collection. The VirtualImageList maintains the slot of the deleted image with an empty icon. You then add an Image to the ImageCollection with the same name as the deleted one. The VirtualImageList reestablishes the connection with the newly added icon automatically. PreserveItems also helps when you switch ImageCollections. Say you have two ImageCollections one suited to dark and one to light colors. With PreserveItems set to true, you can switch ImageCollections without having to reset the images as long as the icon collections have the same image names.

SilverpointDev commented 3 years ago

I still think it's a bug, I get that ImageName has the priority, but there seems to be a difference in behavior between runtime and designtime.

For example, place the following controls on the Form: A) TImageCollection with 4 images: png0, png1, png2, png3 B) TVirtualImageList with AutoFill=False and add only 3 images: png0, png1, png2 C) TSpeedButton, link the Images property and set ImageIndex=1 and ImageName=png1

At designtime delete png0 from the VirtualImageList, the ImageIndex property of the SpeedButton is changed to 0. The image remains the same, because it is linked by the ImageName which is the same, so it only changes ImageIndex.

At runtime calling VirtualImageList.Delete(0) to delete png0 does nothing, ImageIndex is not changed. The image is changed, it is only visible when you hover the mouse over the SpeedButton. The image showed is png2, it still uses the ImageIndex=1 which is now png2, but the ImageName is png1 ! Shouldn't it behave like on designtime? Like I said this happens because Change is not called, the changes are made but remain dormant and are not visible until the next call to Change. For example executing: VirtualImageList.Delete(0); VirtualImageList.Add('png3', 'png3'); // Adding or inserting an image calls Change

This will change ImageIndex to 0, as per designtime behavior.

The old TImageList didn't change the ImageIndex of the controls when the ImageList was changed. I get that VirtualImageList has to change ImageIndex because it is linked to the ImageCollection but I'm not sure it is done correctly.

pyscripter commented 3 years ago

Yes that sounds like a bug. But I cannot reproduce the above with Delphi 10.4.2. VirtualImageList.Delete(0) at run-time triggers Change, which results in calling TCustomImageList.CheckIndexAndName and the ImageIndex gets updated.

pyscripter commented 3 years ago

It works well now! Thanks! You can close the issue.