Hi,

Today i've committed a few graphics changes which might effect existing applications.

1) TBitmap is splitup into 4 separate classes:

TRasterImage (TGraphic)
A mostly virtual baseclass which has a Canvas and the capability of displaying itslef through a BitmapHandle.

TCustomBitmap (TRasterImage)
A baseclass for rasterimages with one image

TFPImageBitmap (TCustomBitmap)
A base class for images which get loaded and saved with the help of FPImage. Most image formats lazarus supports are read this way

TBitmap, TPixmap, TPNGImage.... (TFPImageBitmap)
Implementations of the specific graphic formats. Most define only a reader and a writer.

This means that old code where a XPM is loaded by a TBitmap is not supported anymore (or even worse, a BMP by a TPngImage)

Also assignments like:
var
   b: TBitmap;
begin
   b := TPngImage.Create...
end;
is not supported.

2) Reading of TIcon is implemented. Writing and modification still needs to be done. Since this wasn't possible in the old situation either, I decided to commit first (keeping such changes in sync with svn is a pain)

Marc

There's a regression. See http://bugs.freepascal.org/view.php?id=9512

Luiz

An simple application with a PaintBox and the following event handler in OnPaint does not work anymore. The width and Height of the bitmap are zero.

procedure TForm1.PaintBox1Paint(Sender: TObject);
var
  Bitmap: TBitmap;
begin
  Bitmap := TBitmap.Create;
  Bitmap.Width := PaintBox1.Width;
  Bitmap.Height := PaintBox1.Height;
  Bitmap.Canvas.Brush.Color := clWhite;
  Bitmap.Canvas.FillRect(0, 0, PaintBox1.Width, PaintBox1.Height);
  PaintBox1.Canvas.Draw(0, 0, Bitmap);
  Caption := Format('Bitmap Width: %d, Height: %d', [Bitmap.Width,
Bitmap.Height]);
  Bitmap.Destroy;
end;

Luiz

One old issue i have with TBitmap (not introduced now but that could be resolved with this refactoring) is that when a image is loaded with LoadFromStream or
LoadFromFile a copy of the image buffer is kept in memory. This can speedup if is necessary to access the image data directly but for the more common case
of loading a file to display is waste of memory.

There's also another problem with it: if the image is modified using LCLIntf functions like BitBlt the buffer will not be updated and next time SaveToFile/Stream is
called, the buffer (with the outdated data) will be used.

Maybe a TBufferedImage class or an option can be added.

Luiz

This is no error, this is by design. And is only kept once and as long as the image isn't changed. Indeed, for cases where you know you never want to save it, we can add an property.

There's also another problem with it: if the image is modified using LCLIntf functions like BitBlt the buffer will not be updated and next time SaveToFile/Stream is called, the buffer (with the outdated data) will be used.

Which buffer do you mean ?

Maybe a TBufferedImage class or an option can be added.

Not all modification functionality is fully implemented. IOW, these changes still need some work.

Marc

Yeah, this pretty much destroyed the material thumbnails in my 3D design app. Any workaround or this will come back soon? :-)

Kostas

Revert to r15471

Marc

When do you expect that to fix it so that at least the IDE works again? Should I try something?

Mattias

Don't know if it is related, but after doing an svn update, the debugger no longer works.

On the console, the last thing I see is

[TDebugManager.DoInitDebugger] A

And then it stops.

Michael.

This works here under linux/ubuntu/gtk2/laz15489/fpc 2.2.3 [2008/06/07].

Mattias

That, or make a patch :-P.

I've attached my fix. Seems that TCustomBitmap.SetSize was doing nothing except filling some values in a TDIBSection which weren't used. I kept them there (because i didn't tracked where they're actually used), but also wrote code that updates the width/height info in FImage.Description and allocates data for the pixels. I use RawimageNeeded too to obtain a valid description (i just put the new width and height there).

This makes both the autocompletion and changing TBitmap size code to work.

Kostas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tcustombitmap-setsize-imagedata-patch.patch
Type: application/octet-stream
Size: 642 bytes
Desc: not available
Url : http://www.lazarus.freepascal.org/pipermail/lazarus/attachments/20080621/958ed89f/attachment-0001.obj

ehm.... be more specific...
Before comitting I've tested compilation for linux-gtk1, linux-gtk2, linux-qt, win32,win64,wince osx-gtk1 osx-carbon and tested the IDE on gtk1, carbon and win32 and all work, so what is the problem ?

Marc

autocompletion ??

anyway, I need to think about this. Setting the width/height might be an issue of TRasterImage too. Allocation of data is not needed (yet)

About the DIBsection, that code will be removed and gets replaced by the rawimage.

Marc

this patch only affects LCL and (should) have no influence on the debugger

Marc

The completion box is empty, because the temporary TBitmap Width/Height is 0,0. See Kostas mail about SetSize.

Mattias

Since the most common case is not save it, just display, the default should be not keep a copy of the stream. This my point.

The SaveStream.

Luiz

1. Many images are only loaded, but never displayed.
2. SaveStream often contains information, that is not stored in the corresponding TGraphic class. For example header information or higher resolution.
3. The SaveStream is kept to make sure, that loading/saving creates the same stream. For example the IDE uses this a lot (TGraphics.Equals).

Therefore:
Iff the SaveStream is freed by default, *all* images of the LCL must be checked and the biggest part must be enabled to keep SaveStream at least at designtime.

If you alterthe image then 'Changed' must be called anyway. To free SaveStream and to let TImage invalidate and to call OnChange. Maybe 'Changed' should be make public? What has the VCL here?

> Which buffer do you mean ? The SaveStream.

Mattias

All in all, i have some doubt in the real usefulness of this feature. See the use cases:
- Load a image from file/stream
- The image data is stored in FSharedImage.FSaveStream property
1) If you don't change the image and use the SaveToStream function this will take a shortcut and just copy the data to the destination Stream. Ok. This is supposed to be the main benefit. But how much frequent is the situation where you just load a image and then save back to another stream or file without
modifying?
2) You modify the image using TCanvas methods. The FSharedImage.FSaveStream property is freed. If you call SaveToStream the data will be retrieved from the bitmap handle.
No benefits here.

I will be off in the next days, so excuse me for any lack of further replies.

Luiz

I read about the completionbox, but somehow thought that it happened before my changes.
I'm currently fixing the size issue, I hope to finish it this weekend. The patch sent here only fixes a part of the problem. (Yes, I know it is annoying, I switched to an older lazarus after a while)

Marc

ow ? which ones ?

2. SaveStream often contains information, that is not stored in the corresponding TGraphic class. For example header information or higher resolution.

Not anymore. Most important info is stored in a rawimage description and/or the object itself. The stream is only kept to save an image in its original format.

3. The SaveStream is kept to make sure, that loading/saving creates the same stream. For example the IDE uses this a lot (TGraphics.Equals).

imo, except for jpeg maybe, load and save image code should reproduce the same image. TGraphic.Equals might get adapted to use better comarisation based on imagedata.

Therefore: Iff the SaveStream is freed by default, *all* images of the LCL must be checked and the biggest part must be enabled to keep SaveStream at see no reason to check all images.

Marc

Sorry, maybe 'never displayed' was confusing. I meant 'often never displayed'.
The forms not shown, but loaded hidden (e.g. via CreateForm). And formerly the images of an imagelist were created only if used.

And what about the other attributes?
For example save an image in gimp and it shows for nearly any format some special properties.

What happens to images loaded in higher resolution than the current resolution and is then saved without SaveStream?

Yes, of course, they should. But do they?

For example the xpm reader skips all comments and the extended attributes. And there are many ways to represent the same pixel/color.
And I don't see any extra properties of TPortableNetworkGraphic. Where are they stored?

Ok.

Mattias

Ah, I was ttalking/thinking about cases after a handle was created. Indeed, until a handle (or now a rawimage) is created, the savestream is needed.

And formerly the images of an imagelist were created only if used.

I expected this one ;)

Yes and those extra properties are properties of the coresoponding imageclass (like compressionratio for TJPegImage)

What happens to images loaded in higher resolution than the current resolution and is then saved without SaveStream?

The r15472 implementation reads the raw image data in the same (or better) format. This data is also used for writing. this way no image details are lost. When a BitmapHandle is created, it always gets created in a format supported by the device. This may thus be of lower quality. With this approach, support for the PixelFormat property will be possible.

Bitmap manipulations still happen through a device based canvas, so currently you cannot "draw" a bitmap of a better quality than your current device. (for the far future I'm thinking of a way to make the canvasclass configurable, so that we can have a device independent canvas)

IMO yes, pixel wise.

For example the xpm reader skips all comments and the extended attributes. And there are many ways to represent the same pixel/color.

I don't see that as a problem. Look at a paint application, if you load an image and then save it again, it hgets saved in the format of the application, not the original file.

If a streamcopy is needed, then you should take care of that, not the graphic object.

And I don't see any extra properties of TPortableNetworkGraphic. Where are they stored?

To be implemented ?

Marc

Oops, hit sent to fast.

In case of the IDE, I have no opinion yet on what would be better. Keeping the original stream or write a "similar" one ourselves. In this case a KeepStream property might be a good thing.

Marc

The new image changes made TTrayIcon stop working.

The following trivial code no longer works and crashes (from trayicon example):

procedure TfrmTrayTest.btnPaintTestClick(Sender: TObject);
var
  SecondImage: TIcon;
begin
  SecondImage := TIcon.Create;
  try
    SecondImage.Height := 22;
    SecondImage.Width := 22;
    {$IFDEF FPC}
    SecondImage.Canvas.Draw(0, 0, MyImage);
    {$ENDIF}
    Canvas.Draw(0, 0, SecondImage);
  finally
    SecondImage.Free;
  end;
end;

TApplication.HandleException Cannot change size of icon image Stack trace:
$000BD014 TCUSTOMICON__SETSIZE, line 613 of ./include/icon.inc $000B1C35 TRASTERIMAGE__SETHEIGHT, line 684 of ./include/rasterimage.inc $000284DC TFRMTRAYTEST__BTNPAINTTESTCLICK, line 106 of frmtest.pas $000ECC86 TCONTROL__CLICK, line 2094 of ./include/control.inc $000FF77F TBUTTONCONTROL__CLICK, line 57 of ./include/buttoncontrol.inc $000FFE65 TCUSTOMBUTTON__CLICK, line 185 of ./include/buttons.inc $00100301 TBUTTON__CLICK, line 326 of ./include/buttons.inc $0010001A TCUSTOMBUTTON__WMDEFAULTCLICKED, line 240 of ./include/buttons.inc $0000D289
$000E317F TWINCONTROL__WNDPROC, line 4715 of ./include/wincontrol.inc $00190575 DELIVERMESSAGE, line 103 of lclmessageglue.pas $0019063C SENDSIMPLEMESSAGE, line 129 of lclmessageglue.pas $00190C16 LCLSENDCLICKEDMSG, line 525 of lclmessageglue.pas $001B0688 TCARBONCUSTOMBUTTON__HIT, line 326 of carbonbuttons.pp $001A6658 CARBONCONTROL_HIT, line 46 of carbonprivatecontrol.inc $92DEE4D7
$92DEDB7C

Yes, you cannot change the size of an Icon, since an icon contains on or more images of different sizes/depths

Use Icon.Add() to add a new image with given size and depth, then use TIcon.Current to select the wanted image.

Marc

Should be fixed now in r15510

Marc

Cannot this be done transparently so it won't break the code? Even with a boolean property to enable/disable this, it would be nice. Or if not, make width and size read-only. IMHO it is a bad surprise for the user to get this exception when the object allows the assignment of width and height fields.

Of course this isn't a high priority thing or anything, but it looks more "polished" i think :-)

Kostas

How would this work. We have a SetSize method, but moth users woud set Width and Height separately. Imagine the next scenario: Current Icon is 16x16, size is changed to 32x32 by setting width first so it becomes a 32x16 icon. Should this one be added, or should an Icon looked up with this width ?

I don't think they should automagically be added, but what maybe possible is to select based on width or height

Or if not, make width and size read-only. IMHO it is a bad surprise for the user to get this exception when the object allows the assignment of width and height fields.

Width and Height can be made protected properties, the SetSize method not.

Of course this isn't a high priority thing or anything, but it looks more "polished" i think :-)

Marc

I have not followed the discussion, but if the order of setting Width/Height is the issue, a typical solution is to postpone the resize until image is actually needed, and mark the component as Dirty. This is known as "Lazy Evaluation", a common optimization technique. This way any change to width/height will not take any effect until the image is actually needed, and setting Width/Height will work the same way as using SetSize.

With best regards,
Boian Mitov

-------------------------------------------------------------------- Mitov Software
http://www.mitov.com
--------------------------------------------------------------------