MagickSettings: probable issue with font & fontPointSize?

[Gounlaf]

Looking at ctor of MagicSettings:

Magick.NET/src/Magick.NET/Settings/MagickSettings.cs

Lines 31 to 51 in 5676471

	internal MagickSettings()
	{
	using var instance = new NativeMagickSettings();
	AntiAlias = instance.AntiAlias;
	BackgroundColor = instance.BackgroundColor;
	ColorSpace = instance.ColorSpace;
	ColorType = instance.ColorType;
	Compression = instance.Compression;
	Debug = instance.Debug;
	Density = CreateDensity(instance.Density);
	Depth = instance.Depth;
	Endian = instance.Endian;
	Extract = MagickGeometry.FromString(instance.Extract);
	_font = instance.Font;
	_fontPointsize = instance.FontPointsize;
	Format = EnumHelper.Parse(instance.Format, MagickFormat.Unknown);
	Interlace = instance.Interlace;
	Monochrome = instance.Monochrome;
	Verbose = instance.Verbose;
	Drawing = new DrawingSettings();
	}

font & fontPointSize are set on the backing property ; but getter&setter use the Drawing object.
So, if the default value of Drawing.Font and Drawing.FontPointsize (defined from NativeDrawingSettings) are not the same has default of NativeMagickSettings, it could be an issue.

Setting the Drawing property first, then using Font&FontPointsize should be "safer", no?

    internal MagickSettings()
    {
        Drawing = new DrawingSettings();

        using var instance = new NativeMagickSettings();
        AntiAlias = instance.AntiAlias;
        BackgroundColor = instance.BackgroundColor;
        ColorSpace = instance.ColorSpace;
        ColorType = instance.ColorType;
        Compression = instance.Compression;
        Debug = instance.Debug;
        Density = CreateDensity(instance.Density);
        Depth = instance.Depth;
        Endian = instance.Endian;
        Extract = MagickGeometry.FromString(instance.Extract);
        Font = instance.Font;
        FontPointsize = instance.FontPointsize;
        Format = EnumHelper.Parse(instance.Format, MagickFormat.Unknown);
        Interlace = instance.Interlace;
        Monochrome = instance.Monochrome;
        Verbose = instance.Verbose;
    }

[dlemstra]

I suspect that there was probably a good reason to do it like this years ago. But I do think it makes sense to make this more consistent. So feel free to create a PR to change this.

[Gounlaf]

👋

What I understand is my guess was true.

• NativeMagickSettings.FontPointSize is 0 by default
• DrawingSettings.FontPointSize is 12 by default

So, backed property _font & _fontPointSize are unsync with DrawingSettings.

Which one should have the primacy?

[dlemstra]

Thanks for taking a deep dive into this. We definitely need some unit tests for that then first. And I will also be going to investigate this now. I think that was the reason that I had those backing fields. I will probably not merge your PR then and make the changes myself if I am going to make any changes.

[Gounlaf]

Yup. The MR tests are not working because the tests with Affine are not working anymore (because of the font size).

You are definitely more able to do the changes since I don't know the architecture behind 😅

[dlemstra]

I was able to change the Font field but I must keep the _fontPointsize field because it has a different default value.