Delphi XE2 - Access violation in loadfromfile pngimage

Question

I have a listbox of images that normally works fine, but today it throws a access violation for no apparent reason.

Here is my code:

procedure TfrmSelectIcon.ListBox1DrawItem(Control: TWinControl; Index: integer;
  Rect: TRect; State: TOwnerDrawState);
var
  icone: TImageItem; // Ticone;
  png1: TPngImage;
  ImageIcone: TPngImage;
  TextPosition: integer;
  nomearquivo: string;
  Images: TImageList;
begin

  icone := TImageItem(listaIcone.Items[StrToInt(TListBox(Control).Items.Strings
    [Index])]);
  // Ticone(listaIcone.Items[strtoint(TListBox(Control).Items.Strings[Index])]);
  nomearquivo := Diretorio + icone.arquivo;

  // ShowMessage(nomearquivo);

  TListBox(Control).Canvas.FillRect(Rect);
  if FileExists(nomearquivo) then
  begin
    png1 := TPngImage.Create;
    png1.LoadFromFile(nomearquivo);  //here happen the problem.
    png1.Draw(TListBox(Control).Canvas, Rect);
  end;

end;

The file exists and it's a .png.

The bug happens just on the fifth image.

The first problem I can see (although it won't cause a AV) is that you are not freeing your png1. You just keep creating it. — Graymatter
Please edit to include the exact error message from the access violation, complete with the memory addresses it includes. As @Graymatter says, you have a massive memory leak by not freeing the png1 every time when you've finished with it, but that shouldn't cause an AV. (It may cause your app itself to crash with an out of memory error, but not an AV.) You should also include whether it's VCL or FMX, as both have a TListBox. — Ken White
Graymatter and Ken White, thanks for help. Inexplicable, but this problem was it! I do this way: png1 := TPngImage.Create; png1.LoadFromFile(nomearquivo); png1.Draw(TListBox(Control).Canvas, Rect); FreeAndNil(png1); This way works fine!!! Thanks more once! — pedro.olimpio
Do not load images in a paint event. Prepare it once you know what file to load and keep that object alive (use e.g. a private field, or image list for it). Release it when you don't need it anymore. — TLama
Yes, the paint event is called many, many, many times, for example, when scrolling down the list. — Jerry Dodge

Remy Lebeau Remy Lebeau · Accepted Answer · 2014-04-26T04:22:01

You have a memory leak, as you are not freeing the TPngImage objects you create. But worse, you should NOT be loading image files during a drawing operation to begin with. You should instead load the images once beforehand and then reuse them each time an item need to be drawn.

Try something more like this:

private
  Images: array of TPngImage; // or any other container you want to use

...

procedure TfrmSelectIcon.FormDestroy(Sener: TObject);
var
  I: Integer;
begin
  for I := 0 to High(Images) do
    Images[I].Free;
end;

procedure TfrmSelectIcon.ListBox1DrawItem(Control: TWinControl; Index: integer; Rect: TRect; State: TOwnerDrawState);
var
  png: TPngImage;
begin
  png := Images[Index];
  if (png <> nil) and (not png.Empty) then
    png1.Draw(TListBox(Control).Canvas, Rect);
end;

var
  icone: TImageItem; // Ticone;
  nomearquivo: string;
  I: Integer;
begin
  SetLength(Images, ListBox1.Items.Count);
  for I := 0 to High(Images) do
    Images[I] := nil;
  for I := 0 to High(Images) do
  begin
    // personally, I would suggest storing the TImageItem pointers
    // in the TListBox.Items.Objects[] property for easier access:
    //
    // icone := TImageItem(ListBox1.Items.Objects[I]);
    //
    icone := TImageItem(listaIcone.Items[StrToInt(ListBox1.Items.Strings[I])]);

    nomearquivo := Diretorio + icone.arquivo;
    if FileExists(nomearquivo) then
    begin
      try
        Images[I] := TPngImage.Create;
        Images[I].LoadFromFile(nomearquivo);
      except
      end;
    end;
  end;
end;

Delphi XE2 - Access violation in loadfromfile pngimage

2 Answers