FPImageException while loading Bitmap from resource stream

[TRon]

I don't think so. Because when the stream is a resource stream the stream does not start at the BitmapFileHeader ('BM') but is offset by a few bytes and, in case of the RT_BITMAP, there is not BitmapFileHeader at all.

afaik that statement assumes that loadfromstream should be able to read from whatever stream and decide, based on the binary data in that stream, what type of data needs to be loaded (which seem a bit counterproductive in my eyes). Again, to make sure as I can't test myself, I don't think the Delphi implementation actually does that.

If you truly want to load from a resource, then there are specific loadfromresource functions, that already know about the underlying binary data and as such can act accordingly.

But perhaps I am overlooking something ? You are more versed in the matter than I am.

[wp]

Here is a modified TBitmap.LoadFromStream which works with normal streams, as well as with RT_BITMAP and RT_RCDATA resource streams:

Code: Pascal  [Select][+][-]

1. procedure TBitmap.LoadFromStream(AStream: TStream; ASize: Cardinal);
2. var
3.   S: THeaderStream;
4.   Header: TBitmapFileHeader;
5.   P: Cardinal;
6.   Magic: array[0..1] of AnsiChar;
7. begin
8.   if AStream is TResourceStream then
9.   begin
10.     P := AStream.Position;
11.     AStream.Read(Magic, 2);
12.     AStream.Position := P;
13.     if (Magic[0] = 'B') and (Magic[1] = 'M') then
14.       { Handle the special case of RC_RTDATA resource which contains a
15.         complete, functional bitmap structure. }
16.       inherited LoadfromStream(AStream, ASize)
17.     else
18.     begin
19.       { Normal case of an RT_BITMAP resource lacking the BitmapFileHeader }
20.       FillChar(Header, SizeOf(Header), 0);
21.       { Create a BMP header ordered as it would be on disc, noting that if the CPU
22.         is big-endian this will be the "wrong way round" for numeric operations. }
23.       {$IFNDEF ENDIAN_BIG}
24.       Header.bfType := $4d42;
25.       Header.bfSize := SizeOf(Header) + ASize;
26.       {$ELSE}
27.       Header.bfType := $424d;
28.       Header.bfSize := swap(SizeOf(Header) + ASize);
29.       {$ENDIF}
30.       //Header.bfOffBits := 0; //data imediately follows
31.  
32.       S := THeaderStream.Create(AStream, @Header, SizeOf(Header));
33.       try
34.         inherited LoadFromStream(S, SizeOf(Header) + ASize);
35.       finally
36.         S.Free;
37.       end;
38.     end;
39.   end
40.   else
41.     inherited LoadFromStream(AStream, ASize);
42. end;  

It first checks whether the stream is a resource stream. If not, the inherited method is called. Otherwise it is a resource stream, and there are two cases: It could be RT_BITMAP or RT_RCDATA. Since the stream is at the very beginning of the bitmap structure two bytes are read from the input stream here: if they are 'BM' we have a fully functional bitmap (originating from RT_RCDATA) and the bitmap can be read as usual. If the two bytes are different we have a RT_BITMAP stream and reading must follow the original method, i.e. it introduces a THeaderStream to start reading at the TFileInfoHeader.

[wp]

If you truly want to load from a resource, then there are specific loadfromresource functions, that already know about the underlying binary data and as such can act accordingly.

There are TBitmap.LoadFromResourceName and .LoadFromResourceID which get the raw stream data from the resource, but loading the stream into the bitmap pixel array again leads to TBitmap.LoadFromStream.

[TRon]

There are TBitmap.LoadFromResourceName and .LoadFromResourceID which get the raw stream data from the resource, but loading the stream into the bitmap pixel array again leads to TBitmap.LoadFromStream.

Isn't that the heart/origin of the matter ?

In these methods the code knows what to expect, and can act accordingly. Imho LoadFromStream is not the proper place to be doing that. So if any would it not be a better solution to incorporate the insertion of the header in those methods ? Or better yet introduce a loadfromresource method that does that so that both methods can invoke it (write once, use many) ? That way the LoadFromStream method can be a normal/simple implementation again.

fwiw: I agree that the code you presented in your previous post does work and would be the proper way to go forward if there is currently no need for another solution. (afaik the presented code is still not 100% foolproof but it wasn't before either -> not your fault)

[wp]

There are TBitmap.LoadFromResourceName and .LoadFromResourceID which get the raw stream data from the resource, but loading the stream into the bitmap pixel array again leads to TBitmap.LoadFromStream.

Isn't that the heart/origin of the matter ?

In these methods the code knows what to expect, and can act accordingly. Imho LoadFromStream is not the proper place to be doing that. So if any would it not be a better solution to incorporate the insertion of the header in those methods ? Or better yet introduce a loadfromresource method that does that so that both methods can invoke it (write once, use many) ? That way the LoadFromStream method can be a normal/simple implementation again.

AFAIK that's not what the resource access routines are doing. When a RT_BITMAP resource is written it is written without the BitmapFileHeader, and when it is read, it is sent without it. I guess there are ancient reasons in the early days of Windows why this was done so, maybe saving some memory - a few bytes in those days were as precious as a few Megabytes now. Most important: The resource stream is not able to reconstruct those partial data, it does not "know" about the file structure of a bitmap or whatever data type is handled. Therefore, the correct place to fix this is within the receiving class, here: in the TBitmap.

Delphi does it in the same way. It also checks the 'BM' signature in the LoadFromStream method.

[TRon]

Delphi does it in the same way. It also checks the 'BM' signature in the LoadFromStream method.

So you were able to verify that Delphi is able to use LoadFromStream (by that I mean only LoadFromStream) to actually construct a valid bitmap file (as stored on disk) out of a resource stream so that it actually can be loaded by TBitmap (by using the rest of TBitmaps implemented methods/functions) ?

In case they did then :facepalm: and further discussion would be of no (practical) use.

[Bart]

Delphi does it in the same way. It also checks the 'BM' signature in the LoadFromStream method.

I hope you did not look at the sourceode of the Delphi implementation?

Bart

[wp]

Well, the formulation was a bit inaccurate: They do not "reconstruct" the header. There are two methods: Delphi's TBitmap.LoadFromResourceName calls LoadDIB which starts reading at the BitmapInfoHeader/BitmapCoreHeader (after the BitmapFileHeader), and TBitmap.LoadFromStream reads the BitmapFileHeader first and then continues with LoadDIB. As a consequence Delphi cannot use LoadFromStream to read RT_BITMAP resources provided by a ResourceStream - maybe this is intentional (because RT_BITMAP is "known" to provide an incomplete bitmap stream), but I tend to say that it is a bug because it makes the resourcetype parameter of the TResourceStream constructor obsolete, at least for bitmaps.

Lazarus, on the other hand, uses an internal "THeaderStream" class to fake (reconstruct) the presence of a TBitmapFileHeader for the main reading done by fcl-image.

Similar to Delphi's LoadDIB, the LCL provides also a TLazReaderDIB starting at the BitmapInfo/CoreHeader, as well as a descendant TLazReaderBMP reading the FileHeader first. At first sight it should be possible to adjust the TBitmap.GetReaderClass method to one of these two readers, depending on the caller. But GetReaderClass is a class method, and I don't know an easy way of telling this method whether the reader is needed for LoadFromStream or for LoadFromResourceName. And even if it could be done, it would leave us in the same situation as Delphi where RT_BITMAP resources cannot be read by LoadFromStream.

[TRon]

@Bart:
Don't worry too much. Most of that information that is/was presented can easily be obtained by running an executable through a debugger or disassembler. Unless you count that as "having seen the source-code". For me (but also for most if not all compilers) it is not the same as having looked at the original code. Usually a compiler allows the binary produced data to be distributed without it being copyrighted by the original author/copyright holder of the used compiler. And thank heavens for that as that would be an instant wet dream for all copyright trolls out there.

Well, the formulation was a bit inaccurate:

Yeah, sorry about that but since I can only check fpc's implementation the conclusion from what I've read is that it needs to add the header to the stream in order for the rest of the code to be able to properly load the resource from the stream because the fileheader is expected to exist. It is almost impossible to write that down without a whole lot of cringe feelings about that (the requirement to add something to raw data so that the rest of the code is able to properly process it *chills running down spine*).

They do not "reconstruct" the header.

Thank you very much for that detailed process of how Delphi seem to be doing things. That process makes a whole lot more sense to me as there is no need to add data to anything but just checking if something exist or not and the rest of the code acting accordingly (e.g. skip loading the fileheader but start processing the data that does exist (or is suppose to exist))

As a consequence Delphi cannot use LoadFromStream to read RT_BITMAP resources provided by a ResourceStream - maybe this is intentional (because RT_BITMAP is "known" to provide an incomplete bitmap stream), but I tend to say that it is a bug because it makes the resourcetype parameter of the TResourceStream constructor obsolete, at least for bitmaps.

I think we have a different opinion on that 

For me it makes absolute sense that loadfromstream is not able to load any other arbitrary data to construct a bitmap out of it. And I know I exaggerate with the example here but before you know it people expect to feed it a stream of jpeg data and for that data to be converted into a proper bitmap automagically because, why not ?

If you so must then you can use the designated loadfromresourceXXX functionality, and if that is not enough then perhaps introduce a loadfromresourcestream method that directly loads the resource data stream without the needs to use (fall back to) existing functionality ?

Btw, have you noticed the implementation of function CreateBitmapFromResourceName inside unit graphics ?

Lazarus, on the other hand, uses an internal "THeaderStream" class to fake (reconstruct) the presence of a TBitmapFileHeader for the main reading done by fcl-image.

Yeah I've noticed that Lazarus relies on the LoadFromStream method but not have looked at the readers in detail. fwiw I understand why it would have ended up in LoadFromStream as most (if not all) the other graph formats are loaded from a resource of type RT_DATA only and that is always the raw data as stored on disk.

But GetReaderClass is a class method, and I don't know an easy way of telling this method whether the reader is needed for LoadFromStream or for LoadFromResourceName. And even if it could be done, it would leave us in the same situation as Delphi where RT_BITMAP resources cannot be read by LoadFromStream.

afaik as it stands now only for LoadFromStream() /but/ LoadFromResourceName() depends on LoadFromStream() so indirectly for that function as well. That is the reason this issue exist in the first place.

I have not seen any (online) example in Delphi that directly loads a bitmap from a (bitmap)resource using LoadFromStream, Even embarcadero's examples (that are publicly available) uses the LoadFromResourceXXX functions in order to construct a bitmap out of a (bitmap) resource. So I personally would not regret it one bit if it wasn't possible (anymore) to load a bitmap resource using LoadFromStream() but that is a pure personal opinion on the matter. Just to avoid misunderstandings RT_DATA is ofc just raw data and can be loaded using LoadFromStream.

In one of my earlier replies I stated (I had it removed after I posted) to opt for overloading/reintroducing the LoadFromResourceXXX functionality to do it more in the style Delphi seems to be doing (ans I could now more or less verify it for as far as I was able to understand from your detailed descriptions) but that does have some implications such as not being able to use LoadFromStream for loading a bitmap resource anymore and some care needs to be taken for other formats that can be loaded by TBitmap and that supports loading from a resource other than RT_DATA. I haven't had time yet to look into that into more detail (which also requires some better understanding of the reader).

fwiw it is not about beating the dead horse and trying for someone (else) to revive that horse in case it isn't necessary. Your otherwise excellent solution works as expected/intended just not how I would have done it (and according to your findings Delphi doesn't seem to do so either).

[Bart]

@Bart:
Don't worry too much.

If wp did look at the source code, everything that is based upon his knowledge now is tainted and cannot be in LCL.
And if so, all references to this must also be removed from the bugtracker.
So yes, it is something to worry about.

(We've had similar problems in the past.)

Bart

[TRon]

If wp did look at the source code, everything that is based upon his knowledge now is tainted and cannot be in LCL.

You have a point there (including the consequences) except that I believe that wp wasn't born yesterday.

But even if he was then he is allowed to describe the process as long as he does not mention literal code (to us).

[wp]

Yes, I did look at the source. But I did not publish the code, I just described the basic steps which in my eyes are rather trivial, there are only a few possibilities to do this. If you now consider this thread as being tainted you should ban every user having access to Delphi from the Lazarus development.