[Mono-winforms-list] [Mono-osx] DrawImage alpha blend broken in libgdiplus

[Sebastien Pouliot]

On Thu, 2009-04-16 at 15:38 +0300, Alex Shulgin wrote:
> Alex Shulgin wrote:
> > Sebastien Pouliot wrote:
> >> On Thu, 2009-04-09 at 18:22 +0300, Alex Shulgin wrote:
> >>>
> >>> I'm trying to run the tests in mcs/class/System.Drawing/Test under 
> >>> MonoDevelop and I have 144 failing tests (out of 1377) to begin with...
> >>>
> >>> Is this normal situation or my system is severely broken?
> >>
> >> It's definitively not normal.
> >> It could be some path issues since some tests depends on external files
> >> (and MD compiled output could be at the "wrong" place).
> >>> I'm on x86 Ubuntu 8.10 with the following packages freshly built from 
> >>> svn:
> >>>
> >>> libgdiplus
> >>> mono
> >>> mcs
> >>> gtk-sharp
> >>> mono-addins
> >>> monodevelop
> >>>
> >>> The number of failing tests doesn't change if I apply my patch to 
> >>> libgdiplus, though.
> >>>
> >>> How do I proceed?
> >>
> >> You can try a "make run-test" at the prompt.
> >>
> >> If that works (or you can't make it work) then email a patch with the
> >> new test(s) and I'll try it on my box.
> > 
> > Hi again,
> > 
> > I've tried
> > 
> > ~/src/mono/trunk/mcs/class/System.Drawing$ make run-test MCS=gmcs
> > 
> > where gmcs is built & installed from trunk.
> > 
> > This time I get only 6 failures (two related to XML serialization and 
> > the rest in PrintingPermissionTest), so I've tried to apply my patch and 
> > see what happens.
> > 
> > The result was that some tests that save an image, then load it and 
> > check pixel format failed (on png and .ico).  The tests expect to get 
> > Format32bppArgb, but they get Format32bppPArgb per my change.
> > 
> > Thinking further about this situation, I've looked into pngcodec.c to 
> > see how does it save the image if it's data is in PARGB format.  It 
> > turned out, that there's no special handling for this!
> > 
> > There's a simple code to demonstrate what happens if you load an ARGB 
> > PNG image, save it untouched, then load it again.
> > 
> > using System;
> > using System.Drawing;
> > 
> > namespace AlphaBlend
> > {
> >     class MainClass
> >     {
> >         public static void Main(string[] args)
> >         {
> >             Bitmap pngmask = new Bitmap("gimpmask.png");
> >             Console.WriteLine("orig: PixelFormat={0}, pixel at (4,4): {1}",
> >                               pngmask.PixelFormat, pngmask.GetPixel(4,4));
> > 
> >             pngmask.Save("savedmask.png");
> > 
> >             Bitmap loadedmask = new Bitmap("savedmask.png");
> >             Console.WriteLine("load: PixelFormat={0}, pixel at (4,4): {1}",
> >                               loadedmask.PixelFormat, 
> > loadedmask.GetPixel(4,4));
> >         }
> >     }
> > }
> > 
> > On the attached test image this code gives:
> > 
> > orig: PixelFormat=Format32bppArgb, pixel at (4,4): Color [A=51, R=49, 
> > G=49, B=49]
> > load: PixelFormat=Format32bppArgb, pixel at (4,4): Color [A=51, R=9, 
> > G=9, B=9]
> > 
> > The RGB channels get premultiplied twice...
> > 
> > To resolve this mess I propose the following:
> > 
> > 1. Postpone premultiplication until actual use of cairo functions which 
> > require it.
> > 
> > 2. Add premultiplication code where it's currently missing (as in my 
> > original patch + few similar places).
> > 
> > 3. Add code to handle saving of PARGB bitmaps created by user like this: 
> > new Bitmap(w, h, PixelFormat.Format32bppPArgb).  Possibly just throw 
> > NotImplementedException for now.
> > 
> > This way we'll fix current problem with creating mask in memory and 
> > alpha blending it on some background.
> > 
> > We'll avoid post-multiplication otherwise needed before saving an ARGB 
> > PNG image.  Also, libgdiplus won't lie anymore that loaded image data is 
> > in ARGB format while it's actually in PARGB. :)
> > 
> > What do you think?
> 
> Any comments?

Sounds correct but will requires a lot of testing. Please open a bug
report for this  and copy/paste the email info into it (otherwise we'll
lose track of important data).

Thanks
Sebastien