bugGNUstep - Bugs: bug #34740, gnustep has problems with PNGs...

Group
 
 

bug #34740: gnustep has problems with PNGs that have a sRGB colorspace

Submitter:  julian <julian_>
Submitted:  Thu 03 Nov 2011 06:40:29 PM UTC
   
 
Category:  Gui/AppKit Severity:  3 - Normal
Item Group:  Bug Status:  Fixed
Privacy:  Public Assigned to:  None
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 07 Nov 2011 05:47:16 PM UTC, comment #10: 


>another way, at least on cocoa,


ok thanks, maybe gnustep supports it some day ;)
for cocoa you can go even further and use coregraphics like apple suggests:

http://developer.apple.com/library/mac/documentation/GraphicsImaging/Conceptual/OpenGL-MacProgGuide/opengl_texturedata/opengl_texturedata.html#//apple_ref/doc/uid/TP40001987-CH407-SW31

>so anywhere in GNUstep it screws up, we have bugs


ok will try again and file bugs for all observed problems ;)

julian <julian_>
Mon 07 Nov 2011 02:20:09 AM UTC, comment #9: 


>thanks i can confirm that fixes the problem so this bug can be closed.

ok, cool.

>i can't use cairo. i want to deploy a binary-only with included gnustep as a
>precompiled package for all linux distributions. in this case the only thing i really
>care about is deployability = dependencies. i will therefore try again to get plain X
>backend working and file a bug about the font issue if it still exists.


ah, I see.

>thanks for the warning! actually i don't know a better way except perhaps using
>TIFFRepresentationUsingCompression:NSTIFFCompressionNone, and i believe i tried
>that and the results were not good...i think there was a issue with transparency. if
>you want i can retest this and look if there is bug there compared to cocoa. i believe
>in my case bitmapData (with a few sanity checks) isn't so bad anyway since i have
>complete control over the embedded gnustep version and all images that ever will
>get loaded.


yeah, it's probably fine especially given that you have control over GS.

another way, at least on cocoa, is create NSBitmapImageRep of the format you want to work with, and then call +[NSGraphicsContext graphicsContextWithBitmapImageRep:] to get a context which draws into that bitmap image rep, then draw on that context. Problem is, GNUstep doesn't support that method AFAIK. :-(

> 512x512 down to 1x1 embedded.


that is the ideal format for an app icon, so anywhere in GNUstep it screws up, we have bugs. i was aware of the about panel one (it will use the largest size, so you end up with a huge 512x512 image in the about panel, right?) but never got around to fixing it.

Eric Wasylishen <ericw>
Group Member
Sun 06 Nov 2011 11:59:42 PM UTC, comment #8: 


>It will now use exactly 72dpi (1 point = 1 pixel) if the pixels-per-meter value in the file is close to that.


thanks i can confirm that fixes the problem so this bug can be closed.

>not sure.. see if cairo fixes it.


i can't use cairo. i want to deploy a binary-only with included gnustep as a precompiled package for all linux distributions. in this case the only thing i really care about is deployability = dependencies. i will therefore try again to get plain X backend working and file a bug about the font issue if it still exists.

> the pixel data with -bitmapData is a dangerous idea unless you are prepared to check the bitmap format carefully


thanks for the warning! actually i don't know a better way except perhaps using TIFFRepresentationUsingCompression:NSTIFFCompressionNone, and i believe i tried that and the results were not good...i think there was a issue with transparency. if you want i can retest this and look if there is bug there compared to cocoa. i believe in my case bitmapData (with a few sanity checks) isn't so bad anyway since i have complete control over the embedded gnustep version and all images that ever will get loaded.

>calculating rowsize yourself is wrong becasue the framework


yes thanks. i actually tried bytesPerRow to see if the problem would go away when using it but didn't leave it in the production code since i was afraid it wouldn't work under cocotron/win32 path.

>comes from the resolution independence of NSImage


ok since AFAIK you were working on the resolution independence of NSImage i've got a question. i added a icon to my app some days ago and couldn't find any documentation regarding the recommended size or format of the icon (seems only base is really documented) and therefore added
1.) a tiff file with all sizes from 512x512 down to 1x1 embedded. this fucked up the icon in the taskbar, in gnusteps dock and in the about panel
2.) next i tried a tiff file with a single size 512x512. this just fucked up the about panel if i remember correctly
3.) i settled on a 64x64 tiff which finally worked fine ;)

is this known to be broken or not expected to work or should i file a bug there?

julian <julian_>
Sun 06 Nov 2011 10:49:05 PM UTC, comment #7: 

update: I modified the part of NSBitmapImageRep+PNG.m that reads the DPI. It will now use exactly 72dpi (1 point = 1 pixel) if the pixels-per-meter value in the file is close to that.

Eric Wasylishen <ericw>
Group Member
Sun 06 Nov 2011 10:11:42 PM UTC, comment #6: 

hey, i have a few comments which hopefully help:

regarding backends, you should probably use cairo as it is the best supported.
fonts should still display using xlib, though.

1.) loading PNGs with NSImage results in images that are 1 pixel smaller in both dimensions in some cases.

this is a nasty thing which is sort of my fault, and comes from the resolution independence of NSImage. first, the return value of -[NSImage size] is in points (1 point = 1/72 inch), not pixels. it is calculated from the pixel size of the png using the pHYs chunk, which specifies a relationship between the pixel size and physical units (centimetres). the source of the -[NSImage size] being smaller than the real pixel size comes from the fact that the PNG pHYs chunk can't express 72 DPI exactly, because it stores resolution as 'pixels per meter' in an unsigned integer.

so there are a few alternatives:
-we could put in a hack in the png loading code which check for a resolution close to 72dpi and rounds it to exactly 72dpi (thinking about it now, we should probably do this)
- you could remove the dpi metadata from the png's, and GNUstep will assume they are 72dpi (1 pixel = 1 point). you can remove it with imagemagick, and probably gimp.
-the simple answer is your code should probably use -[NSBitmapImageRep pixelsWide] and -pixelsHigh.

2) not sure.. see if cairo fixes it.

a final few comments, calling -[bitmap initWithFocusedViewRect:] and then accessing the pixel data with -bitmapData is a dangerous idea unless you are prepared to check the bitmap format carefully and be ready to bail out if it isn't what you expect.

oh, calculating rowsize yourself is wrong becasue the framework is allowed to pad rows so they are longer than necessary, to make them a multiple of the word size in length (presumably this makes accessing the memory faster). you should use -[bitmapRep bytesPerRow].

Eric Wasylishen <ericw>
Group Member
Sun 06 Nov 2011 09:25:26 PM UTC, comment #5: 

i am using the art backend. actually i'd like to use the plain x backend but i don't see any fonts when using it ... is this a known issue?

ok following your suggestions i added

 NSLog(@" bpp %i %i %i %i %i %i %i", bpp, (int)[theImg size].width, (int)[theImg size].height, (int)[bitmap size].width, (int)[bitmap size].height, (int)[bitmap bytesPerRow], (int)[bitmap bitsPerPixel]);

for the "broken" png this prints:

bpp 4 191 191 192 192 768 32

and in the good case

bpp 4 192 192 192 192 768 32

so we see that the problem is that:
1.) loading PNGs with NSImage results in images that are 1 pixel smaller in both dimensions in some cases like BUG.png and BUG2.png (thats the real bug)
2.) generating a NSBitmapImageRep via initWithFocusedViewRect can have larger dimensions than the passed rect (does the specification allow this? seems like a bug to me too)

 

julian <julian_>
Sun 06 Nov 2011 05:34:04 PM UTC, comment #4: 

Which GNUstep backend are you using? I remember that some time ago the art backend had a problem that the bitmap returned from initWithFocusedViewRect: could be slightly larger than expected. Something like this could be the case here. Although I don't quite see why this should happen.

An easy way to wrok around that issue is to use the actual width of bitmap when determining the rowsize and the same is of course true for height. There is no reason to expect that the generated bitmap will have exactly the same properties as the original one.

You should report these two values from both bitmaps and see if there is any difference.

Fred Kiefer <FredKiefer>
Group Member
Sun 06 Nov 2011 12:55:44 AM UTC, comment #3: 

it isn't related to the colorspace at all. i have a file here with the very same problem but it doesn't have a colorspace


julian <julian_>
Sat 05 Nov 2011 07:34:03 PM UTC, comment #2: 

sorry for the incomplete problem description.

i don't get crashes, but the pixel data obtained is distorted, see the attached screenshot. all distortions go away when using the same file without the colorspace.

i can confirm that the "problematic" PNGs display fine in image views, just obtaining their raw pixel data is wrong.

>The drawing of the rep in your example code should no longer be needed,
> as I added this after one of your other bug reports to our code.


yes thanks i have removed it now but the behavior regarding the sRGB PNGs is still the same.

i can still try to whip up a small sample exhibiting the issue if you want.


julian <julian_>
Sat 05 Nov 2011 05:29:05 PM UTC, comment #1: 

I don't quite understand what is the problem here?

Do you get a problem with the code you provided, e.g. a segmentation fault. Or is the result of this operation wrong (all zero?), or does it look wrong when further processed?

Displaying this image in an NSImageView looks just like having it displayed with any other image application. It could be that we handle the transparent part wrong, but I cannot determine this without further investigations.

The drawing of the rep in your example code should no longer be needed, as I added this after one of your other bug reports to our code. Could this doubled drawing be causing the problem?

Fred Kiefer <FredKiefer>
Group Member
Thu 03 Nov 2011 06:40:29 PM UTC, original submission:  

i've problem with gnustep and PNGs in sRGB colorspace.

i do not know if these pictures display fine e.g. in NSImageViews, but i know that obtaining raw pixel data from  sRGB-PNG => NSImage => NSBitmapImageRep => bitmapData is broken.

i am using the following code for extracting raw pixel data out of PNGs and it works fine for the attached picture OK.png (256x256) but not for the picture BUG.png (256x256, sRGB):


            NSImage *theImg = [[NSImage alloc] initWithContentsOfURL:_url];
            NSBitmapImageRep *bitmap = [NSBitmapImageRep alloc];
            NSSize imgSize = [theImg size];
            width = imgSize.width;
   height = imgSize.height;
            NSImageRep *rep = [theImg bestRepresentationForDevice: nil];
            [theImg lockFocus];
            [rep drawInRect:NSMakeRect(0,0,[theImg size].width, [theImg size].height)];
            [bitmap initWithFocusedViewRect:NSMakeRect(0.0, 0.0, imgSize.width, imgSize.height)];
            [theImg unlockFocus];
            int bpp = [bitmap bitsPerPixel] / 8;
            const int rowsize = width * bpp;
            _data = (char )calloc( rowsize height, 1);
            memcpy(_data, [bitmap bitmapData], width bpp height);


if you want i can create a more complete test-case e.g. a full application that loads two 2x2 PNGs but i am not sure this is necessary and it would take some time

julian <julian_>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #24309:  BUG2.png added by julian_ (86KiB - image/png)
file #24306:  distortion.png added by julian_ (150KiB - image/png)
file #24293:  OK.png added by julian_ (16KiB - image/png)
file #24294:  BUG.png added by julian_ (34KiB - image/png)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by ericw (Posted a comment)
  • -email is unavailable- added by FredKiefer (Posted a comment)
  • -email is unavailable- added by julian_ (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2011-11-07 ericw StatusNone Fixed
        Open/ClosedOpen Closed
    2011-11-06 julian_ Attached File- Added BUG2.png, #24309
    2011-11-05 julian_ Attached File- Added distortion.png, #24306
    2011-11-03 julian_ Attached File- Added OK.png, #24293
        Attached File- Added BUG.png, #24294

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code