jonaskohl / CapsLockIndicator

A small utility that indicates the state of the Num lock, Caps lock and Scroll lock key.
https://cli.jonaskohl.de/
Apache License 2.0
327 stars 41 forks source link

An option to visualize all states in a single Icon would be great #95

Closed vladtepesch closed 1 year ago

vladtepesch commented 2 years ago

An option to visualize all states in a single Icon would be great since three icons take a large amount of try space.

I would see 2 option for implementing this

  1. for each Key two Icons (On and Off) may be provided and the Icons of the three states will be overlayed and then the result is displayed
  2. a single Icon for each possibe combination is provided

I think the first one may fit the current approach a bit better. So all to do is creating the new iconset and adding a option that, if activated, just combines the states and displays a single icon instead of three

jonaskohl commented 2 years ago

This would be difficult to implement because this would require a significant rewrite of the tray icon system. Furthermore each icon pack would have to be reworked to include 8 additional icons. I'm not sure if this would be feasible

vladtepesch commented 2 years ago

Regarding the backward compatibility: I d not think so. Its only necessary that the selected iconpack supports the option (if enabled). the other one still can be used as they are if this option is disabled. My idea about this was not to add 8 more but only to use 6 (as currently) but instead of displaying them in 3 different icons to just draw them onto each other and show the result in the tray.

Regearding the implementation: I cant say to much about this because I have not took a look at the code too deeply yet and I am not that familar with C#. But it seems that all states are checked and icons are set in UpdateTimerTick. So it would be possible to check the options and create a merged icon at this place.

vladtepesch commented 2 years ago

addition - I implemented this as a proof of concept (hardcoded without the menu option) and it seemed to work with very little changes. But one thing that confuses me was the initial icon loading with ExtractAssociatedIcon that leads to upscaled icons and later to artifacts on the merged icons. Changing the icon initialization to the Icon(string) constructor reads the icon in original size that seems to be more appropriate - but I guess there was a reason for using ExtractAssociatedIcon. I would like to know which.

jonaskohl commented 2 years ago

You are right, I certainly do not need ExtractAssociatedIcon. I have taken a look at your implementation (I just looked at the code, I didn't have time to compile it yet), but I immediately noticed that in your mergeIcons() function you don't dispose of your Graphics object and the old bitmap. This will lead to memory leaks very quickly.

I will try to take a closer look at this over the weekend

vladtepesch commented 2 years ago

oh - I indeed did not investigated the rules about handling gdi objects in c#. I estimated that this is handled by the wrapping objects. May be it would be advisable to create all icon variants at loading time and free at the end and only select the correct one in the timer.

(also the icons are fastly thrown together without any artistic claim ;) )

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.