Fix multiple bugs and conversions for PARGB32 bitmap object and premultiplied Cairo surfaces

mono / libgdiplus

C-based implementation of the GDI+ API

http://www.mono-project.com/

MIT License

334 stars 171 forks source link

Fix multiple bugs and conversions for PARGB32 bitmap object and premultiplied Cairo surfaces #599

Closed filipnavara closed 4 years ago

filipnavara commented 4 years ago

Contributes to #446.

Fixes the following issues:

Converts to/from premultiplied colors in GdipBitmap[Set/Get]Pixel in PARGB32 mode
Converts back and forth underlying data for Cairo surfaces where only PARGB32 is supported and ARGB32 is emulated. This fixes regressions caused by 389cb7bb49006c0a598ee3369d9c4876574740fe and f7775ec5746d242e9c3c0f06a725bfee748f8243 at the expense of slight inaccuracies due to rounding in the ARGB32 -> PARGB32 -> ARGB32 conversion.

filipnavara commented 4 years ago

Should fix the CorrectColorDepthExtracted test failing on Mono PRs. I didn't try it locally yet. The test itself is slightly broken because it expects rounded values (resulting from alpha pre-multiplication) instead of the actual values in the .ico file.

filipnavara commented 4 years ago

/cc @akoeplinger

akoeplinger commented 4 years ago

I tried this locally on OSX and it doesn't fix the issue for me:

    System.Drawing.Tests.IconTests.CorrectColorDepthExtracted [FAIL]
      Assert.Equal() Failure
      Expected: 2275337522
      Actual:   2270394650
      Stack Trace:
        /_/src/System.Drawing.Common/tests/IconTests.cs(864,0): at System.Drawing.Tests.IconTests.CorrectColorDepthExtracted()
        /Users/alexander/dev/mono/netcore/System.Private.CoreLib/src/System.Reflection/RuntimeMethodInfo.cs(339,0): at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)

filipnavara commented 4 years ago

Hmm, that is still the wrong premultiplied value.

akoeplinger commented 4 years ago

/azp run

azure-pipelines[bot] commented 4 years ago

Azure Pipelines successfully started running 1 pipeline(s).

filipnavara commented 4 years ago

Now it should pass all the tests except for this one:

    System.Drawing.Tests.IconTests.CorrectColorDepthExtracted [FAIL]
      Assert.Equal() Failure
      Expected: 2275337522
      Actual:   2275337265

It is consequence of rounding error in ARGB32 -> PARGB32 -> ARGB32 conversion and naive implementation of Bitmap(Bitmap bitmap) constructor. The best fix is on CoreFX side to switch to GdipCloneBitmapArea API which is lossless since it avoids creating GpGraphics object and drawing using Cairo.

akoeplinger commented 4 years ago

Cherry-picked to release/6.0