[Patch] Memory leak in fcl-image when using a ClipRegion

Summary

Clipping in TFPCustomCanvas can be achieved by using the region concept (although only rectangular regions are implemented ATM). However when a clip region is created and assigned to the ClipRegion property of the canvas a memory leak is reported by HeapTrc.

System Information

• Operating system: Windows 11, 64bit
• Processor architecture: x86-64
• Compiler version: 3.3 (892220d6) - Important: the main branch is required because clipping was fixed here. (#24427 (closed))
• Device: Computer

Steps to reproduce, sample project

At first let me mention that this leak is a bit difficult to detect. The following code creates a TFPMemoryImage, an TFPImageCanvas, and a TFPRectRegion and draws a clipped circle which is saved to file at the end.

program no_leak;

uses
  HeapTrc, Types, fpImage, fpCanvas, fpImgCanv, fpWritePng;

var
  canvas: TFPCustomCanvas;
  image: TFPCustomImage;
  writer: TFPCustomImageWriter;
  region: TFPRectRegion = nil;

begin
  { Create an image 100x100 pixels}
  image := TFPMemoryImage.Create(100,100);

  { Attach the image to the canvas }
  canvas := TFPImageCanvas.Create(image);

  { Draw a yellow circle and clip-off its left by means of ClipRegion }
  canvas.Brush.FPColor := colYellow;
  region := TFPRectRegion.Create;
  region.Rect := Rect(50, 0, 100, 100);
  canvas.ClipRegion := region;
  canvas.Clipping := true;
  canvas.Ellipse(10, 10, 90, 90);
  // region.Free; // !!! region is destroyed by the canvas !!!

  { Create the writer }
  writer := TFPWriterPNG.Create;

  { Save to file }
  image.SaveToFile('no_leak.png', writer);

  { Clean up! }
  writer.Free;
  canvas.Free;
  image.Free;
end.  

This project does NOT leak. This is because the destructor of TFPCustomCanvas takes care of destroying the ClipRegion instance. (This is also the reason why region.Free must not be called in the project code - it would create a crash at the end since the canvas would attempt to destroy this region again.)

The following project, however, is leaking:

program leak;

uses
  HeapTrc, Types, fpImage, fpCanvas, fpImgCanv, fpWritePng;

var
  canvas: TFPCustomCanvas;
  image: TFPCustomImage;
  writer: TFPCustomImageWriter;
  region: TFPRectRegion = nil;

begin
  { Create an image 100x100 pixels}
  image := TFPMemoryImage.Create(100,100);

  { Attach the image to the canvas }
  canvas := TFPImageCanvas.Create(image);

  canvas.Brush.FPColor := colWhite;
  canvas.FillRect(0, 0, image.Width,image.Height);

  // Left part of the circle: in red and clipped by means of ClipRect
  canvas.Brush.FPColor := colRed;
  canvas.ClipRect := Rect(0, 0, 50, 100);
  canvas.Clipping := true;
  canvas.Ellipse(10, 10, 90, 90);

  // Right part of the circle in yellow and clipped by ClipRegion
  canvas.Brush.FPColor := colYellow;
  region := TFPRectRegion.Create;
  region.Rect := Rect(50, 0, 100, 100);
  canvas.ClipRegion := region;
  canvas.Ellipse(10, 10, 90, 90);

  { Create the writer }
  writer := TFPWriterPNG.Create;

  { Save to file }
  image.SaveToFile('leak.png', writer);

  { Clean up! }
  writer.Free;
  canvas.Free;
  image.Free;
end.

Now a first clipping action is done by using the ClipRect property (to clip off the right half of a red circle), and then the ClipRegion technique of the previous example is used to clip off the left half of a yellow circle.

HeapTrc reports a memory leak at the end of this program.

Analysis and fix

Using the ClipRect property creates a RectClipRegion internally. The setter of TFPCustomCanvas.ClipRect, SetClipRect, destroys the currently active ClipRegion and assigns the new ClipRegion to the FClipRegion variable.

The externally created ClipRegion (in the second step), however, is only assigned to the FClipRegion variable; there is no setter method which destroys the previously created ClipRegion for the ClipRect.

Therefore, the clipregion created internally for the ClipRect is never destroyed.

The fix is simple: Introduce a setter for the ClipRegion property which destroys the currently active ClipRegion before the newly created ClipRegion is assigned:

procedure TFPCustomCanvas.SetClipRegion(const AValue: TFPCustomRegion);
begin
  if AValue = FClipRegion then exit;
  FClipRegion.Free;
  FClipRegion := AValue;
end;

When this is done, the memory leak is gone.

See also attached patch: clipregion_memoryleak.diff

Remark

TFPCustomCanvas.SetClipRect unnecessarily checks for nil before freeing the FClipRegion:

procedure TFPCustomCanvas.SetClipRect(const AValue: TRect);
var
  lNewRegion: TFPRectRegion;
begin
  lNewRegion := TFPRectRegion.Create;
  lNewRegion.Rect := AValue;
  if FClipRegion <> nil then FClipRegion.Free;  // better: FClipRegion.Free
  FClipRegion := lNewRegion;
end;