BeRoPNG - A very tiny but complete PNG loader

[deathshadow]

What about the Vampyre Imaging Library?

... and it's massive (relying on the monstrous zlib library for png decode), slow (often painfully so), and on the whole completely unsuited to many projects because of it.

Take on windows for example, where a game I've been puttering around on making for a year or so just had it's executable size cut by two-thirds just by switching to this.

Could be worse though, could be SDLImage, were you need 200k in two DLL's just to unpack .png

Thanks BeRo -- this looks to be getting a lot of use from me.

I should clean up my BMP loader (with RLE4 and RLE8 support) and upload it sometime... sad part is I just ported my BMP loader to PHP as GD actually lacks BMP support. (crappy little one-bit WBMP is NOT Windows BMP!)

[deathshadow]

Uhm... one thing...

function Swap32(x:word):word;
begin
 result:=(Swap16(x and $ffff) shl 16) or Swap16((x and $ffff0000) shr 16);
end;

That doesn't make any sense. Shouldn't X be longword? Word and $FFFF0000 should always return zero on this. Also, if you are shifting right off a 32 bit integer by 16, there's no reason to do an AND on it.

I'd probably also hardcode the byte swaps -- the overhead of the function calls slows down each of these functions in turn.

Playing with checking ifdef cpui386 to add assembler optimized versions of your various routines...

{$ifdef cpui386}
function Swap16(x:word):word; assembler;
asm
	mov ax,x
	xchg ah,al { thankfully FPC/Delphi treat AX as the return value }
end;
{$else}
function Swap16(x:word):word;
begin
 result:=(x shl 8) + (x shr 8);
end;
{$endif}

(also another case where the "and" is unnecessary due to the bit width and pascal's strict typecasting.)

also playing with making a TP7 back-port as I'm still churning out DOS games, and PNG support would be really handy there.

[paul_nicholls]

I just realised that I have a problem

I have my LoadPNGFromFile() routine, and I wanted to make a LoadPNGFromStream() version...

The problem is that in the loading code below requires a pointer to a PNG file, and the length!!

function LoadPNGFromFile(FileName: String; Buffer: TImageBuffer32): Boolean;
var
  PNGFile  : TMemoryStream;
  PNGData  : Pointer;
  PNGWidth : Integer;
  PNGHeight: Integer;
  PNGAddr  : PRGBA;
  BufferRow: PRGBAArray;
  x,y      : Integer;
begin
  Result := False;

  if not FileExists(FileName) then Exit;

  PNGFile := TMemoryStream.Create;
  try
    PNGFile.LoadFromFile(FileName);
    Result := LoadPNG(PNGFile.Memory,PNGFile.Size,PNGData,PNGWidth,PNGHeight);
    if not Result then Exit;

    Buffer.SetSize(PNGWidth,PNGHeight);

    PNGAddr := PNGData;
    // copy bitmap pixels to buffer pixels
    for y := 0 to PNGHeight - 1 do
    begin
      BufferRow := Buffer.ScanLine[y];
      for x := 0 to PNGWidth - 1 do
      begin
        BufferRow^[x].r := PNGAddr^.r;
        BufferRow^[x].g := PNGAddr^.g;
        BufferRow^[x].b := PNGAddr^.b;
        BufferRow^[x].a := PNGAddr^.a;
        Inc(PNGAddr);
      end;
    end;
  finally
    PNGFile.Free;
  end;
end;

the loading code in the BeRo file uses the DataSize parameter as an indicator of when the PNG file has been read in, I would like to be able to change this so that the code knows when to stop without the user passing in the size.

Any ideas?

cheers,
Paul

[BeRo]

Any ideas?

cheers,
Paul

Just remove DataSize:longword; and the DataEnd checks from BeRoPNG (change "while (pansichar(DataPtr)+11)<pansichar(DataEnd) do begin
" to "while true do begin" and so on, but without this it's a security leak then. :-)

[paul_nicholls]

Just remove DataSize:longword; and the DataEnd checks from BeRoPNG (change "while (pansichar(DataPtr)+11)<pansichar(DataEnd) do begin
" to "while true do begin" and so on, but without this it's a security leak then. :-)

Thanks I think I might just leave it then haha

cheers,
Paul

[deathshadow]

Still, you're swap32 function should be longword, not word, no?

word and $FFFF0000 === 0 -- ALWAYS
word shl 16 === 0 -- ALWAYS.

So in effect:

result:=(Swap16(x and $ffff) shl 16) or Swap16((x and $ffff0000) shr 16);

is the same thing as saying

result:=0;

Which means on big-endian systems when your swap function is called, you'll get nothing but zero.

Also, you're sending zero to your swap16 function...

function Swap32(x:longword):longword;
begin
 result:=(Swap16(x and $ffff) shl 16) or Swap16(x shr 16);
end;

Is what you should have there. What you have right now won't work... Thankfully it's only called on big endian systems -- and you've probably only tested little endian.

It may also need typecasting forced, since swap16 returns word, not longword meaning that shl 16 could STILL result in zero.

[BeRo]

word and $FFFF0000 === 0 -- ALWAYS
word shl 16 === 0 -- ALWAYS.

Sorry, you're wrong. See my code again:

function Swap32(x:word):word;
begin
 result:=(Swap16(x and $ffff) shl 16) or Swap16((x and $ffff0000) shr 16);
end;

The first swap16 call gets "x and $ffff", and the result of it will shifted 16 bits to left, which's correct.
The second swap16 call gets "(x and $ffff0000) shr 16" or short "x shr 16", and the result of it will not shifted because it was already shifted 16 bits to right, which's also correct.

I guess, you had simply overlook the correct nested levels of the (...) blocks in my code.

[BeRo]

function Swap32(x:word):word;
begin
 result:=(Swap16(x and $ffff) shl 16) or Swap16((x and $ffff0000) shr 16);
end;

BeRoPNG should be portable and small (even at source code level) at the same time, so that it should containing no or only few inline assembler parts, but a good optimizing compiler with inline-support and good code peephole replace templates should optimize this with "bswap eax" on x86-32 and x86-64 (what Delphi&FPC does it not yet, but some C/C++ compilers. My big hope for this is the LLVM backend for FPC). The "and $ffff0000" is just for that it's more readable, the compiler will optimizing it out, but that is something you should know (as you probably already do).