Discussion:
[Lazarus] A bitmap strange issue
Giuliano Colla via Lazarus
2018-04-21 15:18:31 UTC
Permalink
--
Luca Olivetti via Lazarus
2018-04-21 16:06:44 UTC
Permalink
El 21/04/18 a les 17:18, Giuliano Colla via Lazarus ha escrit:
>
> Here's a snippet of code used to show the issue:
>
>> procedure TForm1.Button1Click(Sender: TObject); var Filename: String;
>> begin if OpenPictureDialog1.Execute then begin // MyBitmap :=
>> Image1.Picture.Bitmap; <-------This causes a sigsev in FreeImage

If you follow the implementation of Image1.Picture.LoadFromFile you'll
see that it frees the current bitmap, that's the cause of the sigsev.
If you want to save the bitmap for later use, you should probably make a
copy instead.

>> Filename:= OpenPictureDialog1.FileName;
>> Image1.Picture.LoadFromFile(Filename); If assigned(MyBitmap) then
>> begin MyBitmap.FreeImage; FreeAndNil(MyBitmap); end; end; end;
>> procedure TForm1.Button2Click(Sender: TObject); begin If not
>> Assigned(MyBitmap) then MyBitmap:= TBitmap.Create; MyBitmap.Width:=
>> Image1.Width; MyBitmap.Height:= Image1.Height;
>> MyBitmap.Canvas.Brush.Color:= clNavy; MyBitmap.Canvas.Brush.Style:=
>> bsSolid;
>> MyBitmap.Canvas.FillRect(0,0,MyBitmap.Width,MyBitmap.Height);
>> Image1.Picture.Bitmap := MyBitmap;

And this assignment makes a copy of MyBitmap

>> //MyBitmap.FreeImage; Useless FreeAndNil(MyBitmap); end;

Bye

--
Luca Olivetti
Wetron Automation Technology http://www.wetron.es/
Tel. +34 93 5883004 (Ext.3010) Fax +34 93 5883007

--
Vojtěch Čihák via Lazarus
2018-04-21 16:07:24 UTC
Permalink
Hi,
 
@ MyBitmap := Image1.Picture.Bitmap;
This line only copies pointer, but Image1.Picture.Bitmap belongs to Image1 and it should care itself.
If you need copy of the bitmap you shoud use:
MyBitmap.Assign(Image1.Picture.Bitmap);
 
V.
 
______________________________________________________________
> Od: Giuliano Colla via Lazarus <***@lists.lazarus-ide.org>
> Komu: Lazarus mailing list <***@lists.lazarus-ide.org>
> Datum: 21.04.2018 17:18
> Předmět: [Lazarus] A bitmap strange issue
>
 
procedure TForm1.Button1Click(Sender: TObject); var Filename: String; begin if OpenPictureDialog1.Execute then begin // MyBitmap := Image1.Picture.Bitmap; <-------This causes a sigsev in FreeImage Filename:= OpenPictureDialog1.FileName; Image1.Picture.LoadFromFile(Filename); If assigned(MyBitmap) then begin MyBitmap.FreeImage; FreeAndNil(MyBitmap); end; end; end;Giuliano



----------

--
_______________________________________________
Lazarus mailing list
***@lists.lazarus-ide.org
https://lists.lazarus-ide.org/listinfo/lazarus <https://lists.lazarus-ide.org/listinfo/lazarus>
Giuliano Colla via Lazarus
2018-04-22 10:38:26 UTC
Permalink
Il 21/04/2018 18:07, Vojtěch Čihák via Lazarus ha scritto:

> @ MyBitmap := Image1.Picture.Bitmap;
>
> This line only copies pointer, but Image1.Picture.Bitmap belongs to
> Image1 and it should care itself.
>

That's what I had guessed, but this means that there's a patent
inconsistency, as shown in the Button2Click part of my test:

> Image1.Picture.Bitmap := MyBitmap; FreeAndNil(MyBitmap);

doesn't generate any error. If I don't free MyBitmap I see a memory
leakage in heaptrc.

IOW

Image1.Picture.Bitmap := MyBitmap;

performs an implicit assign, creating a new Bitmap Object for Image1.

The apparently symmetrical

MyBitmap := Image1.Picture.Bitmap;

just copies the pointer. Timage.Picture.GetBitmap and
TImage.Picture.SetBitmap do not follow the same logic.

All of this is maybe somewhat Delphi compatible. Actually Delphi (Rad
Studio 10.1) doesn't generate a SIGSEV with the same code, but rather
just an "Illegal Pointer Operation" warning. However IMO it still
remains quite inconsistent.

Giuliano
Michael Van Canneyt via Lazarus
2018-04-22 11:22:17 UTC
Permalink
On Sun, 22 Apr 2018, Giuliano Colla via Lazarus wrote:

> Il 21/04/2018 18:07, Vojtěch Čihák via Lazarus ha scritto:
>
>> @ MyBitmap := Image1.Picture.Bitmap;
>>
>> This line only copies pointer, but Image1.Picture.Bitmap belongs to Image1
>> and it should care itself.
>>
>
> That's what I had guessed, but this means that there's a patent
> inconsistency, as shown in the Button2Click part of my test:
>
>> Image1.Picture.Bitmap := MyBitmap; FreeAndNil(MyBitmap);
>
> doesn't generate any error. If I don't free MyBitmap I see a memory leakage
> in heaptrc.
>
> IOW
>
> Image1.Picture.Bitmap := MyBitmap;
>
> performs an implicit assign, creating a new Bitmap Object for Image1.
>
> The apparently symmetrical
>
> MyBitmap := Image1.Picture.Bitmap;

This is how all properties that are TPersistent descendents work, so this
behaviour should not come as a surprise.

Michael.
Giuliano Colla via Lazarus
2018-04-22 15:36:19 UTC
Permalink
Il 22/04/2018 13:22, Michael Van Canneyt via Lazarus ha scritto:

> This is how all properties that are  TPersistent descendents work, so
> this
> behaviour should not come as a surprise.

I wouldn't say so.

E.g. TCanvas is a TPersistent descendent, but the Canvas property of a
TCustomControl is declared like that:
>     property Canvas: TCanvas read FCanvas write FCanvas;
This means that after a

SomeControl.Canvas := MyCanvas;

or

MyCanvas := SomeControl.Canvas;

SomeControl.Canvas and MyCanvas will be the same object. Only a pointer
is copied.

On the contrary, the property Bitmap of a TPicture is declare like that:

>     property Bitmap: TBitmap read GetBitmap write SetBitmap;

where GetBitmap actually returns the TPicture.Bitmap Object, while
SetBitmap creates a different Bitmap Object, and assigns it the content
of the supplied bitmap (see TPicture.SetGraphic, called by
TPicture.SetBitmap).

This means that after a

MyBitmap := SomePicture.Bitmap;

MyBitmap and SomePicture.Bitmap will represent the same Object, while
after a

SomePicture.Bitmap := MyBitmap;

MyBitmap and SomePicture.Bitmap will represent different Objects.

This is IMHO a TPicture specific inconsistency.

Giuliano
Ondrej Pokorny via Lazarus
2018-04-22 11:49:25 UTC
Permalink
On 22.04.2018 12:38, Giuliano Colla via Lazarus wrote:
>
> Il 21/04/2018 18:07, Vojtěch Čihák via Lazarus ha scritto:
>
>> @ MyBitmap := Image1.Picture.Bitmap;
>>
>> This line only copies pointer, but Image1.Picture.Bitmap belongs to
>> Image1 and it should care itself.
>>
>
> That's what I had guessed, but this means that there's a patent
> inconsistency, as shown in the Button2Click part of my test:
>> Image1.Picture.Bitmap := MyBitmap; FreeAndNil(MyBitmap);
> doesn't generate any error. If I don't free MyBitmap I see a memory
> leakage in heaptrc.
>
> IOW
> Image1.Picture.Bitmap := MyBitmap;
> performs an implicit assign, creating a new Bitmap Object for Image1.
>
> The apparently symmetrical
> MyBitmap := Image1.Picture.Bitmap;
> just copies the pointer. Timage.Picture.GetBitmap and
> TImage.Picture.SetBitmap do not follow the same logic.

Do you really mean that
Image1.Picture.Bitmap.SomeProperty := SomeValue;
should apply SomeProperty := SomeValue on a copied object of Bitmap and
produce a memory leak?

And the same, that
Image1.Picture.Bitmap := Image2.Picture.Bitmap;
should produce a memory leak because of the temporary bitmap object?

...and taking it into detail because Picture is a property of TImage as
well:
Image1.Picture.Bitmap := Image2.Picture.Bitmap;
would actually produce 3 memory leaks: 2x TPicture and 1x TBitmap.

Ondrej
Giuliano Colla via Lazarus
2018-04-22 15:06:43 UTC
Permalink
Il 22/04/2018 13:49, Ondrej Pokorny via Lazarus ha scritto:

> Do you really mean that
> Image1.Picture.Bitmap.SomeProperty := SomeValue;
> should apply SomeProperty := SomeValue on a copied object of Bitmap
> and produce a memory leak?

No, I mean that like

Image1.Picture.Bitmap := SomeBitmap;

actually executes a

Image1.Picture.Bitmap.assign(SomeBitmap);

(see TPicture.SetGraphic called by TPicture.SetBitmap in picture.inc), so

SomeBitmap := Image1.Picture.BItmap;

should execute (with proper checks)

SomeBitmap.assign(Image1.Picture.Bitmap);

I speak about manipulating the Tpicture.Bitmap property itself, not the
TBitmap properties, which would not be affected.

Giuliano



--
Mattias Gaertner via Lazarus
2018-04-22 15:39:34 UTC
Permalink
On Sun, 22 Apr 2018 17:06:43 +0200
Giuliano Colla via Lazarus <***@lists.lazarus-ide.org> wrote:

>[...]
> SomeBitmap := Image1.Picture.BItmap;
>
> should execute (with proper checks)
>
> SomeBitmap.assign(Image1.Picture.Bitmap);
>
> I speak about manipulating the Tpicture.Bitmap property itself, not the
> TBitmap properties, which would not be affected.

And what to do with

W := Image1.Picture.BItmap.Width?

Mattias
--
Giuliano Colla via Lazarus
2018-04-22 16:55:47 UTC
Permalink
Il 22/04/2018 17:39, Mattias Gaertner via Lazarus ha scritto:

> And what to do with
>
> W := Image1.Picture.BItmap.Width?

You're right, there's no easy way out.

The original sin has been maybe to call Bitmap a TPicture property which
isn't really a TBitmap, as the GetBitmap and SetBitmap methods show.
This makes it easy to put any image into a picture, but creates
unavoidable inconsistencies.

Giuliano




--
Ondrej Pokorny via Lazarus
2018-04-22 17:43:42 UTC
Permalink
On 22.04.2018 17:06, Giuliano Colla via Lazarus wrote:
> Il 22/04/2018 13:49, Ondrej Pokorny via Lazarus ha scritto:
>
>> Do you really mean that
>> Image1.Picture.Bitmap.SomeProperty := SomeValue;
>> should apply SomeProperty := SomeValue on a copied object of Bitmap
>> and produce a memory leak?
>
> No, I mean that like
>
> Image1.Picture.Bitmap := SomeBitmap;
>
> actually executes a
>
> Image1.Picture.Bitmap.assign(SomeBitmap);
>
> (see TPicture.SetGraphic called by TPicture.SetBitmap in picture.inc), so
>
> SomeBitmap := Image1.Picture.BItmap;
>
> should execute (with proper checks)
>
> SomeBitmap.assign(Image1.Picture.Bitmap);

You can definitely do this - but not on on the Image1.Picture side and
on the other side - SomeBitmap:

property SomeBitmap: TBitmap read GetSomeBitmap write SetSomeBitmap;

-> implement SetSomeBitmap to execute .Assign() and you are good.

Ondrej
--
Loading...