[KBSWITCH] Fix keyboard indicator text and font #4723
Conversation
|
English: |
katahiromz
changed the title
|
|
||
| /* Create hdc, hbmColor and hbmMono */ | ||
| hdc = CreateCompatibleDC(NULL); | ||
| hbmColor = CreateCompatibleBitmap(hdc, CX_ICON, CY_ICON); | ||
| hbmColor = CreateDIBSection(hdc, &bmi, DIB_RGB_COLORS, NULL, NULL, 0); |
There was a problem hiding this comment.
(That's also a question for the other CPL PR): Why forcing here RGB_Colors? What's happening if you keep the original code? Is it going to be monochrome then?
There was a problem hiding this comment.
The 24-bpp DIB is a device-independent full color bitmap. So it doesn't need the color table if conversion was correct.
There was a problem hiding this comment.
The device-dependent bitmap (DDB) has a fear that conversion might be failed if the device was changed.
There was a problem hiding this comment.
BEFORE:
Looks like monochrome.
There was a problem hiding this comment.
I guess that if the CreateCompatibleDC call was called on the hdc of the traybar's window, it could have been in color.
But why I'm overall surprised why this code would need any change, is I know that so far, the kbswitch was drawing a perfectly colored icon in the traybar, so why is it that sometimes it would draw a monochrome-looking one instead? 🤔
See for example:
https://i.ytimg.com/vi/Ne88Is2cymQ/maxresdefault.jpg
And switching to french didn't cause problem. But why for japanese (at least in your new tests) the "JA" icon now appears in black with the existing code, is a mystery for me.
EDIT: This is the commit that refactored this CreateTrayIcon:
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=0991cedca782f770e41bf991815d7d57f93ba1fd
One question is whether the problem started to appear there. If so, what are the subtle differences between the old code and the new one that triggered it?
There was a problem hiding this comment.
That DDB bitmap is passed to CreateIconIndirect function. That is non-compatible. Japanese text is antialiased. Antialiasing uses some gray colors. DDB may lack palette.
There was a problem hiding this comment.
DDB was not full color. Rendering Japanese text used antialiasing. Antialiasing used gray colors. The DDB lacks palette. Darkblue color is ignored.
There was a problem hiding this comment.
Maybe our palette optimization was poor.
There was a problem hiding this comment.
If you isolate this "CreateTrayIcon" (the one before the fix from this PR) into a standalone test-program that just creates a tray icon (or any other kind of window on which the icon can be drawn), that you then run on Windows (japanese or not): would the created bitmap/icon be in color or not? (I would suggest testing that on Windows XP/2k3 before looking at e.g. Windows 10.)
If it works there (but not in ReactOS), this could indicate a potential bug in our gdi32/win32k code.
There was a problem hiding this comment.
I made a test program: https://jira.reactos.org/secure/attachment/62796/DDBTest.zip
ReactOS Japanese:
WinXP Japanese:
Win2k3 English:
Win10 Japanese:
Addendum to commits 5f4bb73 and c6ccb92. - GetLocaleInfo() returns an int, not a bool: makes it clear in the test. - No need to use StringCchCopy() to just initialize two chars to the same value. - The question about the test in #4723 (comment) was meant to discover that CreateDIBSection() was unnecessary, since the very original code (before commit 0991ced) did not use it and was working fine in that regard. The simple fix was to use GetDC(NULL). - Use SM_CXSMICON/SM_CYSMICON metrics for the KBSWITCH indicator as well. - Override the font size obtained from SPI_GETICONTITLELOGFONT with a known one (allows to get a correct indicator even if the user font is very large). - Do the initialization in such a way that in case SPI_GETICONTITLELOGFONT or CreateFontIndirect fails, we always fall back to the default stock font that is ensured to always exist. - Initialize *all* the fields of the IconInfo structure.
Purpose
JIRA issue: CORE-10667
Comparison
WinXP Japanese:
BEFORE:
AFTER:
Proposed changes
input.dllin getting indicator text.SPI_GETICONTITLELOGFONTfor font.