After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 678252 - File-bmp: Color Space Information breaks BITMAPV5HEADER Structure
File-bmp: Color Space Information breaks BITMAPV5HEADER Structure
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: Plugins
git master
Other Linux
: Normal normal
: 2.8
Assigned To: GIMP Bugs
GIMP Bugs
: 676219 678250 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2012-06-17 14:59 UTC by Bastian Ilsø
Modified: 2012-10-08 02:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
bmp-write.c with Jigebren's changes applied (30.96 KB, patch)
2012-06-17 14:59 UTC, Bastian Ilsø
none Details | Review
Exported Results from Initial Changes (320.22 KB, application/zip)
2012-06-19 15:24 UTC, Bastian Ilsø
  Details
Patch for bmp-write.c which fixes writing 32-bit RGBA & RGBX Windows Bitmaps. (2.08 KB, patch)
2012-06-20 09:45 UTC, Bastian Ilsø
none Details | Review

Description Bastian Ilsø 2012-06-17 14:59:14 UTC
This bug report is derived from Bug 676219. The bug was initially found by Jigebren:

"- In the file
"http://git.gnome.org/browse/gimp/tree/plug-ins/file-bmp/bmp-write.c" inside
WriteBMP(), inside the "switch (BMPSaveData.rgb_format)" the "mask_info_size"
is not set for case "RGBA_8888". This is later preventing the writing of the
Mask info "if (mask_info_size > 0)", thus breaking the BITMAPV5HEADER structure
in case the color_space_data are also written.

So we should likely have:
        case RGBA_8888:
          BitsPerPixel = 32;
          mask_info_size = 16;
          break;
instead of:
        case RGBA_8888:
          BitsPerPixel = 32;
          break;

(Maybe this bad structure format could be a reason some apps fails reading a
BMP image with color space information saved from Gimp BTW...)"

I have attached the fix which Jigebren is suggesting above as a patch to this bug report.
Comment 1 Bastian Ilsø 2012-06-17 14:59:39 UTC
Created attachment 216605 [details] [review]
bmp-write.c with Jigebren's changes applied
Comment 2 jigebren 2012-06-19 05:18:24 UTC
The broken code above also prevents the use of BI_BITFIELDS format when a writing 32-bit BMP file, which later leads for example GDK to ignore the alpha channel (see Bug 678248).
So I confirm the necessity of the fix above...
Comment 3 Bastian Ilsø 2012-06-19 15:23:56 UTC
I have attached a zip file which shows the result of these changes alone ("changes1.bmp"). As you can see there is an alpha channel, but everything has turned pink and semi-transparent. This is the results regardless of whether colorspace information is written to the RGBA BMP or not.

The zip also contains a file called "changes2.bmp". It shows the result if the changes above + the changes you suggest in Bug 678250 is applied to bmp-write.c. The exported bmp is now blue and still semi-transparent.

The third file is called "no changes.bmp" and is created from a non-modified bmp-write.c.
Comment 4 Bastian Ilsø 2012-06-19 15:24:41 UTC
Created attachment 216756 [details]
Exported Results from Initial Changes
Comment 5 Michael Schumacher 2012-06-19 15:55:40 UTC
You should attach only the diff output of your changes as a patch, not the complete files. Please use either diff -u or git's diff command - the latter will also get you the proper attribution in the revision history if you add your name and mail address to your git setup.
Comment 6 Bastian Ilsø 2012-06-20 09:45:40 UTC
Created attachment 216810 [details] [review]
Patch for bmp-write.c which fixes writing 32-bit RGBA & RGBX Windows Bitmaps.
Comment 7 Bastian Ilsø 2012-06-20 09:48:04 UTC
*** Bug 678250 has been marked as a duplicate of this bug. ***
Comment 8 jigebren 2012-06-21 01:46:52 UTC
A few info about the patch submitted by Bastian above (attachment 216810 [details] [review]).

This path is a merge of the fix from bug 676219, comment 6 and bug 678250, comment 2. It fixes the following cases:
- 32 bits BMP with alpha channel (RGBA_8888) were saved using BI_RGB instead of BI_BITFIELDS format (those corrupted BMP files could not be properly read by some softwares - including GIMP).
- Exporting an image containing an alpha channel as a RGBX_8888 BMP was broken: channels were mixed in totally transparent areas.


BTW, just for info I notice that though RGBX / RGBA names are used in the source code, the actual channel layout (and the options in GIMP interface) is XRGB / ARGB (or BGRX / BGRA, I'm not 100% sure).
Comment 9 Mukund Sivaraman 2012-10-08 02:21:39 UTC
This bug is fixed in the following commits. Note that Firefox still has a bug that it does not use the masks when parsing BMPs. Reporters please verify the fix.

Resolving as FIXED, milestone 2.8.


In master:

commit 800f967930e360e86cbb15b6c4d1ae36443ac9c8
Author: Mukund Sivaraman <muks@banu.com>
Date:   Fri Sep 21 20:11:31 2012 +0530

    file-bmp: Fix order of data in RGBA_8888 images
    
    See #678250, #678252, etc. for example bug reports.
    
    This fix should be sufficient in fixing BMP output, but it looks
    like some apps like Firefox have broken BMP loaders which do not
    care for the masks. We would have to change the masks for them.

commit b100b14111725bf9ad7a48db096ae4ad65a81bc5
Author: Mukund Sivaraman <muks@banu.com>
Date:   Fri Sep 21 20:09:51 2012 +0530

    file-bmp: All 16 and 32-bpp files need the masks to be written
    
    See #678250, #678252, etc. for example bug reports.


In gimp-2-8:

commit b38d44220f2eb47a2e2e7584850a99874242961b
Author: Mukund Sivaraman <muks@banu.com>
Date:   Fri Sep 21 20:11:31 2012 +0530

    file-bmp: Fix order of data in RGBA_8888 images
    
    See #678250, #678252, etc. for example bug reports.
    
    This fix should be sufficient in fixing BMP output, but it looks
    like some apps like Firefox have broken BMP loaders which do not
    care for the masks. We would have to change the masks for them.

commit 2af0e143706128842cae71850f3aa2a3cd418bf7
Author: Mukund Sivaraman <muks@banu.com>
Date:   Fri Sep 21 20:09:51 2012 +0530

    file-bmp: All 16 and 32-bpp files need the masks to be written
    
    See #678250, #678252, etc. for example bug reports.
Comment 10 Mukund Sivaraman 2012-10-08 02:24:59 UTC
*** Bug 676219 has been marked as a duplicate of this bug. ***