Grayscaling an image (memory leak)

[Serge_G 87]

HI, I wrote this unit to grayscale an image

unit ImageUtils;

interface

uses
  System.SysUtils, System.UITypes, System.UIConsts , System.Math,
  FMX.Types, FMX.Graphics, FMX.Utils;

  type Talgorithm = (algnone,algluminosity,algaverage,alglightness);

  function ConvertToGrayscale(const aBitmap: TBitmap; const aMethod : TAlgorithm=algnone) : TBitmap; overload;
  function ConvertToGrayscale(const FileName : String; const aMethod : TAlgorithm=algnone) : TBitmap; overload;


implementation


function Colortogray(const aColor : Talphacolor; const aAlgo : TAlgorithm=algnone) : Talphacolor;
  var
    H,S,L : Single;
    C : TAlphacolorRec;
    // https://www.johndcook.com/blog/2009/08/24/algorithms-convert-color-grayscale/
  begin
    RGBToHSL(aColor,H,S,L);
    c.Color:=acolor;
    case aAlgo of
      algluminosity: begin
                       H:=Trunc(0.2126*c.R) + Trunc(0.7152*c.G) + Trunc(0.0722*C.B);
                       Exit(HSLToRGB(H,S,L));
                     end;
      algaverage: begin
                    var mean : integer := (c.R + c.G + c.B) div 3;
                    c.R:=mean;
                    c.G:=mean;
                    c.B:=mean;
                    Exit(c.Color);
                  end;
      alglightness: begin
                      H:=(maxvalue([TAlphacolorRec(aColor).R,TAlphacolorRec(aColor).G,TAlphacolorRec(aColor).B]) +
                        minvalue([TAlphacolorRec(aColor).R,TAlphacolorRec(aColor).G,TAlphacolorRec(aColor).B])) / 2;
                      Exit(HSLToRGB(H,S,L));
                    end;
      else Exit(HSLtoRGB(0,0, L));
    end;

  end;

function ConvertToGrayscale(const aBitmap: TBitmap; const aMethod : TAlgorithm=algnone): TBitmap;
var
  X, Y: Integer;
  bd1, bd2: TBitmapData;
  p1, p2: PAlphaColorArray;
begin
  Result := TBitmap.Create(Round(aBitmap.Width), Round(aBitmap.Height));
  if (aBitmap.Map(TMapAccess.Read, bd1)
     and Result.Map(TMapAccess.Write, bd2)) then
  begin
    try
      for Y := 0 to (aBitmap.Height - 1) do
      begin
        p1 := PAlphaColorArray(bd1.GetScanline(Y));
        p2 := PAlphaColorArray(bd2.GetScanline(Y));
        for X := 0 to (aBitmap.Width - 1) do
        begin
           p2[X] := Colortogray(p1[X],aMethod);
        end;
      end;
    finally
      aBitmap.Unmap(bd1);
      Result.Unmap(bd2);
    end;
   end;
end;

function ConvertToGrayscale(const FileName : String; const aMethod : TAlgorithm=algnone): TBitmap;
var
  X, Y: Integer;
  bd1, bd2: TBitmapData;
  p1, p2: PAlphaColorArray;
  Source : TBitmap;
begin
  Source:=TBitmap.Create;
  try
   Source.LoadFromFile(FileName);

   Result := TBitmap.Create(Round(Source.Width), Round(Source.Height));
   if (Source.Map(TMapAccess.Read, bd1)
     and Result.Map(TMapAccess.Write, bd2)) then
   begin
    try
      for Y := 0 to (Source.Height - 1) do
      begin
        p1 := PAlphaColorArray(bd1.GetScanline(Y));
        p2 := PAlphaColorArray(bd2.GetScanline(Y));
        for X := 0 to (Source.Width - 1) do
        begin
           p2[X] := Colortogray(p1[X],aMethod);
        end;
      end;
    finally
      Source.Unmap(bd1);
      Result.Unmap(bd2);
      Source.Free;
     end;
  end;
  except
    Source.Free;
    result:=nil;
  end;
end;
end.

But I don't understand where I miss something because I have some memoryleaks

---------------------------
Unexpected Memory Leak
---------------------------
An unexpected memory leak has occurred. The unexpected small block leaks are:

 

29 - 36 bytes: TD2DBitmapHandle x 1, TBitmapImage x 1

45 - 52 bytes: TBitmap x 1

61 - 68 bytes: Unknown x 1

---------------------------
OK   
---------------------------

 

 

[Anders Melander 1783]

You haven't shown how you're handling the bitmap returned from ConvertToGrayscale but besides that your exception handling is wrong. Use this pattern instead:

begin
  Source := TBitmap.Create;
  try
    Result := TBitmap.Create(...);
    try
      ...
    except
      Result.Free;
      Result := nil;
    end;
  finally
    Source.Free;
  end;
end;

Apart from that your grayscale algorithm is horribly inefficient (and wrong). Why use HSLToRGB when you know that for grayscale R=G=B and why use RGBToHSL when you already operate directly on the RGB values...?

[Serge_G 87]

50 minutes ago, Anders Melander said:

You haven't shown how you're handling the bitmap returned from ConvertToGrayscale

Sorry, I tried to edit my first message but don't work

procedure TfrmMain.btnGriserClick(Sender: TObject);
begin
  if OpenDialog1.Execute then
  begin
    Image1.Bitmap.LoadFromFile(OpenDialog1.FileName);
    image2.Bitmap:= ConvertToGrayscale(image1.Bitmap);
//    image3.Bitmap:= ConvertToGrayscale(image1.Bitmap,TAlgorithm.algluminosity);
//    image4.Bitmap:= ConvertToGrayscale(image1.Bitmap,TAlgorithm.algaverage);
//    image5.Bitmap:= ConvertToGrayscale(image1.Bitmap,TAlgorithm.alglightness);
  end;
end;

result

54 minutes ago, Anders Melander said:

Apart from that your grayscale algorithm is horribly inefficient (and wrong).

Well, I try this ones

// https://www.johndcook.com/blog/2009/08/24/algorithms-convert-color-grayscale/

[Lajos Juhász 293]

3 hours ago, Serge_G said:

But I don't understand where I miss something because I have some memoryleaks

---------------------------

It's very simple take a look at TImage.SetBitmap. The TImage object will assign the bitmap to the internal bitmap and will not take the ownership over of the parameter. You've to write:
 

var 
  lbm: TBitmap;
begin
  if OpenDialog1.Execute then
  begin
    Image1.Bitmap.LoadFromFile(OpenDialog1.FileName);
    lbm:=ConvertToGrayscale(image1.Bitmap);
    try
      Image2.Bitmap:=lbm;
    finally
      lbm.Free;
    end;
  end;  
end;

 

Edited January 18, 2022 by Lajos Juhász
Open dialog in the example instead of a hard coded filename.

[Serge_G 87]

Thanks, I was looking for an error in the bad unit !

[Anders Melander 1783]

4 hours ago, Serge_G said:

Well, I try this ones

// https://www.johndcook.com/blog/2009/08/24/algorithms-convert-color-grayscale/

The algorithms in that link are okay. It's your implementation of them that's a problem. Just get rid of all the HSL stuff and you should be fine.

 

The formula you call "luminosity" is using the Rec 709 coefficients for "luminance" used in HDTV video.

I recommend you read these two sections:

• https://en.wikipedia.org/wiki/HSL_and_HSV#Lightness
• https://en.wikipedia.org/wiki/HSL_and_HSV#Disadvantages