First: should i open a bug for this? The problem is from the new libpng 1.4.0: this evening i just updated this lib, and now i get compile error in dlls/windowscodecs/pngformat.c I'm still investigating the issue to find a solution, but the change that introduced this is clear, directly from their README: Removed deprecated functions png_read_init, png_write_init, png_info_init, png_permit_empty_plte, png_set_gray_1_2_4_to_8, and removed the deprecated macro PNG_MAX_UINT.
So, what should WINE do? I see two actions that need to be performed: 1- check the libpng version in configure 2- adjust pngformat.c to work with the new lib.
For a short term solution, needed also in the long term one (in the beginning), configure should give error if libpng isn't < 1.4.0 In the long term, while distros begin to ship libpng 1.4.0, the main developers of windowscodecs should work out a code solution. What is that i cannot say, since i don't really know the code, but i think it won't be a few minutes of work...
This could apply also to libjpeg since it bumped to 8.0, but i didn't try that part yet.
On Tue, Feb 2, 2010 at 8:46 PM, Luca Bennati lucak3@gmail.com wrote:
First: should i open a bug for this? The problem is from the new libpng 1.4.0: this evening i just updated this lib, and now i get compile error in dlls/windowscodecs/pngformat.c I'm still investigating the issue to find a solution, but the change that introduced this is clear, directly from their README: Removed deprecated functions png_read_init, png_write_init, png_info_init, png_permit_empty_plte, png_set_gray_1_2_4_to_8, and removed the deprecated macro PNG_MAX_UINT.
We don't seem to use any of the functions / macros you mentioned. It would be best to figure out what is wrong by checking the compile log. We might be able to run using both 1.2 and 1.4 if the functions we use have stayed the same.
BTW what distribution are you using? Does libpng now link to libpng14.so (previously it linked to libpng12.so).
Roderick
We use png_set_gray_1_2_4_to_8 to expand grayscale PNG's to an RGB format.
It'd be nice to have libpng do this (if there's a non-deprecated way to do it), but it's a trivial operation that we could do without if necessary.
2010/2/3 Vincent Povirk <madewokherd+8cd9@gmail.commadewokherd%2B8cd9@gmail.com
We use png_set_gray_1_2_4_to_8 to expand grayscale PNG's to an RGB format.
It'd be nice to have libpng do this (if there's a non-deprecated way to do it), but it's a trivial operation that we could do without if necessary.
The error is: ccache gcc -c -I. -I. -I../../include -I../../include -D__WINESRC__ -D_REENTRANT -fPIC -Wall -pipe -fno-strict-aliasing -Wdeclaration-after-statement -Wstrict-prototypes -Wtype-limits -Wwrite-strings -Wpointer-arith -g -O2 -o pngformat.o pngformat.c pngformat.c:62: error: ‘png_set_gray_1_2_4_to_8’ undeclared here (not in a function) pngformat.c:62: warning: type defaults to ‘int’ in declaration of ‘ppng_set_gray_1_2_4_to_8’ pngformat.c: In function ‘PngDecoder_Initialize’: pngformat.c:285: error: called object ‘ppng_set_gray_1_2_4_to_8’ is not a function
The distro i'm using is ArchLinux i686, that just shipped the new libraries with massive rebuilds. I already resolved the problem, as the only thing to do was search all "png_set_gray_1_2_4_to_8" and change them to "png_set_expand_gray_1_2_4_to_8" i already wrote a patch for this that make compilation works again, but now as appeared another problem: LD_LIBRARY_PATH="../../libs/wine:$LD_LIBRARY_PATH" ../../tools/wrc/wrc --nostdinc -I. -I. -I../../include -I../../include -D__WINESRC__ -foversion.res version.rc ../../tools/winegcc/winegcc -B../../tools/winebuild --sysroot=../.. -shared ./windowscodecs.spec bmpdecode.o bmpencode.o clsfactory.o converter.o gifformat.o icoformat.o imgfactory.o info.o jpegformat.o main.o palette.o pngformat.o propertybag.o regsvr.o stream.o ungif.o version.res -o windowscodecs.dll.so -luuid -lole32 -ladvapi32 -lkernel32 ../../libs/port/libwine_port.a pngformat.o: In function `PngFrameEncode_Commit': /home/luca/wine-git/dlls/windowscodecs/pngformat.c:987: undefined reference to `png_set_longjmp_fn' pngformat.o: In function `PngEncoder_Initialize': /home/luca/wine-git/dlls/windowscodecs/pngformat.c:1119: undefined reference to `png_set_longjmp_fn' pngformat.o: In function `PngFrameEncode_WritePixels': /home/luca/wine-git/dlls/windowscodecs/pngformat.c:861: undefined reference to `png_set_longjmp_fn' pngformat.o: In function `PngDecoder_Initialize': /home/luca/wine-git/dlls/windowscodecs/pngformat.c:252: undefined reference to `png_set_longjmp_fn' collect2: ld returned 1 exit status
I will now investigate this new one
BTW i confirmed that the libjpeg 7 -> 8 bump doesn't bring problem :)
2010/2/3 Luca Bennati lucak3@gmail.com
LD_LIBRARY_PATH="../../libs/wine:$LD_LIBRARY_PATH" ../../tools/wrc/wrc --nostdinc -I. -I. -I../../include -I../../include -D__WINESRC__ -foversion.res version.rc ../../tools/winegcc/winegcc -B../../tools/winebuild --sysroot=../.. -shared ./windowscodecs.spec bmpdecode.o bmpencode.o clsfactory.o converter.o gifformat.o icoformat.o imgfactory.o info.o jpegformat.o main.o palette.o pngformat.o propertybag.o regsvr.o stream.o ungif.o version.res -o windowscodecs.dll.so -luuid -lole32 -ladvapi32 -lkernel32 ../../libs/port/libwine_port.a pngformat.o: In function `PngFrameEncode_Commit': /home/luca/wine-git/dlls/windowscodecs/pngformat.c:987: undefined reference to `png_set_longjmp_fn' pngformat.o: In function `PngEncoder_Initialize': /home/luca/wine-git/dlls/windowscodecs/pngformat.c:1119: undefined reference to `png_set_longjmp_fn' pngformat.o: In function `PngFrameEncode_WritePixels': /home/luca/wine-git/dlls/windowscodecs/pngformat.c:861: undefined reference to `png_set_longjmp_fn' pngformat.o: In function `PngDecoder_Initialize': /home/luca/wine-git/dlls/windowscodecs/pngformat.c:252: undefined reference to `png_set_longjmp_fn' collect2: ld returned 1 exit status
I will now investigate this new one
I didn't find a clear answer as code seemed ok, so i tried to issue the command: ../../tools/winegcc/winegcc -B../../tools/winebuild --sysroot=../.. -shared ./windowscodecs.spec bmpdecode.o bmpencode.o clsfactory.o converter.o gifformat.o icoformat.o imgfactory.o info.o jpegformat.o main.o palette.o pngformat.o propertybag.o regsvr.o stream.o ungif.o version.res -o windowscodecs.dll.so -luuid -lole32 -ladvapi32 -lkernel32 ../../libs/port/libwine_port.a adding a simple -lpng: ../../tools/winegcc/winegcc -B../../tools/winebuild --sysroot=../.. -shared ./windowscodecs.spec bmpdecode.o bmpencode.o clsfactory.o converter.o gifformat.o icoformat.o imgfactory.o info.o jpegformat.o main.o palette.o pngformat.o propertybag.o regsvr.o stream.o ungif.o version.res -o windowscodecs.dll.so -luuid -lole32 -ladvapi32 -lkernel32 -lpng ../../libs/port/libwine_port.a and it magically works ok
so the previous patch just has to add png to the imports in Makefile.in to make it work Should i send the now correct patch?
On Wed, Feb 3, 2010 at 11:57 AM, Luca Bennati lucak3@gmail.com wrote:
2010/2/3 Luca Bennati lucak3@gmail.com
LD_LIBRARY_PATH="../../libs/wine:$LD_LIBRARY_PATH" ../../tools/wrc/wrc --nostdinc -I. -I. -I../../include -I../../include -D__WINESRC__ -foversion.res version.rc ../../tools/winegcc/winegcc -B../../tools/winebuild --sysroot=../.. -shared ./windowscodecs.spec bmpdecode.o bmpencode.o clsfactory.o converter.o gifformat.o icoformat.o imgfactory.o info.o jpegformat.o main.o palette.o pngformat.o propertybag.o regsvr.o stream.o ungif.o version.res -o windowscodecs.dll.so -luuid -lole32 -ladvapi32 -lkernel32 ../../libs/port/libwine_port.a pngformat.o: In function `PngFrameEncode_Commit': /home/luca/wine-git/dlls/windowscodecs/pngformat.c:987: undefined reference to `png_set_longjmp_fn' pngformat.o: In function `PngEncoder_Initialize': /home/luca/wine-git/dlls/windowscodecs/pngformat.c:1119: undefined reference to `png_set_longjmp_fn' pngformat.o: In function `PngFrameEncode_WritePixels': /home/luca/wine-git/dlls/windowscodecs/pngformat.c:861: undefined reference to `png_set_longjmp_fn' pngformat.o: In function `PngDecoder_Initialize': /home/luca/wine-git/dlls/windowscodecs/pngformat.c:252: undefined reference to `png_set_longjmp_fn' collect2: ld returned 1 exit status
I will now investigate this new one
I didn't find a clear answer as code seemed ok, so i tried to issue the command: ../../tools/winegcc/winegcc -B../../tools/winebuild --sysroot=../.. -shared ./windowscodecs.spec bmpdecode.o bmpencode.o clsfactory.o converter.o gifformat.o icoformat.o imgfactory.o info.o jpegformat.o main.o palette.o pngformat.o propertybag.o regsvr.o stream.o ungif.o version.res -o windowscodecs.dll.so -luuid -lole32 -ladvapi32 -lkernel32 ../../libs/port/libwine_port.a adding a simple -lpng: ../../tools/winegcc/winegcc -B../../tools/winebuild --sysroot=../.. -shared ./windowscodecs.spec bmpdecode.o bmpencode.o clsfactory.o converter.o gifformat.o icoformat.o imgfactory.o info.o jpegformat.o main.o palette.o pngformat.o propertybag.o regsvr.o stream.o ungif.o version.res -o windowscodecs.dll.so -luuid -lole32 -ladvapi32 -lkernel32 -lpng ../../libs/port/libwine_port.a and it magically works ok
so the previous patch just has to add png to the imports in Makefile.in to make it work Should i send the now correct patch?
We are loading png dynamically and we want to keep doing that. Figure out where the longjmp call comes from (perhaps it is some macro or so in the png code). We might have to dynamically load the function..
Regarding that gray conversion function. The only thing that changed is the name of the function the prototype and its behavior looks similar. I'm not sure how we want to fix it. For instance it can be fixed at compile time by checking the png version (I believe png.h defines it) or it can be done at runtime by checking if the old function is around and if it isn't try to load the new function.
Roderick
2010/2/3 Roderick Colenbrander thunderbird2k@gmail.com
We are loading png dynamically and we want to keep doing that. Figure out where the longjmp call comes from (perhaps it is some macro or so in the png code). We might have to dynamically load the function..
Regarding that gray conversion function. The only thing that changed is the name of the function the prototype and its behavior looks similar. I'm not sure how we want to fix it. For instance it can be fixed at compile time by checking the png version (I believe png.h defines it) or it can be done at runtime by checking if the old function is around and if it isn't try to load the new function.
Roderick
Ok, i will investigate more on libpng loading
As for the gray conversion functions, the ones used by windowscodecs were deprecated since long time (it seems from their readme), so changing their name shouldn't break builds with libpng 1.2.x
Could you test the attached patch? It builds with libpng 1.2 and seems to work, but I'd like to verify that it also solves the problems with 1.4.
If it's not too much trouble, I'd like you to also test the functionality.
There's a program at http://code.google.com/p/winezeug/source/browse/trunk/appinstall/tools/image... that tests the ability to load images. It can be invoked as "wine imagetest.exe filename.png".
In particular, I'd like to know whether the following two files from the png test suite (http://www.libpng.org/pub/png/pngsuite.html) work:
* http://www.libpng.org/pub/png/PngSuite/basn4a16.png - This is a grayscale png with an alpha channel. That's the situation where we use the png_set_gray... function that was removed.
* http://www.libpng.org/pub/png/PngSuite/x00n0g01.png - This is a broken png file. It will trigger an error. The program should print "GdipLoadImageFromFile returned 1" when loading this file (rather than crashing).
grayscale png with an alpha channel. That's the situation where we use the png_set_gray... function that was removed.
Errr.. I may actually be thinking of http://www.libpng.org/pub/png/PngSuite/tbbn1g04.png
2010/2/4 Vincent Povirk <madewokherd+8cd9@gmail.commadewokherd%2B8cd9@gmail.com
grayscale png with an alpha channel. That's the situation where we use the png_set_gray... function that was removed.
Errr.. I may actually be thinking of http://www.libpng.org/pub/png/PngSuite/tbbn1g04.png
Those two show correctly, while the other doesn't work, but it's the image's fault, as it's only 49 B and can't be open even in other programs. As a side note, you need to do a similar work on winemenubuilder, as it uses libpng and fails linking without modifications. to test this images i statically linked it to libpng, just to make it work, as it wasn't needed in your test. Thank you for working on this, since i'm not really an expert on this dynamical loading code and couldn't help.
As a side note, you need to do a similar work on winemenubuilder, as it uses libpng and fails linking without modifications. to test this images i statically linked it to libpng, just to make it work, as it wasn't needed in your test. Thank you for working on this, since i'm not really an expert on this dynamical loading code and couldn't help.
Technically, winemenubuilder should be using windowscodecs for its png support, like the rest of wine. I believe Steven Edwards has done some work on this.
2010/2/4 Vincent Povirk <madewokherd+8cd9@gmail.commadewokherd%2B8cd9@gmail.com
As a side note, you need to do a similar work on winemenubuilder, as it
uses
libpng and fails linking without modifications. to test this images i statically linked it to libpng, just to make it work, as it wasn't needed
in
your test. Thank you for working on this, since i'm not really an expert on this dynamical loading code and couldn't help.
Technically, winemenubuilder should be using windowscodecs for its png support, like the rest of wine. I believe Steven Edwards has done some work on this.
Don't know about this. What i know is that currently winemenubuilder shows this problem and it's needed to be worked out if you want to support builds on libpng 1.4.
Hi,
On Thu, Feb 4, 2010 at 7:08 PM, Luca Bennati lucak3@gmail.com wrote:
Technically, winemenubuilder should be using windowscodecs for its png support, like the rest of wine. I believe Steven Edwards has done some work on this.
Don't know about this. What i know is that currently winemenubuilder shows this problem and it's needed to be worked out if you want to support builds on libpng
I was looking in to the overall architectural issues but have not had the time to really hack on it recently. As far as I understand, the current png support in wine's windowscodecs does not support creation of a 32bpp image with transparency, unless I missed some commit messages or totally misunderstood our discussions. I believe from my reading native does support doing the conversions, saving the alpha channel information, applying the masking, etc.
I was working on a patch for winemenubuilder so we would at least have that part part done, even if it only spit out 24bpp images without transparency but never got very far with it. If I recall correctly, the current code parses the raw ico bitmapinfo, passes that to libpng, applies the mask and generates the resulting image. I tried a few different ways, using WIC to open a stream to the icon that was then parsed by the icon decoder feeding the png encoder to generate the image. I also tried using the existing parsing system, feeding that to the bitmap decoder then the png encoder and never could get it to work quite right. With the patch I had, the resulting png image was corrupt possibly due to some RGB/BGR issue, I seem to recall Vincent mentioning something about that as well.
Also I don't understand all of the magic involved in these operations so I got kind of stalled on it until I am motivated to try to merge my MacOS Application Bundle patch for OS X again. For my own education I want to try to write a test for Windows that calls GdiAlphaBlend on a bitmap image, applies the mask and then uses WIC do the same operation and compare do a comparison. Something could be done based on existing WIC tests.
If someone with better understanding feels motivated to do the legwork in WIC, winemenubuilder can be fixed in pretty short order. There is some redundancy in the code because there is an old code path for XPM generation if libpng fails. I don't know if Alexandre really cares about the XPM case, I'd just like to remove it. He did agree that using WIC was the way to go.
Just about everything that supports xpm supports png these days. I wasted a lot of time going through mental cycles thinking about how to abstract the icon creation since on OS X (at least on leopard) the icon format used by application bundles is icns not png, so we would either need to have a special case for OS X anyway. I don't think making a generic interface for corner cases like this should be a show stopper. From what I gathered, WIC has this concept of user defined image codecs. I suspect if we suddenly found people bitching at the lack of XPM fallback support, we COULD go through the trouble of being purists and writing a WIC decoder for these non-standard formats but then again, this app is not meant to run on Windows ever anyway so what's the point of being purists?
If we want to keep a XPM fallback using the old method, it would be simple enough. As far as the icns case goes on OS X, it would also take a lot less code to just call sips on the resulting png to have it do the conversion to icns for us. I am not even sure if this step is even really required on Snow Leopard, it may actually support using png's for appbundles. When I last looked at the Application Bundle documentation it implied that PNG was a supported format but was not clear if it was limited to the iphone OS. Since my last round of RFCs, submissions and groveling for comment on the direction I was going with Application bundles was met with general silence, I've lost interest again for a while. I'll pick it up again at some point and
So again, since this entire email has most likely exceeded the total LOC in required, the simple answer is, I think, even if we fix winemenubuilder to use WIC now, it would result in reduced quality icons until WIC is enhanced due to lack of 32bpp with alpha channel for transparency.
Thanks
I was looking in to the overall architectural issues but have not had the time to really hack on it recently. As far as I understand, the current png support in wine's windowscodecs does not support creation of a 32bpp image with transparency, unless I missed some commit messages or totally misunderstood our discussions. I believe from my reading native does support doing the conversions, saving the alpha channel information, applying the masking, etc.
It's supported 32bpp with transparency from the start. The full list of supported writing formats is here: http://source.winehq.org/source/dlls/windowscodecs/pngformat.c#L680
GUID_WICPixelFormat32bppBGRA is the format you want, I think.
You probably tried to use a format not on that list, which defaults to 24-bit. I'm not sure what the behavior should be in that case.
I was working on a patch for winemenubuilder so we would at least have that part part done, even if it only spit out 24bpp images without transparency but never got very far with it. If I recall correctly, the current code parses the raw ico bitmapinfo, passes that to libpng, applies the mask and generates the resulting image. I tried a few different ways, using WIC to open a stream to the icon that was then parsed by the icon decoder feeding the png encoder to generate the image. I also tried using the existing parsing system, feeding that to the bitmap decoder then the png encoder and never could get it to work quite right. With the patch I had, the resulting png image was corrupt possibly due to some RGB/BGR issue, I seem to recall Vincent mentioning something about that as well.
That's odd. Both WIC and winemenubuilder appear to be using BGR for 32-bit pixel formats and informing libpng of this . I don't know where you could be getting RGB pixels.
On Fri, Feb 5, 2010 at 9:54 AM, Vincent Povirk madewokherd+8cd9@gmail.com wrote:
It's supported 32bpp with transparency from the start. The full list of supported writing formats is here: http://source.winehq.org/source/dlls/windowscodecs/pngformat.c#L680
GUID_WICPixelFormat32bppBGRA is the format you want, I think.
You probably tried to use a format not on that list, which defaults to 24-bit. I'm not sure what the behavior should be in that case.
Thanks, I'll take another look.
That's odd. Both WIC and winemenubuilder appear to be using BGR for 32-bit pixel formats and informing libpng of this . I don't know where you could be getting RGB pixels.
It was just a thought, as I said, my understand of the graphics manipulation is pretty limited, I just assumed that was the problem due to resulting images. I'll try to give it another shot with this information.
Thanks
2010/2/5 Vincent Povirk <madewokherd+8cd9@gmail.commadewokherd%2B8cd9@gmail.com
It's supported 32bpp with transparency from the start. The full list of supported writing formats is here: http://source.winehq.org/source/dlls/windowscodecs/pngformat.c#L680
GUID_WICPixelFormat32bppBGRA is the format you want, I think.
You probably tried to use a format not on that list, which defaults to 24-bit. I'm not sure what the behavior should be in that case.
That's odd. Both WIC and winemenubuilder appear to be using BGR for 32-bit pixel formats and informing libpng of this . I don't know where you could be getting RGB pixels.
I didn't notice earlier, but you sent in a fix for winemenubuilder. I tested compilation with it and it works fine. Thank you for your attention on this problem.
png_jmpbuf used to be a macro that accessed fields of png structures directly. In 1.4.0 it was apparently changed to call png_set_longjmp_fn.
This is probably because the png developers hate that programmers can directly access fields in their structures. As I recall, they whine about this frequently in their manual.
Unfortunately, png_set_longjmp_fn was introduced in 1.4.0. As far as I can tell, there is no function present in both versions that we could call to do this, and of course we can't access the field directly in 1.4.
Maybe we could use png_set_error_fn instead and do the longjmp in the error function? Does that make sense?
-
Luca Bennati -
Roderick Colenbrander -
Steven Edwards -
Vincent Povirk