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

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo updated this revision to Diff 146694.
mstorsjo added a comment.

Back to the original form of the check, but using sizeof(BitmapInfoHeader) instead of a separate constant.


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
@@ -922,33 +922,39 @@
 
   // 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;
   }
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.146694.patch
Type: text/x-patch
Size: 3508 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180514/9a24fa41/attachment.bin>