diff --git a/src/System.Drawing.Common/src/NativeMethods.txt b/src/System.Drawing.Common/src/NativeMethods.txt index 8eaca883d5c..b819d01919c 100644 --- a/src/System.Drawing.Common/src/NativeMethods.txt +++ b/src/System.Drawing.Common/src/NativeMethods.txt @@ -35,6 +35,7 @@ LANG_ARABIC LANG_JAPANESE LevelsEffectGuid LevelsParams +LookupIconIdFromDirectoryEx PALETTEENTRY PaletteFlags PaletteType diff --git a/src/System.Drawing.Common/src/System/Drawing/Icon.cs b/src/System.Drawing.Common/src/System/Drawing/Icon.cs index 0430d26edb1..503db22c32d 100644 --- a/src/System.Drawing.Common/src/System/Drawing/Icon.cs +++ b/src/System.Drawing.Common/src/System/Drawing/Icon.cs @@ -17,8 +17,6 @@ namespace System.Drawing; [TypeForwardedFrom(AssemblyRef.SystemDrawing)] public sealed unsafe partial class Icon : MarshalByRefObject, ICloneable, IDisposable, ISerializable, IIcon { - private static int s_bitDepth; - // The PNG signature is specified at https://www.w3.org/TR/PNG/#5PNG-file-signature private const int PNGSignature1 = 137 + ('P' << 8) + ('N' << 16) + ('G' << 24); private const int PNGSignature2 = 13 + (10 << 8) + (26 << 16) + (10 << 24); @@ -398,6 +396,40 @@ public static Icon FromHandle(IntPtr handle) return new Icon((HICON)handle); } + // Creates RT_GROUP_ICON resource directory from ICO file data + private static byte[] CreateIconResourceDirectory(ReadOnlySpan iconData) + { + var reader = new SpanReader(iconData); + + reader.TryRead(out ICONDIR icoDir); + reader.TryRead(icoDir.idCount, out ReadOnlySpan icoEntries); + + byte[] resDir = GC.AllocateUninitializedArray(sizeof(GRPICONDIR) + icoDir.idCount * sizeof(GRPICONDIRENTRY)); + Span resDirSpan = resDir; + + MemoryMarshal.Write(resDirSpan, new GRPICONDIR { Reserved = 0, ResType = icoDir.idType, ResCount = icoDir.idCount }); + + Span gpEntries = MemoryMarshal.Cast(resDirSpan[sizeof(GRPICONDIR)..]); + + for (int i = 0; i < icoDir.idCount; i++) + { + ref readonly ICONDIRENTRY icoEntry = ref icoEntries[i]; + gpEntries[i] = new GRPICONDIRENTRY + { + Width = icoEntry.bWidth, + Height = icoEntry.bHeight, + ColorCount = icoEntry.bColorCount, + Reserved = icoEntry.bReserved, + Planes = icoEntry.wPlanes, + BitCount = icoEntry.wBitCount, + BytesInRes = icoEntry.dwBytesInRes, + IconCursorId = (ushort)i + }; + } + + return resDir; + } + // Initializes this Image object. This is identical to calling the image's // constructor with picture, but this allows non-constructor initialization, // which may be necessary in some instances. @@ -408,155 +440,48 @@ private void Initialize(int width, int height) throw new InvalidOperationException(SR.Format(SR.IllegalState, GetType().Name)); } - SpanReader reader = new(_iconData); - if (!reader.TryRead(out ICONDIR dir) - || dir.idReserved != 0 - || dir.idType != 1 - || dir.idCount == 0) - { - throw new ArgumentException(SR.Format(SR.InvalidPictureType, "picture", nameof(Icon))); - } + var reader = new SpanReader(_iconData); - // Get the correct width and height. - if (width == 0) + if (!reader.TryRead(out ICONDIR icoDir) || icoDir.idReserved != 0 || icoDir.idType != 1 || icoDir.idCount < 1) { - width = PInvokeCore.GetSystemMetrics(SYSTEM_METRICS_INDEX.SM_CXICON); - } - - if (height == 0) - { - height = PInvokeCore.GetSystemMetrics(SYSTEM_METRICS_INDEX.SM_CYICON); - } - - if (s_bitDepth == 0) - { - using var hdc = GetDcScope.ScreenDC; - s_bitDepth = PInvokeCore.GetDeviceCaps(hdc, GET_DEVICE_CAPS_INDEX.BITSPIXEL); - s_bitDepth *= PInvokeCore.GetDeviceCaps(hdc, GET_DEVICE_CAPS_INDEX.PLANES); - - // If the bit depth is 8, make it 4 because windows does not - // choose a 256 color icon if the display is running in 256 color mode - // due to palette flicker. - if (s_bitDepth == 8) - { - s_bitDepth = 4; - } + throw new ArgumentException(SR.Format(SR.InvalidPictureType, "picture", nameof(Icon))); } - byte bestWidth = 0; - byte bestHeight = 0; - - if (!reader.TryRead(dir.idCount, out ReadOnlySpan entries)) + if (!reader.TryRead(icoDir.idCount, out ReadOnlySpan icoEntries)) { throw new ArgumentException(SR.Format(SR.InvalidPictureType, "picture", nameof(Icon))); } - foreach (ICONDIRENTRY entry in entries) - { - bool fUpdateBestFit = false; - uint iconBitDepth; - if (entry.bColorCount != 0) - { - iconBitDepth = 4; - if (entry.bColorCount < 0x10) - { - iconBitDepth = 1; - } - } - else - { - iconBitDepth = entry.wBitCount; - } + // Prepare fake RT_GROUP_ICON resource for LookupIconIdFromDirectoryEx call. + var resDir = CreateIconResourceDirectory(_iconData); - // If it looks like if nothing is specified at this point then set the bits per pixel to 8. - if (iconBitDepth == 0) - { - iconBitDepth = 8; - } - - // Windows rules for specifying an icon: - // - // 1. The icon with the closest size match. - // 2. For matching sizes, the image with the closest bit depth. - // 3. If there is no color depth match, the icon with the closest color depth that does not exceed the display. - // 4. If all icon color depth > display, lowest color depth is chosen. - // 5. color depth of > 8bpp are all equal. - // 6. Never choose an 8bpp icon on an 8bpp system. - - if (_bestBytesInRes == 0) - { - fUpdateBestFit = true; - } - else - { - int bestDelta = Math.Abs(bestWidth - width) + Math.Abs(bestHeight - height); - int thisDelta = Math.Abs(entry.bWidth - width) + Math.Abs(entry.bHeight - height); - - if ((thisDelta < bestDelta) - || (thisDelta == bestDelta - && ((iconBitDepth <= s_bitDepth && iconBitDepth > _bestBitDepth) - || (_bestBitDepth > s_bitDepth && iconBitDepth < _bestBitDepth)))) - { - fUpdateBestFit = true; - } - } - - if (fUpdateBestFit) - { - bestWidth = entry.bWidth; - bestHeight = entry.bHeight; - _bestImageOffset = entry.dwImageOffset; - _bestBytesInRes = entry.dwBytesInRes; - _bestBitDepth = iconBitDepth; - } - } - - if (_bestImageOffset > int.MaxValue) + int id = 0; + fixed (byte* b = resDir) { - throw new ArgumentException(SR.Format(SR.InvalidPictureType, "picture", nameof(Icon))); + id = PInvoke.LookupIconIdFromDirectoryEx(b, fIcon: true, width, height, IMAGE_FLAGS.LR_DEFAULTSIZE); } - if (_bestBytesInRes > int.MaxValue) - { - throw new Win32Exception((int)WIN32_ERROR.ERROR_INVALID_PARAMETER); - } + var entry = icoEntries[id]; + _bestImageOffset = entry.dwImageOffset; + _bestBytesInRes = entry.dwBytesInRes; + _bestBitDepth = entry.bColorCount != 0 ? (entry.bColorCount < 16 ? 1u : 4u) + : (entry.wBitCount > 0 ? entry.wBitCount : 8u); - uint endOffset; - try - { - endOffset = checked(_bestImageOffset + _bestBytesInRes); - } - catch (OverflowException) + if (_bestBytesInRes > int.MaxValue) { throw new Win32Exception((int)WIN32_ERROR.ERROR_INVALID_PARAMETER); } - if (endOffset > _iconData.Length) + if (checked(_bestImageOffset + _bestBytesInRes) > _iconData.Length) { throw new ArgumentException(SR.Format(SR.InvalidPictureType, "picture", nameof(Icon))); } - // Copy the bytes into an aligned buffer if needed. - - ReadOnlySpan bestImage = reader.Span.Slice((int)_bestImageOffset, (int)_bestBytesInRes); - - if ((_bestImageOffset % sizeof(nint)) != 0) + // CreateIconFromResourceEx expects an aligned RT_ICON resource buffer. + // Just copy best image bytes into a new aligned buffer. + fixed (byte* b = _iconData.AsSpan().Slice((int)_bestImageOffset, (int)_bestBytesInRes).ToArray()) { - // Beginning of icon's content is misaligned. - using BufferScope alignedBuffer = new((int)_bestBytesInRes); - bestImage.CopyTo(alignedBuffer.AsSpan()); - - fixed (byte* b = alignedBuffer) - { - _handle = PInvoke.CreateIconFromResourceEx(b, (uint)bestImage.Length, fIcon: true, 0x00030000, 0, 0, 0); - } - } - else - { - fixed (byte* b = bestImage) - { - _handle = PInvoke.CreateIconFromResourceEx(b, (uint)bestImage.Length, fIcon: true, 0x00030000, 0, 0, 0); - } + _handle = PInvoke.CreateIconFromResourceEx(b, _bestBytesInRes, fIcon: true, 0x00030000, 0, 0, IMAGE_FLAGS.LR_DEFAULTCOLOR); } if (_handle.IsNull) diff --git a/src/System.Drawing.Common/tests/System/Drawing/IconTests.cs b/src/System.Drawing.Common/tests/System/Drawing/IconTests.cs index b146c0bf4c9..af9e9c7fed9 100644 --- a/src/System.Drawing.Common/tests/System/Drawing/IconTests.cs +++ b/src/System.Drawing.Common/tests/System/Drawing/IconTests.cs @@ -24,7 +24,6 @@ using System.ComponentModel; using System.Drawing.Imaging; -using System.Reflection; using Microsoft.DotNet.RemoteExecutor; namespace System.Drawing.Tests; @@ -33,7 +32,6 @@ public class IconTests { [Theory] [InlineData("48x48_multiple_entries_4bit.ico")] - [InlineData("256x256_seven_entries_multiple_bits.ico")] [InlineData("pngwithheight_icon.ico")] public void Ctor_FilePath(string name) { @@ -50,8 +48,8 @@ public static IEnumerable Size_TestData() yield return new object[] { "48x48_multiple_entries_4bit.ico", new Size(-32, -32), new Size(16, 16) }; yield return new object[] { "48x48_multiple_entries_4bit.ico", new Size(32, 16), new Size(32, 32) }; yield return new object[] { "256x256_seven_entries_multiple_bits.ico", new Size(48, 48), new Size(48, 48) }; - yield return new object[] { "256x256_seven_entries_multiple_bits.ico", new Size(0, 0), new Size(32, 32) }; - yield return new object[] { "256x256_seven_entries_multiple_bits.ico", new Size(1, 1), new Size(256, 256) }; + yield return new object[] { "256x256_seven_entries_multiple_bits.ico", new Size(0, 0), new Size(64, 32) }; + yield return new object[] { "256x256_seven_entries_multiple_bits.ico", new Size(1, 1), new Size(16, 16) }; // Unusual size yield return new object[] { "10x16_one_entry_32bit.ico", new Size(16, 16), new Size(10, 16) }; @@ -753,10 +751,7 @@ public void CorrectColorDepthExtracted() using Bitmap bitmap = icon.ToBitmap(); Assert.Equal(new Size(32, 32), bitmap.Size); - int expectedBitDepth; - string fieldName = PlatformDetection.IsNetFramework ? "bitDepth" : "s_bitDepth"; - FieldInfo fi = typeof(Icon).GetField(fieldName, BindingFlags.Static | BindingFlags.NonPublic); - expectedBitDepth = (int)fi.GetValue(null); + int expectedBitDepth = Windows.Forms.Screen.PrimaryScreen.BitsPerPixel; // If the first icon entry was picked, the color would be black: 0xFF000000? diff --git a/src/System.Drawing.Common/tests/System/Drawing/ImageConverterTests.cs b/src/System.Drawing.Common/tests/System/Drawing/ImageConverterTests.cs index 16726a84834..f910767027f 100644 --- a/src/System.Drawing.Common/tests/System/Drawing/ImageConverterTests.cs +++ b/src/System.Drawing.Common/tests/System/Drawing/ImageConverterTests.cs @@ -31,7 +31,6 @@ public ImageConverterTest() [Theory] [InlineData("48x48_multiple_entries_4bit.ico")] - [InlineData("256x256_seven_entries_multiple_bits.ico")] [InlineData("pngwithheight_icon.ico")] public void ImageConverterFromIconTest(string name) { diff --git a/src/System.Private.Windows.Core/src/Windows/Win32/UI/WindowsAndMessaging/GRPICONDIR.cs b/src/System.Private.Windows.Core/src/Windows/Win32/UI/WindowsAndMessaging/GRPICONDIR.cs new file mode 100644 index 00000000000..f83373b5e30 --- /dev/null +++ b/src/System.Private.Windows.Core/src/Windows/Win32/UI/WindowsAndMessaging/GRPICONDIR.cs @@ -0,0 +1,18 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.InteropServices; + +namespace Windows.Win32.UI.WindowsAndMessaging; + +// https://devblogs.microsoft.com/oldnewthing/?p=7083 +// https://devblogs.microsoft.com/oldnewthing/?p=108925 +// https://learn.microsoft.com/en-us/windows/win32/menurc/newheader + +[StructLayout(LayoutKind.Sequential, Pack = 2)] +internal struct GRPICONDIR +{ + public ushort Reserved; + public ushort ResType; + public ushort ResCount; +} diff --git a/src/System.Private.Windows.Core/src/Windows/Win32/UI/WindowsAndMessaging/GRPICONDIRENTRY.cs b/src/System.Private.Windows.Core/src/Windows/Win32/UI/WindowsAndMessaging/GRPICONDIRENTRY.cs new file mode 100644 index 00000000000..3d3b4ff57d9 --- /dev/null +++ b/src/System.Private.Windows.Core/src/Windows/Win32/UI/WindowsAndMessaging/GRPICONDIRENTRY.cs @@ -0,0 +1,23 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.InteropServices; + +namespace Windows.Win32.UI.WindowsAndMessaging; + +// https://devblogs.microsoft.com/oldnewthing/?p=7083 +// https://devblogs.microsoft.com/oldnewthing/?p=108925 +// https://learn.microsoft.com/en-us/windows/win32/menurc/resdir + +[StructLayout(LayoutKind.Sequential, Pack = 2)] +internal struct GRPICONDIRENTRY +{ + public byte Width; + public byte Height; + public byte ColorCount; + public byte Reserved; + public ushort Planes; + public ushort BitCount; + public uint BytesInRes; + public ushort IconCursorId; +}