Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Idea: make the currently-selected preset more visible #55

Closed
6 tasks
denilsonsa opened this issue Aug 7, 2023 · 25 comments
Closed
6 tasks

Idea: make the currently-selected preset more visible #55

denilsonsa opened this issue Aug 7, 2023 · 25 comments
Assignees
Labels
enhancement New feature or request

Comments

@denilsonsa
Copy link
Contributor

The currently set preset is shown at the bottom-right corner of the main window, but also as a tiny text on the system tray. Really tiny. Looks like this:

vdu_controls tray icon

Let's look again, this time zoomed 4×:

vdu_controls tray icon zoomed 4×

Each character is about 3 or 4 pixels wide, and 5 pixels tall. With a white-ish foreground color on top of a light blue background, with contrast ratio between 1.5 and 2.4. In very blurry pixels that are basically unreadable.

Sidenote: the contrast was measured using KDE's Kontrast app (for simplicity), but there are also many browser-based checkers. For comparison, the contrast ratio between the foreground and background colors of my taskbar (e.g. used while rendering the clock) is 8.58. The contrast ratio of this GitHub page is 11.66.


Proposed solution(s):

  • Improving the icon
    • Render the icon text using a bigger font.
    • Render the icon text using better contrast.
    • Render the icon text using a bitmap font, optimized for pixel-perfect rendering on small sizes. There are many fonts for that, or it's possible to use a plain image as a tileset and pick the glyphs from the image.
  • Menu improvements
    • At the systray menu, mark the currently-selected preset with a checkmark. Like a radio box. Whatever it is called when inside the menu. (see screenshot further below)
    • Do the same at the main presets menu (at the bottom-right of the main window).
    • If, for whatever reason, native checkmarks aren't supported in menus, it's possible to fake them using Unicode characters. Like ✓ or ☐ or ☒ or ☑ or others.

I can understand that rendering text inside an icon can quickly become a large problem and a lot of work. Thus, changing the way the menu items are rendered sounds like a simple and yet great solution. Of course, both improvements can be implemented together.


Screenshot of the menu of KolourPaint

@digitaltrails
Copy link
Owner

Some things to try:

  1. Had you considered assigning an icon to each preset? That was my original intention for presets. The preset icon is overlayed over the program icon. The letters are a fallback, used in the absence of an icon. I find the icons are very visible.

image

  1. On KDE the current preset is also prefixed on the tray icon tooltip and the window title. Hovering over the icon to see the tooltip is easier than bringing up the menu.

image

I use a 4k monitor, so the fonts look fine :-). However there is probably room to make the font a bit larger. Also, I use a light theme, on my desktop the letters are drawn in black and are more visible. I think I should disable/ignore theming for annotating the main icon text. Each individual's choice for the size/height of the bottom panel and its icons may also be a factor. On my own desktop the icon is about 40 px wide:

Screenshot_20230807_211916

I'm not that keen on adding more code to change the menus. The codebase is about 7500 lines, and becoming a bit brittle. If it can be done easily, without fighting Qt too much, that probably would be OK.

@digitaltrails
Copy link
Owner

I can definitely see that forcing dark-text (= light theme) for anything superimposed on the program icon is preferable to following the desktop theme. That doesn't look too difficult either.

@denilsonsa
Copy link
Contributor Author

I'm not that keen on adding more code to change the menus. […] If it can be done easily, without fighting Qt too much, that probably would be OK.

That's why I've suggested just prefixing the menu item strings with a checkmark. It's a hacky but simple way to mark the currently selected item without having to rely on any additional GUI capability. ;)

In your screenshots, the text is significantly larger than mine. It might be related to conversion between pt (points) to pixels, based on the current display DPI.

@digitaltrails
Copy link
Owner

digitaltrails commented Aug 7, 2023

I'm not that keen on adding more code to change the menus. […] If it can be done easily, without fighting Qt too much, that probably would be OK.

That's why I've suggested just prefixing the menu item strings with a checkmark. It's a hacky but simple way to mark the currently selected item without having to rely on any additional GUI capability. ;)

Oh, sorry, I misunderstood, I was thinking checkboxes. Even so, it might mean adding code to refresh the menu to track events when other threads activate a preset (Preset-Dialog, preset-scheduler or lux-metering) . But if something can be done with a small amount of code... I'll have to balance out any maintenance nightmare side effects (issue #52).

In your screenshots, the text is significantly larger than mine. It might be related to conversion between pt (points) to pixels, based on the current display DPI.

It's probably because I'm using 4k with no scaling. Partly because of 4K, I think my desktop panel is set to a slightly larger size (as measured in pixels). Anything I do in respect to this issue, I'll test on a normal monitor, preferably a small one (<27").

I'll likely at least make the font color dark and font size bigger, I'll look at the implications for the menu code.

@digitaltrails
Copy link
Owner

It took a couple of goes, but I think I've disabled theming for app icon. Both for letters and icon overlays. From now on the icon and letters will be in dark colors on the app icon, but properly themed everywhere else. A bonus was it did not increase the size of the script.

I have to look at what come be done about the font size.

@digitaltrails
Copy link
Owner

digitaltrails commented Aug 7, 2023

Actually it took three goes - when a preset is transitioning it blinks the target icon and transition icon. I forgot about that.

Font size is proving difficult - I don't fully understand the code - it draws the font into a pixmap, so changing pixmap sizes and font sizes yields various centering and pixelation issues.

I guess the menu should really use radio buttons. But that might look ugly with icons. Maybe icons should be removed from the menu, or moved to the right hand side. As you suggest, a tick on the right-hand side might be easiest.

@digitaltrails
Copy link
Owner

digitaltrails commented Aug 8, 2023

I've added a checkmark - it seems to work. I was hesitant to add more code, but I trimmed elsewhere. The script is four lines shorter now, and only a little more complicated than before. (I did cheat a bit by trimming the lines back via reformatting).

@digitaltrails digitaltrails self-assigned this Aug 8, 2023
@digitaltrails digitaltrails added the enhancement New feature or request label Aug 8, 2023
@denilsonsa
Copy link
Contributor Author

I've disabled theming for app icon

So, is the icon itself supplied with vdu_controls? Because if the icon comes from the user's theme, then we can have issues with certain themes. (Sorry, I haven't checked it myself. That's why I'm asking.)

Font size is proving difficult

Now that you mention that, indeed the font size of the menus is also tiny:

Presets menu screenshot

The script is four lines shorter now

Please, don't look too hard at LOC metric. Adding comments would increase the LOC, but would also increase the maintainability due to better documentation of intent of the code.

@digitaltrails
Copy link
Owner

digitaltrails commented Aug 8, 2023

I've disabled theming for app icon

So, is the icon itself supplied with vdu_controls? Because if the icon comes from the user's theme, then we can have issues with certain themes. (Sorry, I haven't checked it myself. That's why I'm asking.)

The user gets to pick icons. SVG works best. I keep thinking I should supply a set. One day.

Font size is proving difficult

Now that you mention that, indeed the font size of the menus is also tiny:

That might be a Qt/Desktop setting - I'm not setting the size.

Presets menu screenshot

The script is four lines shorter now

Please, don't look too hard at LOC metric. Adding comments would increase the LOC, but would also increase the maintainability due to better documentation of intent of the code.

Yes, I know. It's just the code is becoming very unwieldy and untestable and I'm itching to delete some of it. At least I'm not thinking of adding anything big to it.

@denilsonsa
Copy link
Contributor Author

denilsonsa commented Aug 8, 2023

Now that you mention that, indeed the font size of the menus is also tiny:

That might be a Qt/Desktop setting - I'm not setting the size.

Oh, I wasn't clear in my message. I meant the font size of the dynamically-rendered icons is tiny, and as tiny as the systray icon. The font size of the menu labels themselves is fine.

@digitaltrails
Copy link
Owner

Now that you mention that, indeed the font size of the menus is also tiny:

That might be a Qt/Desktop setting - I'm not setting the size.

Oh, I wasn't clear in my message. I meant the font size of the dynamically-rendered icons is tiny, and as tiny as the systray icon. The font size of the menu labels themselves is fine.

Oh. The menu uses the same generated text "icon" as used for the app-icon. So same problem.

@denilsonsa
Copy link
Contributor Author

denilsonsa commented Sep 11, 2023

Hi! Thanks for the updates! I just wanted to report that the text generation inside an icon is still tiny, but the contrast is good. Look:

image

image

@digitaltrails
Copy link
Owner

This is what I see:

Screenshot_20230911_220928

I never found a way to make it bigger that didn't stuff up other aspects of the icon. So I settled for turning off dark theming for the icon, which means the text is always black. But clearly the text size is different for each of us.

I suppose it could be the icon size in the panel. I have no idea how KDE sets the panel icon size - I'll have to find out. My KDE panel height is 64, maybe it's based on that. I will have to set up a virtualbox with a smaller panel height and see what happens.

@digitaltrails digitaltrails reopened this Sep 11, 2023
@denilsonsa
Copy link
Contributor Author

I'd guess it might be related to the DPI. But that's assuming the font-size is defined using points or other non-pixel-based unit, which then gets converted differently to pixel sizes on different systems. (But that's a guess without poking into the code.)

@digitaltrails
Copy link
Owner

digitaltrails commented Sep 11, 2023

I'd guess it might be related to the DPI. But that's assuming the font-size is defined using points or other non-pixel-based unit, which then gets converted differently to pixel sizes on different systems. (But that's a guess without poking into the code.)

Yes, could be.

I'm also using the system font unaltered, the size (and look) may well vary from system to system. I've altered the font by setting it's size explicitly in pixels. The icon starts out being generated as 32x32 pixels, so I've set the font to 24 pixels. It seems like a trivial change, so I've pushed it, but I need to go test what difference it makes on a lower DPI setup and on gnome, xfce, deepin.

@digitaltrails
Copy link
Owner

digitaltrails commented Sep 11, 2023

I can reproduce the look of the fonts of your sample icon on gnome in a virtual box. Setting the font pixel size seems to have worked, the font size is more reasonable (although the default size of the gnome tray icons is still tiny).
Screenshot_20230912_080823
Screenshot_20230912_081025

I think before I was trying to use scaling. For some reason I had overlooked setting the pixel size of the font.

@denilsonsa
Copy link
Contributor Author

Yes, I've launched the new v1.11.1 version and it is indeed fixed, I can actually read the text (next to the original screenshot from when this issue was reported):

old vdu_controls tray icon new vdu_controls tray icon

Thank you!


P.S. Maybe a bit blurry when compared to the other text on the taskbar. Look:

image
image

Observe how the 2 looks crisper on the clock. Then I tried enabling the font hinting:

diff --git a/vdu_controls.py b/vdu_controls.py
index 9529c8a..4999900 100755
--- a/vdu_controls.py
+++ b/vdu_controls.py
@@ -4915,6 +4915,7 @@ def create_icon_from_text(text: str, themed: bool = True) -> QIcon:
     painter = QPainter(pixmap)
     font = QApplication.font()
     font.setPixelSize(24)
+    font.setHintingPreference(QFont.PreferFullHinting)
     painter.setFont(font)
     painter.setOpacity(1.0)
     painter.setPen(QColor((SVG_DARK_THEME_COLOR if themed and is_dark_theme() else SVG_LIGHT_THEME_COLOR).decode("utf-8")))

The results were underwhelming. They both look very similar:

image

Which one has hinting enabled? Can you guess? Click here to expand! 👈

The left one is v1.11.1 using the default hinting settings.

The one to the right is commit 1ba63d1 with the one-line patch applied.

I guess there is no point in changing the default.

Then I tried playing with the font weight:

image
image

From left to right: Click here to expand! 👈
  • Plain v1.11.1 (probably QFont.Normal, but I haven't checked)
  • font.setHintingPreference(QFont.PreferFullHinting)
  • font.setBold(True), maybe too bold.
  • font.setWeight(QFont.Medium)
  • font.setWeight(QFont.DemiBold)
  • font.setWeight(QFont.Bold)

I can't see any difference between Medium and DemiBold. And I feel like Bold is too much.

Should we settle for Medium? I'm gonna send a PR for this one-line change.

denilsonsa added a commit to denilsonsa/vdu_controls that referenced this issue Sep 25, 2023
It seems the default font weight is Normal or 400. Changing it to Medium (500) seem to make the text more visible. The rendered text looks less thin and less gray, and instead looks more black without looking too bold. Setting it to DemiBold(600) looks the same as Medium. Setting it to Bold (700) is too much.

https://doc.qt.io/qtforpython-6/PySide6/QtGui/QFont.html#PySide6.QtGui.PySide6.QtGui.QFont.Weight

digitaltrails#55 (comment)
@digitaltrails
Copy link
Owner

Try replacing the SVG_LIGHT_THEME_COLOR with b"#000000" (black). The default drawing color for a light theme is a gray (b"#232629"). I've just done this, but my desktop is 4k, I will go and try an old 1080 display (and try the other suggestions).

@digitaltrails
Copy link
Owner

I think changing the font color to black has the greatest effect. Setting the weight seems to help.

I don't think hinting is adding anything, perhaps it's already active, or perhaps it's effect is being lost when the icon is scaled for presentation.

See what you think on your desktop. I've added a new constant SVG_LIGHT_THEME_TEXT_COLOR and set it to black, but it could be set to something more optimal if an appropriate color can be identified.

I added a corresponding SVG_LIGHT_THEME_TEXT_COLOR, but that's never used for the desktop/tray icon (which is now always unthemed). It's only used in the toolbar, and probably needs to stay slightly off-white.

@denilsonsa
Copy link
Contributor Author

The default drawing color for a light theme is a gray (b"#232629").

Oh! That explains why I never got pure black when I was checking the colors of the screenshot!

Indeed, changing it significantly improves the contrast. When checking the colors on that nice KDE tool called Kontrast, the black version achieved "good for normal text", even if true black pixels weren't rendered due to anti-aliasing.

Having colors that follow the dark/light theme might be useful IF the background of the text also follows the theme. Otherwise, I think it's best to hardcode the text color because the background color is already hardcoded.

I think changing the font color to black has the greatest effect. Setting the weight seems to help.

I don't think hinting is adding anything, perhaps it's already active, or perhaps it's effect is being lost when the icon is scaled for presentation.

Fully agree! I don't have to squint my eyes to read the text on the corner of my screen (which is further away than the center of the screen) using the black version, with either weight. So, I'm grateful for the fully-black; and I'm fine if you choose to keep the medium weight or decide to reduce it back to the normal weight.

I also believe this will be very noticeable for people with TN panels. IPS panels have great color rendering on any angle, TN panels are horrible for anything that isn't pure black or pure white — speaking from my own experience from a laptop from years ago, before I knew the difference (and photos about the issue can be seen at the bottom of the Lagom LCD test page).

Screenshot: v1.11.1 (non-gray text, normal weight), 64ce3fe (black text, medium weight), and normal black (black text, default weight).

image
image

Awesome! Thank you for your cooperation! If we look at the first screenshot in this issue, and compare to the current state, it's incredibly better. Even the menu icons are now decent, with good size and contrast.


You mentioned icon resizing… I noticed the icon is being created as a 32x32 pixmap. Should it be created as a high-res pixmap? Maybe 64x64 or 128x128? Would that look good on the system tray? Would that look good on a HiDPI display? Would that always end up with blurry text because it negates the effects of font hinting? I don't know, that's just an area worth exploring if you decide to pursue this rabbit hole. Your choice, because I'm already pleased with the current result, as the icon is now useful and I can read the text. Feel free to close this issue if you think it's done.

@digitaltrails
Copy link
Owner

You mentioned icon resizing… I noticed the icon is being created as a 32x32 pixmap. Should it be created as a high-res pixmap? Maybe 64x64 or 128x128? Would that look good on the system tray? Would that look good on a HiDPI display? Would that always end up with blurry text because it negates the effects of font hinting? I don't know, that's just an area worth exploring if you decide to pursue this rabbit hole. Your choice, because I'm already pleased with the current result, as the icon is now useful and I can read the text. Feel free to close this issue if you think it's done.

I tried 64x64 and 128x128 just now, I could not perceive a difference on my 4K monitor (the monitor isn't scaled). All text icons are eventually used at 32x32, so perhaps I'll leave it as is until I see a reason to change.

I have a HiDPI display, it's 4k, I don't scale it, rather I just adjust font sizes to suit. All the icons look pretty sharp (probably because I would have done something about it otherwise).

I can see that I should probably reduce the main panel and toolbar icon sizes for non HiDPI desktops. I need to get around to that.

@digitaltrails
Copy link
Owner

Actually I can see that I hard wired some of the icon sizes to suit my desktop. Not in the menus, but in the toolbars and in the main panel.

If I take off the hard wiring, then Qt scales appropriately. However, leaving things to Qtt, does leave the HiDPI main panel and toolbar icon sizes a little puny. Investigating options.

@denilsonsa
Copy link
Contributor Author

Feel free to open a separate issue for investigating icon sizes, if you prefer.

Or, if Qt has proper SVG support, you can just tell Qt to use a certain SVG code as the icon. And that has the bonus feature of getting text rendering for free (just modify the content of a <text> element). I know Linux distros ship with several SVG icons, I just don't know how Qt handles them.

If Qt can't read SVG from a string, but instead supports a file-like object, you can try wrapping the string in an io.StringIO object. I don't have much experience yet with Qt, so I have no idea.

@digitaltrails
Copy link
Owner

digitaltrails commented Sep 25, 2023

Qt does scale appropriately and from SVG. The issue arises when I override things to make them bigger the it uses the same size no matter the DPI. Yes, this is a separate problem, to be dealt with elsewhere. Continued in #63

@digitaltrails
Copy link
Owner

I think we've achieved the desired goal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants