Opened 4 years ago

Closed 8 months ago

#12762 closed defect (fixed)

wxBitmaps with alpha data not properly loaded from BMP file

Reported by: snowleopard2 Owned by:
Priority: normal Milestone:
Component: wxMSW Version: dev-latest
Keywords: wxBitmap alpha transparency loading Cc:
Blocked By: Blocking:
Patch: yes

Description

When I load (or even create in memory) data into a wxBitmap which has alpha data in it, the image will become corrupted if I convert it to a wxImage or save as PNG. Note that this bug appeared after 2.9.1.

Copy the file "test.bmp" (attached) to "C:\temp\test.bmp" and then run these lines in the minimal sample:

wxInitAllImageHandlers();
wxBitmap testBmp;
testBmp.LoadFile(L"c:\\temp\\test.bmp", wxBITMAP_TYPE_BMP);//this image has alpha data to it
testBmp.SaveFile(L"c:\\temp\\SavedAsBmp.bmp", wxBITMAP_TYPE_BMP);//this saves identically as the original bitmap
testBmp.SaveFile(L"c:\\temp\\SavedAsPng.png", wxBITMAP_TYPE_PNG);//this saves the alpha channel OK, but there is some "garbage" in the image now

wxBitmap test2Bmp;
test2Bmp.LoadFile(L"c:\\temp\\test.bmp", wxBITMAP_TYPE_BMP);
wxImage img(test2Bmp.ConvertToImage());
img.SaveFile(L"c:\\temp\\ConvertedToWxImageSavedAsBmp.bmp", wxBITMAP_TYPE_BMP);//now alpha is lost and garbage is in image too

Now look at the files in "c:\temp" and not how loading the file and saving as BMP was fine, but saving as PNG caused corruption, and loading the file again and converting to wxImage and saving as BMP led to even worse corruption.

It's not specific to this file, it seems to happen with any wxBitmap with alpha data in it.

Windows XP SP3

Attachments (3)

test.png download (275 bytes) - added by snowleopard2 4 years ago.
test.bmp download (62.7 KB) - added by snowleopard2 4 years ago.
bmp-load-check.patch download (2.1 KB) - added by awi 8 months ago.
Patch implementing some sanity check of loaded bitmap.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 4 years ago by snowleopard2

Hmmm, the test bitmap is too big to attach here (even when zipped). I guess I could email it to someone?

comment:2 Changed 4 years ago by vadz

  • Keywords regression added

Very strange, there has been no significant changes to src/common/imagpng.cpp since a long, long time and definitely not since 2.9.1. Can you please try determine when exactly did this stop working by bisecting the history?

Ah, wait, could this be related to r65959? I don't see how exactly could this break it but this is the only common point between the first and second tests I see anyhow (as the first one doesn't use wxImage and the second one doesn't use PNG). Could you please test with the svn just before and just after it?

As for the bitmap: if the bug is indeed due to premultiplied alpha, it should be visible with any (even tiny 4 pixel) bitmap using alpha values different from 0 and 255. Could you please test if this is the case? And if not, I'd recommend just bisecting your bitmap and attaching its 1/4 or 1/16 part as at least one of them should presumably still show the problem in any case.

TIA!

Changed 4 years ago by snowleopard2

comment:3 Changed 4 years ago by snowleopard2

It definitely seems to do with loading an image and either saving it as a different format or simply converting a wxBitmap to wxImage. I attached a simple PNG file. When I load it and:

  1. Save as PNG--all goes well.
  2. Save as BMP--alpha channel is gone (although bitmap is 32 bpp) and colors are wrong.
  3. Convert wxImage and save that as BMP--alpha is gone, but colors are OK.

I attached this PNG file and here is the new code to reproduce it:

wxBitmap testBmp;
testBmp.LoadFile(L"c:\\temp\\test.png", wxBITMAP_TYPE_PNG);
testBmp.SaveFile(L"c:\\temp\\SavedAsBmp.bmp", wxBITMAP_TYPE_BMP);
testBmp.SaveFile(L"c:\\temp\\SavedAsPng.png", wxBITMAP_TYPE_PNG);

wxBitmap test2Bmp;
test2Bmp.LoadFile(L"c:\\temp\\test.png", wxBITMAP_TYPE_PNG);
wxImage img(test2Bmp.ConvertToImage());
img.SaveFile(L"c:\\temp\\ConvertedToWxImageSavedAsBmp.bmp", wxBITMAP_TYPE_BMP);

comment:4 Changed 4 years ago by snowleopard2

I just noticed, if I load a PNG (with transparency) into a wxBitmap, convert it to wxImage, and save that wxImage as a BMP then the bitmap is 24 bpp--the alpha channel was entirely lost.

When I load the PNG into a wxBitmap and save as BMP then it is 32 bpp, although the image unfortunately is corrupted. Perhaps these are different issues?

comment:5 Changed 4 years ago by jatupper

With #12756, a change involving bitmaps, alpha, and pngs occurred with revision 65898.

comment:6 Changed 4 years ago by vadz

@snowleopard2, could you please test if your problem was there with r65897? TIA!

Changed 4 years ago by snowleopard2

comment:7 Changed 4 years ago by snowleopard2

I tried rolling back the changes found in r65898 and r65959, but it is still broken.

I uploaded "test.bmp" that really shows off the problems. Just load that into a wxBitmap, convert it to a wxImage and then save it as any format and it will look like garbage.

Frankly, it seems like the entire DIB subsystem is failing with alpha data, because I run into problems in memory as well. For example, I load a simple bitmap (with no alpha) into a wxBitmap and then into a wxMemoryDc. At this point, if I "DrawBitmap()" another image on top of it with alpha data then it looks corrupted (never had that problem before).

I can keep trying to roll back small changes, but I'm concerned that there have been a lot of changes in here the last few months to the image processing system. Is there an easy way to grab a build from a couple of months ago? I'm afraid that I don't know how to use SVN (I only download the daily ZIP files), so you would really need to dumb it down for me. Thanks.

comment:8 Changed 4 years ago by LukasK

I suffer from premultiplied alpha problems when saving png on msw since a while. I might look at this some if I can find the time.

Also, probably unrelated but good to know:
I noticed before that there is code in common/imagpng.cpp (for the wxPNGHandler) which detects if an image "requires" alpha, which I believe means that a png which uses only alpha values 255 and 0 will have this replaced by a mask. This surprised me when I last looked at the other alpha problem.

comment:9 follow-up: Changed 4 years ago by LukasK

I guess my main problem was different: MSW uses premultiplied alpha on load, GTK does not. This is problematic when using pixel data iterators.

The wxBitmap alpha->mask conversion that confused me is issue #3019.

Also the documentation for trunk says "While all images have RGB data, not all images have an alpha channel. Before using wxImage::GetAlpha you should check if this image contains an alpha channel with wxImage::HasAlpha. Note that currently only the PNG format has full alpha channel support so only the images loaded from PNG files can have alpha and, if you initialize the image alpha channel yourself using wxImage::SetAlpha, you should save it in PNG format to avoid losing it."

comment:10 in reply to: ↑ 9 ; follow-up: Changed 4 years ago by vadz

Replying to LukasK:

I guess my main problem was different: MSW uses premultiplied alpha on load, GTK does not. This is problematic when using pixel data iterators.

Yes, this is unfortunate but I really don't know what to do about this. In fact, it's even worse than this as MSW doesn't always use premultiplied alpha: it does it by default because this is what's needed to be able to draw bitmaps on wxDC but not when using them in image lists and so the bitmaps created by wxImageList will have non-premultiplied alpha.

Any ideas about how could this be improved would be welcome... One obvious solution is to always use non-premultiplied format but this could seriously hurt the performance.

Anyhow, this doesn't seem to be related to the problem of this ticket per se, is it? I.e. there are no pixel data iterators in sight here AFAICS...

The wxBitmap alpha->mask conversion that confused me is issue #3019.

I replied there.

Also the documentation for trunk says "While all images have RGB data, not all images have an alpha channel. Before using wxImage::GetAlpha you should check if this image contains an alpha channel with wxImage::HasAlpha. Note that currently only the PNG format has full alpha channel support so only the images loaded from PNG files can have alpha and, if you initialize the image alpha channel yourself using wxImage::SetAlpha, you should save it in PNG format to avoid losing it."

Sorry, what's wrong with this?

comment:11 follow-up: Changed 4 years ago by roebling

Yes, this is unfortunate but I really don't know what to do
about this. In fact, it's even worse than this as MSW doesn't
always use premultiplied alpha: it does it by default because
this is what's needed to be able to draw bitmaps on wxDC but
not when using them in image lists and so the bitmaps created
by wxImageList will have non-premultiplied alpha.

Any ideas about how could this be improved would be welcome...
One obvious solution is to always use non-premultiplied format
but this could seriously hurt the performance.

Shouldn't we by default do the same on all platforms and leave
an option under wxMSW that - for max performance - you can also
choose the non-premultiplied alpha format?

comment:12 in reply to: ↑ 10 Changed 4 years ago by LukasK

Replying to vadz:

Anyhow, this doesn't seem to be related to the problem of this ticket per se, is it? I.e. there are no pixel data iterators in sight here AFAICS...

Yes, I got off topic a bit. I have a sort of vague recollection of several problems with bitmap and alpha handling long ago, which as it turned out may not have been relevant for this ticket.

Sorry, what's wrong with this?

Nothing! Sorry, I should have been more clear. I thought it could be relevant for snowleopard2.

comment:13 in reply to: ↑ 11 Changed 4 years ago by snowleopard2

Replying to roebling:

Shouldn't we by default do the same on all platforms and leave
an option under wxMSW that - for max performance - you can also
choose the non-premultiplied alpha format?

Does there already exists a way to make wxMSW work like it used to, some sort of a flag? If I could just turn off this new premultiplied alpha format stuff, would that fix the transparency issues? Performance I suppose is always important, but if you are working with transparency then correct results is more important.

Changed 8 months ago by awi

Patch implementing some sanity check of loaded bitmap.

comment:14 Changed 8 months ago by awi

  • Component changed from base to wxMSW
  • Keywords alpha transparency loading added; wxImage alphablend regression removed
  • Patch set
  • Summary changed from wxBitmaps with alpha data corrupted when converted to wxImage or saved as PNG to wxBitmaps with alpha data not properly loaded from BMP file
  • Version changed from stable-latest to dev-latest

The reason of the reported problem is that input file with bitmap (test.bmp) contains RGBA data in not premultiplied form. These data are loaded directly (without any conversion) into wxBitmap object for which there is an assumption that it contains data in premultiplied form.
If such a wxBitmap object is next converted to wxImage then these not premultiplied data are "de-premultiplied" (unnecessary step in this case) which ends in arithmetic overflows for RGB data and with garbage in the output image.

To prevent from loading not premuliplied data into wxBitmap some sanity check should be applied for input RGBA data taken also from BMP files (which not necessary contain premultplied data, especially if not created under MSW). There is not possible to capture all malformed data but some errors can be eliminated.
Patch implementing some checks is attached.

comment:15 Changed 8 months ago by vadz

Thanks for the patch! I'll apply it, but I think the only really comprehensive solution is to allow storing both plain and pre-multiplied data in wxBitmap and record which one we have. This should fix the confusion (at least we'll always know what are we dealing with) and might also improve efficiency as we could avoid unnecessary conversions if we already have the data in the right format.

comment:16 Changed 8 months ago by VZ

(In [76006]) Add helper Set32bppHDIB() method to wxMSW wxBitmapRefData.

No real changes, just add a helper to allow replacing the HBITMAP stored in
wxBitmapRefData without changing anything else, this is going to be used in
another place soon.

See #12762.

comment:17 Changed 8 months ago by VZ

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

(In [76007]) Fix loading of bitmap with non-pre-multiplied alpha in wxMSW.

Detect when the bitmap file doesn't have pre-multiplied alpha and pre-multiply
it ourselves when loading it in this case.

Closes #12762.

Note: See TracTickets for help on using tickets.