Is TPngImage.Destroy() called when TPngImage goes out of scope if TPngImage.Free() is not called

Question

I have a function that takes a TPicture as a parameter and returns a TPngImage. To preserve the original image I create a TPngImage and copy the TPicture to the TPngImage, apply the effect and return the TPngImage. As so.

function Effect( const Value : TPicture ) : TPngImage;
  var
    AnImage : TPngImage;

  begin
    if( Value.Graphic is TPngImage ) then
      begin
        AnImage := TPngImage.Create();
        AnImage.Assign( TPngImage( Value ) );
        //Apply effect
        Result := AnImage;
        //AnImage.Free(); //error
      end;
  end;

procedure TForm11.Button1Click( Sender : TObject );
  begin
    Image2.Picture.Assign( Effect( Image1.Picture ) );
  end;

As I am creating an object, when does one free the created object. I cannot call TPngImage.Free() in the the function as that destroys the object before assignment. So how do I free the object created? Does TPngImage call its destructor when the object goes out of scope? Not freeing the object would lead to memory leak as far as I can see.

Answer 1

Your code has several bugs in it:

if( Value.Graphic is TPngImage ) then

If the caller's TPicture does not already contain a TPNGImage, you don't return anything at all. The Result is undefined.

In fact, you shouldn't check the Graphic's type at all. Various TGraphic classes can be assigned to each other, converting their image data from one format to another, so you should let that conversion happen when possible.

AnImage.Assign( TPngImage( Value ) );

You are typecasting the TPicture itself. You need to typecast its Graphic instead.

Result := AnImage;
//AnImage.Free();

This requires the caller to take ownership of the TPNGImage and free it, which is generally a bad design.

Image2.Picture.Assign( Effect( Image1.Picture ) );

Case in point, the caller is not taking ownership of the returned TPngImage, so it is leaked.

If you want to return a new TPNGImage, try this instead:

function Effect(Value : TPicture) : TPngImage;
begin
  Result := TPngImage.Create;
  try
    if (Value.Graphic <> nil) and (not Value.Graphic.Empty) then
    begin
      Result.Assign(Value.Graphic);
      //Apply effect
    end;
  except
    Result.Free;
    raise;
  end;
end;

Or

function Effect(Value : TPicture) : TPngImage;
begin
  Result := nil;
  if (Value.Graphic <> nil) and (not Value.Graphic.Empty) then
  begin
    Result := TPngImage.Create;
    try
      Result.Assign(Value.Graphic);
      //Apply effect
    except
      Result.Free;
      raise;
    end;
  end;
end;

Either way, you can then do this:

procedure TForm11.Button1Click(Sender : TObject);
var
  AImage: TPngImage;
begin
  AImage := Effect(Image1.Picture);
  try
    Image2.Picture.Assign(AImage);
  finally
    AImage.Free;
  end;
end;

However, a better design is to not return a new TPngImage at all. Pass in the 2 TPicture objects and let Effect() manipulate them as needed:

procedure Effect(Input, Output : TPicture);
var
  AnImage : TPngImage;
begin
  AnImage := TPngImage.Create;
  try
    if (Input.Graphic <> nil) and (not Input.Graphic.Empty) then
    begin
      AnImage.Assign(Input.Graphic);
      //Apply effect
    end;
    Output.Assign(AnImage);
  finally
    AnImage.Free;
  end;
end;

Or

procedure Effect(Input, Output : TPicture);
var
  AnImage : TPngImage;
begin
  if (Input.Graphic <> nil) and (not Input.Graphic.Empty) then
  begin
    AnImage := TPngImage.Create.Create;
    try
      AnImage.Assign(Input.Graphic);
      //Apply effect
      Output.Assign(AnImage);
    finally
      AnImage.Free;
    end;
  end else
    Output.Assign(nil);
end;

Then you can do this:

procedure TForm11.Button1Click(Sender : TObject);
begin
  Effect(Image1.Picture, Image2.Picture);
end;

Answer 2

The way you are doing this, you are leaking.

You can return an object from a function like this, but after use (in this case, after you assigned it to Image2.Picture, you must Free it. But you can only do that if you have a reference to it. So do this:

procedure TForm11.Button1Click(Sender: TObject);
var
  EffectImage: TPngImage;
begin
  EffectImage := Effect(Image1.Picture);
  Image2.Picture.Assign(EffectImage);
  EffectImage.Free;
end;

Note that if the image is not PNG, your Effect function will not return anything valid. That is an issue you have to address too.

Alternatively, I would do something like this:

procedure ProcEffect(InPicture, OutPicture: TPicture);
var
  AnImage : TPngImage;
begin
  if InPicture.Graphic is TPngImage then
  begin
    AnImage := TPngImage.Create;
    try
      AnImage.Assign(InPicture);
      //Apply effect
      OutPicture.Graphic := AnImage; // same as OutPicture.Assign(AnImage);
    finally
      AnImage.Free;
    end;
  end;
end;

And then call it with:

procedure TForm11.Button1Click(Sender: TObject);
begin
  ProcEffect(Image1.Picture, Image2.Picture);
end;