Icon background and dark mode

[GasDauMin]

Mismatched icon's background ant context menu color when dark mode is on.

[end2endzone]

Hi. Thank you for the feedback. I really appreciate it.

I am guessing that you are referring to Windows 10 dark mode. I don't use dark mode and I develop on Windows 7 so I have never seen this bug before. Good catch.

Dealing with transparent icons is complicated on Windows context menus. Over multiple tries and errors, I found out that its better to "fill" the transparency with the correct background color. I seem to have used the wrong color id. The function FillTransparentPixels() in file Win32Utils.cpp is probably the one to blame. I think I will have to take a look at it.

I am open to contributions and push request. Would you like to contribute to the project and correct the issue yourself? If so, then the following information might help you:

There is a unit test that can be used to validate the correction. You can take a look at unit test testIconToBitmapConversion() in file TestWin32Utils.cpp which extracts all 305 icons of file shell32.dll into the test directory. Many icons uses transparency. The unit test is not a "true unit test" since it never fails but it provides a quick way for validating incorrect behavior in FillTransparentPixels() function. A human must take a look at the results for analysis.

P.S. The menu Open command prompt here have currently an issue discussed at #51.

[GasDauMin]

I don't really like hardcoding, but at the moment i see only one way to deal with this problem and that is harcoding background color for current case of course the better solution would be to get current color of context menu but i can't find a way how to do it.

bool isDarkThemeActive = Win32Utils::IsDarkThemeActive();
if (isDarkThemeActive)
{
    background_color = RGB(43,43,43);
}

My proposed solution solves only half the problem, now highlighted icons are displayed incorrectly.

[GasDauMin]

What about transparent bitmaps I found working example which used in KuShellExtension it is using GDI+ and seems to be working.

And also found third-party library FreeImage but i don't know proper way how to implement third-party libraries to your project.

[end2endzone]

You are right, there seems to be no way of getting the menus background color in Dark Mode. I asked a friend to run the following sample on Windows 10 in dark mode:

#include <Windows.h>
#include <string>

RGBQUAD ToRgbQuad(const DWORD & iColor) {
  RGBQUAD output = {0};
  output.rgbRed   = (BYTE)((iColor&0x000000FF)    );
  output.rgbGreen = (BYTE)((iColor&0x0000FF00)>> 8);
  output.rgbBlue  = (BYTE)((iColor&0x00FF0000)>>16);
  output.rgbReserved = 255;
  return output;
}

std::string GetColorName(int index) {
  std::string str;
  #define CASE_COLOR(name) if (index == name) { if (!str.empty()) str.append(", "); str.append(#name); }
  CASE_COLOR(CTLCOLOR_MSGBOX                )
  CASE_COLOR(CTLCOLOR_EDIT                  )
  CASE_COLOR(CTLCOLOR_LISTBOX               )
  CASE_COLOR(CTLCOLOR_BTN                   )
  CASE_COLOR(CTLCOLOR_DLG                   )
  CASE_COLOR(CTLCOLOR_SCROLLBAR             )
  CASE_COLOR(CTLCOLOR_STATIC                )
  CASE_COLOR(CTLCOLOR_MAX                   )
  CASE_COLOR(COLOR_SCROLLBAR                )
  CASE_COLOR(COLOR_BACKGROUND               )
  CASE_COLOR(COLOR_ACTIVECAPTION            )
  CASE_COLOR(COLOR_INACTIVECAPTION          )
  CASE_COLOR(COLOR_MENU                     )
  CASE_COLOR(COLOR_WINDOW                   )
  CASE_COLOR(COLOR_WINDOWFRAME              )
  CASE_COLOR(COLOR_MENUTEXT                 )
  CASE_COLOR(COLOR_WINDOWTEXT               )
  CASE_COLOR(COLOR_CAPTIONTEXT              )
  CASE_COLOR(COLOR_ACTIVEBORDER             )
  CASE_COLOR(COLOR_INACTIVEBORDER           )
  CASE_COLOR(COLOR_APPWORKSPACE             )
  CASE_COLOR(COLOR_HIGHLIGHT                )
  CASE_COLOR(COLOR_HIGHLIGHTTEXT            )
  CASE_COLOR(COLOR_BTNFACE                  )
  CASE_COLOR(COLOR_BTNSHADOW                )
  CASE_COLOR(COLOR_GRAYTEXT                 )
  CASE_COLOR(COLOR_BTNTEXT                  )
  CASE_COLOR(COLOR_INACTIVECAPTIONTEXT      )
  CASE_COLOR(COLOR_BTNHIGHLIGHT             )
  CASE_COLOR(COLOR_3DDKSHADOW               )
  CASE_COLOR(COLOR_3DLIGHT                  )
  CASE_COLOR(COLOR_INFOTEXT                 )
  CASE_COLOR(COLOR_INFOBK                   )
  CASE_COLOR(COLOR_HOTLIGHT                 )
  CASE_COLOR(COLOR_GRADIENTACTIVECAPTION    )
  CASE_COLOR(COLOR_GRADIENTINACTIVECAPTION  )
  CASE_COLOR(COLOR_MENUHILIGHT              )
  CASE_COLOR(COLOR_MENUBAR                  )
  CASE_COLOR(COLOR_DESKTOP                  )
  CASE_COLOR(COLOR_3DFACE                   )
  CASE_COLOR(COLOR_3DSHADOW                 )
  CASE_COLOR(COLOR_3DHIGHLIGHT              )
  CASE_COLOR(COLOR_3DHILIGHT                )
  CASE_COLOR(COLOR_BTNHILIGHT               )
  #undef CASE_COLOR
  //default
  if (str.empty())
    str.append("Unknown");
  return str;
}

int main(int argc, char* argv[]) {
  for(int i=0; i<50; i++) {
    COLORREF menu_background_color = GetSysColor(i);
    const RGBQUAD rgb_color = ToRgbQuad(menu_background_color);
    printf("GetSysColor(%03d)=RGB(%03d,%03d,%03d,%03d), %s\n", i, rgb_color.rgbRed, rgb_color.rgbGreen, rgb_color.rgbBlue, rgb_color.rgbReserved, GetColorName(i).c_str());
  }
  system("PAUSE");
    return 0;
}

And the output was the following:

GetSysColor(000)=RGB(200,200,200,255), CTLCOLOR_MSGBOX, COLOR_SCROLLBAR
GetSysColor(001)=RGB(000,000,000,255), CTLCOLOR_EDIT, COLOR_BACKGROUND, COLOR_DESKTOP
GetSysColor(002)=RGB(153,180,209,255), CTLCOLOR_LISTBOX, COLOR_ACTIVECAPTION
GetSysColor(003)=RGB(191,205,219,255), CTLCOLOR_BTN, COLOR_INACTIVECAPTION
GetSysColor(004)=RGB(240,240,240,255), CTLCOLOR_DLG, COLOR_MENU
GetSysColor(005)=RGB(255,255,255,255), CTLCOLOR_SCROLLBAR, COLOR_WINDOW
GetSysColor(006)=RGB(100,100,100,255), CTLCOLOR_STATIC, COLOR_WINDOWFRAME
GetSysColor(007)=RGB(000,000,000,255), CTLCOLOR_MAX, COLOR_MENUTEXT
GetSysColor(008)=RGB(000,000,000,255), COLOR_WINDOWTEXT
GetSysColor(009)=RGB(000,000,000,255), COLOR_CAPTIONTEXT
GetSysColor(010)=RGB(180,180,180,255), COLOR_ACTIVEBORDER
GetSysColor(011)=RGB(244,247,252,255), COLOR_INACTIVEBORDER
GetSysColor(012)=RGB(171,171,171,255), COLOR_APPWORKSPACE
GetSysColor(013)=RGB(000,120,215,255), COLOR_HIGHLIGHT
GetSysColor(014)=RGB(255,255,255,255), COLOR_HIGHLIGHTTEXT
GetSysColor(015)=RGB(240,240,240,255), COLOR_BTNFACE, COLOR_3DFACE
GetSysColor(016)=RGB(160,160,160,255), COLOR_BTNSHADOW, COLOR_3DSHADOW
GetSysColor(017)=RGB(109,109,109,255), COLOR_GRAYTEXT
GetSysColor(018)=RGB(000,000,000,255), COLOR_BTNTEXT
GetSysColor(019)=RGB(000,000,000,255), COLOR_INACTIVECAPTIONTEXT
GetSysColor(020)=RGB(255,255,255,255), COLOR_BTNHIGHLIGHT, COLOR_3DHIGHLIGHT, COLOR_3DHILIGHT, COLOR_BTNHILIGHT
GetSysColor(021)=RGB(105,105,105,255), COLOR_3DDKSHADOW
GetSysColor(022)=RGB(227,227,227,255), COLOR_3DLIGHT
GetSysColor(023)=RGB(000,000,000,255), COLOR_INFOTEXT
GetSysColor(024)=RGB(255,255,225,255), COLOR_INFOBK
GetSysColor(025)=RGB(000,000,000,255), Unknown
GetSysColor(026)=RGB(000,102,204,255), COLOR_HOTLIGHT
GetSysColor(027)=RGB(185,209,234,255), COLOR_GRADIENTACTIVECAPTION
GetSysColor(028)=RGB(215,228,242,255), COLOR_GRADIENTINACTIVECAPTION
GetSysColor(029)=RGB(000,120,215,255), COLOR_MENUHILIGHT
GetSysColor(030)=RGB(240,240,240,255), COLOR_MENUBAR
GetSysColor(031)=RGB(000,000,000,255), Unknown
GetSysColor(032)=RGB(000,000,000,255), Unknown
GetSysColor(033)=RGB(000,000,000,255), Unknown
GetSysColor(034)=RGB(000,000,000,255), Unknown
GetSysColor(035)=RGB(000,000,000,255), Unknown
GetSysColor(036)=RGB(000,000,000,255), Unknown
GetSysColor(037)=RGB(000,000,000,255), Unknown
GetSysColor(038)=RGB(000,000,000,255), Unknown
GetSysColor(039)=RGB(000,000,000,255), Unknown
GetSysColor(040)=RGB(000,000,000,255), Unknown
GetSysColor(041)=RGB(000,000,000,255), Unknown
GetSysColor(042)=RGB(000,000,000,255), Unknown
GetSysColor(043)=RGB(000,000,000,255), Unknown
GetSysColor(044)=RGB(000,000,000,255), Unknown
GetSysColor(045)=RGB(000,000,000,255), Unknown
GetSysColor(046)=RGB(000,000,000,255), Unknown
GetSysColor(047)=RGB(000,000,000,255), Unknown
GetSysColor(048)=RGB(000,000,000,255), Unknown
GetSysColor(049)=RGB(000,000,000,255), Unknown
Press any key to continue . . .

The function GetSysColor() may not be the right way to get the background color in dark mode. We are also not the only one to report this issue.

[end2endzone]

Regarding Win32Utils::IsDarkThemeActive(), my guess is that it is also inspired from the same issue.

There is also this article that talks about this guy's project (win32-darkmode) which might help. He created a mapping for using dark mode functionality on generic window. I am really not certain if I want to use undocumented APIs on a shell extension. My point is that Microsoft can deprecate theses API anytime without warning which would result in File Explorer to crash every time a context menu would be displayed.

I do like like hardcoded stuff either. How about we define a fixed property in shell extension that contains the color reference to use? Something like this:

ShellAnything.icons.background.color.red=43
ShellAnything.icons.background.color.green=43
ShellAnything.icons.background.color.blue=43

If the properties are not specified, then ShellAnything uses the GetSysColor(COLOR_MENU) function to get the menus background color. If the these properties are defined (and valid), then ShellAnything would use the color defined by the properties instead. This validation code would be inserted in shellext.cpp, line 353 and the resulting color would be given to Win32Utils::FillTransparentPixels(). This method wont need to rely on Win32Utils::IsDarkThemeActive().

or we could create the following properties:

ShellAnything.themes.dark.icons.background.color.red=43
ShellAnything.themes.dark.icons.background.color.green=43
ShellAnything.themes.dark.icons.background.color.blue=43

and use these properties inside an if (Win32Utils::IsDarkThemeActive()) to force a menu background color.

This proposed solution would not be completed either since highlighted icons would still be displayed incorrectly because we would be replacing transparency with an "hardcoded" background color. In my opinion, I do not find incorrect annoying.

I did not know about project KuShellExtension. That looks like a project similar to mine. We could try using GDI+ for converting an HICON with transparency to HBITMAP and preserving transparency. I am concerned about his comment above the line you specified:

this works in most cases, but generates ugly images for such icons contains alpha channel and depend on GDI+

His comment about transparency seems to be related to our issue. We will see. This might be a better way to preserve transparency information and support highlighted icons.

Please tell me what you think about all these ideas and which one you prefer.

[end2endzone]

Just to clarify, the properties like ShellAnything.themes.dark.icons.background.color.red would be defined by ShellAnything at load time, not in a configuration file. However, if for any reason, Microsoft changes the menu background color in dark mode, one could use a configuration default properties to match the change:

<default>
  <property name="ShellAnything.themes.dark.icons.background.color.red" value="20" />
  <property name="ShellAnything.themes.dark.icons.background.color.green" value="20" />
  <property name="ShellAnything.themes.dark.icons.background.color.blue" value="20" />
</default>

[GasDauMin]

Thank You for fast answer.

In my opinion:

1. Properties always a good solution as they can be used flexibly. But in this situation I see potential problem we can define color for state when menu item is not highlighted as i described before...

   //Remove the invisible background and replace by the default popup menu color
   COLORREF menu_background_color = GetSysColor(COLOR_MENU);
   if (Win32Utils::IsDarkThemeActive())
   menu_background_color = RGB(43, 43, 43);
   Win32Utils::FillTransparentPixels(hBitmap, menu_background_color);

2. Probably the best solution would be to find the proper way of using transparent colors in bitmaps. Now i see diference between generated image transparance and extracted from icon with Gimp. I am not yet detected reason why gimp image have true transparent background and with unit test generated image have opaque black color instead.

Now I am trying to generate GDI+ based bitmap file, due to lack of knowledge the process can take time. If I manage to achieve a positive result I will let you know :)

3. What do you thinking about Free Image library? I have no idea how to correctly integrate it to project but for my opinion is last but not least option for working with images.

[end2endzone]

Hi.

You are right in point 1, we should use your proposed code instead of properties.

I don't understand what you mean in point 2. Icons sometimes have a 1 bit per pixel alpha channel (which means pixel is fully opaque or fully transparent). Other icons have 8 bit per pixel alpha channel. Gimp also uses an 8-bit per pixel alpha channel when loading an icon.

Regarding transparency, maybe I was not clear but I don't think the issue is related to loading a bitmap image with transparency or the proper way of handling transparency in bitmaps. I already tried handling 32 bit per pixel bitmaps before without issue. For example, see unit test TestWin32Utils.testIconToBitmapConversion, if you comment the following lines (63 and 64), the resulting bmp files are 32 bit per pixels with an alpha channel:

      ////Remove the invisible background and replace by red color
      //COLORREF background_color = RGB(255,0,255); //pink
      //Win32Utils::FillTransparentPixels(hBitmap, background_color);

See for example TestWin32Utils.testIconToBitmapConversion.shell32.dll.index000.bmp.

I also make sure that the resulting bitmap is actually 32 bit per pixel with an alpha channel in Win32Utils.cpp, line 200. The assertion code is disabled by default and must be manually enabled for verifying.

You can validate both assertions with Gimp, Photoshop or an hexadecimal viewer.

The issue is specifically related to using bitmap with transparency in a menu:

Dealing with transparent icons is complicated on Windows context menus. Over multiple tries and errors, I found out that its better to "fill" the transparency with the correct background color.

See the following issues for references:

• https://stackoverflow.com/questions/60176811/win32-menu-item-bitmap-with-transparent-background
• https://stackoverflow.com/questions/49799264/how-to-show-menu-bitmaps-with-transparent-background

You can also take a look at my comments in function CopyAsBitmap.

I have heard of FreeImage library before. That's a library for loading and manipulating images (.bmp, .jpg, *.png and others) but the generic interface is FIBITMAP which is not compatible with an HBITMAP. I don't know enough about FreeImage to know if it is possible to load an icon from a dll as an FIBITMAP and to convert it to an HBITMAP.

Also keep in mind that images are first loaded from an icon (HICON) and then converted to HBITMAP and only an HBITMAP can be assigned to a menu. As you already pointed out, KuShellExtension also converts icon to bitmap.

The issue may also be related to the version of the header we used for creating the bitmap. As specified in this wikipedia article, there is a header called BITMAPV4HEADER which might give us more control (for instance more flags) on how to create the bitmap. Our current code uses the "standard" of basic header BITMAPINFOHEADER.

Please, continue to look for a positive solution using GDI+. If that strategy fails, then we can also take a look at this article that claims to know how to use transparent bitmap in a menu: https://www.dreamincode.net/forums/topic/281612-how-to-make-bitmaps-on-menus-transparent-in-c-win32/ (more specifically take a look at 9th and 10th block of code).

[GasDauMin]

Hi, part with GDI+ done i would like to contribute. How can i share my code? Localy i commited changes, but can't push request, because lack of permissions.

[end2endzone]

I am sorry. I think I have missed this comment. I appologize.

I have never done a push or pull request to another repository. That is something new for me.

According to https://akrabat.com/the-beginners-guide-to-contributing-to-a-github-project/#step-3-do-some-work, I think you first need to commit in your repository first. I mean the one on github, usually called origin, not the one on your computer. Looking at your forked repository, you don't seem to have anything new committed.

You should be able to push your local (computer) changes to your github account. If you do not have permission, you can go to the settings, managed access page to add permission to "yourself".

Once you will have something new committed, you should be able to make a push request.

If I look at your forked repository, I see a pull request link:

[end2endzone]

I also found out that in my ShellAnything repository on github, I do have a link called "Invite a collaborator". I think this is what was missing. I have sent you an invite, please accept it.

[GasDauMin]

Thank You as soon as possible, I will try this feature :-)

On Sun, Jul 26, 2020 at 3:59 AM Antoine Beauchamp notifications@github.com wrote:

I also found out that in my ShellAnything repository on github, I do have a link called "Invite a collaborator". I think this is what was missing. I have sent you an invite, please accept it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/end2endzone/ShellAnything/issues/54#issuecomment-663922411, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQKQLIQIZE6JY3IU22J5MDR5N5WBANCNFSM4NZUJMOQ .

--

Pagarbiai,

Mindaugas Ribačonka

• Mobilus *+370 6444 5222

  Skype GasDauMin.alt

  El.paštas GasDauMin@gmail.com

[end2endzone]

Hi. I think I might have found the solution!!!

While I was looking to solve another issue, I got into this stackoverflow article that explains how to properly support HBITMAP with transparency. The important thing to note is the use of the LR_CREATEDIBSECTION flag.

The issue we had with our HBITMAP was that pixels data was stored on the video card instead of in the bitmap. You can find more information in StackOverflow - Why does GetObject return an BITMAP with null bmBits?. There is an anwser that explains the issue.

In other words, all HBITMAP must have a DIB (device-independant-bitmap) section; the field bmBits of the BITMAP structure must not be NULL. I though that using SetBitmapBits() would actually create a DIB but it is not.

Then I looked at the Microsoft Bitmap Alpha-blending example which explains how to "create" a HBITMAP that has a DIB section.

TLDR;

I think the solution is to only modify the function CopyAsBitmap(HICON hIcon, const int bitmap_width, const int bitmap_height) and replace the CreateBitmap() call at line 197 by the following code:

    BITMAPINFO bmi;
    bmi.bmiHeader.biSize = sizeof(BITMAPINFOHEADER);
    bmi.bmiHeader.biWidth = bitmap_width;
    bmi.bmiHeader.biHeight = bitmap_height;
    bmi.bmiHeader.biPlanes = 1;
    bmi.bmiHeader.biBitCount = 32;
    bmi.bmiHeader.biCompression = BI_RGB;
    bmi.bmiHeader.biSizeImage = bitmap_width * bitmap_height * 4;
    VOID *pvBits;
    HBITMAP hBitmap = CreateDIBSection(hDcMem, &bmi, DIB_RGB_COLORS, &pvBits, NULL, 0x0);

which effectively create a DIB section in the bitmap making the future calls to SetBitmapBits() work as expected.

yep that's it, change a single line of code...

Obviously, the call to Win32Utils::FillTransparentPixels() in shellext.cpp, line 354 must also be removed.

Eureka!

I have not tried it with ShellAnything yet because experimenting with an active shell extension often results in a crash of File Explorer or having to force close all File Explorer instances in order to recompile. As you might remember, I also do not have access to Windows 10 machine to get dark mode working. I will way for your feedback but I really think it will work.

See below how I validated the change on Windows 7 with a light-gray background.

Testing the modifications on an hello world project:

I tried this technique on an "hello world" example.

First create a Win32 Project (a native win32 GUI app). Then hijack the WndProc() function and modify how the WM_COMMAND case IDM_ABOUT window message is handled and add the following code:

HMENU hPopupMenu = CreatePopupMenu();
AppendMenuA(hPopupMenu, MF_STRING, IDM_EXIT, "&New\tCtrl+N");
AppendMenuA(hPopupMenu, MF_STRING, IDM_EXIT, "&Open\tCtrl+O");

static const std::string menu_name = "Transparency test!";
MENUITEMINFOA menuinfo = {0};
menuinfo.cbSize = sizeof(MENUITEMINFOA);
menuinfo.fMask = MIIM_FTYPE | MIIM_STRING;
menuinfo.fType = MFT_STRING;
menuinfo.dwTypeData = (char*)menu_name.c_str();
menuinfo.cch = (UINT)menu_name.size();

const char * file_path = "G:\\test_menu_length\\transparent.gimp.ico";

// Here I used LoadImageA() instead of ExtractIconEx(). This is ONLY for debugging. Please keep the existing ExtractIconEx() usage.
HICON hIcon = (HICON)LoadImageA(NULL, file_path, IMAGE_ICON, 0, 0, LR_CREATEDIBSECTION | LR_DEFAULTSIZE | LR_LOADFROMFILE);

// Note that I also modified CopyAsBitmap() function to create a DIB section.
HBITMAP hBitmap = Win32Utils::CopyAsBitmap(hIcon, 16, 16);

// Enable bitmap handling for the menu
menuinfo.fMask |= MIIM_BITMAP; 
menuinfo.hbmpItem = hBitmap;

DestroyIcon(hIcon);
hIcon = NULL;

BOOL inserted = InsertMenuItemA(hPopupMenu, GetMenuItemCount(hPopupMenu), FALSE, &menuinfo);

SetForegroundWindow(hWnd);
TrackPopupMenu(hPopupMenu, TPM_BOTTOMALIGN | TPM_LEFTALIGN, 0, 0, 0, hWnd, NULL);

Launch the app, click on menu Help and then click on menu About.... This should display a context menu with a menu named "Transparency test!" that have a transparent background!

Waiting for your feedback.

[GasDauMin]

Thanks, briliant it works flawlessly, my solution with usage of GDI+ has no longer makes sense. :)👍

[end2endzone]

I am sorry to hear that the solution that you had already found is not longer required. I hope you don't have a bad feeling about this experience with another user on Github.

Thank you for quickly trying the proposed solution on ShellAnything. I am really happy that this is finally working! Man I have worked on other Shell Extensions in the past and have been struggling with this issue for years!

Please commit and push your changes on your forked repository. Also add references about this issue in the code so we can link to this issue. Once you are done, make a push request and I will be able to merge your changes back to my repository. Don't forget about CHANGES and AUTHORS files. :-)

Again, I really appreciate your help.

[end2endzone]

Any progress on this issue?

[GasDauMin]

Hi, I don’t have time for a hobby at the moment, the primary activity and holidays fill up all the time. Sorry, I'll be back later :-)

On Mon, Aug 24, 2020 at 12:12 AM Antoine Beauchamp notifications@github.com wrote:

Any progress on this issue?

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/end2endzone/ShellAnything/issues/54#issuecomment-678825542, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQKQLI4BJPT76PQLZN7S4DSCGA45ANCNFSM4NZUJMOQ .

--

Pagarbiai,

Mindaugas Ribačonka

• Mobilus *+370 6444 5222

  Skype GasDauMin.alt

  El.paštas GasDauMin@gmail.com

[end2endzone]

Do you think you will have time to work on this issue by the end of october? I would like to release version 0.5.0 soon and there are 2 issues remaining in the 0.5.0 milestone. Both are assigned to you.

As we already discussed, I think this issue would be good starting point to learn github's collaboration process together. Since the solution for this issue is already known, and is very small (AFAIK, there is only 1 or 2 LoC that needs to be changed), it would not require much effort from your part. Keep me posted. Thanks.

[GasDauMin]

Yes, I think I will be able to demonstrate the result before the beginning of October. Let’s set a deadline of October 4th, if I run into problems I’ll let you know. Is it right for you?

[end2endzone]

That will be perfect. I will monitor this issue every 24h for questions.

[GasDauMin]

Adjustments complete and changes commited. Now I want to ask is my programming and versioning style right for you?

Please also note the changes in the install_googletest.bat file. To correctly prebuild the code in Visual Studio 2019 x64 environment I had to modify the GoogleTest building flags.

There is one of mine tests.

[end2endzone]

Good! I was expecting the changes on your fork and then a push request so that I could benefit from the "code review" tool that is provided by github. Then I remembered that I have sent you a "collaborator invite" for the project. I guess a collaborator means "write access to my repository". Don't worry, I'm fine with it. I have reviewed your changes directly on my machine and the process was really easy.

I do not have any issues about your coding style. As long as you use meaningful names and that you document your code where documentation would be required by another developer to understand a portion of your code. This is what I try to do as well. I am working on this project for a long time and my coding style have changed over the years so I am not that meticulous about this.

I have take a look at the new _SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING preprocessor when building googletest. This have no impact on my side either (I am using VS2010). This is also what I would have done.

Regarding the code that used to call the FillTransparentPixels() function, do you have a reason for keeping the code in commented form with #if 0 ? My guess is that if we ever want to go back with filling pixels instead of using true transparency.

I see that you created a new public function in Win32Utils.h called CreateBitmap(). The reason I created this header file is to provide high level utility functions while staying away from win32 api as much as possible. I find the Win32 api way too difficult to remember. Looking at the function, the code in file Win32Utils.cpp, line 169 should be changed: the hardcoded 4 should be replaced by (biBitCount/8). In other words, for a 32 bpp bitmap, you have 4 bytes per pixels, but if you call CreateBitmap() with biBitCount=24, then you need to use the value 3. This correction is only valid for biBitCount=24 and biBitCount=32. Even that, I am uncertain about the 24 bpp because bitmap pixels are 4-bytes-aligned which means that pixels data have padding at the end of each rows. In other words, this function may only be safe to call for 32 bpp bitmap... Also, calling the function with values such as 1, 4 or 16 is not supported.

Since the purpose of this issue was to support bitmap with transparency, I would like to propose the following changes:

1. Remove biPlanes and use hardcoded value 1 in the cpp file instead. I have never seen use case scenario about creating a multi-layered bitmap. For this reason, the argument biPlanes should always be 1 which makes the need for an argument not really required.
2. Remove biBitCount argument, use hardcoded value 32 in the cpp file instead.
3. Rename the function to CreateBitmapWithTransparency(), or CreateTransparentBitmap(), or CreateBitmapWithAlphaChannel(), or something along those lines.

Optionally, the HDC hDc argument could also be removed to reduce the complexity of the user having to remember "how to properly create and destroy an HDC" to be able to call this function. I know I would not know by heart. The cpp file could use

    HWND hWndDesktop = GetDesktopWindow();
    HDC hdcDesktop = GetDC(hWndDesktop);
    HDC hDcMem = CreateCompatibleDC(hdcDesktop);

and properly destroy the HDC at the end of the function.

Let me know what you think about the propositions.

Now that you have provided help with the project, you might want to add your name (or a reference) in the AUTHORS file.

And thanks for the star :)

[GasDauMin]

Thanks for the reasoned comments.

I agree that it is better to follow the general order and leave the code clean, since we using versioning there are no reasons to store obsolete code like parts of FillTransparentPixels().

What about proposed changes I Am understand your position but in principle when hardcoding is not necessary I trying to avoid them because using hardcode makes the code less flexible.

I didn’t understand much about the HDC part. Do you have in mind every time where we need desktop window handler better to create it localy but not carry it through the arguments?

[end2endzone]

What about proposed changes I Am understand your position but in principle when hardcoding is not necessary I trying to avoid them because using hardcode makes the code less flexible.

You are right. It is always better to have the flexibility but in this particular case, I don't think it applies. My point is that even if you created the function CreateBitmap() it is not much flexible:

1. biPlanes should always be 1.
2. biBitCount only supports value 32.

That's why I suggested to rename the function to something like CreateTransparentBitmap(width, height, hdc). By the name, the function implies biPlanes=1 and biBitCount=32 and still offers the most flexibility.

However, I am not too stubborn and we will proceed with your original commit.

I didn’t understand much about the HDC part. Do you have in mind every time where we need desktop window handler better to create it localy but not carry it through the arguments?

Yes exactly. But like I said, this correction was really optionnal and since we do not proceed with my proposed modifications, there is no need for this one.

Feel free to leave your code as it is.

[GasDauMin]

All fixes commited with optional corrections.

[end2endzone]

Implemented in aa627ad2cd058e3e64d651bb0ceb3722d06f8a3f and 8659617f1b4a97bc5ad301561346a4e0a10f0d0a.

FYI, one nice feature I like about github is that you can you can use #54 in your commit message to allow github to automatically track commits about this issue. This allow tracability between the issue and the code. If we later come back and want to know what was changed about a specific issue, the information is available.