Opened 3 years ago

Closed 7 months ago

Last modified 7 months ago

#13650 closed defect (fixed)

Unable to load BMP file - image.cpp(1570) in wxImage::GetData(): invalid image

Reported by: ssbarnea Owned by: disc
Priority: low Milestone:
Component: GUI-all Version: dev-latest
Keywords: wxBitmap wxDib DIB Cc:
Blocked By: Blocking:
Patch: yes

Description

The attached BMP file fails to load giving:

"C:\Python27\lib\site-packages\wx-2.8-msw-unicode\wx\_core.py", line 2653, in init

_core_.Image_swiginit(self,_core_.new_Image(*args, kwargs))

PyAssertionError: C++ assertion "Ok()" failed at ..\..\src\common\image.cpp(1570) in wxImage::GetData(): invalid image

The same BMP file loads successfully in Paint, Paint.NET, GDI+ and ImageMagick.

Attachments (5)

sample.zip download (95.1 KB) - added by ssbarnea 3 years ago.
sample.png download (149.5 KB) - added by troelsk 3 years ago.
Win7 trunk screenshot, of sample.bmp
bitmap-load.patch download (4.4 KB) - added by awi 7 months ago.
Patch to fix loading of bitmaps.
images-loaded.png download (122.3 KB) - added by awi 7 months ago.
Screenshot of loaded bitmaps.
bitmap-load_v2.patch download (871 bytes) - added by awi 7 months ago.
Patch to fix loading of bitmaps (v2).

Download all attachments as: .zip

Change History (23)

Changed 3 years ago by ssbarnea

comment:1 Changed 3 years ago by disc

  • Component changed from wxPython to GUI-all
  • Owner set to disc
  • Status changed from new to accepted

This was previously fixed in wx' trunk. A recent wxPython 2.9 should also contain the fix. I will backport the change to the 2.8 branch of wx.

comment:2 Changed 3 years ago by DS

  • Resolution set to fixed
  • Status changed from accepted to closed

(In [69732]) Backport of r68766 (without the unit test changes).

(
Improved BMP decoding.

The BMP decoder did not handle images that are not stored upside down but straight up (in which case the height is negative). Also with RLE4 or RLE8 compressed images the 'end of scanline' RLE marker was not handled correctly. Fixed the issues and added a unit test for them.
)

Fixes #13650.

comment:3 Changed 3 years ago by troelsk

Not fixed. If opening sample.bmp with Ctrl+O in the image sample it comes up wrong, all grey, on Windows. In short, bmp support is incomplete. See also trac.wxwidgets.org/ticket/3433 "Add support for v1 bitmap files to wxBMPHandler"

comment:4 Changed 3 years ago by disc

That's also how I tested it and before it didn't load at all and now it does. What you are seeing is a result of the (quite awkward) alpha channel of this image. With 2.9 it does display correctly but I think this is related to a blitting problem with 2.8 (or a 'feature' of the sample in 2.8 when using images with alpha), not image decoding. Try covering and uncovering the window, here the background is not refreshed. Tested with XP.

comment:5 Changed 3 years ago by troelsk

Yes it loads, but no, not correctly, in trunk. Comes up all grey, on Win7. Didn't try stable.

comment:6 Changed 3 years ago by disc

For this ticket I didn't make any changes in trunk, they were in stable (it was a backport of a trunk revision though, handling negative heights specified in the BMP).
Just to be sure, by "all grey" you mean completely grey and don't see anything of the image, right? I'll try to reproduce with Windows 7 (and the tip of the trunk).

Changed 3 years ago by troelsk

Win7 trunk screenshot, of sample.bmp

comment:7 Changed 3 years ago by troelsk

Attachment, "Win7 trunk screenshot, of sample.bmp", on Win7

comment:8 Changed 3 years ago by disc

That's what I'm seeing here. Given the alpha in the image and the background colour of the window this is what I expect. The result is not nice and looking at the (again) awkward alpha channel I'd think it might have been encoded wrong, but we can't determine that with this kind of image AFAICS (unlike when specifying 32 bits/pixel and all alpha pixels being transparent). In this case the best one can do is manually remove the alpha from the wxImage I guess.

comment:9 Changed 3 years ago by troelsk

Just tried with stable. Fair enough, the image is broken. But now it can be opened and displayed at least, thanks.

comment:10 Changed 3 years ago by ssbarnea

  • Resolution fixed deleted
  • Status changed from closed to reopened

I reopened the bug in order to fully fix the issue (properly load 32bpp images).

I'm far from an expert in BMP format but I found these two interesting links: * http://www.imagemagick.org/discourse-server/viewtopic.php?t=8518#p49137 * http://msdn.microsoft.com/en-us/library/dd183376(VS.85).aspx - The right info source regarding BMP format (as opposed to fileformat.info)

Quoting: The bitmap has a maximum of 232 colors. If the biCompression member of the BITMAPINFOHEADER is BI_RGB, the bmiColors member of BITMAPINFO is NULL. Each DWORD in the bitmap array represents the relative intensities of blue, green, and red, respectively, for a pixel. The high byte in each DWORD is not used.

Am I wrong or WX is does not do that?

comment:11 follow-up: Changed 3 years ago by ssbarnea

I checked the image and the pixels and here is the summary:

  • biWidth is negative
  • biPlanes = 1
  • biBitCount = 32
  • biCompression = 0
  • biSizeImage= 0
  • biClrUsed = 0
  • biClrImportant = 0

Pixels are stored starting from top-left corner using BGRA, BGRA, ... format with the remark that, for the above header, the loader must ignore the values of "A" channel. The test image contains information on this channel but based on the file header it must not treat it as transparency.

Or we can describe the issue differently, officially BMP files do not support transparency so any image loaded should ignore the 4th hannel in 32bpp BMP files (still it could be useful to be able to load it if you really want, but by default it should be ignored).

comment:12 Changed 3 years ago by ssbarnea

  • Keywords Windows added

comment:13 in reply to: ↑ 11 Changed 3 years ago by disc

Replying to ssbarnea:

I checked the image and the pixels and here is the summary:

  • biWidth is negative
  • biPlanes = 1
  • biBitCount = 32
  • biCompression = 0
  • biSizeImage= 0
  • biClrUsed = 0
  • biClrImportant = 0

Pixels are stored starting from top-left corner using BGRA, BGRA, ... format with the remark that, for the above header, the loader must ignore the values of "A" channel. The test image contains information on this channel but based on the file header it must not treat it as transparency.

What would a file header look like that shouldn't have its transparency ignored? AFAIK it looks the same as the above one (though width is always positive, as with this sample.bmp. It's the height that can be negative but this is not related to alpha).

Or we can describe the issue differently, officially BMP files do not support transparency so any image loaded should ignore the 4th hannel in 32bpp BMP files (still it could be useful to be able to load it if you really want, but by default it should be ignored).

You can already do the opposite and ignore it afterwards (using ClearAlpha() in 2.9 and making a copy or setting all alpha values to opaque in 2.8). I don't think the current behaviour should change anymore at this point (there's also software writing BMP with alpha, notably Photoshop), and if it would then only in 2.9.

Changed 7 months ago by awi

Patch to fix loading of bitmaps.

Changed 7 months ago by awi

Screenshot of loaded bitmaps.

comment:14 Changed 7 months ago by awi

  • Keywords wxBitmap wxDib DIB added; bmp Windows removed
  • Patch set
  • Version changed from 2.8.x to dev-latest

Problematic bitmap (sample.bmp) cannot be loaded into wxBitmap because its height has a negative value. This is a legitimate value but apparently LoadImage() Win API which is used (in wxDIB::Load) to load images into wxBitmap doesn't handle top-down DIBs.
Such DIBs must be loaded another way - patch attached.

Loaded image is very pale because it is very transparent - it contains valid transparency data with alpha values generally < 10. If alpha channel would be removed that it looks like in any other graphic tool (it seems that graphic programs (like Paint, IrfanView, GIMP) simply ignore alpha channel in bitmaps). Screenshot attached.
Screenshot of loaded bitmaps.

Minimal patch to reproduce the issue below.

  • \samples\minimal\minimal.cpp

    old new  
    172172    CreateStatusBar(2); 
    173173    SetStatusText("Welcome to wxWidgets!"); 
    174174#endif // wxUSE_STATUSBAR 
     175    wxString file(wxT("sample.bmp")); 
     176    wxInitAllImageHandlers(); 
     177    int  y = 0; 
     178    wxBitmap bmp1; 
     179    bmp1.LoadFile(file, wxBITMAP_TYPE_BMP); 
     180    // Loaded wxBitmap 
     181    if(bmp1.IsOk()) 
     182    { 
     183        new wxStaticText(this, wxID_ANY, wxT("Loaded wxBitmap"), wxPoint(0,y)); 
     184        new wxStaticBitmap(this, wxID_ANY, bmp1, wxPoint(0,y+20), bmp1.GetSize()); 
     185        y += 20 + bmp1.GetHeight(); 
     186        // wxImage from wxBitmap (alpha removed) 
     187        wxImage img1 = bmp1.ConvertToImage(); 
     188        if(img1.HasAlpha()) 
     189            img1.ClearAlpha(); 
     190        wxBitmap bmp2(img1); 
     191        new wxStaticText(this, wxID_ANY, wxT("wxImage from wxBitmap (alpha removed)"), wxPoint(0,y)); 
     192        new wxStaticBitmap(this, wxID_ANY, bmp2, wxPoint(0,y+20), bmp2.GetSize()); 
     193        y += 20 + bmp2.GetHeight(); 
     194    } 
     195    // Loaded wxImage 
     196    wxImage img2; 
     197    img2.LoadFile(file, wxBITMAP_TYPE_BMP); 
     198    wxBitmap bmp3(img2); 
     199    new wxStaticText(this, wxID_ANY, wxT("Loaded wxImage"), wxPoint(0,y)); 
     200    new wxStaticBitmap(this, wxID_ANY, bmp3, wxPoint(0,y+20), bmp3.GetSize()); 
    175201} 
    176202 
    177203 

comment:15 Changed 7 months ago by vadz

  • Priority changed from normal to low

Do I understand correctly that wxImage already loads this file correctly (forgetting about alpha for the moment)? If so, wouldn't it be better to fall back to it instead of having a 3rd version of bitmap loading code in wxDIB?

As for transparency, I think it's too late to change this, there could well be a lot of code relying on loading BMP files with alpha by now, even if officially it doesn't exist in the BMP format, I think it is used in practice.

Changed 7 months ago by awi

Patch to fix loading of bitmaps (v2).

comment:16 Changed 7 months ago by awi

Indeed, wxImage has its own bitmap loader which is apparently independent of Win API.
Use it inside wxDib and then convert wxImage -> wxBitmap -> wxDib seems to be a bit extravagant solution so perhaps maybe it could be used on a higher level when direct loading of bitmap into wxDib failed.
Proposed solution attached.

comment:17 Changed 7 months ago by VZ

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [76041]) Fix loading of top to bottom BMP files in wxMSW wxBitmap.

The native LoadImage() function used by wxBMPFileHandler only supports the
standard bottom to top BMPs, fall back to our own implementation in wxImage
wxBMPHandler if it fails to also support the top to bottom ones.

Closes #13650.

comment:18 Changed 7 months ago by VZ

(In [76043]) Fix loading of top to bottom BMP files in wxMSW wxBitmap.

The native LoadImage() function used by wxBMPFileHandler only supports the
standard bottom to top BMPs, fall back to our own implementation in wxImage
wxBMPHandler if it fails to also support the top to bottom ones.

Closes #13650.

Note: See TracTickets for help on using tickets.