Robmaister / SharpFont

Cross-platform FreeType bindings for .NET - Need maintainer
Other
289 stars 106 forks source link

FTBitmap to Bitmap 8bppindexed palette issue #55

Open lukadt opened 9 years ago

lukadt commented 9 years ago

I have downloaded Sharpfont sources and run simple code example on my x86 windows machine and on my ubuntu machine using mono. There's a problem in the function ToGdipBitmap with a bitmap 8bpp and gray pixelmode. After Bitmap palette manipulation the background of the glyph is rendered as a black box instead of transparent after the assignment bmp.Palette = palette

    ColorPalette palette = bmp.Palette;
    for (int i = 0; i < palette.Entries.Length; i++)
        {
            float a = i / 255f;
            palette.Entries[i] = Color.FromArgb(i, (int)(color.R * a), (int)(color.G * a), (int)(color.B * a));
        }                 

        bmp.Palette = palette;

Windows Output sharpfontwin Linux Mono Output

sharpfontlin

I was able to workaround the issue with a negative ColorMatrix, but it's really a hack. Does this behaviour depends on Mono or a wrong palette code manipulation? Thanks a lot in advance

Robmaister commented 9 years ago

I would assume this is an issue within libgdiplus as part of Mono or at least a regression of it from GDI+.

I'll take a look at it and find a fix that will make it transparent on both Mono and .NET

lukadt commented 9 years ago

Thanks a lot Robert, I will try to run the code using Mono on Windows. If it works then as you assume it's an issue with libgdiplus .

I will wait for your fix, Luca

Robmaister commented 9 years ago

There is also a chance that Mono will use the real GDI+ when available (on Windows), so don't be too surprised if it still works on Mono on Windows

Robmaister commented 8 years ago

I just emailed you back, but for the sake of keeping things public, I just tested this out in OpenSUSE with KDE. Looks like it's worse on Qt!

sharpfont-opensuse

Robmaister commented 8 years ago

So I finally got it working after digging through libgdiplus source code for a while.

sharpfont-opensuse_working

Just before bmp.Palette = palette in FTBitmap.ToGdipBitmap() (for all cases except LCD), insert the following line:

typeof(ColorPalette).GetField ("flags", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic).SetValue (palette, palette.Flags | 1);

The 1 in this case is PaletteFlagsHasAlpha

This issue is caused because of:

I couldn't quickly find a normal way to set this flag or build a new ColorPalette with it, so I went ahead and solved it with reflection. If there is no clean way to do this, this patch may have to be included in SharpFont for the foreseeable future and tested against the Windows GDI+ implementation.

I would consider this a regression of libgdiplus from Windows GDI+, which means a bug report should be submitted. I'll go ahead and do that in the morning

Robmaister commented 8 years ago

Finally submitted to Xamarin:

https://bugzilla.xamarin.com/show_bug.cgi?id=37608

lukadt commented 8 years ago

Thanks a lot Robert for the bug submission !!!

HinTak commented 8 years ago

MSDN ( https://msdn.microsoft.com/en-us/library/system.drawing.imaging.colorpalette%28v=vs.110%29.aspx ) seems to suggest that the flag is read-only, hence there is only a getter for flags ( https://github.com/mono/mono/blob/master/mcs/class/System.Drawing/System.Drawing.Imaging/ColorPalette.cs ) . Using reflection to set flags after the fact seems to be a bit naughty.

Perhaps the correct solution is to see why the flag was not set correctly on creation?

It seems that the palette is created around line 604-622 of https://github.com/mono/mono/blob/master/mcs/class/System.Drawing/System.Drawing/Image.cs - from calling GDIPlus.GdipGetImagePalette() then ret.setFromGDIPalette(). The latter is common to windows and libgdiplus, so I would guess we need to look at GDIPlus.GdipGetImagePalette().

Actually, would calling GDIPlus.GdipSetImagePalette() explicitly to set the palette explicitly be a better solution, if it works?

Robmaister commented 8 years ago

When I was exploring this bug, I could not find any bit of publicly accessible code that sets the flags. The code that sets palettes does nothing to check whether or not the palette has any alpha values, just marshals the data back and forth blindly.

The problem is here, where the alpha values are blindly forced to be 0xFF https://github.com/mono/libgdiplus/blob/e1fb6fe92d4b1691d3ded9ba5356a7e782cfa234/src/bitmap.c#L2349

A quick search of both the libgdiplus and mono repositories show no useful references to the alpha flag at all. https://github.com/mono/libgdiplus/search?utf8=%E2%9C%93&q=PaletteFlagsHasAlpha https://github.com/mono/mono/search?utf8=%E2%9C%93&q=ColorPalette.flags

I believe when I was testing on Windows, only the Halftone flag was set, but that seems to be enough to allow transparent palette values to be copied properly. Perhaps the check against that flag is entirely unnecessary, but I don't know the logic that went into the writing of that function, so I just submitted a bug report to Xamarin who would know better than I.

Calling GdipSetImagePalette() wouldn't change much, it's essentially just a memcpy: https://github.com/mono/libgdiplus/blob/e1fb6fe92d4b1691d3ded9ba5356a7e782cfa234/src/image.c#L1920

I'd have to construct my own ColorPalette to call the function with, which would require changing the private/internal properties of ColorPalette, which means I'd still be using reflection to change the flag. As far as I know, my solution is the simplest way to work around this bug.

HinTak commented 8 years ago

Yes. The reflection trick works, and MS GDI+ does not seem to mind the extra code either.

in both mono and MS GDI+, bmp.Palette.Flags is 4.

HinTak commented 8 years ago

... but I don't know the logic that went into the writing of that function, ...

I use 'git blame' for that sort of thing...

Robmaister commented 8 years ago

What I mostly meant by that was that I haven't bothered looking at the sort of tests they're running for libgdiplus. I may do that later and submit a PR, but it may have side effects in a lot of other programs, seeing as it was last updated almost 10 years ago.

There are a few ways of applying the patch that will change the way the function works and if there's anyone on the planet left who understands what broader effects that will have on other code, it's probably someone at Xamarin. Either way, with the slow uptake of new Mono versions in slower-moving distributions like Debian, a patch on our end will have to exist for probably 4-5 years before a libgdiplus fix will be almost universally available.