Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility for Fl_PNG_Image, 4/8bit + alpha channel #234

Closed
rageworx opened this issue May 22, 2021 · 6 comments
Closed

Compatibility for Fl_PNG_Image, 4/8bit + alpha channel #234

rageworx opened this issue May 22, 2021 · 6 comments
Assignees
Labels
invalid This doesn't seem right wontfix This will not be worked on

Comments

@rageworx
Copy link

rageworx commented May 22, 2021

Dear, FLTK developers.

I've been found an issue for Fl_PNG_Image cannot read some PNG files of this condition with @fire-eggs for this issue ( rageworx/fl_imgtk#16 )

  • If PNG is grayscale image up to 8bit depth with has alpha channel ( transparency bits ),
  • Fl_PNG_Image recognized this image as 2 depth ( as RGB555, or RGB565 ).

So I tested with my own clonned Fl_PNG_Image to solve this issue here.

If alpha channel existed, array converting to RGBA, then process to each graphics driver.
My Fl_PNG_Image may not the best choice, but it should be make better to load variable PNG images.

Regards, Raph.K.

@Albrecht-S
Copy link
Member

Thanks for your report, I'll look into it...

@Albrecht-S
Copy link
Member

@rageworx: Thanks for your "heart" emoji. A first (preliminary) comment: FLTK supports grayscale with alpha channel, hence I think it wouldn't be necessary to convert all gray+alpha images to RGBA which makes them larger than necessary (in terms of memory usage). Gray + alpha (in FLTK images: d() == 2) would suffice (see below).

I didn't test with example "1 and 2 bpp" images yet but I'm going to test this later. I'll try to modify your patch accordingly.

FYI: There are currently 11 "gray+alpha" images in the FLTK source distribution which show that FLTK can load gray+alpha images, at least 8bpp:

$ file documentation/src/Fl_Light_Button.png 
documentation/src/Fl_Light_Button.png: PNG image data, 130 x 30, 8-bit gray+alpha, non-interlaced

I tried the test program you posted here as pngtest1.zip but this test program (built on Linux) seems to always issue warnings and errors (on all images I tested so far) like this;

$ pngtest documentation/src/Fl_Light_Button.png
> load file = documentation/src/Fl_Light_Button.png
> PNG version : 1.6.37
> color type = 00000004
> alpha channel existed
> tRNS ( transparency ) = 0
> color bit depth = 8
libpng warning: IDAT: bad parameters to zlib
libpng error: [00][00][00][00]: invalid chunk type

The "IDAT: bad parameters to zlib" warning seems to be always, but the error is different. I hoped this test would help but unfortunately it doesn't seem so. The errors seem to be like those you posted but they have nothing to do with FLTK (the test program is not using FLTK). Is there anything wrong with this test program?

Looks like I need to dive deeper into this...

PS: Full list of "gray+alpha" images in FLTK docs (file *.png|grep 'gray+alpha'):

adjuster1.png:         PNG image data, 125 x 120, 8-bit gray+alpha, non-interlaced
counter.png:           PNG image data, 210 x 115, 8-bit gray+alpha, non-interlaced
Fl_Light_Button.png:   PNG image data, 130 x 30, 8-bit gray+alpha, non-interlaced
Fl_Return_Button.png:  PNG image data, 160 x 30, 8-bit gray+alpha, non-interlaced
Fl_Roller.png:         PNG image data, 145 x 120, 8-bit gray+alpha, non-interlaced
Fl_Round_Button.png:   PNG image data, 148 x 29, 8-bit gray+alpha, non-interlaced
Fl_Value_Input.png:    PNG image data, 140 x 50, 8-bit gray+alpha, non-interlaced
Fl_Value_Output.png:   PNG image data, 135 x 50, 8-bit gray+alpha, non-interlaced
scrollbar.png:         PNG image data, 130 x 120, 8-bit gray+alpha, non-interlaced
tabs.png:              PNG image data, 309 x 209, 8-bit gray+alpha, non-interlaced
text.png:              PNG image data, 213 x 187, 8-bit gray+alpha, non-interlaced

I'm sure that FLTK can load and display all these images. All these images have d() == 2 after loading (just tested).

@Albrecht-S
Copy link
Member

Albrecht-S commented May 22, 2021

Okay, I think I found the "problem". See original post above:

Fl_PNG_Image recognized this image as 2 depth ( as RGB555, or RGB565 ).

@rageworx: This seems to be a misunderstanding on your part. In Fl_Image and derived classes image "depth" d() == 2 is clearly defined as 2 channels (gray + alpha), each channel one byte (8 bits), not - as you assume - "RGB555, or RGB565". Note: 1 = gray, 2 = gray+alpha, 3 = RGB, 4 = RGBA.

I used the image tbbn1g04_depth2.png posted by Kevin (@fire-eggs) for my tests:

$ file tbbn1g04_depth2.png 
tbbn1g04_depth2.png: PNG image data, 32 x 32, 4-bit grayscale, non-interlaced

which is clearly a 4-bit grayscale image with alpha (the latter is not displayed by file but is true). I added a few printf statements to your patched version which (IMHO) clearly shows that the patch is not correct. Besides there being no need to make a 4-channel image from a 2-channel image, you seem to multiply the byte values of the gray and alpha channels by mulb = (8/bd); which is obviously not correct:

Image channels = 2, num_trans = 1, convreq = 1
[load_png_] /path/to/tbbn1g04_depth2.png(w,h,d,bd) = (32, 32, 2, 4)
C[  19] = (170, 255),  mulb = 2
C[  20] = (170, 255),  mulb = 2
C[  50] = (153, 255),  mulb = 2
C[  51] = ( 51, 255),  mulb = 2
C[  52] = (102, 255),  mulb = 2
C[  80] = (153, 255),  mulb = 2
C[  81] = ( 68, 255),  mulb = 2
C[  82] = (  0, 255),  mulb = 2
C[  83] = (  0, 255),  mulb = 2
...

The code in question is (slightly modified):

    uchar* p = (uchar*)array;
    if (array) {
      int mulb = (8/bd);
      for (i=0; i<w()*h(); i++) {
        int temp1 = int(pvarray[i*2])*mulb;
        int temp2 = int(pvarray[i*2+1])*mulb;
        if (temp1 > 255 || temp2 > 255)
          printf("C[%4d] = (%3d, %3d),  mulb = %d\n", i, pvarray[i*2], pvarray[i*2+1], mulb);
        p[i*4] = p[i*4+1] = p[i*4+2] = (pvarray[i*2]*mulb);
        p[i*4+3] = (pvarray[i*2+1]*mulb);
      }
      channels = 4;
      d(channels);
    }

As you can see the channel values * mulb (2) would obviously be > 255 which is wrong.

Conclusion: FLTK's code is correct WRT handling bit depth 4 gray+alpha PNG images and your patch can't be accepted.

As a proof here's a screenshot of four images:

tbbn1g04_views

image

  • top left: FLTK original
  • top middle: firefox
  • top right: your patched version
  • bottom: eog (image viewer)

There are slight differences in brightness, maybe also caused by background diffs, but I'd say that your patched version is definitely the "worst" display and, as shown before, obviously not correct.

So, unless I missed something essential, FLTK is correct and this issue should be closed.

@rageworx
Copy link
Author

rageworx commented May 23, 2021

Dear @Albrecht-S , and @fire-eggs .

Thank you for all,
I was confused to Fl_BMP_Image 16bit ( 2depth ) conversion in this code,

case 16 : // 16-bit 5:5:5 or 5:6:5 RGB
.
I will rollback my repo to origin Fl_PNG_Image.cxx.

Thought all 2depth will be processed to RGB555 or RGB565 to RGB(3depth).
Plus, I will process Fl_RGB_Image to 2depth to gray+alpha in other tool kits ( fl_imgtk or others ).

Regards, Raph.K.

@rageworx
Copy link
Author

rageworx commented May 23, 2021

As you can see the channel values * mulb (2) would obviously be > 255 which is wrong.

Yes, you right, It wasn't tested as well, it's my fault. (and it will be back to previous codes)

  • top left: FLTK original

  • top middle: firefox

  • top right: your patched version

  • bottom: eog (image viewer)

Anyway, Firefox renders different way ... what's different, should we know ?
Or just defference from interpolated image ... ?

@Albrecht-S
Copy link
Member

Thanks for the feedback and for closing this issue.

Anyway, Firefox renders different way ... what's different, should we know ?

I tried it with Google Chrome and Gimp as well. Looks like Firefox is wrong here, making the image brighter than it should be and emphasizing the lighter border color. That's something I won't pursue (now) because the majority including FLTK renders the image very similar.

@Albrecht-S Albrecht-S self-assigned this May 23, 2021
@Albrecht-S Albrecht-S added invalid This doesn't seem right wontfix This will not be worked on labels May 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants