[llvm-rc] Read the Planes/BitCount fields from BITMAPINFOHEADER for icons

Previously these fields were only read from this header for cursors,
while Planes was hardcoded to 1 for icons (with a comment that it was
unknown why this was needed) and BitCount was left at the value
read originally in the RESDIRENTRY.

This fixes the single byte that was differing for the icon/cursor test
compared to rc.exe.

This is based on research/testing by Nico Weber.

Differential Revision: https://reviews.llvm.org/D46816

llvm-svn: 332328

2 files changed, 27 insertions, 21 deletions

diff --git a/llvm/test/tools/llvm-rc/tag-icon-cursor.test b/llvm/test/tools/llvm-rc/tag-icon-cursor.test
index 59fae38a34a..44ef6841165 100644
--- a/llvm/test/tools/llvm-rc/tag-icon-cursor.test
+++ b/llvm/test/tools/llvm-rc/tag-icon-cursor.test
@@ -156,7 +156,7 @@
 ; CHECK-NEXT: Characteristics: 0
 ; CHECK-NEXT: Data size: 62
 ; CHECK-NEXT: Data: (
-; CHECK-NEXT:   0000: 00000100 04001010 00000100 00006804  |..............h.|
+; CHECK-NEXT:   0000: 00000100 04001010 00000100 20006804  |............ .h.|
 ; CHECK-NEXT:   0010: 00000300 18180000 01002000 88090000  |.......... .....|
 ; CHECK-NEXT:   0020: 04002020 00000100 2000A810 00000500  |..  .... .......|
 ; CHECK-NEXT:   0030: 30300000 01002000 A8250000 0600      |00.... ..%....|
diff --git a/llvm/tools/llvm-rc/ResourceFileWriter.cpp b/llvm/tools/llvm-rc/ResourceFileWriter.cpp
index f468fa67f52..70274b6e051 100644
--- a/llvm/tools/llvm-rc/ResourceFileWriter.cpp
+++ b/llvm/tools/llvm-rc/ResourceFileWriter.cpp
@@ -922,33 +922,39 @@ Error ResourceFileWriter::visitIconOrCursorResource(const RCResource *Base) {
 
   // Now, write all the headers concatenated into a separate resource.
   for (size_t ID = 0; ID < NumItems; ++ID) {
-    if (Type == IconCursorGroupType::Icon) {
-      // rc.exe seems to always set NumPlanes to 1. No idea why it happens.
-      ItemEntries[ID].Planes = 1;
-      continue;
-    }
-
-    // We need to rewrite the cursor headers.
+    // We need to rewrite the cursor headers, and fetch actual values
+    // for Planes/BitCount.
     const auto &OldHeader = ItemEntries[ID];
-    ResourceDirEntryStart NewHeader;
-    NewHeader.Cursor.Width = OldHeader.Icon.Width;
-    // Each cursor in fact stores two bitmaps, one under another.
-    // Height provided in cursor definition describes the height of the
-    // cursor, whereas the value existing in resource definition describes
-    // the height of the bitmap. Therefore, we need to double this height.
-    NewHeader.Cursor.Height = OldHeader.Icon.Height * 2;
+    ResourceDirEntryStart NewHeader = OldHeader;
+
+    if (Type == IconCursorGroupType::Cursor) {
+      NewHeader.Cursor.Width = OldHeader.Icon.Width;
+      // Each cursor in fact stores two bitmaps, one under another.
+      // Height provided in cursor definition describes the height of the
+      // cursor, whereas the value existing in resource definition describes
+      // the height of the bitmap. Therefore, we need to double this height.
+      NewHeader.Cursor.Height = OldHeader.Icon.Height * 2;
+
+      // Two WORDs were written at the beginning of the resource (hotspot
+      // location). This is reflected in Size field.
+      NewHeader.Size += 2 * sizeof(uint16_t);
+    }
 
     // Now, we actually need to read the bitmap header to find
     // the number of planes and the number of bits per pixel.
     Reader.setOffset(ItemOffsets[ID]);
     const BitmapInfoHeader *BMPHeader;
     RETURN_IF_ERROR(Reader.readObject(BMPHeader));
-    NewHeader.Planes = BMPHeader->Planes;
-    NewHeader.BitCount = BMPHeader->BitCount;
-
-    // Two WORDs were written at the beginning of the resource (hotspot
-    // location). This is reflected in Size field.
-    NewHeader.Size = OldHeader.Size + 2 * sizeof(uint16_t);
+    if (BMPHeader->Size == sizeof(BitmapInfoHeader)) {
+      NewHeader.Planes = BMPHeader->Planes;
+      NewHeader.BitCount = BMPHeader->BitCount;
+    } else {
+      // A PNG .ico file.
+      // https://blogs.msdn.microsoft.com/oldnewthing/20101022-00/?p=12473
+      // "The image must be in 32bpp"
+      NewHeader.Planes = 1;
+      NewHeader.BitCount = 32;
+    }
 
     ItemEntries[ID] = NewHeader;
   }