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

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo created this revision.
mstorsjo added reviewers: thakis, zturner, amccarth.

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.


Repository:
  rL LLVM

https://reviews.llvm.org/D46816

Files:
  test/tools/llvm-rc/tag-icon-cursor.test
  tools/llvm-rc/ResourceFileWriter.cpp


Index: tools/llvm-rc/ResourceFileWriter.cpp
===================================================================
--- tools/llvm-rc/ResourceFileWriter.cpp
+++ tools/llvm-rc/ResourceFileWriter.cpp
@@ -919,33 +919,40 @@
 
   // 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);
+    constexpr size_t BITMAPINFOHEADER_size = 0x28;
+    if (BMPHeader->Size == BITMAPINFOHEADER_size) {
+      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;
   }
Index: test/tools/llvm-rc/tag-icon-cursor.test
===================================================================
--- test/tools/llvm-rc/tag-icon-cursor.test
+++ 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.... ..%....|


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46816.146542.patch
Type: text/x-patch
Size: 3557 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180514/9477be71/attachment.bin>